value_terminator unexpectedly eating with multiple multi-length arguments

[kleptog]

Please complete the following tasks

• I have searched the discussions
• I have searched the open and rejected issues

Rust Version

rustc 1.69.0 (84c898d65 2023-04-16)

Clap Version

master (3fa7b8f)

Minimal reproducible code

clap::{self, Arg, ArgAction};

fn main() {
    // Define the command line interface using Clap
    let matches = clap::Command::new("Tester")
        .version("1.0")
        .about("Testing")
        .arg(
            Arg::new("program1")
                .action(ArgAction::Append)
                .required(true)
                .allow_hyphen_values(true)
                .value_terminator("--")
                .help("Arguments for the first command line program")
                .num_args(0..)
                .last(false),
        )
        .arg(
            Arg::new("program2")
                .action(ArgAction::Append)
                // .required(true)
                .allow_hyphen_values(true)
                .help("Arguments for the second command line program")
                .num_args(0..)
                .last(true),
        )
        .get_matches();

    // Extract the values of the command line arguments
    let program1: Vec<&str> = matches.get_many::<String>("program1")
        .unwrap_or_default()
        .map(|v| v.as_str())
        .collect();
    let program2: Vec<&str> = matches.get_many::<String>("program2")
        .unwrap_or_default()
        .map(|v| v.as_str())
        .collect();

    // Print the extracted values (for demonstration purposes)
    eprintln!("Program 1: {:?}", program1);
    eprintln!("Program 2: {:?}", program2);
}

Steps to reproduce the bug with the above code

cargo run -- echo foo -- echo bar

Actual Behaviour

Program 1: ["echo", "foo", "echo", "bar"]
Program 2: []

Expected Behaviour

Program 1: ["echo", "foo"]
Program 2: ["echo", "bar"]

Additional Context

What I'm trying to do is have a command line parser that accepts two multivalue arguments which are going to be used to spawn two programs. And it almost works. If you comment out the two lines:

                .allow_hyphen_values(true)
                .value_terminator("--")

It actually appear to works as expected. However, if you try to the pass argument to the programs, like so:

cargo run -- echo -e foo -- echo bar

it breaks because clap assumes the "-e" is for it rather than for the subprogram. Setting allow_hyphen_values(true) should fix that, except it now swallows the entire rest of the arguments into program1. I thought value_terminator() should fix that, except it just vanishes.

Now, it may be that this isn't the right way to do this, but the current behavior violates the Principle of Least Astonishment. Either the double-hyphen should terminate the argument, or it should be included in the result. Making the second argument required or not makes no difference.

I tried following the code to figure out exactly which bit needs changing, but I just can't figure it out. I think the point where it checks for the terminator it should do something other than ignoring it (the debug line about "ignoring terminator result").

Thanks in advance.

Debug Output

