Support multiple values in native completions

[epage]

Blocked on #3920

Both for flags and positional arguments

• code
• tests
• argcomplete and cobra can serve as examples

Tasks

• Support multiple values in native completions #5601

[shannmu]

I have added support to complete multiple values for flags.
But I have some confusion for positional arguments. How does clap::Parser parse the parsing of positional arguments, especially when there are multiple positional arguments with num_args setting? I have found the relevant code

clap/clap_builder/src/parser/parser.rs

Lines 289 to 404 in 6b18d77

	pos_counter = {
	let is_second_to_last = pos_counter + 1 == positional_count;
	// The last positional argument, or second to last positional
	// argument may be set to .multiple_values(true) or `.multiple_occurrences(true)`
	let low_index_mults = is_second_to_last
	&& self.cmd.get_positionals().any(|a| {
	a.is_multiple() && (positional_count != a.get_index().unwrap_or(0))
	})
	&& self
	.cmd
	.get_positionals()
	.last()
	.map(|p_name| !p_name.is_last_set())
	.unwrap_or_default();
	let is_terminated = self
	.cmd
	.get_keymap()
	.get(&pos_counter)
	.map(|a| a.get_value_terminator().is_some())
	.unwrap_or_default();
	let missing_pos = self.cmd.is_allow_missing_positional_set()
	&& is_second_to_last
	&& !trailing_values;
	debug!("Parser::get_matches_with: Positional counter...{pos_counter}");
	debug!("Parser::get_matches_with: Low index multiples...{low_index_mults:?}");
	if (low_index_mults || missing_pos) && !is_terminated {
	let skip_current = if let Some(n) = raw_args.peek(&args_cursor) {
	if let Some(arg) = self
	.cmd
	.get_positionals()
	.find(|a| a.get_index() == Some(pos_counter))
	{
	// If next value looks like a new_arg or it's a
	// subcommand, skip positional argument under current
	// pos_counter(which means current value cannot be a
	// positional argument with a value next to it), assume
	// current value matches the next arg.
	self.is_new_arg(&n, arg)
	|| self
	.possible_subcommand(n.to_value(), valid_arg_found)
	.is_some()
	} else {
	true
	}
	} else {
	true
	};
	if skip_current {
	debug!("Parser::get_matches_with: Bumping the positional counter...");
	pos_counter + 1
	} else {
	pos_counter
	}
	} else if trailing_values
	&& (self.cmd.is_allow_missing_positional_set() || contains_last)
	{
	// Came to -- and one positional has .last(true) set, so we go immediately
	// to the last (highest index) positional
	debug!("Parser::get_matches_with: .last(true) and --, setting last pos");
	positional_count
	} else {
	pos_counter
	}
	};
	if let Some(arg) = self.cmd.get_keymap().get(&pos_counter) {
	if arg.is_last_set() && !trailing_values {
	let _ = self.resolve_pending(matcher);
	// Its already considered a positional, we don't need to suggest turning it
	// into one
	let suggested_trailing_arg = false;
	return Err(ClapError::unknown_argument(
	self.cmd,
	arg_os.display().to_string(),
	None,
	suggested_trailing_arg,
	Usage::new(self.cmd).create_usage_with_title(&[]),
	));
	}
	if arg.is_trailing_var_arg_set() {
	trailing_values = true;
	}
	if matcher.pending_arg_id() != Some(arg.get_id()) || !arg.is_multiple_values_set() {
	ok!(self.resolve_pending(matcher));
	}
	parse_state =
	if let Some(parse_result) = self.check_terminator(arg, arg_os.to_value_os()) {
	debug_assert_eq!(parse_result, ParseResult::ValuesDone);
	pos_counter += 1;
	ParseState::ValuesDone
	} else {
	let arg_values = matcher.pending_values_mut(
	arg.get_id(),
	Some(Identifier::Index),
	trailing_values,
	);
	arg_values.push(arg_os.to_value_os().to_owned());
	// Only increment the positional counter if it doesn't allow multiples
	if !arg.is_multiple() {
	pos_counter += 1;
	ParseState::ValuesDone
	} else {
	ParseState::Pos(arg.get_id().clone())
	}
	};
	valid_arg_found = true;
	} else if let Some(external_parser) =

it would be better if there were documentation explaining it.

[epage]

Another relevant piece of code is

clap/clap_builder/src/builder/debug_asserts.rs

