From 9ee45f7f3dc659b3c7b283d2e863c3ca280db70d Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 28 Sep 2022 14:33:59 -0700 Subject: [PATCH] fix(complete): Fix `git diff log ` for Bash This continues the work started with the fix for #4273. There was another bug caused by using the subcommand names without considering their position in the argument list. If the user enters `git diff log `, we build up a string that identifies the subcommand. We ended up making the string `git__diff__log` in this case because we appended `__log` without considering the current state. Since `git__diff__log` does not correspond to an actual command, we wouldn't provide any suggestions. This commit restructures the code so we walk subcommands and subsubcommands in `bash.rs`. While walking those, we build up a list containing triples of the parent `$cmd` name (e.g. `git__diff`), the current command's name (e.g. `log`), and the `$cmd` for the current command. We then build the shell script's case arms based on that information. We could instead have fixed #4280 by using the second element in the pair returned from `utils::all_subcommands()` (a stringified list of the subcommand path) instead of the first one. However, that would not have helped us solve #4265. Closes #4280 --- clap_complete/src/shells/bash.rs | 53 +++++++++++++------ clap_complete/tests/snapshots/basic.bash | 14 +++-- .../tests/snapshots/feature_sample.bash | 14 +++-- clap_complete/tests/snapshots/quoting.bash | 49 ++++++++++++----- .../tests/snapshots/special_commands.bash | 35 ++++++++---- .../tests/snapshots/sub_subcommands.bash | 37 ++++++++++--- 6 files changed, 145 insertions(+), 57 deletions(-) diff --git a/clap_complete/src/shells/bash.rs b/clap_complete/src/shells/bash.rs index 76e37b75cd1..6ea962c5e49 100644 --- a/clap_complete/src/shells/bash.rs +++ b/clap_complete/src/shells/bash.rs @@ -75,26 +75,45 @@ complete -F _{name} -o bashdefault -o default {name} fn all_subcommands(cmd: &Command) -> String { debug!("all_subcommands"); - let mut subcmds = vec![String::new()]; - let mut scs = utils::all_subcommands(cmd) - .iter() - .map(|x| x.0.clone()) - .collect::>(); - - scs.sort(); - scs.dedup(); + fn add_command( + parent_fn_name: &str, + cmd: &Command, + subcmds: &mut Vec<(String, String, String)>, + ) { + let fn_name = format!( + "{parent_fn_name}__{cmd_name}", + parent_fn_name = parent_fn_name, + cmd_name = cmd.get_name().to_string().replace('-', "__") + ); + subcmds.push(( + parent_fn_name.to_string(), + cmd.get_name().to_string(), + fn_name.clone(), + )); + for subcmd in cmd.get_subcommands() { + add_command(&fn_name, subcmd, subcmds); + } + } + let mut subcmds = vec![]; + let fn_name = cmd.get_name().replace('-', "__"); + for subcmd in cmd.get_subcommands() { + add_command(&fn_name, subcmd, &mut subcmds); + } + subcmds.sort(); - subcmds.extend(scs.iter().map(|sc| { - format!( - "*,{name}) - cmd+=\"__{fn_name}\" + let mut cases = vec![String::new()]; + for (parent_fn_name, name, fn_name) in subcmds { + cases.push(format!( + "{parent_fn_name},{name}) + cmd=\"{fn_name}\" ;;", - name = sc, - fn_name = sc.replace('-', "__") - ) - })); + parent_fn_name = parent_fn_name, + name = name, + fn_name = fn_name, + )); + } - subcmds.join("\n ") + cases.join("\n ") } fn subcommand_details(cmd: &Command) -> String { diff --git a/clap_complete/tests/snapshots/basic.bash b/clap_complete/tests/snapshots/basic.bash index 1f7c154a25d..70946e79ca7 100644 --- a/clap_complete/tests/snapshots/basic.bash +++ b/clap_complete/tests/snapshots/basic.bash @@ -12,11 +12,17 @@ _my-app() { ",$1") cmd="my__app" ;; - *,help) - cmd+="__help" + my__app,help) + cmd="my__app__help" ;; - *,test) - cmd+="__test" + my__app,test) + cmd="my__app__test" + ;; + my__app__help,help) + cmd="my__app__help__help" + ;; + my__app__help,test) + cmd="my__app__help__test" ;; *) ;; diff --git a/clap_complete/tests/snapshots/feature_sample.bash b/clap_complete/tests/snapshots/feature_sample.bash index 7854f3d7abb..8ea49145b5a 100644 --- a/clap_complete/tests/snapshots/feature_sample.bash +++ b/clap_complete/tests/snapshots/feature_sample.bash @@ -12,11 +12,17 @@ _my-app() { ",$1") cmd="my__app" ;; - *,help) - cmd+="__help" + my__app,help) + cmd="my__app__help" ;; - *,test) - cmd+="__test" + my__app,test) + cmd="my__app__test" + ;; + my__app__help,help) + cmd="my__app__help__help" + ;; + my__app__help,test) + cmd="my__app__help__test" ;; *) ;; diff --git a/clap_complete/tests/snapshots/quoting.bash b/clap_complete/tests/snapshots/quoting.bash index 2ac46bcd740..e387c0bc52a 100644 --- a/clap_complete/tests/snapshots/quoting.bash +++ b/clap_complete/tests/snapshots/quoting.bash @@ -12,26 +12,47 @@ _my-app() { ",$1") cmd="my__app" ;; - *,cmd-backslash) - cmd+="__cmd__backslash" + my__app,cmd-backslash) + cmd="my__app__cmd__backslash" ;; - *,cmd-backticks) - cmd+="__cmd__backticks" + my__app,cmd-backticks) + cmd="my__app__cmd__backticks" ;; - *,cmd-brackets) - cmd+="__cmd__brackets" + my__app,cmd-brackets) + cmd="my__app__cmd__brackets" ;; - *,cmd-double-quotes) - cmd+="__cmd__double__quotes" + my__app,cmd-double-quotes) + cmd="my__app__cmd__double__quotes" ;; - *,cmd-expansions) - cmd+="__cmd__expansions" + my__app,cmd-expansions) + cmd="my__app__cmd__expansions" ;; - *,cmd-single-quotes) - cmd+="__cmd__single__quotes" + my__app,cmd-single-quotes) + cmd="my__app__cmd__single__quotes" ;; - *,help) - cmd+="__help" + my__app,help) + cmd="my__app__help" + ;; + my__app__help,cmd-backslash) + cmd="my__app__help__cmd__backslash" + ;; + my__app__help,cmd-backticks) + cmd="my__app__help__cmd__backticks" + ;; + my__app__help,cmd-brackets) + cmd="my__app__help__cmd__brackets" + ;; + my__app__help,cmd-double-quotes) + cmd="my__app__help__cmd__double__quotes" + ;; + my__app__help,cmd-expansions) + cmd="my__app__help__cmd__expansions" + ;; + my__app__help,cmd-single-quotes) + cmd="my__app__help__cmd__single__quotes" + ;; + my__app__help,help) + cmd="my__app__help__help" ;; *) ;; diff --git a/clap_complete/tests/snapshots/special_commands.bash b/clap_complete/tests/snapshots/special_commands.bash index 900a02bc036..31b5354aa7b 100644 --- a/clap_complete/tests/snapshots/special_commands.bash +++ b/clap_complete/tests/snapshots/special_commands.bash @@ -12,20 +12,35 @@ _my-app() { ",$1") cmd="my__app" ;; - *,help) - cmd+="__help" + my__app,help) + cmd="my__app__help" ;; - *,some-cmd-with-hyphens) - cmd+="__some__cmd__with__hyphens" + my__app,some-cmd-with-hyphens) + cmd="my__app__some__cmd__with__hyphens" ;; - *,some-hidden-cmd) - cmd+="__some__hidden__cmd" + my__app,some-hidden-cmd) + cmd="my__app__some__hidden__cmd" ;; - *,some_cmd) - cmd+="__some_cmd" + my__app,some_cmd) + cmd="my__app__some_cmd" ;; - *,test) - cmd+="__test" + my__app,test) + cmd="my__app__test" + ;; + my__app__help,help) + cmd="my__app__help__help" + ;; + my__app__help,some-cmd-with-hyphens) + cmd="my__app__help__some__cmd__with__hyphens" + ;; + my__app__help,some-hidden-cmd) + cmd="my__app__help__some__hidden__cmd" + ;; + my__app__help,some_cmd) + cmd="my__app__help__some_cmd" + ;; + my__app__help,test) + cmd="my__app__help__test" ;; *) ;; diff --git a/clap_complete/tests/snapshots/sub_subcommands.bash b/clap_complete/tests/snapshots/sub_subcommands.bash index 41ee58c57d5..fe9c8459704 100644 --- a/clap_complete/tests/snapshots/sub_subcommands.bash +++ b/clap_complete/tests/snapshots/sub_subcommands.bash @@ -12,17 +12,38 @@ _my-app() { ",$1") cmd="my__app" ;; - *,help) - cmd+="__help" + my__app,help) + cmd="my__app__help" ;; - *,some_cmd) - cmd+="__some_cmd" + my__app,some_cmd) + cmd="my__app__some_cmd" ;; - *,sub_cmd) - cmd+="__sub_cmd" + my__app,test) + cmd="my__app__test" ;; - *,test) - cmd+="__test" + my__app__help,help) + cmd="my__app__help__help" + ;; + my__app__help,some_cmd) + cmd="my__app__help__some_cmd" + ;; + my__app__help,test) + cmd="my__app__help__test" + ;; + my__app__help__some_cmd,sub_cmd) + cmd="my__app__help__some_cmd__sub_cmd" + ;; + my__app__some_cmd,help) + cmd="my__app__some_cmd__help" + ;; + my__app__some_cmd,sub_cmd) + cmd="my__app__some_cmd__sub_cmd" + ;; + my__app__some_cmd__help,help) + cmd="my__app__some_cmd__help__help" + ;; + my__app__some_cmd__help,sub_cmd) + cmd="my__app__some_cmd__help__sub_cmd" ;; *) ;;