Repository: clap-rs/clap
Pull request #5539 (2024-06-19 08:55:04)

feat(clap_complete): Support flags with values `--flag bar`and `-f bar` in native completions
related issue: https://github.com/clap-rs/clap/issues/3920
what is left
- support `-fbar` and `-f=bar`
- error parsing for `--flag=bar` in bash
- `foo --bar\t` offering both `--bar <value>` and `--bare`

shannmu: The cases of -fbar and -f=bar have not been handled yet.
shannmu: I have added support for `-fbar` and `-f=bar` completion.
https://github.com/clap-rs/clap/blob/469d847d35731a987235d320517700c07ba729a4/clap_complete/src/dynamic/completer.rs#L141-L146
This code didn't consider the case `-f=bar[TAB]`, so it could generate `-f=barf` in completions. I will fix it later.
epage: While talked about this in the call, to not lose track of this

> I have added support for -fbar and -f=bar completion.

Please split that out into another branch for a later PR so we can focus on getting the current PR merged.

Patch:
diff --git a/clap_builder/src/util/color.rs b/clap_builder/src/util/color.rs
index f446c016..d13a6df9 100644
--- a/clap_builder/src/util/color.rs
+++ b/clap_builder/src/util/color.rs
@@ -8,6 +8,10 @@ pub enum ColorChoice {
     ///
     /// **NOTE:** This is the default behavior of `clap`.
     ///
+    /// # Platform Specific
+    ///
+    /// This setting only applies to Unix, Linux, and macOS (i.e. non-Windows platforms).
+    ///
     /// # Examples
     ///
     /// ```rust
@@ -23,6 +27,10 @@ pub enum ColorChoice {
 
     /// Enables colored output regardless of whether or not the output is going to a terminal/TTY.
     ///
+    /// # Platform Specific
+    ///
+    /// This setting only applies to Unix, Linux, and macOS (i.e. non-Windows platforms).
+    ///
     /// # Examples
     ///
     /// ```rust
@@ -38,6 +46,10 @@ pub enum ColorChoice {
 
     /// Disables colored output no matter if the output is going to a terminal/TTY, or not.
     ///
+    /// # Platform Specific
+    ///
+    /// This setting only applies to Unix, Linux, and macOS (i.e. non-Windows platforms)
+    ///
     /// # Examples
     ///
     /// ```rust
diff --git a/clap_complete/src/dynamic/completer.rs b/clap_complete/src/dynamic/completer.rs
index 19966f47..5f540c3d 100644
--- a/clap_complete/src/dynamic/completer.rs
+++ b/clap_complete/src/dynamic/completer.rs
@@ -53,9 +53,10 @@ pub fn complete(
     let mut current_cmd = &*cmd;
     let mut pos_index = 1;
     let mut is_escaped = false;
+    let mut state = ParseState::ValueDone;
     while let Some(arg) = raw_args.next(&mut cursor) {
         if cursor == target_cursor {
-            return complete_arg(&arg, current_cmd, current_dir, pos_index, is_escaped);
+            return complete_arg(&arg, current_cmd, current_dir, pos_index, state);
         }
 
         debug!("complete::next: Begin parsing '{:?}'", arg.to_value_os(),);
@@ -64,18 +65,103 @@ pub fn complete(
             if let Some(next_cmd) = current_cmd.find_subcommand(value) {
                 current_cmd = next_cmd;
                 pos_index = 1;
+                state = ParseState::ValueDone;
                 continue;
             }
         }
 
         if is_escaped {
             pos_index += 1;
+            state = ParseState::Pos(pos_index);
         } else if arg.is_escape() {
             is_escaped = true;
-        } else if let Some(_long) = arg.to_long() {
-        } else if let Some(_short) = arg.to_short() {
+            state = ParseState::ValueDone;
+        } else if let Some((flag, value)) = arg.to_long() {
+            if let Ok(flag) = flag {
+                let opt = current_cmd.get_arguments().find(|a| {
+                    let longs = a.get_long_and_visible_aliases();
+                    let is_find = longs.map(|v| {
+                        let mut iter = v.into_iter();
+                        let s = iter.find(|s| *s == flag);
+                        s.is_some()
+                    });
+                    is_find.unwrap_or(false)
+                });
+                state = match opt.map(|o| o.get_action()) {
+                    Some(clap::ArgAction::Set) | Some(clap::ArgAction::Append) => {
+                        if value.is_some() {
+                            ParseState::ValueDone
+                        } else {
+                            ParseState::Opt(opt.unwrap().clone())
+                        }
+                    }
+                    Some(clap::ArgAction::SetTrue) | Some(clap::ArgAction::SetFalse) => {
+                        ParseState::ValueDone
+                    }
+                    Some(clap::ArgAction::Count) => ParseState::ValueDone,
+                    Some(clap::ArgAction::Version) => ParseState::ValueDone,
+                    Some(clap::ArgAction::Help)
+                    | Some(clap::ArgAction::HelpLong)
+                    | Some(clap::ArgAction::HelpShort) => ParseState::ValueDone,
+                    Some(_) => ParseState::ValueDone,
+                    None => ParseState::ValueDone,
+                };
+            } else {
+                state = ParseState::ValueDone;
+            }
+        } else if let Some(mut short) = arg.to_short() {
+            let mut takes_value = false;
+            loop {
+                if let Some(Ok(opt)) = short.next_flag() {
+                    let opt = current_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 == opt);
+                            c.is_some()
+                        });
+                        is_find.unwrap_or(false)
+                    });
+
+                    state = match opt.map(|o| o.get_action()) {
+                        Some(clap::ArgAction::Set) | Some(clap::ArgAction::Append) => {
+                            takes_value = true;
+                            if short.next_value_os().is_some() {
+                                ParseState::ValueDone
+                            } else {
+                                ParseState::Opt(opt.unwrap().clone())
+                            }
+                        }
+                        Some(clap::ArgAction::SetTrue) | Some(clap::ArgAction::SetFalse) => {
+                            ParseState::ValueDone
+                        }
+                        Some(clap::ArgAction::Count) => ParseState::ValueDone,
+                        Some(clap::ArgAction::Version) => ParseState::ValueDone,
+                        Some(clap::ArgAction::Help)
+                        | Some(clap::ArgAction::HelpShort)
+                        | Some(clap::ArgAction::HelpLong) => ParseState::ValueDone,
+                        Some(_) => ParseState::ValueDone,
+                        None => ParseState::ValueDone,
+                    };
+
+                    if takes_value {
+                        break;
+                    }
+                } else {
+                    state = ParseState::ValueDone;
+                    break;
+                }
+            }
         } else {
-            pos_index += 1;
+            match state {
+                ParseState::ValueDone | ParseState::Pos(_) => {
+                    pos_index += 1;
+                    state = ParseState::ValueDone;
+                }
+                ParseState::Opt(_) => {
+                    state = ParseState::ValueDone;
+                }
+            }
         }
     }
 
