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

Multi valued flags and required argument do not work together very well #1125

Open
rsertelon opened this issue Dec 8, 2017 · 6 comments
Open
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing

Comments

@rsertelon
Copy link

Rust Version

  • 1.22.1

Affected Version of clap

  • 2.29.0

The habitat project uses clap for its command line parsing. There are currently two issues opened related to how clap generates help message and/or how it parses the command line arguments.

Issues are:

In the source code of habitat, here's the interesting bit.

The subcommand load has both a required @arg PKG_IDENT_OR_ARTIFACT + required + takes_value and a multivalued @arg BIND: --bind +takes_value +multiple

The problem is that the help message prints:

USAGE:
    hab-sup load [FLAGS] [OPTIONS] <PKG_IDENT>

But if you try to actually follow the help message while using the --bind flag and specify the <PKG_IDENT> as the last argument, it won't work as clap thinks it's still a --bind value. However, if --bind argument is placed as the last one, it'll work as expected.

So there's either a discrepancy in the help message, or/and a multivalue parsing that may be to greedy for when there's an expected required argument.

I understand this problem is not really trivial, I've seen an issue that might address this, but it feels weird to have to add a terminator to the --bind flag from a user perspective.

Hope the problem has been made clearly, don't hesitate if you need further information!

@kbknapp
Copy link
Member

kbknapp commented Jan 9, 2018

@rsertelon sorry for the long delay! Holidays and travel have had me gone for a while.

Although this isn't a trivial problem (how to tell clap when --bind is done and <PKG_IDENT> starts), there is a trivial fix if it works with your use case.

  • Limit --bind to a single value per occurrence. So for example, --bind value1 --bind value2 is legal, but --bind value1 value2 is not. This is the easiest fix and my recommended solution. To do this, declare --bind like so: @arg BIND: --bind +takes_value +multiple number_of_values(1)

There are some other things you could try like Arg::last(true) (or +last for the macro version). Which will change your usage string to hab-sup load [FLAGS] [OPTIONS] [--] <PKG_IDENT> the downside it that it requires -- to be used to access <PKG_IDENT>.

Finally, you could also just set a custom usage string for that one subcommand, to make it hab-sup load [FLAGS] <PKG_IDENT> [OPTIONS].

@kbknapp
Copy link
Member

kbknapp commented Jan 9, 2018

I have some ideas on things that could fix this...but I'm far from implementing them yet and they wouldn't probably happen until v3. I'm thinking of things like filters, where one could say, "This particular arg only accepts values that meet some criteria...if they don't fit, look at other args."

@rsertelon
Copy link
Author

Hey @kbknapp no problem! Thanks for your answer. I'll discuss this with the dev team at habitat. The workaround might be a good solution, filters might be nice too! :)

@pksunkara pksunkara added C: args E-hard Call for participation: Experience needed to fix: Hard / a lot and removed T: RFC / question labels Apr 1, 2020
@pksunkara pksunkara added this to the 3.0 milestone Feb 21, 2021
@pksunkara pksunkara removed the W: 3.x label May 26, 2021
@pksunkara pksunkara removed this from the 3.0 milestone May 26, 2021
@epage epage added A-parsing Area: Parser's logic and needs it changed somehow. C-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing and removed C: args E-hard Call for participation: Experience needed to fix: Hard / a lot labels Dec 8, 2021
@marcospb19
Copy link
Contributor

+1, I'd like to share the code I expected to succeed.

Here's the minified example for the issue I found in Ouch:

use std::{ffi::OsString, path::PathBuf};

use clap::Parser;

#[derive(Parser, Debug)]
pub enum Subcommand {
    Compress {
        #[arg(required = true)]
        files: Vec<PathBuf>,

        #[arg(required = true)]
        output: PathBuf,

        #[arg(short, long)]
        format: Option<OsString>,
    },
}

fn main() {
    let inputs = [
        "ouch compress a b c output --format tar.gz",
        "ouch compress a b c --format tar.gz output",
        "ouch compress a b --format tar.gz c output",
        "ouch compress a --format tar.gz b c output",
        "ouch compress --format tar.gz a b c output",
    ];
    for input in inputs {
        let result = Subcommand::try_parse_from(input.split_whitespace());
        let result = result.map(|_| ()).map_err(|_| ());
        println!("{input}, {result:?}");
    }
}

I was expecting it all to be Ok(()), but instead some Err(())s:

"ouch compress a b c output --format tar.gz", Ok(())
"ouch compress a b c --format tar.gz output", Err(())
"ouch compress a b --format tar.gz c output", Err(())
"ouch compress a --format tar.gz b c output", Err(())
"ouch compress --format tar.gz a b c output", Ok(())

To my surprise, the exact same applies for flags that don't take a value:

"ouch compress a b c output --verbose", Ok(())
"ouch compress a b c --verbose output", Err(())
"ouch compress a b --verbose c output", Err(())
"ouch compress a --verbose b c output", Err(())
"ouch compress --verbose a b c output", Ok(())

Is there a way around this?

(I'd consider this to be C-bug instead of C-enhancement 🤔 but that's just because I expect CLI tools in general to extract flags out of the way of positional arguments, so you never have to worry where you insert a flag)

@epage
Copy link
Member

epage commented Sep 8, 2023

@marcospb19 I suspect your case's root cause is independent of this issue and I'd recommend making your own and following the template.

@marcospb19
Copy link
Contributor

👍 I made the example smaller and created #5115.

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-enhancement Category: Raise on the bar on expectations S-waiting-on-design Status: Waiting on user-facing design to be resolved before implementing
Projects
None yet
Development

No branches or pull requests

5 participants