clap icon indicating copy to clipboard operation
clap copied to clipboard

`TrailingVarArg` doesn't work without `--`

Open naftulikay opened this issue 4 years ago • 17 comments

Maintainer's notes

  • Workaround: call Command::allow_hyphen_values(true)

Rust Version

Stable

Affected Version of clap

Latest

Bug or Feature Request Summary

I am unable to write a program which accepts a variable amount of final args which include flag-like values without using a delimiter.

Expected Behavior Summary

I'd like to be able to have my variable amount of args be of any format, including flag-like options.

Actual Behavior Summary

Parsing fails.

Steps to Reproduce the issue

I have created a playground example which demonstrates the problem directly.

Sample Code or Link to Sample Code

Playground

fn parser<'a, 'b>() -> App<'a, 'b> {
    App::new("varargs")
        .setting(AppSettings::TrailingVarArg)
        // boolean
        .arg(Arg::with_name("prog-flag")
            .long("prog-flag")
            .takes_value(false))
        // path to executable
        .arg(Arg::with_name("EXECUTABLE")
            .takes_value(true)
            .required(true))
        // list of arguments to pass to executable
        .arg(Arg::with_name("ARGS")
            .takes_value(true)
            .multiple(true)
            .allow_hyphen_values(true))
}

#[test]
fn test_with_delimiter() {
    parser().get_matches_from_safe(vec!["--prog-flag", "executable", "--", "-a", "1"]).unwrap();
}

#[test]
fn test_without_delimiter() {
    parser().get_matches_from_safe(vec!["--prog-flag", "executable", "-a", "1"]).unwrap();
}

My exact use-case is that I have a positional argument which is a path to an executable, followed by a list of arguments to pass to that executable.

Example:

./my-program --prog-flag /usr/bin/watch -n 1 echo true

Internally, I'm using --prog-flag to customize my application's behavior, the executable path to find the binary to execute, and the variable length of args to pass to the program as arguments.

I can't seem to find a way to write the above without needing a delimiter (--) between executable and the argument list.

naftulikay avatar Aug 31 '19 00:08 naftulikay

Do we really want this?

ldm0 avatar Aug 14 '21 06:08 ldm0

Yeah, this looks like a bug.

pksunkara avatar Aug 14 '21 09:08 pksunkara

+ 1 on this one. This is behavior that's quite common seen when using getopts_long. Is there a way to stop clap-rs from parsing the the incoming args as soon as it hit something unknown? That would solve this.

kristof-mattei avatar Oct 27 '21 22:10 kristof-mattei

This would be very useful for me. Is there a work-around?

lpil avatar Jan 13 '22 17:01 lpil

+1 This is a semi-common scenario. I have the same use case as the OP. Any plans to implement this feature?

alexreg avatar Jan 30 '22 01:01 alexreg

The work-around for me right now is just to get rid of the EXECUTABLE arg (see the original post) and extract that from the first item of ARGS. That's not too bad in fairness.

alexreg avatar Jan 30 '22 02:01 alexreg

I just ported the example code to clap3 and it seems to work for me

fn main() {
    let m = parser().get_matches();
    dbg!(m.is_present("prog-flag"));
    dbg!(m
        .values_of("EXECUTABLE")
        .unwrap_or_default()
        .collect::<Vec<_>>());
    dbg!(m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>());
}

fn parser<'a>() -> clap::App<'a> {
    clap::App::new("varargs")
        .setting(clap::AppSettings::TrailingVarArg)
        // boolean
        .arg(
            clap::Arg::new("prog-flag")
                .long("prog-flag")
                .takes_value(false),
        )
        // path to executable
        .arg(
            clap::Arg::new("EXECUTABLE")
                .takes_value(true)
                .required(true),
        )
        // list of arguments to pass to executable
        .arg(
            clap::Arg::new("ARGS")
                .takes_value(true)
                .multiple_values(true)
                .allow_hyphen_values(true),
        )
}

#[test]
fn test_with_delimiter() {
    parser()
        .try_get_matches_from(vec!["--prog-flag", "executable", "--", "-a", "1"])
        .unwrap();
}

#[test]
fn test_without_delimiter() {
    parser()
        .try_get_matches_from(vec!["--prog-flag", "executable", "-a", "1"])
        .unwrap();
}

Am I missing something or does it look like this was fixed and we can close it?

epage avatar Feb 01 '22 20:02 epage

@epage It seems that particular example works, but it still fails if you follow executable with a double-dash option. For example:

#[test]
fn test_without_delimiter() {
    parser()
        .try_get_matches_from(vec!["--prog-flag", "executable", "--foo", "1"])
        .unwrap();
}

alexreg avatar Feb 01 '22 21:02 alexreg

