fix(complete): Handle multi-valued arguments

zsh completions for commands that end with two Vec arguments require
special care to handle the transition from the penultimate multi-valued
argument and the final multi-valued argument.

We can have two Vec args separated with a value terminator.
We can also have two Vec args with no value terminators specified.
Both of these scenarios require a terminator in the zsh completions.

Special-case the penultimate multi-valued argument and force it to emit
a value terminator when the final argument also takes multiple values
or has the `last` option set.

Closes clap-rs#3022
Signed-off-by: David Aguilar <davvid@gmail.com>

clap_complete/src/shells/zsh.rs

@@ -622,18 +622,49 @@ fn write_positionals_of(p: &Command) -> String {
 
     let mut ret = vec![];
 
-    for arg in p.get_positionals() {
+    // The index of the 2nd-to-last positional argument.
+    let positionals: Vec<_> = p.get_positionals().collect();
+    let penultimate_idx = if positionals.len() > 1 {
+        Some(positionals.len() - 2)
+    } else {
+        None
+    };
+    // Completions for commands that end with two Vec arguments require special care:
+    // - You can have two Vec args separated with a value_terminator.
+    // - You can have two Vec args with the second one set to last (raw sets last)
+    //   which will force a separator of -- before the second.
+    // zsh requires a terminator in both of these situations.
+    let needs_terminator = match positionals.last() {
+        Some(arg) => arg.is_last_set() || arg.get_num_args().expect("built").max_values() > 1,
+        None => false,
+    };
+
+    for (idx, arg) in positionals.iter().enumerate() {
         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 {
+            // The current argument takes multiple values.
+            // Is it the penultimate argument?
+            let is_penultimate = penultimate_idx.map(|value| value == idx).unwrap_or(false);
             match arg.get_value_terminator() {
                 Some(terminator) => {
                     cardinality_value = format!("*{}:", escape_value(terminator));
                     cardinality_value.as_str()
                 }
-                None => "*:",
+                None => {
+                    // If this is the penultimate argument (which takes multiple
+                    // arguments), and the next (final) argument also takes multiple
+                    // values, then a terminator must be used to separate the arguments.
+                    // "--" is used when no value terminator has been specified.
+                    // Otherwise, no terminator is needed and we can use "*".
+                    if is_penultimate && needs_terminator {
+                        "*--:"
+                    } else {
+                        "*:"
+                    }
+                }
             }
         } else if !arg.is_required_set() {
             ":"

clap_complete/tests/bash.rs

@@ -111,3 +111,15 @@ fn register_minimal() {
         .action_env("SNAPSHOTS")
         .matches_path("tests/snapshots/register_minimal.bash", buf);
 }
+
+#[test]
+fn two_multi_valued_arguments() {
+    let name = "my-app";
+    let cmd = common::two_multi_valued_arguments_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/two_multi_valued_arguments.bash",
+        clap_complete::shells::Bash,
+        cmd,
+        name,
+    );
+}

clap_complete/tests/common.rs

@@ -261,6 +261,20 @@ pub fn value_terminator_command(name: &'static str) -> clap::Command {
     )
 }
 
+pub fn two_multi_valued_arguments_command(name: &'static str) -> clap::Command {
+    clap::Command::new(name)
+        .arg(
+            clap::Arg::new("first")
+                .help("first multi-valued argument")
+                .num_args(1..),
+        )
+        .arg(
+            clap::Arg::new("second")
+                .help("second multi-valued argument")
+                .raw(true),
+        )
+}
+
 pub fn assert_matches_path(
     expected_path: impl AsRef<std::path::Path>,
     gen: impl clap_complete::Generator,

clap_complete/tests/elvish.rs

@@ -95,3 +95,15 @@ fn value_terminator() {
         name,
     );
 }
+
+#[test]
+fn two_multi_valued_arguments() {
+    let name = "my-app";
+    let cmd = common::two_multi_valued_arguments_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/two_multi_valued_arguments.elvish",
+        clap_complete::shells::Elvish,
+        cmd,
+        name,
+    );
+}

clap_complete/tests/fish.rs

@@ -95,3 +95,15 @@ fn value_terminator() {
         name,
     );
 }
+
+#[test]
+fn two_multi_valued_arguments() {
+    let name = "my-app";
+    let cmd = common::two_multi_valued_arguments_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/two_multi_valued_arguments.fish",
+        clap_complete::shells::Fish,
+        cmd,
+        name,
+    );
+}

clap_complete/tests/powershell.rs

@@ -95,3 +95,15 @@ fn value_terminator() {
         name,
     );
 }
+
+#[test]
+fn two_multi_valued_arguments() {
+    let name = "my-app";
+    let cmd = common::two_multi_valued_arguments_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/two_multi_valued_arguments.ps1",
+        clap_complete::shells::PowerShell,
+        cmd,
+        name,
+    );
+}

clap_complete/tests/snapshots/two_multi_valued_arguments.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 [first]... [second]..."
+            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

clap_complete/tests/snapshots/two_multi_valued_arguments.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]
+}

clap_complete/tests/snapshots/two_multi_valued_arguments.fish

@@ -0,0 +1 @@
+complete -c my-app -s h -l help -d 'Print help'

clap_complete/tests/snapshots/two_multi_valued_arguments.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
+}

clap_complete/tests/snapshots/two_multi_valued_arguments.zsh

@@ -0,0 +1,31 @@
+#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]' \
+'*--::first -- first multi-valued argument:' \
+'*::second -- second multi-valued argument:' \
+&& ret=0
+}
+
+(( $+functions[_my-app_commands] )) ||
+_my-app_commands() {
+    local commands; commands=()
+    _describe -t commands 'my-app commands' commands "$@"
+}
+
+_my-app "$@"

clap_complete/tests/zsh.rs

@@ -95,3 +95,15 @@ fn value_terminator() {
         name,
     );
 }
+
+#[test]
+fn two_multi_valued_arguments() {
+    let name = "my-app";
+    let cmd = common::two_multi_valued_arguments_command(name);
+    common::assert_matches_path(
+        "tests/snapshots/two_multi_valued_arguments.zsh",
+        clap_complete::shells::Zsh,
+        cmd,
+        name,
+    );
+}