From 7169c47fede202831e972a7939452c5ecca4bfd6 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 8 Oct 2021 12:55:56 -0400 Subject: [PATCH 1/8] imp(version flag): no longer generates a version unconditionally This commit changes the default behavior of clap to no longer generate a `-V, --version` flag when no version information has been provided. Version information can be provided via `App::version`, `App::long_version`, or via `App::mut_arg("version", |_| ..)`. Using any of the above is the only way to have clap auto-generate the version flag. Technically, clap still generates a version flag, however it is removed prior to parsing if the user has not provided any version information via one of the mentioned methods. Relates to [#2812](https://github.com/clap-rs/clap/issues/2812) --- src/build/app/mod.rs | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 7698be3d..d7900830 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -2395,18 +2395,13 @@ impl<'help> App<'help> { // We have to create a new scope in order to tell rustc the borrow of `sc` is // done and to recursively call this method { - if $_self - .settings - .is_set(AppSettings::DisableVersionForSubcommands) - { - $sc.set(AppSettings::DisableVersionFlag); - } - - if $_self.settings.is_set(AppSettings::PropagateVersion) - && $sc.version.is_none() - && $_self.version.is_some() - { - $sc.version = Some($_self.version.unwrap()); + if $_self.settings.is_set(AppSettings::PropagateVersion) { + if $sc.version.is_none() && $_self.version.is_some() { + $sc.version = Some($_self.version.unwrap()); + } + if $sc.long_version.is_none() && $_self.long_version.is_some() { + $sc.long_version = Some($_self.long_version.unwrap()); + } } $sc.settings = $sc.settings | $_self.g_settings; @@ -2464,7 +2459,14 @@ impl<'help> App<'help> { } } - if self.is_set(AppSettings::DisableVersionFlag) + // Determine if we should remove the generated --version flag + // + // Note that if only mut_arg() was used, the first expression will evaluate to `true` + // however inside the condition block, we only check for Generated args, not + // GeneratedMutated args, so the `mut_arg("version", ..) will be skipped and fall through + // to the following condition below (Adding the short `-V`) + if self.settings.is_set(AppSettings::DisableVersionFlag) + || (self.version.is_none() && self.long_version.is_none()) || self.args.args().any(|x| { x.provider == ArgProvider::User && (x.long == Some("version") || x.id == Id::version_hash()) @@ -2476,6 +2478,8 @@ impl<'help> App<'help> { { debug!("App::_check_help_and_version: Removing generated version"); + // This is the check mentioned above that only checks for Generated, not + // GeneratedMuated args by design. let generated_version_pos = self .args .args() @@ -2484,7 +2488,16 @@ impl<'help> App<'help> { if let Some(index) = generated_version_pos { self.args.remove(index); } - } else { + } + + // If we still have a generated --version flag, determine if we can apply the short `-V` + if self.args.args().any(|x| { + x.id == Id::version_hash() + && matches!( + x.provider, + ArgProvider::Generated | ArgProvider::GeneratedMutated + ) + }) { let other_arg_has_short = self.args.args().any(|x| x.short == Some('V')); let version = self .args @@ -2520,6 +2533,7 @@ impl<'help> App<'help> { .args_mut() .filter(|a| !a.is_positional()) .filter(|a| a.disp_ord == 999) + .filter(|a| a.provider != ArgProvider::Generated) .enumerate() { a.disp_ord = i; From 0addd938a660d7b806b1c1145dd5a97a962a50cc Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 8 Oct 2021 13:04:24 -0400 Subject: [PATCH 2/8] tests(Examples): fixes examples for no-auto generated version --- clap_derive/examples/value_hints_derive.rs | 1 - clap_generate/examples/value_hints.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/clap_derive/examples/value_hints_derive.rs b/clap_derive/examples/value_hints_derive.rs index d0be2c00..4815ecad 100644 --- a/clap_derive/examples/value_hints_derive.rs +++ b/clap_derive/examples/value_hints_derive.rs @@ -34,7 +34,6 @@ enum GeneratorChoice { name = "value_hints_derive", // AppSettings::TrailingVarArg is required to use ValueHint::CommandWithArguments setting = AppSettings::TrailingVarArg, - setting = AppSettings::DisableVersionFlag )] struct Opt { /// If provided, outputs the completion file for given shell diff --git a/clap_generate/examples/value_hints.rs b/clap_generate/examples/value_hints.rs index 43113883..a3fdba64 100644 --- a/clap_generate/examples/value_hints.rs +++ b/clap_generate/examples/value_hints.rs @@ -19,7 +19,6 @@ use std::io; fn build_cli() -> App<'static> { App::new("value_hints") - .setting(AppSettings::DisableVersionFlag) // AppSettings::TrailingVarArg is required to use ValueHint::CommandWithArguments .setting(AppSettings::TrailingVarArg) .arg(Arg::new("generator").long("generate").possible_values([ From 14c8850019d9021efe5a343841a34bce092cc0d6 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 8 Oct 2021 13:05:40 -0400 Subject: [PATCH 3/8] tests: fixes test to new default behavior This commit corrects tests to not expect the `-V,--version` flag when no version information has been provided. --- clap_generate/src/generators/mod.rs | 1 + clap_generate/tests/completions/bash.rs | 2 + clap_generate/tests/completions/elvish.rs | 2 + clap_generate/tests/completions/fish.rs | 3 + clap_generate/tests/completions/powershell.rs | 2 + clap_generate/tests/completions/zsh.rs | 7 +- clap_generate/tests/value_hints.rs | 1 - src/parse/errors.rs | 1 + tests/app_settings.rs | 70 ++- tests/cargo.rs | 6 +- tests/derive_order.rs | 10 +- tests/display_order.rs | 3 +- tests/double_require.rs | 9 +- tests/flag_subcommands.rs | 9 +- tests/groups.rs | 3 +- tests/help.rs | 86 ++-- tests/hidden_args.rs | 46 +- tests/macros.rs | 1 - tests/version.rs | 417 ++++++++---------- 19 files changed, 322 insertions(+), 357 deletions(-) diff --git a/clap_generate/src/generators/mod.rs b/clap_generate/src/generators/mod.rs index 53f14cf0..010626bd 100644 --- a/clap_generate/src/generators/mod.rs +++ b/clap_generate/src/generators/mod.rs @@ -193,6 +193,7 @@ mod tests { fn common() -> App<'static> { let mut app = App::new("myapp") + .version("3.0") .subcommand( App::new("test").subcommand(App::new("config")).arg( Arg::new("file") diff --git a/clap_generate/tests/completions/bash.rs b/clap_generate/tests/completions/bash.rs index 29db63a5..d3c2f79c 100644 --- a/clap_generate/tests/completions/bash.rs +++ b/clap_generate/tests/completions/bash.rs @@ -6,6 +6,7 @@ fn build_app() -> App<'static> { fn build_app_with_name(s: &'static str) -> App<'static> { App::new(s) + .version("3.0") .about("Tests completions") .arg( Arg::new("file") @@ -261,6 +262,7 @@ fn bash_with_aliases() { fn build_app_with_aliases() -> App<'static> { App::new("cmd") + .version("3.0") .about("testing bash completions") .arg( Arg::new("flag") diff --git a/clap_generate/tests/completions/elvish.rs b/clap_generate/tests/completions/elvish.rs index f93d4588..ace58f60 100644 --- a/clap_generate/tests/completions/elvish.rs +++ b/clap_generate/tests/completions/elvish.rs @@ -6,6 +6,7 @@ fn build_app() -> App<'static> { fn build_app_with_name(s: &'static str) -> App<'static> { App::new(s) + .version("3.0") .about("Tests completions") .arg( Arg::new("file") @@ -160,6 +161,7 @@ fn elvish_with_aliases() { fn build_app_with_aliases() -> App<'static> { App::new("cmd") + .version("3.0") .about("testing bash completions") .arg( Arg::new("flag") diff --git a/clap_generate/tests/completions/fish.rs b/clap_generate/tests/completions/fish.rs index 0d9aa792..50b0037a 100644 --- a/clap_generate/tests/completions/fish.rs +++ b/clap_generate/tests/completions/fish.rs @@ -6,6 +6,7 @@ fn build_app() -> App<'static> { fn build_app_with_name(s: &'static str) -> App<'static> { App::new(s) + .version("3.0") .about("Tests completions") .arg( Arg::new("file") @@ -84,6 +85,7 @@ fn fish_with_special_help() { fn build_app_special_help() -> App<'static> { App::new("my_app") + .version("3.0") .arg( Arg::new("single-quotes") .long("single-quotes") @@ -130,6 +132,7 @@ fn fish_with_aliases() { fn build_app_with_aliases() -> App<'static> { App::new("cmd") + .version("3.0") .about("testing bash completions") .arg( Arg::new("flag") diff --git a/clap_generate/tests/completions/powershell.rs b/clap_generate/tests/completions/powershell.rs index 1d3fbfb6..011d2ce4 100644 --- a/clap_generate/tests/completions/powershell.rs +++ b/clap_generate/tests/completions/powershell.rs @@ -6,6 +6,7 @@ fn build_app() -> App<'static> { fn build_app_with_name(s: &'static str) -> App<'static> { App::new(s) + .version("3.0") .about("Tests completions") .arg( Arg::new("file") @@ -192,6 +193,7 @@ fn powershell_with_aliases() { fn build_app_with_aliases() -> App<'static> { App::new("cmd") + .version("3.0") .about("testing bash completions") .arg( Arg::new("flag") diff --git a/clap_generate/tests/completions/zsh.rs b/clap_generate/tests/completions/zsh.rs index a17974c7..d9fdc1b6 100644 --- a/clap_generate/tests/completions/zsh.rs +++ b/clap_generate/tests/completions/zsh.rs @@ -6,6 +6,7 @@ fn build_app() -> App<'static> { fn build_app_with_name(s: &'static str) -> App<'static> { App::new(s) + .version("3.0") .about("Tests completions") .arg( Arg::new("file") @@ -250,6 +251,7 @@ fn zsh_with_special_help() { fn build_app_special_help() -> App<'static> { App::new("my_app") + .version("3.0") .arg( Arg::new("single-quotes") .long("single-quotes") @@ -324,7 +326,9 @@ fn zsh_with_nested_subcommands() { } fn build_app_nested_subcommands() -> App<'static> { - App::new("first").subcommand(App::new("second").subcommand(App::new("third"))) + App::new("first") + .version("3.0") + .subcommand(App::new("second").subcommand(App::new("third"))) } static ZSH_NESTED_SUBCOMMANDS: &str = r#"#compdef my_app @@ -431,6 +435,7 @@ fn zsh_with_aliases() { fn build_app_with_aliases() -> App<'static> { App::new("cmd") + .version("3.0") .about("testing bash completions") .arg( Arg::new("flag") diff --git a/clap_generate/tests/value_hints.rs b/clap_generate/tests/value_hints.rs index 12b6f949..ae452c9e 100644 --- a/clap_generate/tests/value_hints.rs +++ b/clap_generate/tests/value_hints.rs @@ -6,7 +6,6 @@ mod completions; pub fn build_app_with_value_hints() -> App<'static> { App::new("my_app") - .setting(AppSettings::DisableVersionFlag) .setting(AppSettings::TrailingVarArg) .arg( Arg::new("choice") diff --git a/src/parse/errors.rs b/src/parse/errors.rs index f0585dc4..0b98255b 100644 --- a/src/parse/errors.rs +++ b/src/parse/errors.rs @@ -394,6 +394,7 @@ pub enum ErrorKind { /// ```rust /// # use clap::{App, Arg, ErrorKind}; /// let result = App::new("prog") + /// .version("3.0") /// .try_get_matches_from(vec!["prog", "--version"]); /// assert!(result.is_err()); /// assert_eq!(result.unwrap_err().kind, ErrorKind::DisplayVersion); diff --git a/tests/app_settings.rs b/tests/app_settings.rs index 5468ac8b..8fae6107 100644 --- a/tests/app_settings.rs +++ b/tests/app_settings.rs @@ -116,9 +116,6 @@ ARGS: FLAGS: -h, --help Print help information - - -V, --version - Print version information "; static LONG_FORMAT_FOR_NESTED_HELP_SUBCOMMAND: &str = "myprog-test-nested @@ -131,12 +128,9 @@ USAGE: FLAGS: -h, --help Print help information - - -V, --version - Print version information "; -static LONG_FORMAT_SINGLE_ARG_HELP_SUBCOMMAND: &str = "myprog +static LONG_FORMAT_SINGLE_ARG_HELP_SUBCOMMAND: &str = "myprog 1.0 USAGE: myprog [foo] [SUBCOMMAND] @@ -1167,6 +1161,7 @@ fn nested_help_subcommand_with_global_setting() { #[test] fn single_arg_help_with_long_format_setting() { let m = App::new("myprog") + .version("1.0") .setting(AppSettings::UseLongFormatForHelpSubcommand) .subcommand(App::new("test")) .arg( @@ -1200,3 +1195,64 @@ fn use_long_format_for_help_subcommand_with_setting() { false )); } + +#[test] +fn no_auto_help() { + let app = App::new("myprog") + .setting(AppSettings::NoAutoHelp) + .subcommand(App::new("foo")); + + let result = app.clone().try_get_matches_from("myprog --help".split(" ")); + + assert!(result.is_ok()); + assert!(result.unwrap().is_present("help")); + + let result = app.clone().try_get_matches_from("myprog -h".split(" ")); + + assert!(result.is_ok()); + assert!(result.unwrap().is_present("help")); + + let result = app.clone().try_get_matches_from("myprog help".split(" ")); + + assert!(result.is_ok()); + assert_eq!(result.unwrap().subcommand_name(), Some("help")); +} + +#[test] +fn no_auto_version() { + let app = App::new("myprog") + .version("3.0") + .setting(AppSettings::NoAutoVersion); + + let result = app + .clone() + .try_get_matches_from("myprog --version".split(" ")); + + assert!(result.is_ok()); + assert!(result.unwrap().is_present("version")); + + let result = app.clone().try_get_matches_from("myprog -V".split(" ")); + + assert!(result.is_ok()); + assert!(result.unwrap().is_present("version")); +} + +#[test] +fn no_auto_version_mut_arg() { + let app = App::new("myprog") + .version("3.0") + .mut_arg("version", |v| v.about("custom about")) + .setting(AppSettings::NoAutoVersion); + + let result = app + .clone() + .try_get_matches_from("myprog --version".split(" ")); + + assert!(result.is_ok()); + assert!(result.unwrap().is_present("version")); + + let result = app.clone().try_get_matches_from("myprog -V".split(" ")); + + assert!(result.is_ok()); + assert!(result.unwrap().is_present("version")); +} diff --git a/tests/cargo.rs b/tests/cargo.rs index d5a1d8ec..1b50eab1 100644 --- a/tests/cargo.rs +++ b/tests/cargo.rs @@ -67,10 +67,12 @@ fn crate_authors() { #[test] fn crate_name() { - let res = App::new(crate_name!()).try_get_matches_from(vec!["clap", "--version"]); + let res = App::new(crate_name!()) + .version("3.0") + .try_get_matches_from(vec!["clap", "--version"]); assert!(res.is_err()); let err = res.unwrap_err(); assert_eq!(err.kind, ErrorKind::DisplayVersion); - assert_eq!(err.to_string(), "clap \n"); + assert_eq!(err.to_string(), "clap 3.0\n"); } diff --git a/tests/derive_order.rs b/tests/derive_order.rs index 6207e620..9f9220b5 100644 --- a/tests/derive_order.rs +++ b/tests/derive_order.rs @@ -26,10 +26,10 @@ USAGE: test [FLAGS] [OPTIONS] FLAGS: - -h, --help Print help information - -V, --version Print version information --flag_b first flag --flag_a second flag + -h, --help Print help information + -V, --version Print version information OPTIONS: --option_b first option @@ -56,12 +56,12 @@ USAGE: test [OPTIONS] OPTIONS: - -h, --help Print help information - -V, --version Print version information --flag_b first flag --option_b first option --flag_a second flag --option_a second option + -h, --help Print help information + -V, --version Print version information "; static DERIVE_ORDER_SC_PROP: &str = "test-sub 1.2 @@ -128,10 +128,10 @@ USAGE: test [FLAGS] FLAGS: - -V, --version Print version information -h, --help Print help message --flag_b first flag --flag_a second flag + -V, --version Print version information "; static PREFER_USER_HELP_SUBCMD_DERIVE_ORDER: &str = "test-sub 1.2 diff --git a/tests/display_order.rs b/tests/display_order.rs index 3d778350..001d1cf2 100644 --- a/tests/display_order.rs +++ b/tests/display_order.rs @@ -15,8 +15,7 @@ USAGE: test [SUBCOMMAND] FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information SUBCOMMANDS: help Print this message or the help of the given subcommand(s) diff --git a/tests/double_require.rs b/tests/double_require.rs index 239fff54..eecba346 100644 --- a/tests/double_require.rs +++ b/tests/double_require.rs @@ -6,11 +6,10 @@ USAGE: prog [FLAGS] FLAGS: - -a - -b - -c - -h, --help Print help information - -V, --version Print version information + -a + -b + -c + -h, --help Print help information "; static ONLY_B_ERROR: &str = "error: The following required arguments were not provided: diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index b366d154..8c6b694f 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -431,8 +431,7 @@ USAGE: pacman {query, --query, -Q} [OPTIONS] FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information OPTIONS: -i, --info ... view package information @@ -489,8 +488,7 @@ USAGE: pacman {query, --query} [OPTIONS] FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information OPTIONS: -i, --info ... view package information @@ -546,8 +544,7 @@ USAGE: pacman {query, -Q} [OPTIONS] FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information OPTIONS: -i, --info ... view package information diff --git a/tests/groups.rs b/tests/groups.rs index 0e7ea4b0..d2854124 100644 --- a/tests/groups.rs +++ b/tests/groups.rs @@ -266,8 +266,7 @@ ARGS: FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information "; let app = App::new("prog") .arg(Arg::new("a").value_name("A")) diff --git a/tests/help.rs b/tests/help.rs index 7dbe83c8..d1db65b7 100644 --- a/tests/help.rs +++ b/tests/help.rs @@ -403,9 +403,6 @@ FLAGS: -h, --help Print help information - - -V, --version - Print version information "; static HELP_CONFLICT: &str = "conflict @@ -414,9 +411,8 @@ USAGE: conflict [FLAGS] FLAGS: - -h - --help Print help information - -V, --version Print version information + -h + --help Print help information "; static LAST_ARG: &str = "last 0.1 @@ -612,8 +608,7 @@ ARGS: FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information "; static ISSUE_1364: &str = "demo @@ -625,9 +620,8 @@ ARGS: ... FLAGS: - -f - -h, --help Print help information - -V, --version Print version information + -f + -h, --help Print help information "; static OPTION_USAGE_ORDER: &str = "order @@ -643,7 +637,6 @@ FLAGS: -s --select_file --select_folder - -V, --version Print version information -x "; @@ -919,9 +912,6 @@ FLAGS: --no-git-push Do not push generated commit and tags to git remote - - -V, --version - Print version information "; let app = App::new("test") .term_width(67) @@ -971,7 +961,6 @@ FLAGS: --no-git-commit Do not commit version changes --no-git-push Do not push generated commit and tags to git remote - -V, --version Print version information "; let app = App::new("test") .term_width(68) @@ -1335,8 +1324,7 @@ USAGE: hello [OPTIONS] FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information OPTIONS: -p, --package @@ -2267,7 +2255,6 @@ fn option_usage_order() { fn about_in_subcommands_list() { let app = App::new("about-in-subcommands-list") .setting(AppSettings::ColorNever) - .global_setting(AppSettings::DisableVersionFlag) .subcommand( App::new("sub") .long_about("long about sub") @@ -2296,7 +2283,6 @@ ARGS: FLAGS: -h, --help Print help information --option1 - -V, --version Print version information "; let app = clap::App::new("hello") @@ -2359,7 +2345,8 @@ USAGE: test FLAGS: - -h, --help Print help information + -h, --help Print help information + -V, --version Print version information NETWORKING: -s, --speed How fast @@ -2369,7 +2356,6 @@ NETWORKING: fn only_custom_heading_opts() { let app = App::new("test") .version("1.4") - .setting(AppSettings::DisableVersionFlag) .help_heading("NETWORKING") .arg(Arg::from("-s, --speed [SPEED] 'How fast'")); @@ -2390,7 +2376,8 @@ ARGS: Which gear FLAGS: - -h, --help Print help information + -h, --help Print help information + -V, --version Print version information NETWORKING: How fast @@ -2400,7 +2387,6 @@ NETWORKING: fn custom_heading_pos() { let app = App::new("test") .version("1.4") - .setting(AppSettings::DisableVersionFlag) .arg(Arg::new("gear").about("Which gear")) .help_heading("NETWORKING") .arg(Arg::new("speed").about("How fast")); @@ -2419,7 +2405,8 @@ USAGE: test [speed] FLAGS: - -h, --help Print help information + -h, --help Print help information + -V, --version Print version information NETWORKING: How fast @@ -2429,7 +2416,6 @@ NETWORKING: fn only_custom_heading_pos() { let app = App::new("test") .version("1.4") - .setting(AppSettings::DisableVersionFlag) .help_heading("NETWORKING") .arg(Arg::new("speed").about("How fast")); @@ -2538,8 +2524,7 @@ USAGE: my_app [OPTIONS] FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information OPTIONS: --some_arg @@ -2568,8 +2553,7 @@ ARGS: FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information ", false )); @@ -2596,8 +2580,7 @@ ARGS: ... FLAGS: - -h, --help Print help information - -V, --version Print version information + -h, --help Print help information ", false )); @@ -2605,38 +2588,27 @@ FLAGS: #[test] fn disabled_help_flag() { - let app = App::new("foo") + let res = App::new("foo") .subcommand(App::new("sub")) - .setting(AppSettings::DisableHelpFlag); - assert!(utils::compare_output( - app, - "foo a", - "error: Found argument 'a' which wasn't expected, or isn't valid in this context - -USAGE: - foo [SUBCOMMAND] - -For more information try help -", - true - )); + .setting(AppSettings::DisableHelpFlag) + .try_get_matches_from("foo a".split(" ")); + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::UnrecognizedSubcommand); + assert_eq!(err.info, &["a"]); } #[test] fn disabled_help_flag_and_subcommand() { - let app = App::new("foo") + let res = App::new("foo") .subcommand(App::new("sub")) .setting(AppSettings::DisableHelpFlag) - .setting(AppSettings::DisableHelpSubcommand); - assert!(utils::compare_output( - app, - "foo help", - "error: Found argument 'help' which wasn't expected, or isn't valid in this context - -USAGE: - foo [SUBCOMMAND]", - true - )); + .setting(AppSettings::DisableHelpSubcommand) + .try_get_matches_from("foo help".split(" ")); + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::UnrecognizedSubcommand); + assert_eq!(err.info, &["help"]); } #[test] diff --git a/tests/hidden_args.rs b/tests/hidden_args.rs index bd3f8427..7c2a1b45 100644 --- a/tests/hidden_args.rs +++ b/tests/hidden_args.rs @@ -233,8 +233,8 @@ OPTIONS: fn hidden_flag_args() { let app = App::new("test") .version("1.4") - .setting(AppSettings::DisableVersionFlag) .mut_arg("help", |a| a.hidden(true)) + .mut_arg("version", |a| a.hidden(true)) .args(&[Arg::from("--option [opt] 'some option'")]); assert!(utils::compare_output( @@ -251,19 +251,17 @@ USAGE: test [FLAGS] FLAGS: - --flag some flag - -h, --help Print help information + --flag some flag + -h, --help Print help information + -V, --version Print version information "; #[test] fn hidden_opt_args() { - let app = App::new("test") - .version("1.4") - .setting(AppSettings::DisableVersionFlag) - .args(&[ - Arg::from("--flag 'some flag'"), - Arg::from("--option [opt] 'some option'").hidden(true), - ]); + let app = App::new("test").version("1.4").args(&[ + Arg::from("--flag 'some flag'"), + Arg::from("--option [opt] 'some option'").hidden(true), + ]); assert!(utils::compare_output( app, @@ -282,18 +280,16 @@ ARGS: another pos FLAGS: - -h, --help Print help information + -h, --help Print help information + -V, --version Print version information "; #[test] fn hidden_pos_args() { - let app = App::new("test") - .version("1.4") - .setting(AppSettings::DisableVersionFlag) - .args(&[ - Arg::new("pos").about("some pos").hidden(true), - Arg::new("another").about("another pos"), - ]); + let app = App::new("test").version("1.4").args(&[ + Arg::new("pos").about("some pos").hidden(true), + Arg::new("another").about("another pos"), + ]); assert!(utils::compare_output( app, @@ -309,14 +305,14 @@ USAGE: test FLAGS: - -h, --help Print help information + -h, --help Print help information + -V, --version Print version information "; #[test] fn hidden_subcmds() { let app = App::new("test") .version("1.4") - .setting(AppSettings::DisableVersionFlag) .subcommand(App::new("sub").setting(AppSettings::Hidden)); assert!(utils::compare_output( @@ -339,9 +335,9 @@ After help fn hidden_flag_args_only() { let app = App::new("test") .version("1.4") - .setting(AppSettings::DisableVersionFlag) .after_help("After help") - .mut_arg("help", |a| a.hidden(true)); + .mut_arg("help", |a| a.hidden(true)) + .mut_arg("version", |a| a.hidden(true)); assert!(utils::compare_output( app, @@ -363,9 +359,9 @@ After help fn hidden_opt_args_only() { let app = App::new("test") .version("1.4") - .setting(AppSettings::DisableVersionFlag) .after_help("After help") .mut_arg("help", |a| a.hidden(true)) + .mut_arg("version", |a| a.hidden(true)) .args(&[Arg::from("--option [opt] 'some option'").hidden(true)]); assert!(utils::compare_output( @@ -388,9 +384,9 @@ After help fn hidden_pos_args_only() { let app = App::new("test") .version("1.4") - .setting(AppSettings::DisableVersionFlag) .after_help("After help") .mut_arg("help", |a| a.hidden(true)) + .mut_arg("version", |a| a.hidden(true)) .args(&[Arg::new("pos").about("some pos").hidden(true)]); assert!(utils::compare_output( @@ -413,9 +409,9 @@ After help fn hidden_subcmds_only() { let app = App::new("test") .version("1.4") - .setting(AppSettings::DisableVersionFlag) .after_help("After help") .mut_arg("help", |a| a.hidden(true)) + .mut_arg("version", |a| a.hidden(true)) .subcommand(App::new("sub").setting(AppSettings::Hidden)); assert!(utils::compare_output( diff --git a/tests/macros.rs b/tests/macros.rs index 982a6486..1ce22fb5 100644 --- a/tests/macros.rs +++ b/tests/macros.rs @@ -32,7 +32,6 @@ fn basic() { (about: "tests clap library") (author: "Kevin K. ") (@global_setting ColorNever) - (@setting DisableVersionForSubcommands) (@arg opt: -o --option +takes_value ... "tests options") (@arg positional: index(1) "tests positionals") (@arg flag: -f --flag ... +global "tests flags") diff --git a/tests/version.rs b/tests/version.rs index 7b1077e1..f7c4da51 100644 --- a/tests/version.rs +++ b/tests/version.rs @@ -4,216 +4,152 @@ use std::str; use clap::{App, AppSettings, Arg, ErrorKind}; -static VERSION: &str = "clap-test v1.4.8\n"; +fn common() -> App<'static> { + App::new("foo") +} + +fn with_version() -> App<'static> { + common().version("3.0") +} + +fn with_long_version() -> App<'static> { + common().long_version("3.0 (abcdefg)") +} + +fn with_subcommand() -> App<'static> { + with_version().subcommand(App::new("bar").subcommand(App::new("baz"))) +} #[test] -fn version_short() { - let m = App::new("test") - .author("Kevin K.") - .about("tests stuff") - .version("1.3") - .long_version("1.3 (abcdef12)") - .try_get_matches_from(vec!["myprog", "-V"]); +fn no_version_flag_short() { + let res = common().try_get_matches_from("foo -V".split(" ")); - assert!(m.is_err()); - let err = m.unwrap_err(); + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, clap::ErrorKind::UnknownArgument); + assert_eq!(err.info, ["-V"]); +} + +#[test] +fn no_version_flag_long() { + let res = common().try_get_matches_from("foo --version".split(" ")); + + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, clap::ErrorKind::UnknownArgument); + assert_eq!(err.info, ["--version"]); +} + +#[test] +fn version_flag_from_version_short() { + let res = with_version().try_get_matches_from("foo -V".split(" ")); + + assert!(res.is_err()); + let err = res.unwrap_err(); assert_eq!(err.kind, ErrorKind::DisplayVersion); - assert_eq!(err.to_string(), "test 1.3\n"); + assert_eq!(err.to_string(), "foo 3.0\n"); } #[test] -fn version_long() { - let m = App::new("test") - .author("Kevin K.") - .about("tests stuff") - .version("1.3") - .long_version("1.3 (abcdef12)") - .try_get_matches_from(vec!["myprog", "--version"]); +fn version_flag_from_version_long() { + let res = with_version().try_get_matches_from("foo --version".split(" ")); - assert!(m.is_err()); - let err = m.unwrap_err(); + assert!(res.is_err()); + let err = res.unwrap_err(); assert_eq!(err.kind, ErrorKind::DisplayVersion); - assert_eq!(err.to_string(), "test 1.3 (abcdef12)\n"); + assert_eq!(err.to_string(), "foo 3.0\n"); } #[test] -fn complex_version_output() { - let mut a = App::new("clap-test").version("v1.4.8"); - let _ = a.try_get_matches_from_mut(vec![""]); +fn version_flag_from_long_version_short() { + let res = with_long_version().try_get_matches_from("foo -V".split(" ")); - // Now we check the output of print_version() - assert_eq!(a.render_version(), VERSION); -} - -fn prefer_user_app() -> App<'static> { - App::new("test") - .version("1.3") - .arg( - Arg::new("version1") - .long("version") - .short('V') - .about("some version"), - ) - .subcommand( - App::new("foo").arg( - Arg::new("version1") - .long("version") - .short('V') - .about("some version"), - ), - ) + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::DisplayVersion); + assert_eq!(err.to_string(), "foo 3.0 (abcdefg)\n"); } #[test] -fn prefer_user_version_long() { - let m = prefer_user_app().try_get_matches_from(vec!["test", "--version"]); +fn version_flag_from_long_version_long() { + let res = with_long_version().try_get_matches_from("foo --version".split(" ")); - assert!(m.is_ok()); - assert!(m.unwrap().is_present("version1")); + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::DisplayVersion); + assert_eq!(err.to_string(), "foo 3.0 (abcdefg)\n"); } #[test] -fn prefer_user_version_short() { - let m = prefer_user_app().try_get_matches_from(vec!["test", "-V"]); +fn override_version_long_with_user_flag() { + let res = with_version() + .arg(Arg::new("ver").long("version")) + .try_get_matches_from("foo --version".split(" ")); - assert!(m.is_ok()); - assert!(m.unwrap().is_present("version1")); + assert!(res.is_ok()); + let m = res.unwrap(); + assert!(m.is_present("ver")); } #[test] -fn prefer_user_subcmd_version_long() { - let m = prefer_user_app().try_get_matches_from(vec!["test", "foo", "--version"]); +fn override_version_long_with_user_flag_no_version_flag() { + let res = with_version() + .arg(Arg::new("ver").long("version")) + .try_get_matches_from("foo -V".split(" ")); - assert!(m.is_ok()); - assert!(m - .unwrap() - .subcommand_matches("foo") - .unwrap() - .is_present("version1")); + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::UnknownArgument); } #[test] -fn prefer_user_subcmd_version_short() { - let m = prefer_user_app().try_get_matches_from(vec!["test", "foo", "-V"]); +fn override_version_short_with_user_flag() { + let res = with_version() + .arg(Arg::new("ver").short('V')) + .try_get_matches_from("foo -V".split(" ")); - assert!(m.is_ok()); - assert!(m - .unwrap() - .subcommand_matches("foo") - .unwrap() - .is_present("version1")); + assert!(res.is_ok()); + let m = res.unwrap(); + assert!(m.is_present("ver")); } -static OVERRIDE_VERSION_SHORT: &str = "test 1.3 +#[test] +fn override_version_short_with_user_flag_long_still_works() { + let res = with_version() + .arg(Arg::new("ver").short('V')) + .try_get_matches_from("foo --version".split(" ")); + + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::DisplayVersion); +} + +#[test] +fn mut_version_short() { + let res = with_version() + .mut_arg("version", |a| a.short('z')) + .try_get_matches_from("foo -z".split(" ")); + + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::DisplayVersion); +} + +#[test] +fn mut_version_long() { + let res = with_version() + .mut_arg("version", |a| a.long("qux")) + .try_get_matches_from("foo --qux".split(" ")); + + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::DisplayVersion); +} + +static VERSION_ABOUT_MULTI_SC: &str = "foo-bar-baz 3.0 USAGE: - test - -FLAGS: - -h, --help Print help information - -v, --version Print version information -"; - -#[test] -fn override_version_short() { - let app = App::new("test") - .version("1.3") - .mut_arg("version", |a| a.short('v')); - - let m = app.clone().try_get_matches_from(vec!["test", "-v"]); - - assert!(m.is_err()); - let err = m.unwrap_err(); - assert_eq!(err.kind, ErrorKind::DisplayVersion); - assert_eq!(err.to_string(), "test 1.3\n"); - - let m = app.clone().try_get_matches_from(vec!["test", "--version"]); - - assert!(m.is_err()); - let err = m.unwrap_err(); - assert_eq!(err.kind, ErrorKind::DisplayVersion); - assert_eq!(err.to_string(), "test 1.3\n"); - - assert!(utils::compare_output( - app, - "test -h", - OVERRIDE_VERSION_SHORT, - false - )); -} - -static OVERRIDE_VERSION_LONG: &str = "test 1.3 - -USAGE: - test [FLAGS] - -FLAGS: - -h, --help Print help information - -V, --vers Print version information -"; - -#[test] -fn override_version_long() { - let app = App::new("test") - .version("1.3") - .mut_arg("version", |a| a.long("vers")); - - let m = app.clone().try_get_matches_from(vec!["test", "-V"]); - - assert!(m.is_err()); - let err = m.unwrap_err(); - assert_eq!(err.kind, ErrorKind::DisplayVersion); - assert_eq!(err.to_string(), "test 1.3\n"); - - let m = app.clone().try_get_matches_from(vec!["test", "--vers"]); - - assert!(m.is_err()); - let err = m.unwrap_err(); - assert_eq!(err.kind, ErrorKind::DisplayVersion); - assert_eq!(err.to_string(), "test 1.3\n"); - - assert!(utils::compare_output( - app, - "test -h", - OVERRIDE_VERSION_LONG, - false - )); -} - -static OVERRIDE_VERSION_ABOUT: &str = "test 1.3 - -USAGE: - test - -FLAGS: - -h, --help Print help information - -V, --version version info -"; - -#[test] -fn override_version_about() { - let app = App::new("test") - .version("1.3") - .mut_arg("version", |a| a.about("version info")); - - assert!(utils::compare_output( - app.clone(), - "test -h", - OVERRIDE_VERSION_ABOUT, - false - )); - assert!(utils::compare_output( - app, - "test --help", - OVERRIDE_VERSION_ABOUT, - false - )); -} - -static VERSION_ABOUT_MULTI_SC: &str = "myapp-subcmd-multi 1.0 - -USAGE: - myapp subcmd multi + foo bar baz FLAGS: -h, --help Print help information @@ -222,92 +158,87 @@ FLAGS: #[test] fn version_about_multi_subcmd() { - let app = App::new("myapp") + let app = with_subcommand() .mut_arg("version", |a| a.about("Print custom version about text")) - .subcommand(App::new("subcmd").subcommand(App::new("multi").version("1.0"))); - - assert!(utils::compare_output( - app.clone(), - "myapp help subcmd multi", - VERSION_ABOUT_MULTI_SC, - false - )); - assert!(utils::compare_output( - app.clone(), - "myapp subcmd multi -h", - VERSION_ABOUT_MULTI_SC, - false - )); - assert!(utils::compare_output( - app, - "myapp subcmd multi --help", - VERSION_ABOUT_MULTI_SC, - false - )); -} - -static VERSION_ABOUT_MULTI_SC_OVERRIDE: &str = "myapp-subcmd-multi 1.0 - -USAGE: - myapp subcmd multi - -FLAGS: - -h, --help Print help information - -V, --version Print custom version about text from multi -"; - -#[test] -fn version_about_multi_subcmd_override() { - let app = App::new("myapp") - .mut_arg("version", |a| a.about("Print custom version about text")) - .subcommand(App::new("subcmd").subcommand( - App::new("multi").version("1.0").mut_arg("version", |a| { - a.about("Print custom version about text from multi") - }), - )); + .global_setting(AppSettings::PropagateVersion); assert!(utils::compare_output( app, - "myapp help subcmd multi", - VERSION_ABOUT_MULTI_SC_OVERRIDE, + "foo bar baz -h", + VERSION_ABOUT_MULTI_SC, false )); } #[test] -fn disabled_version_long() { - let app = App::new("foo").setting(AppSettings::DisableVersionFlag); - let res = app.try_get_matches_from(&["foo", "--version"]); +fn no_propagation_by_default_long() { + // Version Flag should not be propagated to subcommands + let res = with_subcommand().try_get_matches_from("foo bar --version".split(" ")); + assert!(res.is_err()); let err = res.unwrap_err(); - assert_eq!(err.kind, clap::ErrorKind::UnknownArgument); - assert_eq!(err.info, ["--version"]); + assert_eq!(err.kind, ErrorKind::UnknownArgument); + assert_eq!(err.info, &["--version"]); } #[test] -fn disabled_version_short() { - let app = App::new("foo").setting(AppSettings::DisableVersionFlag); - let res = app.try_get_matches_from(&["foo", "-V"]); +fn no_propagation_by_default_short() { + let res = with_subcommand().try_get_matches_from("foo bar -V".split(" ")); + assert!(res.is_err()); let err = res.unwrap_err(); - assert_eq!(err.kind, clap::ErrorKind::UnknownArgument); - assert_eq!(err.info, ["-V"]); + assert_eq!(err.kind, ErrorKind::UnknownArgument); + assert_eq!(err.info, &["-V"]); } #[test] -fn override_version_using_long() { - let app = App::new("foo") - .setting(AppSettings::DisableVersionFlag) - .arg(Arg::new("version").long("version")); - let matches = app.get_matches_from(&["foo", "--version"]); - assert!(matches.is_present("version")); +fn propagate_version_long() { + let res = with_subcommand() + .setting(AppSettings::PropagateVersion) + .try_get_matches_from("foo bar --version".split(" ")); + + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::DisplayVersion); } #[test] -fn override_version_using_short() { - let app = App::new("foo") - .arg(Arg::new("version").short('V')) - .setting(AppSettings::DisableVersionFlag); - let matches = app.get_matches_from(&["foo", "-V"]); - assert!(matches.is_present("version")); +fn propagate_version_short() { + let res = with_subcommand() + .setting(AppSettings::PropagateVersion) + .try_get_matches_from("foo bar -V".split(" ")); + + assert!(res.is_err()); + let err = res.unwrap_err(); + assert_eq!(err.kind, ErrorKind::DisplayVersion); +} + +#[cfg(debug_assertions)] +#[test] +#[should_panic = "Used App::mut_arg(\"version\", ..) without providing App::version, App::long_version or using AppSettings::NoAutoVersion"] +fn mut_arg_version_panic() { + let _res = common() + .mut_arg("version", |v| v.short('z')) + .try_get_matches_from("foo -z".split(" ")); +} + +#[test] +fn mut_arg_version_no_auto_version() { + let res = common() + .mut_arg("version", |v| v.short('z')) + .setting(AppSettings::NoAutoVersion) + .try_get_matches_from("foo -z".split(" ")); + + assert!(res.is_ok()); + assert!(res.unwrap().is_present("version")); +} + +#[cfg(debug_assertions)] +#[test] +#[should_panic = "No version information via App::version or App::long_version to propagate"] +fn propagate_version_no_version_info() { + let _res = common() + .setting(AppSettings::PropagateVersion) + .subcommand(App::new("bar")) + .try_get_matches_from("foo".split(" ")); } From 07aae5ac543751d7f8bd9ecca28870128611798e Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 8 Oct 2021 13:07:47 -0400 Subject: [PATCH 4/8] docs(AppSettings): documents NoAutoHelp and NoAutoVersion Since these tie in closely with the new default behavior of not auto-generating the `-V/--version` flags when no information has been provided they are now documented. --- src/build/app/settings.rs | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/build/app/settings.rs b/src/build/app/settings.rs index 48569b95..045d966b 100644 --- a/src/build/app/settings.rs +++ b/src/build/app/settings.rs @@ -1036,10 +1036,45 @@ pub enum AppSettings { /// ``` WaitOnError, - /// @TODO-v3: @docs write them...maybe rename + /// Tells clap to treat the auto-generated `-h, --help` flags just like any other flag, and + /// *not* print the help message. This allows one to handle printing of the help message + /// manually. + /// + /// ```rust + /// # use clap::{App, AppSettings}; + /// let result = App::new("myprog") + /// .setting(AppSettings::NoAutoHelp) + /// .try_get_matches_from("myprog --help".split(" ")); + /// + /// // Normally, if `--help` is used clap prints the help message and returns an + /// // ErrorKind::DisplayHelp + /// // + /// // However, `--help` was treated like a normal flag + /// + /// assert!(result.is_ok()); + /// assert!(result.unwrap().is_present("help")); + /// ``` NoAutoHelp, - /// @TODO-v3: @docs write them...maybe rename + /// Tells clap to treat the auto-generated `-V, --version` flags just like any other flag, and + /// *not* print the version message. This allows one to handle printing of the version message + /// manually. + /// + /// ```rust + /// # use clap::{App, AppSettings}; + /// let result = App::new("myprog") + /// .version("3.0") + /// .setting(AppSettings::NoAutoVersion) + /// .try_get_matches_from("myprog --version".split(" ")); + /// + /// // Normally, if `--version` is used clap prints the version message and returns an + /// // ErrorKind::DisplayVersion + /// // + /// // However, `--version` was treated like a normal flag + /// + /// assert!(result.is_ok()); + /// assert!(result.unwrap().is_present("version")); + /// ``` NoAutoVersion, #[doc(hidden)] From bb26ed1c8bdb500d92aeb8b843dcadc1962b6f42 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 8 Oct 2021 13:49:21 -0400 Subject: [PATCH 5/8] imp: adds debug_asserts against generating meaningless flags This commit adds several debug asserts that ensure the user has not accidentally generated a version flag that has no information. The exception to this case is when the user wants to generate a version flag, but then handle the version display manually. I.e. one can still generate a "meaningless" version flag, but use `AppSettings::NoAutoVersion` which is allowed. --- src/build/app/debug_asserts.rs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/build/app/debug_asserts.rs b/src/build/app/debug_asserts.rs index 38ac11ea..b451f66a 100644 --- a/src/build/app/debug_asserts.rs +++ b/src/build/app/debug_asserts.rs @@ -1,4 +1,8 @@ -use crate::{build::arg::debug_asserts::assert_arg, App, AppSettings, ArgSettings, ValueHint}; +use crate::{ + build::arg::{debug_asserts::assert_arg, ArgProvider}, + util::Id, + App, AppSettings, ArgSettings, ValueHint, +}; use std::cmp::Ordering; #[derive(Eq)] @@ -44,6 +48,26 @@ pub(crate) fn assert_app(app: &App) { let mut short_flags = vec![]; let mut long_flags = vec![]; + // Invalid version flag settings + if app.version.is_none() && app.long_version.is_none() { + // PropagateVersion is meaningless if there is no version + assert!( + !app.settings.is_set(AppSettings::PropagateVersion), + "No version information via App::version or App::long_version to propagate" + ); + + // Used `App::mut_arg("version", ..) but did not provide any version information to display + let has_mutated_version = app + .args + .args() + .any(|x| x.id == Id::version_hash() && x.provider == ArgProvider::GeneratedMutated); + + if has_mutated_version { + assert!(app.settings.is_set(AppSettings::NoAutoVersion), + "Used App::mut_arg(\"version\", ..) without providing App::version, App::long_version or using AppSettings::NoAutoVersion"); + } + } + for sc in &app.subcommands { if let Some(s) = sc.short_flag.as_ref() { short_flags.push(Flag::App(format!("-{}", s), &sc.name)); From 0eba8015e3268cacff0b184894106765160d3c36 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 8 Oct 2021 13:51:58 -0400 Subject: [PATCH 6/8] chore: silence warning in example code --- clap_derive/examples/example.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clap_derive/examples/example.rs b/clap_derive/examples/example.rs index d87172bc..f00ac41e 100644 --- a/clap_derive/examples/example.rs +++ b/clap_derive/examples/example.rs @@ -50,5 +50,5 @@ struct Opt { fn main() { let opt = Opt::parse(); - println!("{:?}", opt); + println!("{:?}", opt.skipped); } From 9fbe5841a7ff2d0b4fb3ec51dfeb206eaf6d2932 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 8 Oct 2021 14:20:40 -0400 Subject: [PATCH 7/8] chore: clippy lint fixes --- src/build/app/mod.rs | 27 +++++++++------------------ src/build/arg/mod.rs | 20 +++++--------------- 2 files changed, 14 insertions(+), 33 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index d7900830..0148bd65 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1179,9 +1179,7 @@ impl<'help> App<'help> { /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` pub fn short_flag_alias(mut self, name: char) -> Self { - if name == '-' { - panic!("short alias name cannot be `-`"); - } + assert!(!(name == '-'), "short alias name cannot be `-`"); self.short_flag_aliases.push((name, false)); self } @@ -1261,9 +1259,7 @@ impl<'help> App<'help> { /// ``` pub fn short_flag_aliases(mut self, names: &[char]) -> Self { for s in names { - if s == &'-' { - panic!("short alias name cannot be `-`"); - } + assert!(s != &'-', "short alias name cannot be `-`"); self.short_flag_aliases.push((*s, false)); } self @@ -1341,9 +1337,7 @@ impl<'help> App<'help> { /// ``` /// [`App::short_flag_alias`]: App::short_flag_alias() pub fn visible_short_flag_alias(mut self, name: char) -> Self { - if name == '-' { - panic!("short alias name cannot be `-`"); - } + assert!(name != '-', "short alias name cannot be `-`"); self.short_flag_aliases.push((name, true)); self } @@ -1414,9 +1408,7 @@ impl<'help> App<'help> { /// [`App::short_flag_aliases`]: App::short_flag_aliases() pub fn visible_short_flag_aliases(mut self, names: &[char]) -> Self { for s in names { - if s == &'-' { - panic!("short alias name cannot be `-`"); - } + assert!(!(s == &'-'), "short alias name cannot be `-`"); self.short_flag_aliases.push((*s, true)); } self @@ -2323,13 +2315,11 @@ impl<'help> App<'help> { .map(|arg| String::from(arg.name)) .collect(); - if !args_missing_help.is_empty() { - panic!( + assert!(args_missing_help.is_empty(), "AppSettings::HelpRequired is enabled for the App {}, but at least one of its arguments does not have either `help` or `long_help` set. List of such arguments: {}", self.name, args_missing_help.join(", ") ); - } } for sub_app in &self.subcommands { @@ -2661,9 +2651,10 @@ impl<'help> App<'help> { #[inline] pub(crate) fn contains_short(&self, s: char) -> bool { - if !self.is_set(AppSettings::Built) { - panic!("If App::_build hasn't been called, manually search through Arg shorts"); - } + assert!( + self.is_set(AppSettings::Built), + "If App::_build hasn't been called, manually search through Arg shorts" + ); self.args.contains(s) } diff --git a/src/build/arg/mod.rs b/src/build/arg/mod.rs index 064052f3..84ae2583 100644 --- a/src/build/arg/mod.rs +++ b/src/build/arg/mod.rs @@ -352,9 +352,7 @@ impl<'help> Arg<'help> { /// [`short`]: Arg::short() #[inline] pub fn short(mut self, s: char) -> Self { - if s == '-' { - panic!("short option name cannot be `-`"); - } + assert!(s != '-', "short option name cannot be `-`"); self.short = Some(s); self @@ -447,9 +445,7 @@ impl<'help> Arg<'help> { /// assert_eq!(m.value_of("test"), Some("cool")); /// ``` pub fn short_alias(mut self, name: char) -> Self { - if name == '-' { - panic!("short alias name cannot be `-`"); - } + assert!(name != '-', "short alias name cannot be `-`"); self.short_aliases.push((name, false)); self @@ -502,9 +498,7 @@ impl<'help> Arg<'help> { /// ``` pub fn short_aliases(mut self, names: &[char]) -> Self { for s in names { - if s == &'-' { - panic!("short alias name cannot be `-`"); - } + assert!(s != &'-', "short alias name cannot be `-`"); self.short_aliases.push((*s, false)); } self @@ -554,9 +548,7 @@ impl<'help> Arg<'help> { /// ``` /// [`App::alias`]: Arg::short_alias() pub fn visible_short_alias(mut self, name: char) -> Self { - if name == '-' { - panic!("short alias name cannot be `-`"); - } + assert!(name != '-', "short alias name cannot be `-`"); self.short_aliases.push((name, true)); self @@ -603,9 +595,7 @@ impl<'help> Arg<'help> { /// [`App::aliases`]: Arg::short_aliases() pub fn visible_short_aliases(mut self, names: &[char]) -> Self { for n in names { - if n == &'-' { - panic!("short alias name cannot be `-`"); - } + assert!(n != &'-', "short alias name cannot be `-`"); self.short_aliases.push((*n, true)); } self From 7b45695878eeb8ba3e815d10b30c080f33ac1cf0 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 8 Oct 2021 19:26:36 -0400 Subject: [PATCH 8/8] breaking(DisableVersionForSubcommands): removed This commit removes `AppSettings::DisableVersionForSubcommand` as it's now a moot setting with clap's default functionality of not building a version flag unless there actually exists version information. `clap_up` must still be changed to remove this variant instead of the current configuration to simply rename the variant. --- CHANGELOG.md | 3 +++ benches/06_rustup.rs | 6 ------ clap_up/src/lib.rs | 1 + src/build/app/settings.rs | 29 ----------------------------- 4 files changed, 4 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5271d3c..9d127ba7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ TODO: `YamlLoader` * **Renamed Features** * `unicode_help` to `unicode` to encompass more functionality +* **Removed Settings** + * `AppSettings::DisableVersionForSubcommands` + ## v3.0.0-beta.4 (2021-08-14) diff --git a/benches/06_rustup.rs b/benches/06_rustup.rs index bf48af2f..faf0a1cb 100644 --- a/benches/06_rustup.rs +++ b/benches/06_rustup.rs @@ -26,7 +26,6 @@ fn build_cli() -> App<'static> { .version("0.9.0") // Simulating .about("The Rust toolchain installer") .after_help(RUSTUP_HELP) - .setting(AppSettings::DisableVersionForSubcommands) .setting(AppSettings::DeriveDisplayOrder) // .setting(AppSettings::SubcommandRequiredElseHelp) .arg( @@ -107,7 +106,6 @@ fn build_cli() -> App<'static> { .subcommand( App::new("target") .about("Modify a toolchain's supported targets") - .setting(AppSettings::DisableVersionForSubcommands) .setting(AppSettings::DeriveDisplayOrder) // .setting(AppSettings::SubcommandRequiredElseHelp) .subcommand( @@ -163,7 +161,6 @@ fn build_cli() -> App<'static> { .subcommand( App::new("component") .about("Modify a toolchain's installed components") - .setting(AppSettings::DisableVersionForSubcommands) .setting(AppSettings::DeriveDisplayOrder) // .setting(AppSettings::SubcommandRequiredElseHelp) .subcommand( @@ -210,7 +207,6 @@ fn build_cli() -> App<'static> { App::new("override") .about("Modify directory toolchain overrides") .after_help(OVERRIDE_HELP) - .setting(AppSettings::DisableVersionForSubcommands) .setting(AppSettings::DeriveDisplayOrder) // .setting(AppSettings::SubcommandRequiredElseHelp) .subcommand(App::new("list").about("List directory toolchain overrides")) @@ -304,7 +300,6 @@ fn build_cli() -> App<'static> { .subcommand( App::new("self") .about("Modify the rustup installation") - .setting(AppSettings::DisableVersionForSubcommands) .setting(AppSettings::DeriveDisplayOrder) .subcommand(App::new("update").about("Download and install updates to rustup")) .subcommand( @@ -318,7 +313,6 @@ fn build_cli() -> App<'static> { App::new("telemetry") .about("rustup telemetry commands") .setting(AppSettings::Hidden) - .setting(AppSettings::DisableVersionForSubcommands) .setting(AppSettings::DeriveDisplayOrder) .subcommand(App::new("enable").about("Enable rustup telemetry")) .subcommand(App::new("disable").about("Disable rustup telemetry")) diff --git a/clap_up/src/lib.rs b/clap_up/src/lib.rs index ee87ec79..ffe45ae9 100644 --- a/clap_up/src/lib.rs +++ b/clap_up/src/lib.rs @@ -42,6 +42,7 @@ pub fn runner() -> Runner { &[ ["DisableHelpFlags", "DisableHelpFlag"], ["DisableVersion", "DisableVersionFlag"], + // @TODO @v3 shoud be removed, not renamed ["VersionlessSubcommands", "DisableVersionForSubcommands"], ], ) diff --git a/src/build/app/settings.rs b/src/build/app/settings.rs index 045d966b..5efd8c7a 100644 --- a/src/build/app/settings.rs +++ b/src/build/app/settings.rs @@ -130,8 +130,6 @@ impl_settings! { AppSettings, AppFlags, => Flags::NEXT_LINE_HELP, IgnoreErrors("ignoreerrors") => Flags::IGNORE_ERRORS, - DisableVersionForSubcommands("disableversionforsubcommands") - => Flags::DISABLE_VERSION_FOR_SC, WaitOnError("waitonerror") => Flags::WAIT_ON_ERROR, Built("built") @@ -653,27 +651,6 @@ pub enum AppSettings { /// ``` DisableVersionFlag, - /// Disables `-V` and `--version` for all [`subcommands`] of this [`App`]. - /// - /// # Examples - /// - /// ```rust - /// # use clap::{App, AppSettings, ErrorKind}; - /// let res = App::new("myprog") - /// .version("v1.1") - /// .setting(AppSettings::DisableVersionForSubcommands) - /// .subcommand(App::new("test")) - /// .try_get_matches_from(vec![ - /// "myprog", "test", "-V" - /// ]); - /// assert!(res.is_err()); - /// assert_eq!(res.unwrap_err().kind, ErrorKind::UnknownArgument); - /// ``` - /// - /// [`subcommands`]: crate::App::subcommand() - /// [`App`]: crate::App - DisableVersionForSubcommands, - /// Displays the arguments and [`subcommands`] in the help message in the order that they were /// declared in, and not alphabetically which is the default. /// @@ -1215,12 +1192,6 @@ mod test { "unifiedhelpmessage".parse::().unwrap(), AppSettings::UnifiedHelpMessage ); - assert_eq!( - "disableversionforsubcommands" - .parse::() - .unwrap(), - AppSettings::DisableVersionForSubcommands - ); assert_eq!( "waitonerror".parse::().unwrap(), AppSettings::WaitOnError