Thanks for calling out that other case! I was able to reproduce that. I threw together #3389 as part of looking at this. long arguments are missing some of the checks that short arguments have. This PR just copied over one of them. Some more work is needed on it. I'm giving myself some pause to consider how disruptive of a change any of this is. My guess is I'll consider it to not be a breaking change but will want it called out in a compatibility section in the CHANGELOG (like Rust does). I want to reserve those for minor releases (I am preparing for a minor release atm).

If anyone wants to pick this up, that'd be a big help.

epage avatar Feb 02 '22 19:02 epage

@epage Thanks a lot for looking into this. That sounds reasonable, regarding how you treat this change in behaviour.

I was just thinking of corner cases, and what stands out is the situation when the first of the multiple positional arguments can be interpreted as a flag.

Example:

    App::new("varargs")
        .setting(AppSettings::TrailingVarArg)
        .arg(Arg::with_name("prog-flag")
            .long("prog-flag")
            .takes_value(false))
        // Note that `EXECUTABLE` is no longer present.
        .arg(Arg::with_name("ARGS")
            .takes_value(true)
            .multiple(true)
            .allow_hyphen_values(true))
["--prog-flag", "executable", "--foo", "1"]

Here, --prog-flag should of course get processed as one of the CLI flags. If we changed it instead to

["--fake-flag", "executable", "--foo", "1"]

Would the idea be to treat --fake-flag as the first value of ARGS, since it is not recognised as an actual CLI flag?

alexreg avatar Feb 02 '22 22:02 alexreg

Could you create a separate issue for that?

I ran into those cases as well though I've still been mulling over my thoughts on what should be done.

As for current behavior, It looks like we start processing ARGS as soon as its the next positional argument, even if that means we can't process any flags. This seems like an issue and has probably more compatibility concerns. This also applies to short and long.

One of the questions I have for this is if we should check for flags between EXECUTABLE and ARGS. Its been a bit since I dug into this but I thought clap allows intermixing but provided a way to not intermix (might also be remembering of the other libs I've been researching).

Another is a concern over backwards compatibility for user applications. If we start ARGS on the first unrecognized flag, then any new flag added breaks compatibility.

epage avatar Feb 03 '22 00:02 epage

Just to hopefully clarify things, as I don't think I mentioned this in my bug report, my use case is creating something like sudo. sudo has options (like -i or -u), and then you pass the executable name and all subsequent arguments are passed to the child process exec call.

The project in question is naftulikay/escalator. As opposed to sudo, escalator does not spawn a child process but rather replaces the current process with the spawned one.

Examples of usage:

  • escalator my-program -a -b --c d
  • escalator -u some-user --verbose my-program -a -b --c d

Presently, as can be seen in the docs for escalator, I must pass -- between the executable name and the arguments. I have not tested with clap 3 yet.

naftulikay avatar Feb 03 '22 21:02 naftulikay

@epage One issue with some of the test cases listed in this issue: the calls to try_get_matches_from need to include the program name as the first argument. Fixing that makes some of the examples work that don't work otherwise.

joshtriplett avatar Apr 28 '22 05:04 joshtriplett

Thanks! Thats easy to overlook

epage avatar Apr 28 '22 14:04 epage

Did some more comparisons

#!/usr/bin/env -S rust-script --debug

//! ```cargo
//! [dependencies]
//! #clap = { path = "../clap" }
//! clap = "3"
//! ```

fn main() {
    let all_args: &[&[&str]] = &[
        &["cmd", "--prog-flag", "executable", "--apple", "1"],
        &["cmd", "--prog-flag", "executable", "--", "--apple", "1"],
        &["cmd", "--prog-flag", "executable", "-a", "1"],
        &["cmd", "--prog-flag", "executable", "--", "-a", "1"],
    ];
    for args in all_args {
        dbg!(args);
        let cmd = clap::App::new("varargs")
            .setting(clap::AppSettings::TrailingVarArg)
            .setting(clap::AppSettings::AllowLeadingHyphen)
            // boolean
            .arg(
                clap::Arg::with_name("prog-flag")
                    .long("prog-flag")
                    .takes_value(false),
            )
            // path to executable
            .arg(
                clap::Arg::with_name("EXECUTABLE")
                    .takes_value(true)
                    .required(true),
            )
            // list of arguments to pass to executable
            .arg(
                clap::Arg::with_name("ARGS")
                    .takes_value(true)
                    .multiple(true)
                    .allow_hyphen_values(true),
            );
        let m = cmd.get_matches_from_safe(*args);
        match m {
            Ok(m) => {
                dbg!(m.is_present("prog-flag"));
                dbg!(m
                    .values_of("EXECUTABLE")
                    .unwrap_or_default()
                    .collect::<Vec<_>>());
                dbg!(m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>());
            }
            Err(err) => {
                eprintln!("{}", err);
            }
        }
    }
}

clap v2 without setting(clap::AppSettings::AllowLeadingHyphen)

[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--apple",
    "1",
]
error: Found argument '--apple' which wasn't expected, or isn't valid in this context

USAGE:
    cmd <EXECUTABLE> --prog-flag

For more information try --help

[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--",
    "--apple",
    "1",
]
[clap-1538.rs:41] m.is_present("prog-flag") = true
[clap-1538.rs:42] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:46] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "--apple",
    "1",
]
[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "-a",
    "1",
]
error: Found argument '-a' which wasn't expected, or isn't valid in this context