Lines 487 to 674 in 6b18d77

	#[cfg(debug_assertions)]
	fn _verify_positionals(cmd: &Command) -> bool {
	debug!("Command::_verify_positionals");
	// Because you must wait until all arguments have been supplied, this is the first chance
	// to make assertions on positional argument indexes
	//
	// First we verify that the index highest supplied index, is equal to the number of
	// positional arguments to verify there are no gaps (i.e. supplying an index of 1 and 3
	// but no 2)
	let highest_idx = cmd
	.get_keymap()
	.keys()
	.filter_map(|x| {
	if let KeyType::Position(n) = x {
	Some(*n)
	} else {
	None
	}
	})
	.max()
	.unwrap_or(0);
	let num_p = cmd.get_keymap().keys().filter(|x| x.is_position()).count();
	assert!(
	highest_idx == num_p,
	"Found positional argument whose index is {highest_idx} but there \
	are only {num_p} positional arguments defined",
	);
	for arg in cmd.get_arguments() {
	if arg.index.unwrap_or(0) == highest_idx {
	assert!(
	!arg.is_trailing_var_arg_set() || !arg.is_last_set(),
	"{}:{}: `Arg::trailing_var_arg` and `Arg::last` cannot be used together",
	cmd.get_name(),
	arg.get_id()
	);
	if arg.is_trailing_var_arg_set() {
	assert!(
	arg.is_multiple(),
	"{}:{}: `Arg::trailing_var_arg` must accept multiple values",
	cmd.get_name(),
	arg.get_id()
	);
	}
	} else {
	assert!(
	!arg.is_trailing_var_arg_set(),
	"{}:{}: `Arg::trailing_var_arg` can only apply to last positional",
	cmd.get_name(),
	arg.get_id()
	);
	}
	}
	// Next we verify that only the highest index has takes multiple arguments (if any)
	let only_highest = |a: &Arg| a.is_multiple() && (a.get_index().unwrap_or(0) != highest_idx);
	if cmd.get_positionals().any(only_highest) {
	// First we make sure if there is a positional that allows multiple values
	// the one before it (second to last) has one of these:
	// * a value terminator
	// * ArgSettings::Last
	// * The last arg is Required
	// We can't pass the closure (it.next()) to the macro directly because each call to
	// find() (iterator, not macro) gets called repeatedly.
	let last = &cmd.get_keymap()[&KeyType::Position(highest_idx)];
	let second_to_last = &cmd.get_keymap()[&KeyType::Position(highest_idx - 1)];
	// Either the final positional is required
	// Or the second to last has a terminator or .last(true) set
	let ok = last.is_required_set()
	|| (second_to_last.terminator.is_some() || second_to_last.is_last_set())
	|| last.is_last_set();
	assert!(
	ok,
	"Positional argument `{last}` *must* have `required(true)` or `last(true)` set \
	because a prior positional argument (`{second_to_last}`) has `num_args(1..)`"
	);
	// We make sure if the second to last is Multiple the last is ArgSettings::Last
	let ok = second_to_last.is_multiple() || last.is_last_set();
	assert!(
	ok,
	"Only the last positional argument, or second to last positional \
	argument may be set to `.num_args(1..)`"
	);
	// Next we check how many have both Multiple and not a specific number of values set
	let count = cmd
	.get_positionals()
	.filter(|p| {
	p.is_multiple_values_set()
	&& p.get_value_terminator().is_none()
	&& !p.get_num_args().expect(INTERNAL_ERROR_MSG).is_fixed()
	})
	.count();
	let ok = count <= 1
	|| (last.is_last_set()
	&& last.is_multiple()
	&& second_to_last.is_multiple()
	&& count == 2);
	assert!(
	ok,
	"Only one positional argument with `.num_args(1..)` set is allowed per \
	command, unless the second one also has .last(true) set"
	);
	}
	let mut found = false;
	if cmd.is_allow_missing_positional_set() {
	// Check that if a required positional argument is found, all positions with a lower
	// index are also required.
	let mut foundx2 = false;
	for p in cmd.get_positionals() {
	if foundx2 && !p.is_required_set() {
	assert!(
	p.is_required_set(),
	"Found non-required positional argument with a lower \
	index than a required positional argument by two or more: {:?} \
	index {:?}",
	p.get_id(),
	p.get_index()
	);
	} else if p.is_required_set() && !p.is_last_set() {
	// Args that .last(true) don't count since they can be required and have
	// positionals with a lower index that aren't required
	// Imagine: prog <req1> [opt1] -- <req2>
	// Both of these are valid invocations:
	// $ prog r1 -- r2
	// $ prog r1 o1 -- r2
	if found {
	foundx2 = true;
	continue;
	}
	found = true;
	continue;
	} else {
	found = false;
	}
	}
	} else {
	// Check that if a required positional argument is found, all positions with a lower
	// index are also required
	for p in (1..=num_p).rev().filter_map(|n| cmd.get_keymap().get(&n)) {
	if found {
	assert!(
	p.is_required_set(),
	"Found non-required positional argument with a lower \
	index than a required positional argument: {:?} index {:?}",
	p.get_id(),
	p.get_index()
	);
	} else if p.is_required_set() && !p.is_last_set() {
	// Args that .last(true) don't count since they can be required and have
	// positionals with a lower index that aren't required
	// Imagine: prog <req1> [opt1] -- <req2>
	// Both of these are valid invocations:
	// $ prog r1 -- r2
	// $ prog r1 o1 -- r2
	found = true;
	continue;
	}
	}
	}
	assert!(
	cmd.get_positionals().filter(|p| p.is_last_set()).count() < 2,
	"Only one positional argument may have last(true) set. Found two."
	);
	if cmd
	.get_positionals()
	.any(|p| p.is_last_set() && p.is_required_set())
	&& cmd.has_subcommands()
	&& !cmd.is_subcommand_negates_reqs_set()
	{
	panic!(
	"Having a required positional argument with .last(true) set *and* child \
	subcommands without setting SubcommandsNegateReqs isn't compatible."
	);
	}
	true
	}

