From 389ff4ff219c25756163bca22cbcb65ce8f9ab36 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 22 Jul 2022 14:58:27 -0500 Subject: [PATCH] fix(help): Subcommand display order respects `Command::next_display_order` Previous behavior: - They'd be sorted by default - They'd derive display order if `DeriveDisplayOrder` was set - This could be set recursively - The initial display order value for subcommands was 0 New behavior: - Sorted order is derived by default - Sorting is turned on by `cmd.next_display_order(None)` - This is not recursive, it must be set on each level - The display order incrementing is mixed with arguments - This does make it slightly more difficult to predict --- CHANGELOG.md | 1 + examples/derive_ref/interop_tests.md | 2 +- examples/git-derive.md | 12 ++-- examples/git.md | 12 ++-- examples/multicall-busybox.md | 2 +- examples/pacman.md | 2 +- examples/tutorial_builder/01_quick.md | 2 +- examples/tutorial_derive/01_quick.md | 2 +- src/builder/command.rs | 19 ++++-- tests/builder/derive_order.rs | 71 +++++++++++++++++++++ tests/builder/help.rs | 12 ++-- tests/builder/subcommands.rs | 70 +------------------- tests/builder/template_help.rs | 4 +- tests/ui/arg_required_else_help_stderr.toml | 2 +- tests/ui/h_flag_stdout.toml | 2 +- tests/ui/help_cmd_stdout.toml | 4 +- tests/ui/help_flag_stdout.toml | 4 +- 17 files changed, 117 insertions(+), 106 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87cc8daa..a960d86b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `ErrorKind::EmptyValue` replaced with `ErrorKind::InvalidValue` - `ErrorKind::UnrecognizedSubcommand` replaced with `ErrorKind::InvalidSubcommand` - `arg!` now sets `ArgAction::SetTrue`, `ArgAction::Count`, `ArgAction::Set`, or `ArgAction::Append` as appropriate +- *(help)* Subcommand display order respects `Command::next_display_order` instead of `DeriveDisplayOrder` and using its own initial display order value - *(env)* Parse `--help` and `--version` like any `ArgAction::SetTrue` flag - *(derive)* `subcommand_required(true).arg_required_else_help(true)` is set instead of `SubcommandRequiredElseHelp` diff --git a/examples/derive_ref/interop_tests.md b/examples/derive_ref/interop_tests.md index a455c535..6ec02d2e 100644 --- a/examples/derive_ref/interop_tests.md +++ b/examples/derive_ref/interop_tests.md @@ -112,8 +112,8 @@ OPTIONS: SUBCOMMANDS: add - help Print this message or the help of the given subcommand(s) remove + help Print this message or the help of the given subcommand(s) ``` diff --git a/examples/git-derive.md b/examples/git-derive.md index edf5849d..a0a57633 100644 --- a/examples/git-derive.md +++ b/examples/git-derive.md @@ -16,11 +16,11 @@ OPTIONS: -h, --help Print help information SUBCOMMANDS: - add adds things clone Clones repos - help Print this message or the help of the given subcommand(s) push pushes things + add adds things stash + help Print this message or the help of the given subcommand(s) $ git-derive help git @@ -33,11 +33,11 @@ OPTIONS: -h, --help Print help information SUBCOMMANDS: - add adds things clone Clones repos - help Print this message or the help of the given subcommand(s) push pushes things + add adds things stash + help Print this message or the help of the given subcommand(s) $ git-derive help add git-add @@ -89,10 +89,10 @@ OPTIONS: -m, --message SUBCOMMANDS: + push + pop apply help Print this message or the help of the given subcommand(s) - pop - push $ git-derive stash push -h git-stash-push diff --git a/examples/git.md b/examples/git.md index d836775f..951d260f 100644 --- a/examples/git.md +++ b/examples/git.md @@ -14,11 +14,11 @@ OPTIONS: -h, --help Print help information SUBCOMMANDS: - add adds things clone Clones repos - help Print this message or the help of the given subcommand(s) push pushes things + add adds things stash + help Print this message or the help of the given subcommand(s) $ git help git @@ -31,11 +31,11 @@ OPTIONS: -h, --help Print help information SUBCOMMANDS: - add adds things clone Clones repos - help Print this message or the help of the given subcommand(s) push pushes things + add adds things stash + help Print this message or the help of the given subcommand(s) $ git help add git-add @@ -87,10 +87,10 @@ OPTIONS: -m, --message SUBCOMMANDS: + push + pop apply help Print this message or the help of the given subcommand(s) - pop - push $ git stash push -h git-stash-push diff --git a/examples/multicall-busybox.md b/examples/multicall-busybox.md index 51b3afa8..cfa085ff 100644 --- a/examples/multicall-busybox.md +++ b/examples/multicall-busybox.md @@ -35,8 +35,8 @@ OPTIONS: --install Install hardlinks for all subcommands in path APPLETS: + true does nothing successfully false does nothing unsuccessfully help Print this message or the help of the given subcommand(s) - true does nothing successfully ``` diff --git a/examples/pacman.md b/examples/pacman.md index e81594df..5d55fb45 100644 --- a/examples/pacman.md +++ b/examples/pacman.md @@ -47,9 +47,9 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) query -Q --query Query the package database. sync -S --sync Synchronize packages. + help Print this message or the help of the given subcommand(s) $ pacman -S -h pacman-sync diff --git a/examples/tutorial_builder/01_quick.md b/examples/tutorial_builder/01_quick.md index 4a9e3fca..89180af5 100644 --- a/examples/tutorial_builder/01_quick.md +++ b/examples/tutorial_builder/01_quick.md @@ -16,8 +16,8 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test does testing things + help Print this message or the help of the given subcommand(s) ``` diff --git a/examples/tutorial_derive/01_quick.md b/examples/tutorial_derive/01_quick.md index 4a9e3fca..89180af5 100644 --- a/examples/tutorial_derive/01_quick.md +++ b/examples/tutorial_derive/01_quick.md @@ -16,8 +16,8 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test does testing things + help Print this message or the help of the given subcommand(s) ``` diff --git a/src/builder/command.rs b/src/builder/command.rs index 02bcf46c..f5e1e434 100644 --- a/src/builder/command.rs +++ b/src/builder/command.rs @@ -402,7 +402,13 @@ impl<'help> Command<'help> { #[inline] #[must_use] pub fn subcommand>(mut self, subcmd: S) -> Self { - self.subcommands.push(subcmd.into()); + let mut subcmd = subcmd.into(); + if let Some(current_disp_ord) = self.current_disp_ord.as_mut() { + let current = *current_disp_ord; + subcmd.disp_ord.get_or_insert(current); + *current_disp_ord = current + 1; + } + self.subcommands.push(subcmd); self } @@ -426,8 +432,12 @@ impl<'help> Command<'help> { I: IntoIterator, T: Into, { - for subcmd in subcmds.into_iter() { - self.subcommands.push(subcmd.into()); + let subcmds = subcmds.into_iter(); + let (lower, _) = subcmds.size_hint(); + self.subcommands.reserve(lower); + + for subcmd in subcmds { + self = self.subcommand(subcmd); } self } @@ -4378,9 +4388,6 @@ To change `help`s short, call `cmd.arg(Arg::new(\"help\")...)`.", { a.disp_ord.make_explicit(); } - for (i, sc) in &mut self.subcommands.iter_mut().enumerate() { - sc.disp_ord.get_or_insert(i); - } } for sc in &mut self.subcommands { sc._derive_display_order(); diff --git a/tests/builder/derive_order.rs b/tests/builder/derive_order.rs index 52778064..d35ef06e 100644 --- a/tests/builder/derive_order.rs +++ b/tests/builder/derive_order.rs @@ -291,3 +291,74 @@ fn prefer_user_help_in_subcommand_with_derive_order() { false, ); } + +#[test] +fn subcommand_sorted_display_order() { + static SUBCMD_ALPHA_ORDER: &str = "test 1 + +USAGE: + test [SUBCOMMAND] + +OPTIONS: + -h, --help Print help information + -V, --version Print version information + +SUBCOMMANDS: + a1 blah a1 + b1 blah b1 + help Print this message or the help of the given subcommand(s) +"; + + let app_subcmd_alpha_order = Command::new("test") + .version("1") + .next_display_order(None) + .subcommands(vec![ + Command::new("b1") + .about("blah b1") + .arg(Arg::new("test").short('t')), + Command::new("a1") + .about("blah a1") + .arg(Arg::new("roster").short('r')), + ]); + + utils::assert_output( + app_subcmd_alpha_order, + "test --help", + SUBCMD_ALPHA_ORDER, + false, + ); +} + +#[test] +fn subcommand_derived_display_order() { + static SUBCMD_DECL_ORDER: &str = "test 1 + +USAGE: + test [SUBCOMMAND] + +OPTIONS: + -h, --help Print help information + -V, --version Print version information + +SUBCOMMANDS: + b1 blah b1 + a1 blah a1 + help Print this message or the help of the given subcommand(s) +"; + + let app_subcmd_decl_order = Command::new("test").version("1").subcommands(vec![ + Command::new("b1") + .about("blah b1") + .arg(Arg::new("test").short('t')), + Command::new("a1") + .about("blah a1") + .arg(Arg::new("roster").short('r')), + ]); + + utils::assert_output( + app_subcmd_decl_order, + "test --help", + SUBCMD_DECL_ORDER, + false, + ); +} diff --git a/tests/builder/help.rs b/tests/builder/help.rs index b96498f5..b089390a 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -43,8 +43,8 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) subcmd tests subcommands + help Print this message or the help of the given subcommand(s) "; static SC_NEGATES_REQS: &str = "prog 1.0 @@ -62,8 +62,8 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test + help Print this message or the help of the given subcommand(s) "; static ARGS_NEGATE_SC: &str = "prog 1.0 @@ -82,8 +82,8 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test + help Print this message or the help of the given subcommand(s) "; static AFTER_HELP: &str = "some text that comes before the help @@ -410,8 +410,8 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test some + help Print this message or the help of the given subcommand(s) "; static LAST_ARG_REQ: &str = "last 0.1 @@ -445,8 +445,8 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test some + help Print this message or the help of the given subcommand(s) "; static HIDE_DEFAULT_VAL: &str = "default 0.1 @@ -602,8 +602,8 @@ OPTIONS: -h, --help Print help information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) sub short about sub + help Print this message or the help of the given subcommand(s) "; fn setup() -> Command<'static> { diff --git a/tests/builder/subcommands.rs b/tests/builder/subcommands.rs index 9bc4728d..40eb91b2 100644 --- a/tests/builder/subcommands.rs +++ b/tests/builder/subcommands.rs @@ -12,8 +12,8 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test Some help [aliases: dongle, done] + help Print this message or the help of the given subcommand(s) "; static INVISIBLE_ALIAS_HELP: &str = "clap-test 2.6 @@ -26,37 +26,7 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test Some help -"; - -static SUBCMD_ALPHA_ORDER: &str = "test 1 - -USAGE: - test [SUBCOMMAND] - -OPTIONS: - -h, --help Print help information - -V, --version Print version information - -SUBCOMMANDS: - a1 blah a1 - b1 blah b1 - help Print this message or the help of the given subcommand(s) -"; - -static SUBCMD_DECL_ORDER: &str = "test 1 - -USAGE: - test [SUBCOMMAND] - -OPTIONS: - -h, --help Print help information - -V, --version Print version information - -SUBCOMMANDS: - b1 blah b1 - a1 blah a1 help Print this message or the help of the given subcommand(s) "; @@ -169,44 +139,6 @@ fn subcommand_multiple() { ); } -#[test] -fn subcommand_display_order() { - let app_subcmd_alpha_order = Command::new("test").version("1").subcommands(vec![ - Command::new("b1") - .about("blah b1") - .arg(Arg::new("test").short('t')), - Command::new("a1") - .about("blah a1") - .arg(Arg::new("roster").short('r')), - ]); - - utils::assert_output( - app_subcmd_alpha_order, - "test --help", - SUBCMD_ALPHA_ORDER, - false, - ); - - let app_subcmd_decl_order = Command::new("test") - .version("1") - .setting(clap::AppSettings::DeriveDisplayOrder) - .subcommands(vec![ - Command::new("b1") - .about("blah b1") - .arg(Arg::new("test").short('t')), - Command::new("a1") - .about("blah a1") - .arg(Arg::new("roster").short('r')), - ]); - - utils::assert_output( - app_subcmd_decl_order, - "test --help", - SUBCMD_DECL_ORDER, - false, - ); -} - #[test] fn single_alias() { let m = Command::new("myprog") diff --git a/tests/builder/template_help.rs b/tests/builder/template_help.rs index cf3642a6..f74c4aea 100644 --- a/tests/builder/template_help.rs +++ b/tests/builder/template_help.rs @@ -40,8 +40,8 @@ OPTIONS: ARGS: Sets an optional output file SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test does testing things + help Print this message or the help of the given subcommand(s) "; static SIMPLE_TEMPLATE: &str = "MyApp 1.0 @@ -61,8 +61,8 @@ OPTIONS: -V, --version Print version information SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) test does testing things + help Print this message or the help of the given subcommand(s) "; #[test] diff --git a/tests/ui/arg_required_else_help_stderr.toml b/tests/ui/arg_required_else_help_stderr.toml index 5ac94784..89edc2da 100644 --- a/tests/ui/arg_required_else_help_stderr.toml +++ b/tests/ui/arg_required_else_help_stderr.toml @@ -14,6 +14,6 @@ OPTIONS: --verbose log SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) more + help Print this message or the help of the given subcommand(s) """ diff --git a/tests/ui/h_flag_stdout.toml b/tests/ui/h_flag_stdout.toml index 66af2e46..e5d4fceb 100644 --- a/tests/ui/h_flag_stdout.toml +++ b/tests/ui/h_flag_stdout.toml @@ -13,7 +13,7 @@ OPTIONS: --verbose log SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) more + help Print this message or the help of the given subcommand(s) """ stderr = "" diff --git a/tests/ui/help_cmd_stdout.toml b/tests/ui/help_cmd_stdout.toml index 881d53ee..a7641bd5 100644 --- a/tests/ui/help_cmd_stdout.toml +++ b/tests/ui/help_cmd_stdout.toml @@ -18,9 +18,9 @@ OPTIONS: more log SUBCOMMANDS: - help - Print this message or the help of the given subcommand(s) more + help + Print this message or the help of the given subcommand(s) """ stderr = "" diff --git a/tests/ui/help_flag_stdout.toml b/tests/ui/help_flag_stdout.toml index 2615875b..c8ddca4e 100644 --- a/tests/ui/help_flag_stdout.toml +++ b/tests/ui/help_flag_stdout.toml @@ -18,9 +18,9 @@ OPTIONS: more log SUBCOMMANDS: - help - Print this message or the help of the given subcommand(s) more + help + Print this message or the help of the given subcommand(s) """ stderr = ""