Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC of Approach to Supporting Internationalization #5853

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
258 changes: 249 additions & 9 deletions Cargo.lock

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ snapbox = "0.6.16"
shlex = "1.3.0"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just adding a new example to demonstrate, and some dev-deps to support the example.

automod = "1.0.14"
clap-cargo = { version = "0.14.1", default-features = false }
rust-i18n = "3.1.2"
serde_yml = "0.0.12"

[[example]]
name = "demo"
Expand Down Expand Up @@ -505,6 +507,12 @@ path = "examples/derive_ref/flatten_hand_args.rs"
required-features = ["derive"]
doc-scrape-examples = true

[[example]]
name = "i18n"
path = "examples/i18n/main.rs"
required-features = ["derive"]
doc-scrape-examples = true

[profile.test]
opt-level = 1

Expand Down
4 changes: 2 additions & 2 deletions clap_bench/benches/complex.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(elided_lifetimes_in_paths)] // needed for divan

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll see a lot of cases of text_provider::DEFAULT_TEXT_PROVIDER being added throughout this PR. This is just to resolve compile issues. The final implementation would ideally be structured in a way to avoid needing to make these changes; however, to keep the POC simple I opted to not resolve that issue until later.

use clap::{arg, ArgMatches, Command};
use clap::{arg, text_provider::DEFAULT_TEXT_PROVIDER, ArgMatches, Command};

macro_rules! create_app {
() => {{
Expand Down Expand Up @@ -48,7 +48,7 @@ fn build() -> Command {

#[divan::bench(args=COMPLEX_ARGS)]
fn startup(args: &Args) -> ArgMatches {
create_app!().get_matches_from(args.args())
create_app!().get_matches_from(args.args(), &*DEFAULT_TEXT_PROVIDER)
}

#[divan::bench]
Expand Down
3 changes: 2 additions & 1 deletion clap_bench/benches/empty.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![allow(elided_lifetimes_in_paths)] // needed for divan

use clap::text_provider::DEFAULT_TEXT_PROVIDER;
use clap::ArgMatches;
use clap::Command;

Expand All @@ -16,7 +17,7 @@ fn build() -> Command {

#[divan::bench]
fn startup() -> ArgMatches {
create_app!().get_matches_from(vec![""])
create_app!().get_matches_from(vec![""], &*DEFAULT_TEXT_PROVIDER)
}

#[divan::bench]
Expand Down
8 changes: 5 additions & 3 deletions clap_bench/benches/ripgrep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,13 @@ mod render_help {
}

mod startup {
use clap::text_provider::DEFAULT_TEXT_PROVIDER;

use super::{app_short, ArgMatches};

#[divan::bench]
fn simple() -> ArgMatches {
app_short().get_matches_from(vec!["rg", "pat"])
app_short().get_matches_from(vec!["rg", "pat"], &*DEFAULT_TEXT_PROVIDER)
}

#[divan::bench]
Expand All @@ -62,7 +64,7 @@ mod startup {
"-C5",
"--follow",
"-e some",
])
], &*DEFAULT_TEXT_PROVIDER)
}

#[divan::bench]
Expand Down Expand Up @@ -228,7 +230,7 @@ mod startup {
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
])
], &*DEFAULT_TEXT_PROVIDER)
}
}

Expand Down
6 changes: 4 additions & 2 deletions clap_bench/benches/rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,18 @@ fn build() -> Command {
}

mod startup {
use clap::text_provider::DEFAULT_TEXT_PROVIDER;

use super::{build_cli, ArgMatches};

#[divan::bench]
fn empty() -> ArgMatches {
build_cli().get_matches_from([""])
build_cli().get_matches_from([""], &*DEFAULT_TEXT_PROVIDER)
}

#[divan::bench]
fn sc() -> ArgMatches {
build_cli().get_matches_from(["rustup override add stable"])
build_cli().get_matches_from(["rustup override add stable"], &*DEFAULT_TEXT_PROVIDER)
}
}