[clap_builder::builder::command]Command::_do_parse
[clap_builder::builder::command]Command::_build: name="Interactive Output Comparator"
[clap_builder::builder::command]Command::_propagate:Interactive Output Comparator
[clap_builder::builder::command]Command::_check_help_and_version:Interactive Output Comparator expand_help_tree=false
[clap_builder::builder::command]Command::long_help_exists
[clap_builder::builder::command]Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]Command::_check_help_and_version: Building default --version
[clap_builder::builder::command]Command::_propagate_global_args:Interactive Output Comparator
[clap_builder::builder::debug_asserts]Command::_debug_asserts
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:program1
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:program2
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:version
[clap_builder::builder::debug_asserts]Command::_verify_positionals
[clap_builder::parser::parser]Parser::get_matches_with
[clap_builder::parser::parser]Parser::get_matches_with: Begin parsing '"echo"'
[clap_builder::parser::parser]Parser::possible_subcommand: arg=Ok("echo")
[clap_builder::parser::parser]Parser::get_matches_with: sc=None
[clap_builder::parser::parser]Parser::get_matches_with: Positional counter...1
[clap_builder::parser::parser]Parser::get_matches_with: Low index multiples...false
[clap_builder::parser::parser]Parser::get_matches_with: Begin parsing '"foo"'
[clap_builder::parser::parser]Parser::get_matches_with: Positional counter...1
[clap_builder::parser::parser]Parser::get_matches_with: Low index multiples...false
[clap_builder::parser::parser]Parser::get_matches_with: Begin parsing '"--"'
[clap_builder::parser::parser]Parser::get_matches_with: Positional counter...1
[clap_builder::parser::parser]Parser::get_matches_with: Low index multiples...false
[clap_builder::parser::parser]Parser::check_terminator: terminator=Some("--")
[clap_builder::parser::parser]Parser::get_matches_with: ignoring terminator result ValuesDone
[clap_builder::parser::parser]Parser::get_matches_with: Begin parsing '"echo"'
[clap_builder::parser::parser]Parser::get_matches_with: Positional counter...1
[clap_builder::parser::parser]Parser::get_matches_with: Low index multiples...false
[clap_builder::parser::parser]Parser::get_matches_with: Begin parsing '"bar"'
[clap_builder::parser::parser]Parser::get_matches_with: Positional counter...1
[clap_builder::parser::parser]Parser::get_matches_with: Low index multiples...false
[clap_builder::parser::parser]Parser::resolve_pending: id="program1"
[clap_builder::parser::parser]Parser::react action=Append, identifier=Some(Index), source=CommandLine
[clap_builder::parser::parser]Parser::remove_overrides: id="program1"
[clap_builder::parser::arg_matcher]ArgMatcher::start_custom_arg: id="program1", source=CommandLine
[clap_builder::builder::command]Command::groups_for_arg: id="program1"
[clap_builder::parser::parser]Parser::push_arg_values: ["echo", "foo", "echo", "bar"]
[clap_builder::parser::parser]Parser::add_single_val_to_arg: cur_idx:=1
[clap_builder::parser::parser]Parser::add_single_val_to_arg: cur_idx:=2
[clap_builder::parser::parser]Parser::add_single_val_to_arg: cur_idx:=3
[clap_builder::parser::parser]Parser::add_single_val_to_arg: cur_idx:=4
[clap_builder::parser::arg_matcher]ArgMatcher::needs_more_vals: o=program1, pending=0
[clap_builder::parser::arg_matcher]ArgMatcher::needs_more_vals: expected=0..=18446744073709551615, actual=0
[clap_builder::parser::parser]Parser::react not enough values passed in, leaving it to the validator to complain
[clap_builder::parser::parser]Parser::add_defaults
[clap_builder::parser::parser]Parser::add_defaults:iter:program1:
[clap_builder::parser::parser]Parser::add_default_value: doesn't have conditional defaults
[clap_builder::parser::parser]Parser::add_default_value:iter:program1: doesn't have default vals
[clap_builder::parser::parser]Parser::add_defaults:iter:program2:
[clap_builder::parser::parser]Parser::add_default_value: doesn't have conditional defaults
[clap_builder::parser::parser]Parser::add_default_value:iter:program2: doesn't have default vals
[clap_builder::parser::parser]Parser::add_defaults:iter:help:
[clap_builder::parser::parser]Parser::add_default_value: doesn't have conditional defaults
[clap_builder::parser::parser]Parser::add_default_value:iter:help: doesn't have default vals
[clap_builder::parser::parser]Parser::add_defaults:iter:version:
[clap_builder::parser::parser]Parser::add_default_value: doesn't have conditional defaults
[clap_builder::parser::parser]Parser::add_default_value:iter:version: doesn't have default vals
[clap_builder::parser::validator]Validator::validate
[clap_builder::builder::command]Command::groups_for_arg: id="program1"
[clap_builder::parser::validator]Conflicts::gather_direct_conflicts id="program1", conflicts=[]
[clap_builder::parser::validator]Validator::validate_conflicts
[clap_builder::parser::validator]Validator::validate_exclusive
[clap_builder::parser::validator]Validator::validate_conflicts::iter: id="program1"
[clap_builder::parser::validator]Conflicts::gather_conflicts: arg="program1"
[clap_builder::parser::validator]Conflicts::gather_conflicts: conflicts=[]
[clap_builder::parser::validator]Validator::validate_required: required=ChildGraph([Child { id: "program1", children: [] }])
[clap_builder::parser::validator]Validator::gather_requires
[clap_builder::parser::validator]Validator::gather_requires:iter:"program1"
[clap_builder::parser::validator]Validator::validate_required: is_exclusive_present=false
[clap_builder::parser::arg_matcher]ArgMatcher::get_global_values: global_arg_vec=[]

[epage]

4.3.0 is out with the fix

[kleptog]

Wow, thanks for the quick fix. I've checked it out and I've made it work.

The only weird bit is that if I leave the .last(true) in program2 like in my example program, then it breaks with the error:

error: unexpected argument 'echo' found

which seems weird. It's not a problem because I can just leave .last(false) (which is the default), but it's just unexpected to me.

[epage]

Its giving you that error because it is processing the last argument without coming across the required escape (--). Instead -- is being treated as an argument to program1 due to allow_hyphen_values and you are (cleverly) leveraging value_terminator to emulate escaping but its not the same thing and so last won't work.

[kleptog]

Thank you. In retrospect it is obvious. Everything is working as expected now.

Thanks again!