Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TrailingVarArg doesn't work without -- #1538

Closed
naftulikay opened this issue Aug 31, 2019 · 18 comments · Fixed by #4187
Closed

TrailingVarArg doesn't work without -- #1538

naftulikay opened this issue Aug 31, 2019 · 18 comments · Fixed by #4187
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-bug Category: Updating dependencies S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.

Comments

@naftulikay
Copy link

naftulikay commented Aug 31, 2019

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.

@CreepySkeleton CreepySkeleton self-assigned this Feb 1, 2020
@CreepySkeleton CreepySkeleton changed the title Unable to Parse Variable-Length Flag-Like Args Without Delimiter TrailingVarArg doesn't work if the first trailing args starts with - Feb 6, 2020
@CreepySkeleton CreepySkeleton changed the title TrailingVarArg doesn't work if the first trailing args starts with - TrailingVarArg doesn't work if the first trailing arg starts with - Feb 6, 2020
@CreepySkeleton CreepySkeleton added C: args A-parsing Area: Parser's logic and needs it changed somehow. C-bug Category: Updating dependencies and removed cc: CreepySkeleton labels Feb 6, 2020
@CreepySkeleton CreepySkeleton added this to the 3.1 milestone Feb 6, 2020
@CreepySkeleton CreepySkeleton removed their assignment Mar 3, 2020
@pksunkara pksunkara removed the W: 3.x label Jun 5, 2021
@ldm0 ldm0 changed the title TrailingVarArg doesn't work if the first trailing arg starts with - TrailingVarArg doesn't work without -- Aug 14, 2021
@ldm0
Copy link
Member

ldm0 commented Aug 14, 2021

Do we really want this?

@pksunkara
Copy link
Member

Yeah, this looks like a bug.

@kristof-mattei
Copy link

kristof-mattei commented Oct 27, 2021

+ 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.

@epage epage removed C: args labels Dec 8, 2021
@epage epage removed this from the 3.1 milestone Dec 9, 2021
@epage epage added S-waiting-on-mentor Status: Needs elaboration on the details before doing a 'Call for participation' and removed P2: need to have labels Dec 9, 2021
@lpil
Copy link

lpil commented Jan 13, 2022

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

@alexreg
Copy link

alexreg commented Jan 30, 2022

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

@alexreg
Copy link

alexreg commented Jan 30, 2022

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.

@epage
Copy link
Member

epage commented Feb 1, 2022

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 epage added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-mentor Status: Needs elaboration on the details before doing a 'Call for participation' labels Feb 1, 2022
@alexreg
Copy link

alexreg commented Feb 1, 2022

@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();
}

@epage
Copy link
Member

epage commented Feb 2, 2022

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.

@alexreg
Copy link

alexreg commented Feb 2, 2022

@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?

@epage
Copy link
Member

epage commented Feb 3, 2022

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.

@naftulikay
Copy link
Author

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.

@joshtriplett
Copy link
Contributor

@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.

@epage
Copy link
Member

epage commented Apr 28, 2022

Thanks! Thats easy to overlook

@epage
Copy link
Member

epage commented Aug 9, 2022

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
Copy link
Member

epage commented Aug 9, 2022

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 #3450

@epage
Copy link
Member

epage commented Aug 9, 2022

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

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 pushed a commit to epage/clap that referenced this issue Sep 7, 2022
…first pos

This makes it match up with `Command::allow_hyphen_values` which was the
guiding factor for what the behavior should be.

This supersedes clap-rs#4039

Fixes clap-rs#3880
Fixes clap-rs#1538
@epage
Copy link
Member

epage commented Sep 7, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-bug Category: Updating dependencies S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
9 participants