Positionals must either be

• A fixed number of values (num_args(5))
• last / trailing_var_arg
• second to last argument and last is set on the last

I might be missing some corner cases in there but that is also likely the order of importance for supporting this. We can start with just num_args(N) and have separate issues for last and trailing_var_arg support.

[shannmu]

use clap::{CommandFactory, Parser};
use clap_complete::dynamic::shells::CompleteCommand;
#[derive(Parser, Debug)]
#[clap(name = "dynamic", about = "A dynamic command line tool")]
struct Cli {
    /// The subcommand to run complete
    #[command(subcommand)]
    complete: Option<CompleteCommand>,
    #[clap(value_parser = ["pos_a"], num_args = 2, required = true, index = 1)]
    positional_a: Option<String>,
    #[clap(value_parser = ["pos_b"], num_args = 2, required = true, index = 2)]
    positional_b: Option<String>,
    #[clap(value_parser = ["pos_c"], num_args = 2, last = true, index = 3)]
    positional_c: Option<String>,
}
fn main() {
    let cli = Cli::parse();
    if let Some(completions) = cli.complete {
        completions.complete(&mut Cli::command());
    }
    // normal logic continues...
}

Command line:

dynamic pos_a pos_a pos_b pos_b -- pos_c pos_c

will report an error:

error: 2 values required for '<POSITIONAL_A> <POSITIONAL_A>' but 4 were provided

Usage: dynamic <POSITIONAL_A> <POSITIONAL_A> <POSITIONAL_B> <POSITIONAL_B> [-- <POSITIONAL_C> <POSITIONAL_C>]

For more information, try '--help'.

How should I use this cli?

[epage]

Huh, looks like something isn't working quite right. I even tried throwing in --divider flags to see if that would cause the positionals to resolve_pending and advance the counter and it doesn't.

Updated code

#!/usr/bin/env nargo
---
[dependencies]
clap = { path = "../clap", features = ["derive"] }
---

use clap::Parser;
use clap::builder::ArgAction;

#[derive(Parser, Debug)]
#[command(name = "dynamic", about = "A dynamic command line tool")]
struct Cli {
    #[arg(value_parser = ["pos_a"], num_args = 2, action = ArgAction::Set, required = true)]
    positional_a: Vec<String>,
    #[arg(value_parser = ["pos_b"], num_args = 2, action = ArgAction::Set, required = true)]
    positional_b: Vec<String>,
    #[arg(value_parser = ["pos_c"], num_args = 2, action = ArgAction::Set, last = true)]
    positional_c: Vec<String>,
    #[arg(short, long, action = ArgAction::Count)]
    divider: u8,
}
fn main() {
    let cli = Cli::parse();
    print!("{cli:?}");
}

[shannmu]

Is it possible that positional arguments were not designed to be used this way (multiple positional arguments with num_args set)? If not, this might be a Parser related bug. Understanding this is pretty important for resolving the issue, as it affects when to complete the positional arguments.
I'll keep an eye on this issue. If there's a bug related to the Parser, I'd like to fix it.

[epage]

Some thought was put into positionals and num_args but not in fully scaling it out like this. These are bugs and would ideally be fixed but are not blockers for the work being done here.

[epage]

#5601 got us multi-value support.

It left out

• trailing_var_arg: noted as unblocking Stablize Rust-Native Completion Engine Tracking Issue #3166
• last: noted as unblocking Stablize Rust-Native Completion Engine Tracking Issue #3166
• dynamic --flag=bar1 [TAB]: this isn't support in clap's parser

By controlling the scope of this issue, it looks like we can close it out.