Skip to content

Commit

Permalink
fix(complete): Handle multi-valued arguments
Browse files Browse the repository at this point in the history
zsh completions for commands that have multiple Vec arguments require
special care.

We can have two Vec args separated with a value terminator.
We can also have two Vec args with no value terminators specified
where the final arg uses 'raw' and thus requires '--' to be used.

The 2nd of these scenarios requires special handling to avoid
emitting a duplicate '*:arguments' completion entry.

Currently, the zsh completions generate an error in this scenario:

    $ my-app <TAB>
    _arguments:...: doubled rest argument definition:
    *::second -- second set of of multi-length arguments:

We already use the '-S' option when calling _arguments.
This option makes it so that completion stops after '--' is encountered.
This means that the handling for trailing 'raw' arguments does not need
to specified.

Special-case multi-valued arguments so that we can skip emitting
the final multi-valued argument if a previous multi-valued argument
has already been emitted.

Closes clap-rs#3022
Signed-off-by: David Aguilar <davvid@gmail.com>
  • Loading branch information
davvid committed Jan 10, 2023
1 parent b86c259 commit 170bd59
Show file tree
Hide file tree
Showing 12 changed files with 231 additions and 2 deletions.
32 changes: 30 additions & 2 deletions clap_complete/src/shells/zsh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,18 +622,46 @@ fn write_positionals_of(p: &Command) -> String {

let mut ret = vec![];

// Completions for commands that end with two Vec arguments require special care.
// - You can have two Vec args separated with a custom value terminator.
// - You can have two Vec args with the second one set to last (raw sets last)
// which will require a '--' separator to be used before the second argument
// on the command-line.
//
// We use the '-S' _arguments option to disable completion after '--'. Thus, the
// completion for the second argument in scenario (B) does not need to be emitted
// because it is implicitly handled by the '-S' option.
// We only need to emit the first catch-all.
//
// Have we already emitted a catch-all multi-valued positional argument
// without a custom value terminator?
let mut catch_all_emitted = false;

for arg in p.get_positionals() {
debug!("write_positionals_of:iter: arg={}", arg.get_id());

let num_args = arg.get_num_args().expect("built");
let is_multi_valued = num_args.max_values() > 1;

if catch_all_emitted && (arg.is_last_set() || is_multi_valued) {
// This is the final argument and it also takes multiple arguments.
// We've already emitted a catch-all positional argument so we don't need
// to emit anything for this argument because it is implicitly handled by
// the use of the '-S' _arguments option.
continue;
}

let cardinality_value;
let cardinality = if num_args.max_values() > 1 {
let cardinality = if is_multi_valued {
match arg.get_value_terminator() {
Some(terminator) => {
cardinality_value = format!("*{}:", escape_value(terminator));
cardinality_value.as_str()
}
None => "*:",
None => {
catch_all_emitted = true;
"*:"
}
}
} else if !arg.is_required_set() {
":"
Expand Down
12 changes: 12 additions & 0 deletions clap_complete/tests/bash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
14 changes: 14 additions & 0 deletions clap_complete/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 12 additions & 0 deletions clap_complete/tests/elvish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
12 changes: 12 additions & 0 deletions clap_complete/tests/fish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
12 changes: 12 additions & 0 deletions clap_complete/tests/powershell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
38 changes: 38 additions & 0 deletions clap_complete/tests/snapshots/two_multi_valued_arguments.bash
Original file line number Diff line number Diff line change
@@ -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
26 changes: 26 additions & 0 deletions clap_complete/tests/snapshots/two_multi_valued_arguments.elvish
Original file line number Diff line number Diff line change
@@ -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]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
complete -c my-app -s h -l help -d 'Print help'
32 changes: 32 additions & 0 deletions clap_complete/tests/snapshots/two_multi_valued_arguments.ps1
Original file line number Diff line number Diff line change
@@ -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
}
30 changes: 30 additions & 0 deletions clap_complete/tests/snapshots/two_multi_valued_arguments.zsh
Original file line number Diff line number Diff line change
@@ -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]' \
'*::first -- first multi-valued argument:' \
&& ret=0
}

(( $+functions[_my-app_commands] )) ||
_my-app_commands() {
local commands; commands=()
_describe -t commands 'my-app commands' commands "$@"
}

_my-app "$@"
12 changes: 12 additions & 0 deletions clap_complete/tests/zsh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}

0 comments on commit 170bd59

Please sign in to comment.