-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
base: master
Are you sure you want to change the base?
Changes from all commits
db6705e
75af6df
a005f66
70f6629
fa3e506
d2435e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
#![allow(elided_lifetimes_in_paths)] // needed for divan | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'll see a lot of cases of |
||
use clap::{arg, ArgMatches, Command}; | ||
use clap::{arg, text_provider::DEFAULT_TEXT_PROVIDER, ArgMatches, Command}; | ||
|
||
macro_rules! create_app { | ||
() => {{ | ||
|
@@ -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] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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}; | ||
|
@@ -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. | ||
|
@@ -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. | ||
|
@@ -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) | ||
}) | ||
} | ||
|
||
|
@@ -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); | ||
|
@@ -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() | ||
} | ||
|
@@ -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() | ||
} | ||
|
@@ -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 { | ||
|
@@ -4729,7 +4734,7 @@ impl Command { | |
.help("Print help (see more with '--help')") | ||
.long_help("Print help (see a summary with '-h')"); | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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` | ||
|
@@ -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 | ||
|
@@ -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)"), | ||
) | ||
}; | ||
|
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) | ||
|
@@ -29,6 +31,11 @@ impl StyledStr { | |
Self(String::new()) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 + '_ { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
} | ||
} | ||
} | ||
|
@@ -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) | ||
} | ||
} | ||
} | ||
|
@@ -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) | ||
} | ||
} | ||
|
||
|
@@ -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> | ||
|
There was a problem hiding this comment.
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.