USAGE:
    cmd <EXECUTABLE> --prog-flag

For more information try --help

[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--",
    "-a",
    "1",
]
[clap-1538.rs:41] m.is_present("prog-flag") = true
[clap-1538.rs:42] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:46] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "-a",
    "1",
]

clap v2 with setting(clap::AppSettings::AllowLeadingHyphen)

[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--apple",
    "1",
]
[clap-1538.rs:42] m.is_present("prog-flag") = true
[clap-1538.rs:43] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:47] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "--apple",
    "1",
]
[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--",
    "--apple",
    "1",
]
[clap-1538.rs:42] m.is_present("prog-flag") = true
[clap-1538.rs:43] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:47] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "--apple",
    "1",
]
[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "-a",
    "1",
]
[clap-1538.rs:42] m.is_present("prog-flag") = true
[clap-1538.rs:43] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:47] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "-a",
    "1",
]
[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--",
    "-a",
    "1",
]
[clap-1538.rs:42] m.is_present("prog-flag") = true
[clap-1538.rs:43] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:47] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "-a",
    "1",
]

clap v3 without setting(clap::AppSettings::AllowLeadingHyphen)

[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--apple",
    "1",
]
error: Found argument '--apple' which wasn't expected, or isn't valid in this context

	If you tried to supply `--apple` as a value rather than a flag, use `-- --apple`

USAGE:
    cmd --prog-flag <EXECUTABLE>

For more information try --help

[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--",
    "--apple",
    "1",
]
[clap-1538.rs:41] m.is_present("prog-flag") = true
[clap-1538.rs:42] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:46] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "--apple",
    "1",
]
[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "-a",
    "1",
]
[clap-1538.rs:41] m.is_present("prog-flag") = true
[clap-1538.rs:42] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:46] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "-a",
    "1",
]
[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--",
    "-a",
    "1",
]
[clap-1538.rs:41] m.is_present("prog-flag") = true
[clap-1538.rs:42] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:46] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "-a",
    "1",
]

clap v3 with setting(clap::AppSettings::AllowLeadingHyphen)

[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--apple",
    "1",
]
[clap-1538.rs:42] m.is_present("prog-flag") = true
[clap-1538.rs:43] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:47] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "--apple",
    "1",
]
[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--",
    "--apple",
    "1",
]
[clap-1538.rs:42] m.is_present("prog-flag") = true
[clap-1538.rs:43] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:47] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "--apple",
    "1",
]
[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "-a",
    "1",
]
[clap-1538.rs:42] m.is_present("prog-flag") = true
[clap-1538.rs:43] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:47] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "-a",
    "1",
]
[clap-1538.rs:16] args = [
    "cmd",
    "--prog-flag",
    "executable",
    "--",
    "-a",
    "1",
]
[clap-1538.rs:42] m.is_present("prog-flag") = true
[clap-1538.rs:43] m.values_of("EXECUTABLE").unwrap_or_default().collect::<Vec<_>>() = [
    "executable",
]
[clap-1538.rs:47] m.values_of("ARGS").unwrap_or_default().collect::<Vec<_>>() = [
    "-a",
    "1",
]

epage avatar Aug 09 '22 19:08 epage

The fact that Arg::allow_hyphen_values doesn't fix the issue but Command::allow_hyphen_values is confusing and an example of why we need to address https://github.com/clap-rs/clap/issues/3450

epage avatar Aug 09 '22 20:08 epage

As noted in #4039, since Command::allow_hyphen_values looks to be a workaround, I am de-prioritizing an immediate fix so we can evaluate some of the larger questions

  • What changes are breaking changes and can be backported to v3-master and what changes must stay in master (v4)
    • What is the justification for it no being breaking?
    • What is the scope and impact of a breaking change? Can we accept that cost, even for v4?
  • Why do we need allow_hyphen_values at both Command and Arg level, see https://github.com/clap-rs/clap/issues/3450
  • Why do we need Command::trailing_var_arg rather than assuming it from the last argument?
  • Why is trailing_var_arg on Command rather than Arg, like last?

As well, when we resolve this, we should

  • Ensure trailing_var_arg and last are cross-linked
  • Ensure trailing_var_arg discusses the role of allow_hyphen_values

epage avatar Aug 09 '22 21:08 epage

I'm feeling fairly confident in the behavior in #4187

epage avatar Sep 07 '22 02:09 epage