Expand Down
8 changes: 5 additions & 3 deletions clap_bench/benches/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,23 @@ fn build() -> Command {
}

mod startup {
use clap::text_provider::DEFAULT_TEXT_PROVIDER;

use super::{arg, ArgMatches, Command};

#[divan::bench]
fn flag() -> ArgMatches {
create_app!().get_matches_from(vec!["myprog", "-f"])
create_app!().get_matches_from(vec!["myprog", "-f"], &*DEFAULT_TEXT_PROVIDER)
}

#[divan::bench]
fn opt() -> ArgMatches {
create_app!().get_matches_from(vec!["myprog", "-o", "option1"])
create_app!().get_matches_from(vec!["myprog", "-o", "option1"], &*DEFAULT_TEXT_PROVIDER)
}

#[divan::bench]
fn pos() -> ArgMatches {
create_app!().get_matches_from(vec!["myprog", "arg1"])
create_app!().get_matches_from(vec!["myprog", "arg1"], &*DEFAULT_TEXT_PROVIDER)
}
}

Expand Down
2 changes: 2 additions & 0 deletions clap_builder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ anstyle = "1.0.8"
terminal_size = { version = "0.4.0", optional = true }
backtrace = { version = "0.3.73", optional = true }
unicode-width = { version = "0.2.0", optional = true }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New depts. serde_yml is for deserializing the external config file where the hardcoded texts are extracted to.

serde_yml = "0.0.12"
lazy_static = "1.5.0"

