From a126149a809f01586ccb6d9299eb2fc2c87d8fe0 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 20 Sep 2024 13:59:16 -0500 Subject: [PATCH 1/7] refactor(complete): Remove redundant dedup Our `id` de-duping takes care of this. --- clap_complete/src/engine/complete.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index 9ed091b8b14..7d4ce65968f 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -394,7 +394,6 @@ fn complete_subcommand(value: &str, cmd: &clap::Command) -> Vec>(); scs.sort(); - scs.dedup(); scs } From a4aa5dffdf46a2975f2b2d51ce7e618e19250ead Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 20 Sep 2024 14:04:40 -0500 Subject: [PATCH 2/7] feat(complete): Group by tag --- clap_complete/src/engine/candidate.rs | 14 ++++++++++++++ clap_complete/src/engine/complete.rs | 9 +++++++++ 2 files changed, 23 insertions(+) diff --git a/clap_complete/src/engine/candidate.rs b/clap_complete/src/engine/candidate.rs index 6edf7d2ce24..202fea0e971 100644 --- a/clap_complete/src/engine/candidate.rs +++ b/clap_complete/src/engine/candidate.rs @@ -9,6 +9,7 @@ pub struct CompletionCandidate { value: OsString, help: Option, id: Option, + tag: Option, hidden: bool, } @@ -36,6 +37,14 @@ impl CompletionCandidate { self } + /// Group candidates by tag + /// + /// Future: these may become user-visible + pub fn tag(mut self, tag: Option) -> Self { + self.tag = tag; + self + } + /// Set the visibility of the completion candidate /// /// Only shown when there is no visible candidate for completing the current argument. @@ -74,6 +83,11 @@ impl CompletionCandidate { self.id.as_ref() } + /// Get the grouping tag + pub fn get_tag(&self) -> Option<&StyledStr> { + self.tag.as_ref() + } + /// Get the visibility of the completion candidate pub fn is_hide_set(&self) -> bool { self.hidden diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index 7d4ce65968f..6e96cd8bc42 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -279,6 +279,15 @@ fn complete_arg( } }); + let mut tags = Vec::new(); + for candidate in &completions { + let tag = candidate.get_tag().cloned(); + if !tags.contains(&tag) { + tags.push(tag); + } + } + completions.sort_by_key(|c| tags.iter().position(|t| c.get_tag() == t.as_ref())); + Ok(completions) } From 144879194081212046d5566572bfa15ea9bcca35 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 20 Sep 2024 14:29:05 -0500 Subject: [PATCH 3/7] fix(complete): Specify tags for built-in candiates --- clap_complete/src/engine/complete.rs | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index 6e96cd8bc42..b9aed9a567b 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -364,6 +364,17 @@ fn complete_arg_value( .map(|comp| comp.add_prefix(prefix)) .collect(); } + values = values + .into_iter() + .map(|comp| { + if comp.get_tag().is_some() { + comp + } else { + comp.tag(Some(arg.to_string().into())) + } + }) + .collect(); + values } @@ -418,6 +429,9 @@ fn longs_and_visible_aliases(p: &clap::Command) -> Vec { CompletionCandidate::new(format!("--{}", s)) .help(a.get_help().cloned()) .id(Some(format!("arg::{}", a.get_id()))) + .tag(Some( + a.get_help_heading().unwrap_or("Options").to_owned().into(), + )) .hide(a.is_hide_set()) }) }) @@ -437,6 +451,9 @@ fn hidden_longs_aliases(p: &clap::Command) -> Vec { CompletionCandidate::new(format!("--{}", s)) .help(a.get_help().cloned()) .id(Some(format!("arg::{}", a.get_id()))) + .tag(Some( + a.get_help_heading().unwrap_or("Options").to_owned().into(), + )) .hide(true) }) }) @@ -461,6 +478,9 @@ fn shorts_and_visible_aliases(p: &clap::Command) -> Vec { .or_else(|| a.get_long().map(|long| format!("--{long}").into())), ) .id(Some(format!("arg::{}", a.get_id()))) + .tag(Some( + a.get_help_heading().unwrap_or("Options").to_owned().into(), + )) .hide(a.is_hide_set()) }) }) @@ -495,12 +515,24 @@ fn subcommands(p: &clap::Command) -> Vec { CompletionCandidate::new(s.to_string()) .help(sc.get_about().cloned()) .id(Some(format!("command::{}", sc.get_name()))) + .tag(Some( + p.get_subcommand_help_heading() + .unwrap_or("Commands") + .to_owned() + .into(), + )) .hide(sc.is_hide_set()) }) .chain(sc.get_aliases().map(|s| { CompletionCandidate::new(s.to_string()) .help(sc.get_about().cloned()) .id(Some(format!("command::{}", sc.get_name()))) + .tag(Some( + p.get_subcommand_help_heading() + .unwrap_or("Commands") + .to_owned() + .into(), + )) .hide(true) })) }) From 59a61e188fbb9999db52f567e3951d3720204084 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 20 Sep 2024 14:36:07 -0500 Subject: [PATCH 4/7] refactor(complete): Pull out common candidate code --- clap_complete/src/engine/complete.rs | 83 +++++++++++++--------------- 1 file changed, 38 insertions(+), 45 deletions(-) diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index b9aed9a567b..ed0c3a64900 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -426,13 +426,7 @@ fn longs_and_visible_aliases(p: &clap::Command) -> Vec { .filter_map(|a| { a.get_long_and_visible_aliases().map(|longs| { longs.into_iter().map(|s| { - CompletionCandidate::new(format!("--{}", s)) - .help(a.get_help().cloned()) - .id(Some(format!("arg::{}", a.get_id()))) - .tag(Some( - a.get_help_heading().unwrap_or("Options").to_owned().into(), - )) - .hide(a.is_hide_set()) + populate_arg_candidate(CompletionCandidate::new(format!("--{}", s)), a) }) }) }) @@ -448,12 +442,7 @@ fn hidden_longs_aliases(p: &clap::Command) -> Vec { .filter_map(|a| { a.get_aliases().map(|longs| { longs.into_iter().map(|s| { - CompletionCandidate::new(format!("--{}", s)) - .help(a.get_help().cloned()) - .id(Some(format!("arg::{}", a.get_id()))) - .tag(Some( - a.get_help_heading().unwrap_or("Options").to_owned().into(), - )) + populate_arg_candidate(CompletionCandidate::new(format!("--{}", s)), a) .hide(true) }) }) @@ -471,17 +460,11 @@ fn shorts_and_visible_aliases(p: &clap::Command) -> Vec { .filter_map(|a| { a.get_short_and_visible_aliases().map(|shorts| { shorts.into_iter().map(|s| { - CompletionCandidate::new(s.to_string()) - .help( - a.get_help() - .cloned() - .or_else(|| a.get_long().map(|long| format!("--{long}").into())), - ) - .id(Some(format!("arg::{}", a.get_id()))) - .tag(Some( - a.get_help_heading().unwrap_or("Options").to_owned().into(), - )) - .hide(a.is_hide_set()) + populate_arg_candidate(CompletionCandidate::new(s.to_string()), a).help( + a.get_help() + .cloned() + .or_else(|| a.get_long().map(|long| format!("--{long}").into())), + ) }) }) }) @@ -489,6 +472,19 @@ fn shorts_and_visible_aliases(p: &clap::Command) -> Vec { .collect() } +fn populate_arg_candidate(candidate: CompletionCandidate, arg: &clap::Arg) -> CompletionCandidate { + candidate + .help(arg.get_help().cloned()) + .id(Some(format!("arg::{}", arg.get_id()))) + .tag(Some( + arg.get_help_heading() + .unwrap_or("Options") + .to_owned() + .into(), + )) + .hide(arg.is_hide_set()) +} + /// Get the possible values for completion fn possible_values(a: &clap::Arg) -> Option> { if !a.get_num_args().expect("built").takes_values() { @@ -511,34 +507,31 @@ fn subcommands(p: &clap::Command) -> Vec { .flat_map(|sc| { sc.get_name_and_visible_aliases() .into_iter() - .map(|s| { - CompletionCandidate::new(s.to_string()) - .help(sc.get_about().cloned()) - .id(Some(format!("command::{}", sc.get_name()))) - .tag(Some( - p.get_subcommand_help_heading() - .unwrap_or("Commands") - .to_owned() - .into(), - )) - .hide(sc.is_hide_set()) - }) + .map(|s| populate_command_candidate(CompletionCandidate::new(s.to_string()), p, sc)) .chain(sc.get_aliases().map(|s| { - CompletionCandidate::new(s.to_string()) - .help(sc.get_about().cloned()) - .id(Some(format!("command::{}", sc.get_name()))) - .tag(Some( - p.get_subcommand_help_heading() - .unwrap_or("Commands") - .to_owned() - .into(), - )) + populate_command_candidate(CompletionCandidate::new(s.to_string()), p, sc) .hide(true) })) }) .collect() } +fn populate_command_candidate( + candidate: CompletionCandidate, + cmd: &clap::Command, + subcommand: &clap::Command, +) -> CompletionCandidate { + candidate + .help(subcommand.get_about().cloned()) + .id(Some(format!("command::{}", subcommand.get_name()))) + .tag(Some( + cmd.get_subcommand_help_heading() + .unwrap_or("Commands") + .to_owned() + .into(), + )) + .hide(subcommand.is_hide_set()) +} /// Parse the short flags and find the first `takes_values` option. fn parse_shortflags<'c, 's>( cmd: &'c clap::Command, From 67d9fef9cee8f1b297aed73e6d95f78744b43a27 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 20 Sep 2024 14:39:18 -0500 Subject: [PATCH 5/7] feat(complete): Give control over display order --- clap_complete/src/engine/candidate.rs | 12 ++++++++++++ clap_complete/src/engine/complete.rs | 7 ++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/clap_complete/src/engine/candidate.rs b/clap_complete/src/engine/candidate.rs index 202fea0e971..af629f0f7c0 100644 --- a/clap_complete/src/engine/candidate.rs +++ b/clap_complete/src/engine/candidate.rs @@ -10,6 +10,7 @@ pub struct CompletionCandidate { help: Option, id: Option, tag: Option, + display_order: Option, hidden: bool, } @@ -45,6 +46,12 @@ impl CompletionCandidate { self } + /// Sort weight within a [`CompletionCandidate::tag`] + pub fn display_order(mut self, order: Option) -> Self { + self.display_order = order; + self + } + /// Set the visibility of the completion candidate /// /// Only shown when there is no visible candidate for completing the current argument. @@ -88,6 +95,11 @@ impl CompletionCandidate { self.tag.as_ref() } + /// Get the grouping tag + pub fn get_display_order(&self) -> Option { + self.display_order + } + /// Get the visibility of the completion candidate pub fn is_hide_set(&self) -> bool { self.hidden diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index ed0c3a64900..359af90e743 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -286,7 +286,12 @@ fn complete_arg( tags.push(tag); } } - completions.sort_by_key(|c| tags.iter().position(|t| c.get_tag() == t.as_ref())); + completions.sort_by_key(|c| { + ( + tags.iter().position(|t| c.get_tag() == t.as_ref()), + c.get_display_order(), + ) + }); Ok(completions) } From c6b5d627a0c17527630e4086a4fe83589365d1f0 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 20 Sep 2024 14:43:16 -0500 Subject: [PATCH 6/7] feat(builder): Expose get_display_order --- clap_builder/src/builder/arg.rs | 11 ++++++----- clap_builder/src/builder/command.rs | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/clap_builder/src/builder/arg.rs b/clap_builder/src/builder/arg.rs index b5450d16e35..ec5444cb994 100644 --- a/clap_builder/src/builder/arg.rs +++ b/clap_builder/src/builder/arg.rs @@ -3870,6 +3870,12 @@ impl Arg { self.long_help.as_ref() } + /// Get the placement within help + #[inline] + pub fn get_display_order(&self) -> usize { + self.disp_ord.unwrap_or(999) + } + /// Get the help heading specified for this argument, if any #[inline] pub fn get_help_heading(&self) -> Option<&str> { @@ -4422,11 +4428,6 @@ impl Arg { pub(crate) fn is_multiple(&self) -> bool { self.is_multiple_values_set() || matches!(*self.get_action(), ArgAction::Append) } - - #[cfg(feature = "help")] - pub(crate) fn get_display_order(&self) -> usize { - self.disp_ord.unwrap_or(999) - } } impl From<&'_ Arg> for Arg { diff --git a/clap_builder/src/builder/command.rs b/clap_builder/src/builder/command.rs index 32e6ea4fb0f..d050e4943bd 100644 --- a/clap_builder/src/builder/command.rs +++ b/clap_builder/src/builder/command.rs @@ -3462,6 +3462,12 @@ impl Command { self.long_version.as_deref() } + /// Get the placement within help + #[inline] + pub fn get_display_order(&self) -> usize { + self.disp_ord.unwrap_or(999) + } + /// Get the authors of the cmd. #[inline] pub fn get_author(&self) -> Option<&str> { @@ -4777,11 +4783,6 @@ impl Command { .map(|sc| sc.get_name()) } - #[cfg(feature = "help")] - pub(crate) fn get_display_order(&self) -> usize { - self.disp_ord.unwrap_or(999) - } - pub(crate) fn write_help_err(&self, mut use_long: bool) -> StyledStr { debug!( "Command::write_help_err: {}, use_long={:?}", From 232ee106615e997f0841bb3de97eed126b929fe3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 20 Sep 2024 14:45:54 -0500 Subject: [PATCH 7/7] fix(complete): Use existing display order for Arg/Command --- clap_complete/src/engine/complete.rs | 8 ++++---- clap_complete/tests/testsuite/bash.rs | 10 +++++----- clap_complete/tests/testsuite/engine.rs | 10 +++++----- clap_complete/tests/testsuite/fish.rs | 15 +++++++-------- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs index 359af90e743..b60e084ae3a 100644 --- a/clap_complete/src/engine/complete.rs +++ b/clap_complete/src/engine/complete.rs @@ -414,12 +414,10 @@ fn complete_subcommand(value: &str, cmd: &clap::Command) -> Vec>(); - scs.sort(); - scs + .collect() } /// Gets all the long options, their visible aliases and flags of a [`clap::Command`] with formatted `--` prefix. @@ -487,6 +485,7 @@ fn populate_arg_candidate(candidate: CompletionCandidate, arg: &clap::Arg) -> Co .to_owned() .into(), )) + .display_order(Some(arg.get_display_order())) .hide(arg.is_hide_set()) } @@ -535,6 +534,7 @@ fn populate_command_candidate( .to_owned() .into(), )) + .display_order(Some(subcommand.get_display_order())) .hide(subcommand.is_hide_set()) } /// Parse the short flags and find the first `takes_values` option. diff --git a/clap_complete/tests/testsuite/bash.rs b/clap_complete/tests/testsuite/bash.rs index f1580fd750d..8d63c68992f 100644 --- a/clap_complete/tests/testsuite/bash.rs +++ b/clap_complete/tests/testsuite/bash.rs @@ -255,8 +255,8 @@ fn complete_dynamic_env_toplevel() { let input = "exhaustive \t\t"; let expected = snapbox::str![[r#" % -action help last quote --global --help -alias hint pacman value --generate --version +action value last hint --global --help +quote pacman alias help --generate --version "#]]; let actual = runtime.complete(input, &term).unwrap(); assert_data_eq!(actual, expected); @@ -275,9 +275,9 @@ fn complete_dynamic_env_quoted_help() { let input = "exhaustive quote \t\t"; let expected = snapbox::str![[r#" % -cmd-backslash cmd-double-quotes escape-help --double-quotes --brackets --global -cmd-backticks cmd-expansions help --backticks --expansions --help -cmd-brackets cmd-single-quotes --single-quotes --backslash --choice --version +cmd-single-quotes cmd-backslash escape-help --global --backslash --choice +cmd-double-quotes cmd-brackets help --double-quotes --brackets --help +cmd-backticks cmd-expansions --single-quotes --backticks --expansions --version "#]]; let actual = runtime.complete(input, &term).unwrap(); assert_data_eq!(actual, expected); diff --git a/clap_complete/tests/testsuite/engine.rs b/clap_complete/tests/testsuite/engine.rs index 59a5a64deaa..56f1e1bafd8 100644 --- a/clap_complete/tests/testsuite/engine.rs +++ b/clap_complete/tests/testsuite/engine.rs @@ -30,8 +30,8 @@ fn suggest_subcommand_subset() { assert_data_eq!( complete!(cmd, "he"), snapbox::str![[r#" -hello-moon hello-world +hello-moon help Print this message or the help of the given subcommand(s) "#]], ); @@ -105,8 +105,8 @@ fn suggest_subcommand_aliases() { assert_data_eq!( complete!(cmd, "hello"), snapbox::str![[r#" -hello-moon hello-world +hello-moon "#]], ); } @@ -1099,16 +1099,16 @@ fn sort_and_filter() { assert_data_eq!( complete!(cmd, " [TAB]"), snapbox::str![[r#" -help Print this message or the help of the given subcommand(s) sub +help Print this message or the help of the given subcommand(s) pos-a pos-b pos-c --required-flag --optional-flag --long-flag ---help Print help -s +--help Print help "#]] ); assert_data_eq!( @@ -1116,9 +1116,9 @@ pos-c snapbox::str![[r#" -r --required-flag -o --optional-flag +--long-flag -s -h Print help ---long-flag "#]] ); assert_data_eq!( diff --git a/clap_complete/tests/testsuite/fish.rs b/clap_complete/tests/testsuite/fish.rs index 47ff82e7e0c..1d9f0385491 100644 --- a/clap_complete/tests/testsuite/fish.rs +++ b/clap_complete/tests/testsuite/fish.rs @@ -192,10 +192,9 @@ fn complete_dynamic_env_toplevel() { let input = "exhaustive \t\t"; let expected = snapbox::str![[r#" % exhaustive action -action last --global (everywhere) -alias pacman --generate (generate) -help (Print this message or the help of the given subcommand(s)) quote --help (Print help) -hint value --version (Print version) +action pacman hint --generate (generate) +quote last help (Print this message or the help of the given subcommand(s)) --help (Print help) +value alias --global (everywhere) --version (Print version) "#]]; let actual = runtime.complete(input, &term).unwrap(); assert_data_eq!(actual, expected); @@ -214,22 +213,22 @@ fn complete_dynamic_env_quoted_help() { let input = "exhaustive quote \t\t"; let expected = snapbox::str![[r#" % exhaustive quote -cmd-backslash (Avoid '/n') +cmd-single-quotes (Can be 'always', 'auto', or 'never') +cmd-double-quotes (Can be "always", "auto", or "never") cmd-backticks (For more information see `echo test`) +cmd-backslash (Avoid '/n') cmd-brackets (List packages [filter]) -cmd-double-quotes (Can be "always", "auto", or "never") cmd-expansions (Execute the shell command with $SHELL) -cmd-single-quotes (Can be 'always', 'auto', or 'never') escape-help (/tab "') help (Print this message or the help of the given subcommand(s)) --single-quotes (Can be 'always', 'auto', or 'never') +--global (everywhere) --double-quotes (Can be "always", "auto", or "never") --backticks (For more information see `echo test`) --backslash (Avoid '/n') --brackets (List packages [filter]) --expansions (Execute the shell command with $SHELL) --choice ---global (everywhere) --help (Print help (see more with '--help')) --version (Print version) "#]];