@@ -85,96 +171,123 @@ pub fn complete(
     ))
 }
 
+#[derive(Debug, PartialEq, Eq, Clone)]
+enum ParseState {
+    /// Parsing a value done, there is no state to record.
+    ValueDone,
+
+    /// Parsing a positional argument after `--`
+    Pos(usize),
+
+    /// Parsing a optional flag argument
+    Opt(clap::Arg),
+}
+
 fn complete_arg(
     arg: &clap_lex::ParsedArg<'_>,
     cmd: &clap::Command,
     current_dir: Option<&std::path::Path>,
     pos_index: usize,
-    is_escaped: bool,
+    state: ParseState,
 ) -> Result<Vec<CompletionCandidate>, std::io::Error> {
     debug!(
-        "complete_arg: arg={:?}, cmd={:?}, current_dir={:?}, pos_index={}, is_escaped={}",
+        "complete_arg: arg={:?}, cmd={:?}, current_dir={:?}, pos_index={:?}, state={:?}",
         arg,
         cmd.get_name(),
         current_dir,
         pos_index,
-        is_escaped
+        state
     );
     let mut completions = Vec::<CompletionCandidate>::new();
 
-    if !is_escaped {
-        if let Some((flag, value)) = arg.to_long() {
-            if let Ok(flag) = flag {
-                if let Some(value) = value {
-                    if let Some(arg) = cmd.get_arguments().find(|a| a.get_long() == Some(flag)) {
-                        completions.extend(
-                            complete_arg_value(value.to_str().ok_or(value), arg, current_dir)
-                                .into_iter()
-                                .map(|comp| {
-                                    CompletionCandidate::new(format!(
-                                        "--{}={}",
-                                        flag,
-                                        comp.get_content().to_string_lossy()
-                                    ))
-                                    .help(comp.get_help().cloned())
-                                    .visible(comp.is_visible())
-                                }),
-                        );
+    match state {
+        ParseState::ValueDone => {
+            if let Some((flag, value)) = arg.to_long() {
+                if let Ok(flag) = flag {
+                    if let Some(value) = value {
+                        if let Some(arg) = cmd.get_arguments().find(|a| a.get_long() == Some(flag))
+                        {
+                            completions.extend(
+                                complete_arg_value(value.to_str().ok_or(value), arg, current_dir)
+                                    .into_iter()
+                                    .map(|comp| {
+                                        CompletionCandidate::new(format!(
+                                            "--{}={}",
+                                            flag,
+                                            comp.get_content().to_string_lossy()
+                                        ))
+                                        .help(comp.get_help().cloned())
+                                        .visible(comp.is_visible())
+                                    }),
+                            );
+                        }
+                    } else {
+                        completions.extend(longs_and_visible_aliases(cmd).into_iter().filter(
+                            |comp| {
+                                comp.get_content()
+                                    .starts_with(format!("--{}", flag).as_str())
+                            },
+                        ));
+
+                        completions.extend(hidden_longs_aliases(cmd).into_iter().filter(|comp| {
+                            comp.get_content()
+                                .starts_with(format!("--{}", flag).as_str())
+                        }))
                     }
-                } else {
-                    completions.extend(longs_and_visible_aliases(cmd).into_iter().filter(|comp| {
-                        comp.get_content()
-                            .starts_with(format!("--{}", flag).as_str())
-                    }));
-
-                    completions.extend(hidden_longs_aliases(cmd).into_iter().filter(|comp| {
-                        comp.get_content()
-                            .starts_with(format!("--{}", flag).as_str())
-                    }))
                 }
-            }
-        } else if arg.is_escape() || arg.is_stdio() || arg.is_empty() {
-            // HACK: Assuming knowledge of is_escape / is_stdio
-            completions.extend(longs_and_visible_aliases(cmd));
+            } else if arg.is_escape() || arg.is_stdio() || arg.is_empty() {
+                // HACK: Assuming knowledge of is_escape / is_stdio
+                completions.extend(longs_and_visible_aliases(cmd));
 
-            completions.extend(hidden_longs_aliases(cmd));
-        }
+                completions.extend(hidden_longs_aliases(cmd));
+            }
 
-        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(|comp| {
-                        CompletionCandidate::new(format!(
-                            "{}{}",
-                            dash_or_arg,
-                            comp.get_content().to_string_lossy()
-                        ))
-                        .help(comp.get_help().cloned())
-                        .visible(true)
-                    }),
-            );
-        }
-    }
+            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(|comp| {
+                            CompletionCandidate::new(format!(
+                                "{}{}",
+                                dash_or_arg,
+                                comp.get_content().to_string_lossy()
+                            ))
+                            .help(comp.get_help().cloned())
+                            .visible(true)
+                        }),
+                );
+            }
 
-    if let Some(positional) = cmd
-        .get_positionals()
-        .find(|p| p.get_index() == Some(pos_index))
-    {
-        completions.extend(complete_arg_value(arg.to_value(), positional, current_dir).into_iter());
-    }
+            if let Some(positional) = cmd
+                .get_positionals()
+                .find(|p| p.get_index() == Some(pos_index))
+            {
+                completions.extend(complete_arg_value(arg.to_value(), positional, current_dir));
+            }
 
-    if let Ok(value) = arg.to_value() {
-        completions.extend(complete_subcommand(value, cmd));
+            if let Ok(value) = arg.to_value() {
+                completions.extend(complete_subcommand(value, cmd));
+            }
+        }
+        ParseState::Pos(_) => {
+            if let Some(positional) = cmd
+                .get_positionals()
+                .find(|p| p.get_index() == Some(pos_index))
+            {
+                completions.extend(complete_arg_value(arg.to_value(), positional, current_dir));
+            }
+        }
+        ParseState::Opt(opt) => {
+            completions.extend(complete_arg_value(arg.to_value(), &opt, current_dir));
+        }
     }