[dev-dependencies]
static_assertions = "1.1.0"
Expand Down
29 changes: 17 additions & 12 deletions clap_builder/src/builder/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ use crate::mkeymap::MKeyMap;
use crate::output::fmt::Stream;
use crate::output::{fmt::Colorizer, write_help, Usage};
use crate::parser::{ArgMatcher, ArgMatches, Parser};
use crate::text_provider::TextProvider;
use crate::text_provider::DEFAULT_TEXT_PROVIDER;
use crate::util::ChildGraph;
use crate::util::{color::ColorChoice, Id};
use crate::{Error, INTERNAL_ERROR_MSG};
Expand Down Expand Up @@ -609,8 +611,8 @@ impl Command {
/// [`env::args_os`]: std::env::args_os()
/// [`Command::try_get_matches_from_mut`]: Command::try_get_matches_from_mut()
#[inline]
pub fn get_matches(self) -> ArgMatches {
self.get_matches_from(env::args_os())
pub fn get_matches(self, texts: &impl TextProvider) -> ArgMatches {
self.get_matches_from(env::args_os(), texts)
}

/// Parse [`env::args_os`], [exiting][Error::exit] on failure.
Expand All @@ -633,9 +635,9 @@ impl Command {
/// ```
/// [`env::args_os`]: std::env::args_os()
/// [`Command::get_matches`]: Command::get_matches()
pub fn get_matches_mut(&mut self) -> ArgMatches {
pub fn get_matches_mut(&mut self, texts: &impl TextProvider) -> ArgMatches {
self.try_get_matches_from_mut(env::args_os())
.unwrap_or_else(|e| e.exit())
.unwrap_or_else(|e| e.exit(texts))
}

/// Parse [`env::args_os`], returning a [`clap::Result`] on failure.
Expand Down Expand Up @@ -704,14 +706,14 @@ impl Command {
/// [`Command::get_matches`]: Command::get_matches()
/// [`clap::Result`]: Result
/// [`Vec`]: std::vec::Vec
pub fn get_matches_from<I, T>(mut self, itr: I) -> ArgMatches
pub fn get_matches_from<I, T>(mut self, itr: I, texts: &impl TextProvider) -> ArgMatches
where
I: IntoIterator<Item = T>,
T: Into<OsString> + Clone,
{
self.try_get_matches_from_mut(itr).unwrap_or_else(|e| {
drop(self);
e.exit()
e.exit(texts)
})
}

Expand Down Expand Up @@ -814,7 +816,6 @@ impl Command {
{
let mut raw_args = clap_lex::RawArgs::new(itr);
let mut cursor = raw_args.cursor();

if self.settings.is_set(AppSettings::Multicall) {
if let Some(argv0) = raw_args.next_os(&mut cursor) {
let argv0 = Path::new(&argv0);
Expand Down Expand Up @@ -878,6 +879,8 @@ impl Command {
let usage = Usage::new(self);
write_help(&mut styled, self, &usage, false);

styled.render_text(&*DEFAULT_TEXT_PROVIDER);

let c = Colorizer::new(Stream::Stdout, color).with_content(styled);
c.print()
}
Expand Down Expand Up @@ -905,7 +908,7 @@ impl Command {
let mut styled = StyledStr::new();
let usage = Usage::new(self);
write_help(&mut styled, self, &usage, true);

styled.render_text(&*DEFAULT_TEXT_PROVIDER);
let c = Colorizer::new(Stream::Stdout, color).with_content(styled);
c.print()
}
Expand Down Expand Up @@ -4278,7 +4281,9 @@ impl Command {

// do the real parsing
let mut parser = Parser::new(self);

if let Err(error) = parser.get_matches_with(&mut matcher, raw_args, args_cursor) {

if self.is_set(AppSettings::IgnoreErrors) && error.use_stderr() {
debug!("Command::_do_parse: ignoring error: {error}");
} else {
Expand Down Expand Up @@ -4729,7 +4734,7 @@ impl Command {
.help("Print help (see more with '--help')")
.long_help("Print help (see a summary with '-h')");
} else {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example of one case where we have removed hardcoded English text and replaced it with a template string containing a text lookup key.

arg = arg.help("Print help");
arg = arg.help("{clap.help.short-help}");
}
// Avoiding `arg_internal` to not be sensitive to `next_help_heading` /
// `next_display_order`
Expand All @@ -4741,15 +4746,15 @@ impl Command {
.short('V')
.long("version")
.action(ArgAction::Version)
.help("Print version");
.help("{clap.version.short-help}");
// Avoiding `arg_internal` to not be sensitive to `next_help_heading` /
// `next_display_order`
self.args.push(arg);
}

if !self.is_set(AppSettings::DisableHelpSubcommand) {
debug!("Command::_check_help_and_version: Building help subcommand");
let help_about = "Print this message or the help of the given subcommand(s)";
let help_about = "{clap.help.about}";

let mut help_subcmd = if expand_help_tree {
// Slow code path to recursively clone all other subcommand subtrees under help
Expand All @@ -4771,7 +4776,7 @@ impl Command {
Arg::new("subcommand")
.action(ArgAction::Append)
.num_args(..)
.value_name("COMMAND")
.value_name("{clap.help.command.value-name}")
.help("Print help for the subcommand(s)"),
)
};
Expand Down
7 changes: 7 additions & 0 deletions clap_builder/src/builder/styled_str.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#![cfg_attr(not(feature = "usage"), allow(dead_code))]

use crate::{text_provider::TextProvider, util::interpolate};

/// Terminal-styling container
///
/// Styling may be encoded as [ANSI Escape Code](https://en.wikipedia.org/wiki/ANSI_escape_code)
Expand Down Expand Up @@ -29,6 +31,11 @@ impl StyledStr {
Self(String::new())
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new interpolate function replaces the text lookup keys with the actual texts. There is probably a better place to put this, but for the purposes of demonstrating the approach, I thought this was good enough.

/// Interpolate values into the text
pub fn render_text(&mut self, texts: &impl TextProvider) {
self.0 = interpolate(&self.0, texts);
}

/// Display using [ANSI Escape Code](https://en.wikipedia.org/wiki/ANSI_escape_code) styling
#[cfg(feature = "color")]
pub fn ansi(&self) -> impl std::fmt::Display + '_ {
Expand Down
28 changes: 18 additions & 10 deletions clap_builder/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//! macros in `clap_derive`.

use crate::builder::PossibleValue;
use crate::text_provider::{TextProvider, DEFAULT_TEXT_PROVIDER};
use crate::{ArgMatches, Command, Error};

use std::ffi::OsString;
Expand All @@ -28,15 +29,22 @@ use std::ffi::OsString;
pub trait Parser: FromArgMatches + CommandFactory + Sized {
/// Parse from `std::env::args_os()`, [exit][Error::exit] on error.
fn parse() -> Self {
let mut matches = <Self as CommandFactory>::command().get_matches();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New parse_with_texts is the API which allows a consumer to substitute the OOTB English texts with their own texts. This could perhaps be adjusted to allow a reference to the texts to be added to the Parser struct itself so that we don't need to pass them down through many layers of function calls as we do here. This change might have required making changes to clap_derive in order to get a minimally working example ready, so I decided against it.

Self::parse_with_texts(&*DEFAULT_TEXT_PROVIDER)
}

/// Parse from `std::env::args_os()`, [exit][Error::exit] on error, but replace the default Clap [`TextProvider`] with
/// a custom implementation.
fn parse_with_texts(texts: &impl TextProvider) -> Self {
let mut matches = <Self as CommandFactory>::command().get_matches(texts);
let res = <Self as FromArgMatches>::from_arg_matches_mut(&mut matches)
.map_err(format_error::<Self>);

match res {
Ok(s) => s,
Err(e) => {
// Since this is more of a development-time error, we aren't doing as fancy of a quit
// as `get_matches`
e.exit()
e.exit(texts)
}
}
}
Expand All @@ -48,20 +56,20 @@ pub trait Parser: FromArgMatches + CommandFactory + Sized {
}

/// Parse from iterator, [exit][Error::exit] on error.
fn parse_from<I, T>(itr: I) -> Self
fn parse_from<I, T>(itr: I, texts: &impl TextProvider) -> Self
where
I: IntoIterator<Item = T>,
T: Into<OsString> + Clone,
{
let mut matches = <Self as CommandFactory>::command().get_matches_from(itr);
let mut matches = <Self as CommandFactory>::command().get_matches_from(itr, texts);
let res = <Self as FromArgMatches>::from_arg_matches_mut(&mut matches)
.map_err(format_error::<Self>);
match res {
Ok(s) => s,
Err(e) => {
// Since this is more of a development-time error, we aren't doing as fancy of a quit
// as `get_matches_from`
e.exit()
e.exit(texts)
}
}
}
Expand All @@ -81,18 +89,18 @@ pub trait Parser: FromArgMatches + CommandFactory + Sized {
/// Unlike [`Parser::parse`], this works with an existing instance of `self`.
/// The assumption is that all required fields are already provided and any [`Args`] or
/// [`Subcommand`]s provided by the user will modify only what is specified.
fn update_from<I, T>(&mut self, itr: I)
fn update_from<I, T>(&mut self, itr: I, texts: &impl TextProvider)
where
I: IntoIterator<Item = T>,
T: Into<OsString> + Clone,
{
let mut matches = <Self as CommandFactory>::command_for_update().get_matches_from(itr);
let mut matches = <Self as CommandFactory>::command_for_update().get_matches_from(itr, texts);
let res = <Self as FromArgMatches>::update_from_arg_matches_mut(self, &mut matches)
.map_err(format_error::<Self>);
if let Err(e) = res {
// Since this is more of a development-time error, we aren't doing as fancy of a quit
// as `get_matches_from`
e.exit()
e.exit(texts)
}
}

Expand Down Expand Up @@ -321,12 +329,12 @@ impl<T: Parser> Parser for Box<T> {
<T as Parser>::try_parse().map(Box::new)
}

fn parse_from<I, It>(itr: I) -> Self
fn parse_from<I, It>(itr: I, texts: &impl TextProvider) -> Self
where
I: IntoIterator<Item = It>,
It: Into<OsString> + Clone,
{
Box::new(<T as Parser>::parse_from(itr))
Box::new(<T as Parser>::parse_from(itr, texts))
}

fn try_parse_from<I, It>(itr: I) -> Result<Self, Error>
Expand Down
Loading