Repository: clap-rs/clap
Pull request #4624 (2023-01-10 20:25:32)

fix(completion): emit value terminators on zsh
Subset of #4612


Patch:
diff --git a/clap_complete/src/shells/zsh.rs b/clap_complete/src/shells/zsh.rs
index 580de77a..15a9ed12 100644
--- a/clap_complete/src/shells/zsh.rs
+++ b/clap_complete/src/shells/zsh.rs
@@ -626,8 +626,15 @@ fn write_positionals_of(p: &Command) -> String {
         debug!("write_positionals_of:iter: arg={}", arg.get_id());
 
         let num_args = arg.get_num_args().expect("built");
+        let cardinality_value;
         let cardinality = if num_args.max_values() > 1 {
-            "*:"
+            match arg.get_value_terminator() {
+                Some(terminator) => {
+                    cardinality_value = format!("*{}:", escape_value(terminator));
+                    cardinality_value.as_str()
+                }
+                None => "*:",
+            }
         } else if !arg.is_required_set() {
             ":"
         } else {
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
index 19df8111..ee16eaac 100644
--- a/clap_derive/src/derives/args.rs
+++ b/clap_derive/src/derives/args.rs
@@ -298,14 +298,6 @@ pub fn gen_augment(
                         }
                     }
 
-                    Ty::VecVec | Ty::OptionVecVec => {
-                        quote_spanned! { ty.span() =>
-                            .value_name(#value_name)
-                            #value_parser
-                            #action
-                        }
-                    }
-
                     Ty::Other => {
                         let required = item.find_default_method().is_none();
                         // `ArgAction::takes_values` is assuming `ArgAction::default_value` will be
@@ -460,9 +452,7 @@ pub fn gen_constructor(fields: &[(&Field, Item)]) -> TokenStream {
                     Ty::Unit |
                     Ty::Vec |
                     Ty::OptionOption |
-                    Ty::OptionVec |
-                    Ty::VecVec |
-                    Ty::OptionVecVec => {
+                    Ty::OptionVec => {
                         abort!(
                             ty.span(),
                             "{} types are not supported for subcommand",
@@ -501,9 +491,7 @@ pub fn gen_constructor(fields: &[(&Field, Item)]) -> TokenStream {
                     Ty::Unit |
                     Ty::Vec |
                     Ty::OptionOption |
-                    Ty::OptionVec |
-                    Ty::VecVec |
-                    Ty::OptionVecVec => {
+                    Ty::OptionVec => {
                         abort!(
                             ty.span(),
                             "{} types are not supported for flatten",
@@ -642,7 +630,6 @@ fn gen_parsers(
     let id = item.id();
     let get_one = quote_spanned!(span=> remove_one::<#convert_type>);
     let get_many = quote_spanned!(span=> remove_many::<#convert_type>);
-    let get_occurrences = quote_spanned!(span=> remove_occurrences::<#convert_type>);
     let deref = quote!(|s| s);
     let parse = quote_spanned!(span=> |s| ::std::result::Result::Ok::<_, clap::Error>(s));
 
@@ -699,17 +686,6 @@ fn gen_parsers(
             }
         }
 
-        Ty::VecVec => quote_spanned! { ty.span()=>
-            #arg_matches.#get_occurrences(#id)
-                .map(|g| g.map(::std::iter::Iterator::collect).collect::<Vec<Vec<_>>>())
-                .unwrap_or_else(Vec::new)
-        },
-
-        Ty::OptionVecVec => quote_spanned! { ty.span()=>
-            #arg_matches.#get_occurrences(#id)
-                .map(|g| g.map(::std::iter::Iterator::collect).collect::<Vec<Vec<_>>>())
-        },
-
         Ty::Other => {
             quote_spanned! { ty.span()=>
                 #arg_matches.#get_one(#id)
diff --git a/clap_derive/src/item.rs b/clap_derive/src/item.rs
index 5e8272ac..a068e360 100644
--- a/clap_derive/src/item.rs
+++ b/clap_derive/src/item.rs
@@ -1121,7 +1121,7 @@ impl Action {
 fn default_action(field_type: &Type, span: Span) -> Method {
     let ty = Ty::from_syn_ty(field_type);
     let args = match *ty {
-        Ty::Vec | Ty::OptionVec | Ty::VecVec | Ty::OptionVecVec => {
+        Ty::Vec | Ty::OptionVec => {
             quote_spanned! { span=>
                 clap::ArgAction::Append
             }
diff --git a/clap_derive/src/utils/ty.rs b/clap_derive/src/utils/ty.rs
index 9349bc29..1cf05144 100644
--- a/clap_derive/src/utils/ty.rs
+++ b/clap_derive/src/utils/ty.rs
@@ -11,11 +11,9 @@ use syn::{
 pub enum Ty {
     Unit,
     Vec,
-    VecVec,
     Option,
     OptionOption,
     OptionVec,
-    OptionVecVec,
     Other,
 }
 
@@ -26,13 +24,13 @@ impl Ty {
 
         if is_unit_ty(ty) {
             t(Unit)
-        } else if let Some(vt) = get_vec_ty(ty, Vec, VecVec) {
-            t(vt)
+        } else if is_generic_ty(ty, "Vec") {
+            t(Vec)
         } else if let Some(subty) = subty_if_name(ty, "Option") {
             if is_generic_ty(subty, "Option") {
                 t(OptionOption)
-            } else if let Some(vt) = get_vec_ty(subty, OptionVec, OptionVecVec) {
-                t(vt)
+            } else if is_generic_ty(subty, "Vec") {
+                t(OptionVec)
             } else {
                 t(Option)
             }
@@ -48,8 +46,6 @@ impl Ty {
             Self::Option => "Option<T>",
             Self::OptionOption => "Option<Option<T>>",
             Self::OptionVec => "Option<Vec<T>>",
-            Self::VecVec => "Vec<Vec<T>>",
-            Self::OptionVecVec => "Option<Vec<Vec<T>>>",
             Self::Other => "...other...",
         }
     }
@@ -59,13 +55,9 @@ pub fn inner_type(field_ty: &syn::Type) -> &syn::Type {
     let ty = Ty::from_syn_ty(field_ty);
     match *ty {
         Ty::Vec | Ty::Option => sub_type(field_ty).unwrap_or(field_ty),
-        Ty::OptionOption | Ty::OptionVec | Ty::VecVec => {
+        Ty::OptionOption | Ty::OptionVec => {
             sub_type(field_ty).and_then(sub_type).unwrap_or(field_ty)
         }
-        Ty::OptionVecVec => sub_type(field_ty)
-            .and_then(sub_type)
-            .and_then(sub_type)
-            .unwrap_or(field_ty),
         _ => field_ty,
     }
 }
@@ -147,19 +139,3 @@ where
 {
     iter.next().filter(|_| iter.next().is_none())
 }
-
-#[cfg(feature = "unstable-v5")]
-fn get_vec_ty(ty: &Type, vec_ty: Ty, vecvec_ty: Ty) -> Option<Ty> {
-    subty_if_name(ty, "Vec").map(|subty| {
-        if is_generic_ty(subty, "Vec") {
-            vecvec_ty
-        } else {
-            vec_ty
-        }
-    })
-}
-
-#[cfg(not(feature = "unstable-v5"))]
-fn get_vec_ty(ty: &Type, vec_ty: Ty, _vecvec_ty: Ty) -> Option<Ty> {
-    is_generic_ty(ty, "Vec").then_some(vec_ty)
-}
diff --git a/src/builder/arg.rs b/src/builder/arg.rs
index 53f68075..bea0807f 100644
--- a/src/builder/arg.rs
+++ b/src/builder/arg.rs
@@ -3858,6 +3858,13 @@ impl Arg {
         self.val_delim
     }
 
+    /// Get the value terminator for this argument. The value_terminator is a value
+    /// that terminates parsing of multi-valued arguments.
+    #[inline]
+    pub fn get_value_terminator(&self) -> Option<&Str> {
+        self.terminator.as_ref()
+    }
+
     /// Get the index of this argument, if any
     #[inline]
     pub fn get_index(&self) -> Option<usize> {


Test patch:
diff --git a/clap_complete/tests/bash.rs b/clap_complete/tests/bash.rs
index a3e454de..d01651f2 100644
--- a/clap_complete/tests/bash.rs
+++ b/clap_complete/tests/bash.rs
@@ -84,6 +84,18 @@ fn value_hint() {
     );
 }
 
+#[test]
+fn value_terminator() {
+    let name = "my-app";
+    let cmd = common::value_terminator_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/value_terminator.bash",
+        clap_complete::shells::Bash,
+        cmd,
+        name,
+    );
+}
+
 #[cfg(feature = "unstable-dynamic")]
 #[test]
 fn register_minimal() {
diff --git a/clap_complete/tests/common.rs b/clap_complete/tests/common.rs
index ab52fe93..3f8cb437 100644
--- a/clap_complete/tests/common.rs
+++ b/clap_complete/tests/common.rs
@@ -252,6 +252,15 @@ pub fn value_hint_command(name: &'static str) -> clap::Command {
         )
 }
 
+pub fn value_terminator_command(name: &'static str) -> clap::Command {
+    clap::Command::new(name).arg(
+        clap::Arg::new("arguments")
+            .help("multi-valued argument with a value terminator")
+            .num_args(1..)
+            .value_terminator(";"),
+    )
+}
+
 pub fn assert_matches_path(
     expected_path: impl AsRef<std::path::Path>,
     gen: impl clap_complete::Generator,
diff --git a/clap_complete/tests/elvish.rs b/clap_complete/tests/elvish.rs
index 76beb983..26075a67 100644
--- a/clap_complete/tests/elvish.rs
+++ b/clap_complete/tests/elvish.rs
@@ -83,3 +83,15 @@ fn value_hint() {
         name,
     );
 }
+
+#[test]
+fn value_terminator() {
+    let name = "my-app";
+    let cmd = common::value_terminator_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/value_terminator.elvish",
+        clap_complete::shells::Elvish,
+        cmd,
+        name,
+    );
+}
diff --git a/clap_complete/tests/fish.rs b/clap_complete/tests/fish.rs
index 92c1c4a4..9393ed69 100644
--- a/clap_complete/tests/fish.rs
+++ b/clap_complete/tests/fish.rs
@@ -83,3 +83,15 @@ fn value_hint() {
         name,
     );
 }
+
+#[test]
+fn value_terminator() {
+    let name = "my-app";
+    let cmd = common::value_terminator_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/value_terminator.fish",
+        clap_complete::shells::Fish,
+        cmd,
+        name,
+    );
+}
diff --git a/clap_complete/tests/powershell.rs b/clap_complete/tests/powershell.rs
index fa6657d3..1f18618b 100644
--- a/clap_complete/tests/powershell.rs
+++ b/clap_complete/tests/powershell.rs
@@ -83,3 +83,15 @@ fn value_hint() {
         name,
     );
 }
+
+#[test]
+fn value_terminator() {
+    let name = "my-app";
+    let cmd = common::value_terminator_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/value_terminator.ps1",
+        clap_complete::shells::PowerShell,
+        cmd,
+        name,
+    );
+}
diff --git a/clap_complete/tests/snapshots/value_terminator.bash b/clap_complete/tests/snapshots/value_terminator.bash
new file mode 100644
index 00000000..3b4bc730
--- /dev/null
+++ b/clap_complete/tests/snapshots/value_terminator.bash
@@ -0,0 +1,38 @@
+_my-app() {
+    local i cur prev opts cmds
+    COMPREPLY=()
+    cur="${COMP_WORDS[COMP_CWORD]}"
+    prev="${COMP_WORDS[COMP_CWORD-1]}"
+    cmd=""
+    opts=""
+
+    for i in ${COMP_WORDS[@]}
+    do
+        case "${cmd},${i}" in
+            ",$1")
+                cmd="my__app"
+                ;;
+            *)
+                ;;
+        esac
+    done
+
+    case "${cmd}" in
+        my__app)
+            opts="-h --help [arguments]..."
+            if [[ ${cur} == -* || ${COMP_CWORD} -eq 1 ]] ; then
+                COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
+                return 0
+            fi
+            case "${prev}" in
+                *)
+                    COMPREPLY=()
+                    ;;
+            esac
+            COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
+            return 0
+            ;;
+    esac
+}
+
+complete -F _my-app -o bashdefault -o default my-app
diff --git a/clap_complete/tests/snapshots/value_terminator.elvish b/clap_complete/tests/snapshots/value_terminator.elvish
new file mode 100644
index 00000000..89b26e00
--- /dev/null
+++ b/clap_complete/tests/snapshots/value_terminator.elvish
@@ -0,0 +1,26 @@
+
+use builtin;
+use str;
+
+set edit:completion:arg-completer[my-app] = {|@words|
+    fn spaces {|n|
+        builtin:repeat $n ' ' | str:join ''
+    }
+    fn cand {|text desc|
+        edit:complex-candidate $text &display=$text' '(spaces (- 14 (wcswidth $text)))$desc
+    }
+    var command = 'my-app'
+    for word $words[1..-1] {
+        if (str:has-prefix $word '-') {
+            break
+        }
+        set command = $command';'$word
+    }
+    var completions = [
+        &'my-app'= {
+            cand -h 'Print help'
+            cand --help 'Print help'
+        }
+    ]
+    $completions[$command]
+}
diff --git a/clap_complete/tests/snapshots/value_terminator.fish b/clap_complete/tests/snapshots/value_terminator.fish
new file mode 100644
index 00000000..a33c671a
--- /dev/null
+++ b/clap_complete/tests/snapshots/value_terminator.fish
@@ -0,0 +1,1 @@
+complete -c my-app -s h -l help -d 'Print help'
diff --git a/clap_complete/tests/snapshots/value_terminator.ps1 b/clap_complete/tests/snapshots/value_terminator.ps1
new file mode 100644
index 00000000..30262afe
--- /dev/null
+++ b/clap_complete/tests/snapshots/value_terminator.ps1
@@ -0,0 +1,32 @@
+
+using namespace System.Management.Automation
+using namespace System.Management.Automation.Language
+
+Register-ArgumentCompleter -Native -CommandName 'my-app' -ScriptBlock {
+    param($wordToComplete, $commandAst, $cursorPosition)
+
+    $commandElements = $commandAst.CommandElements
+    $command = @(
+        'my-app'
+        for ($i = 1; $i -lt $commandElements.Count; $i++) {
+            $element = $commandElements[$i]
+            if ($element -isnot [StringConstantExpressionAst] -or
+                $element.StringConstantType -ne [StringConstantType]::BareWord -or
+                $element.Value.StartsWith('-') -or
+                $element.Value -eq $wordToComplete) {
+                break
+        }
+        $element.Value
+    }) -join ';'
+
+    $completions = @(switch ($command) {
+        'my-app' {
+            [CompletionResult]::new('-h', 'h', [CompletionResultType]::ParameterName, 'Print help')
+            [CompletionResult]::new('--help', 'help', [CompletionResultType]::ParameterName, 'Print help')
+            break
+        }
+    })
+
+    $completions.Where{ $_.CompletionText -like "$wordToComplete*" } |
+        Sort-Object -Property ListItemText
+}
diff --git a/clap_complete/tests/snapshots/value_terminator.zsh b/clap_complete/tests/snapshots/value_terminator.zsh
new file mode 100644
index 00000000..656d8ea1
--- /dev/null
+++ b/clap_complete/tests/snapshots/value_terminator.zsh
@@ -0,0 +1,30 @@
+#compdef my-app
+
+autoload -U is-at-least
+
+_my-app() {
+    typeset -A opt_args
+    typeset -a _arguments_options
+    local ret=1
+
+    if is-at-least 5.2; then
+        _arguments_options=(-s -S -C)
+    else
+        _arguments_options=(-s -C)
+    fi
+
+    local context curcontext="$curcontext" state line
+    _arguments "${_arguments_options[@]}" \
+'-h[Print help]' \
+'--help[Print help]' \
+'*;::arguments -- multi-valued argument with a value terminator:' \
+&& ret=0
+}
+
+(( $+functions[_my-app_commands] )) ||
+_my-app_commands() {
+    local commands; commands=()
+    _describe -t commands 'my-app commands' commands "$@"
+}
+
+_my-app "$@"
diff --git a/clap_complete/tests/zsh.rs b/clap_complete/tests/zsh.rs
index df6a2778..57040bde 100644
--- a/clap_complete/tests/zsh.rs
+++ b/clap_complete/tests/zsh.rs
@@ -83,3 +83,15 @@ fn value_hint() {
         name,
     );
 }
+
+#[test]
+fn value_terminator() {
+    let name = "my-app";
+    let cmd = common::value_terminator_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/value_terminator.zsh",
+        clap_complete::shells::Zsh,
+        cmd,
+        name,
+    );
+}
diff --git a/tests/derive/main.rs b/tests/derive/main.rs
index 072aab96..0c6ea7bd 100644
--- a/tests/derive/main.rs
+++ b/tests/derive/main.rs
@@ -22,7 +22,6 @@ mod macros;
 mod naming;
 mod nested_subcommands;
 mod non_literal_attributes;
-mod occurrences;
 mod options;
 mod privacy;
 mod raw_bool_literal;
diff --git a/tests/derive/occurrences.rs b/tests/derive/occurrences.rs
deleted file mode 100644
index db4ae37e..00000000
--- a/tests/derive/occurrences.rs
+++ /dev/null
@@ -1,82 +0,0 @@
-#![cfg(all(feature = "unstable-grouped", feature = "unstable-v5"))]
-use clap::Parser;
-
-#[test]
-fn test_vec_of_vec() {
-    #[derive(Parser, Debug, PartialEq)]
-    struct Opt {
-        #[arg(short = 'p', num_args = 2)]
-        points: Vec<Vec<i32>>,
-    }
-
-    assert_eq!(
-        Opt {
-            points: vec![vec![1, 2], vec![0, 0]]
-        },
-        Opt::try_parse_from(&["test", "-p", "1", "2", "-p", "0", "0"]).unwrap()
-    );
-}
-
-#[test]
-fn test_vec_of_vec_opt_out() {
-    fn parser(s: &str) -> Result<Vec<String>, std::convert::Infallible> {
-        Ok(s.split(',').map(str::to_owned).collect())
-    }
-
-    #[derive(Parser, PartialEq, Debug)]
-    struct Opt {
-        #[arg(value_parser = parser, short = 'p')]
-        arg: Vec<::std::vec::Vec<String>>,
-    }
-
-    assert_eq!(
-        Opt {
-            arg: vec![vec!["1".into(), "2".into()], vec!["a".into(), "b".into()]],
-        },
-        Opt::try_parse_from(["test", "-p", "1,2", "-p", "a,b"]).unwrap(),
-    );
-}
-
-#[test]
-fn test_vec_vec_empty() {
-    #[derive(Parser, Debug, PartialEq)]
-    struct Opt {
-        #[arg(short = 'p', num_args = 2)]
-        points: Vec<Vec<i32>>,
-    }
-
-    assert_eq!(
-        Opt { points: vec![] },
-        Opt::try_parse_from(&["test"]).unwrap()
-    );
-}
-
-#[test]
-fn test_option_vec_vec() {
-    #[derive(Parser, Debug, PartialEq)]
-    struct Opt {
-        #[arg(short = 'p', num_args = 2)]
-        points: Option<Vec<Vec<i32>>>,
-    }
-
-    assert_eq!(
-        Opt {
-            points: Some(vec![vec![1, 2], vec![3, 4]])
-        },
-        Opt::try_parse_from(&["test", "-p", "1", "2", "-p", "3", "4"]).unwrap()
-    );
-}
-
-#[test]
-fn test_option_vec_vec_empty() {
-    #[derive(Parser, Debug, PartialEq)]
-    struct Opt {
-        #[arg(short = 'p', num_args = 2)]
-        points: Option<Vec<Vec<i32>>>,
-    }
-
-    assert_eq!(
-        Opt { points: None },
-        Opt::try_parse_from(&["test"]).unwrap()
-    );
-}