-
     if completions.iter().any(|a| a.is_visible()) {
         completions.retain(|a| a.is_visible())
     }


Test patch:
diff --git a/clap_complete/tests/testsuite/dynamic.rs b/clap_complete/tests/testsuite/dynamic.rs
index 6eff7f29..7087a8e0 100644
--- a/clap_complete/tests/testsuite/dynamic.rs
+++ b/clap_complete/tests/testsuite/dynamic.rs
@@ -1,5 +1,6 @@
 #![cfg(feature = "unstable-dynamic")]
 
+use std::fs;
 use std::path::Path;
 
 use clap::{builder::PossibleValue, Command};
@@ -9,7 +10,7 @@ macro_rules! complete {
     ($cmd:expr, $input:expr$(, current_dir = $current_dir:expr)? $(,)?) => {
         {
             #[allow(unused)]
-            let current_dir = None;
+            let current_dir: Option<&Path> = None;
             $(let current_dir = $current_dir;)?
             complete(&mut $cmd, $input, current_dir)
         }
@@ -288,6 +289,121 @@ goodbye-world
     );
 }
 
+#[test]
+fn suggest_argument_value() {
+    let mut cmd = Command::new("dynamic")
+        .arg(
+            clap::Arg::new("input")
+                .long("input")
+                .short('i')
+                .value_hint(clap::ValueHint::FilePath),
+        )
+        .arg(
+            clap::Arg::new("format")
+                .long("format")
+                .short('F')
+                .value_parser(["json", "yaml", "toml"]),
+        )
+        .arg(
+            clap::Arg::new("count")
+                .long("count")
+                .short('c')
+                .action(clap::ArgAction::Count),
+        )
+        .arg(clap::Arg::new("positional").value_parser(["pos_a", "pos_b", "pos_c"]))
+        .args_conflicts_with_subcommands(true);
+
+    let testdir = snapbox::dir::DirRoot::mutable_temp().unwrap();
+    let testdir_path = testdir.path().unwrap();
+
+    fs::write(testdir_path.join("a_file"), "").unwrap();
+    fs::write(testdir_path.join("b_file"), "").unwrap();
+    fs::create_dir_all(testdir_path.join("c_dir")).unwrap();
+    fs::create_dir_all(testdir_path.join("d_dir")).unwrap();
+
+    assert_data_eq!(
+        complete!(cmd, "--input [TAB]", current_dir = Some(testdir_path)),
+        snapbox::str![
+            "a_file
+b_file
+c_dir/
+d_dir/"
+        ],
+    );
+
+    assert_data_eq!(
+        complete!(cmd, "-i [TAB]", current_dir = Some(testdir_path)),
+        snapbox::str![
+            "a_file
+b_file
+c_dir/
+d_dir/"
+        ],
+    );
+
+    assert_data_eq!(
+        complete!(cmd, "--input a[TAB]", current_dir = Some(testdir_path)),
+        snapbox::str!["a_file"],
+    );
+
+    assert_data_eq!(
+        complete!(cmd, "-i b[TAB]", current_dir = Some(testdir_path)),
+        snapbox::str!["b_file"],
+    );
+
+    assert_data_eq!(
+        complete!(cmd, "--format [TAB]"),
+        snapbox::str![
+            "json
+yaml
+toml"
+        ],
+    );
+
+    assert_data_eq!(
+        complete!(cmd, "-F [TAB]"),
+        snapbox::str![
+            "json
+yaml
+toml"
+        ],
+    );
+
+    assert_data_eq!(complete!(cmd, "--format j[TAB]"), snapbox::str!["json"],);
+
+    assert_data_eq!(complete!(cmd, "-F j[TAB]"), snapbox::str!["json"],);
+
+    assert_data_eq!(complete!(cmd, "--format t[TAB]"), snapbox::str!["toml"],);
+
+    assert_data_eq!(complete!(cmd, "-F t[TAB]"), snapbox::str!["toml"],);
+
+    assert_data_eq!(
+        complete!(cmd, "-cccF [TAB]"),
+        snapbox::str![
+            "json
+yaml
+toml"
+        ]
+    );
+
+    assert_data_eq!(
+        complete!(cmd, "--input a_file [TAB]"),
+        snapbox::str![
+            "--input
+--format
+--count
+--help\tPrint help
+-i
+-F
+-c
+-h\tPrint help
+pos_a
+pos_b
+pos_c"
+        ]
+    );
+}
+
 fn complete(cmd: &mut Command, args: impl AsRef<str>, current_dir: Option<&Path>) -> String {
     let input = args.as_ref();
     let mut args = vec![std::ffi::OsString::from(cmd.get_name())];