fix(clap_complete): Fix the unexpected completion -f=barf and `-fba…

…rf`.
clap-rs · Jul 5, 2024 · 80f13cf · 80f13cf
1 parent 62b7615
commit 80f13cf
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 58 deletions.
diff --git a/clap_complete/src/dynamic/completer.rs b/clap_complete/src/dynamic/completer.rs
@@ -222,58 +222,85 @@ fn complete_arg(
             );
         }
 
-        if arg.is_empty() || arg.is_stdio() || arg.is_short() {
-            let dash_or_arg = if arg.is_empty() {
-                "-".into()
-            } else {
-                arg.to_value_os().to_string_lossy()
-            };
-            // HACK: Assuming knowledge of is_stdio
-            completions.extend(
-                shorts_and_visible_aliases(cmd)
-                    .into_iter()
-                    // HACK: Need better `OsStr` manipulation
-                    .map(|(f, help)| (format!("{}{}", dash_or_arg, f).into(), help)),
-            );
-        }
-
+        // TODO: loop through ShortFlags to check if it has append/set action flag.
         if let Some(mut short) = arg.to_short() {
             if !short.is_negative_number() {
-                if let Some(s) = short.next_value_os() {
-                    let (flag, has_equal, value) = match s.split_once("=") {
-                        Some((flag, value)) => (flag, true, value),
-                        None => (s, false, OsStr::new("")),
-                    };
-                    if let Some(last_c) = flag.to_string_lossy().chars().last() {
-                        if let Some(arg) = cmd.get_arguments().find(|a| {
+                let prev_opt;
+                let mut prev_flags = OsString::new();
+                loop {
+                    if let Some(Ok(opt)) = short.next_flag() {
+                        prev_flags.push(opt.to_string());
+                        let opt = cmd.get_arguments().find(|a| {
                             let shorts = a.get_short_and_visible_aliases();
                             let is_find = shorts.map(|v| {
                                 let mut iter = v.into_iter();
-                                let c = iter.find(|c| *c == last_c);
+                                let c = iter.find(|c| *c == opt);
                                 c.is_some()
                             });
                             is_find.unwrap_or(false)
-                        }) {
-                            completions.extend(
-                                complete_arg_value(value.to_str().ok_or(value), arg, current_dir)
-                                    .into_iter()
-                                    .map(|(os, help)| {
-                                        (
-                                            format!(
-                                                "-{}{}{}",
-                                                flag.to_string_lossy(),
-                                                if has_equal { "=" } else { "" },
-                                                os.to_string_lossy()
-                                            )
-                                            .into(),
-                                            help,
-                                        )
-                                    }),
-                            );
+                        });
+                        if let Some(opt) = opt {
+                            match opt.get_action() {
+                                clap::ArgAction::Set | clap::ArgAction::Append => {
+                                    prev_opt = Some(opt);
+                                    break;
+                                }
+                                clap::ArgAction::SetTrue
+                                | clap::ArgAction::SetFalse
+                                | clap::ArgAction::Count
+                                | clap::ArgAction::Version
+                                | clap::ArgAction::Help
+                                | clap::ArgAction::HelpShort
+                                | clap::ArgAction::HelpLong => (),
+                                _ => (),
+                            };
                         }
                     }
                 }
+                if let Some(opt) = prev_opt {
+                    let has_equal = if let Some(Ok('=')) = short.next_flag() {
+                        true
+                    } else {
+                        false
+                    };
+                    let value = short.next_value_os().unwrap_or(OsStr::new(""));
+                    completions.extend(
+                        complete_arg_value(value.to_str().ok_or(value), opt, current_dir)
+                            .into_iter()
+                            .map(|(os, help)| {
+                                (
+                                    format!(
+                                        "-{}{}{}",
+                                        prev_flags.to_string_lossy(),
+                                        if has_equal { "=" } else { "" },
+                                        os.to_string_lossy()
+                                    )
+                                    .into(),
+                                    help,
+                                )
+                            }),
+                    );
+                } else {
+                    completions.extend(
+                        shorts_and_visible_aliases(cmd)
+                            .into_iter()
+                            .map(|(f, help)| (format!("-{}", f).into(), help)),
+                    );
+                }
             }
+        } else if arg.is_empty() || arg.is_stdio() {
+            let dash_or_arg = if arg.is_empty() {
+                "-".into()
+            } else {
+                arg.to_value_os().to_string_lossy()
+            };
+            // HACK: Assuming knowledge of is_stdio
+            completions.extend(
+                shorts_and_visible_aliases(cmd)
+                    .into_iter()
+                    // HACK: Need better `OsStr` manipulation
+                    .map(|(f, help)| (format!("{}{}", dash_or_arg, f).into(), help)),
+            );
         }
     } else if let ParseState::Opt(opt) = &state {
         completions.extend(complete_arg_value(arg.to_value(), opt, current_dir));

diff --git a/clap_complete/tests/testsuite/dynamic.rs b/clap_complete/tests/testsuite/dynamic.rs
@@ -246,7 +246,6 @@ toml"
 
     assert_data_eq!(complete!(cmd, "-chF j[TAB]"), snapbox::str!["json"]);
 
-    // NOTE: Treat `F` as a value of `-i`, so pressing [TAB] will complete other arguments and subcommands.
     assert_data_eq!(
         complete!(cmd, "-ciF [TAB]"),
         snapbox::str![
@@ -264,11 +263,7 @@ toml"
     assert_data_eq!(
         complete!(cmd, "-ci[TAB]", current_dir = Some(testdir_path)),
         snapbox::str![
-            "-cii
--ciF
--cic
--cih	Print help
--cia_file
+            "-cia_file
 -cib_file
 -cic_dir/
 -cid_dir/"
@@ -278,11 +273,7 @@ toml"
     assert_data_eq!(
         complete!(cmd, "-ci=[TAB]", current_dir = Some(testdir_path)),
         snapbox::str![
-            "-ci=i
--ci=F
--ci=c
--ci=h	Print help
--ci=a_file
+            "-ci=a_file
 -ci=b_file
 -ci=c_dir/
 -ci=d_dir/"
@@ -291,13 +282,7 @@ toml"
 
     assert_data_eq!(
         complete!(cmd, "-ci=a[TAB]", current_dir = Some(testdir_path)),
-        snapbox::str![
-            "-ci=ai
--ci=aF
--ci=ac
--ci=ah	Print help
--ci=a_file"
-        ]
+        snapbox::str!["-ci=a_file"]
     )
 }