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

Document that multi-value options do not get along with trailing subcommands #1721

Closed
nagisa opened this issue Mar 4, 2020 · 13 comments · Fixed by #1834
Closed

Document that multi-value options do not get along with trailing subcommands #1721

nagisa opened this issue Mar 4, 2020 · 13 comments · Fixed by #1834
Labels
A-docs Area: documentation, including docs.rs, readme, examples, etc...
Milestone

Comments

@nagisa
Copy link

nagisa commented Mar 4, 2020

Rust Version

rustc 1.43.0-nightly (7760cd0fb 2020-02-19)

Code

use clap::*;

fn main() {
    let matches = App::new("My Super Program")
                      .subcommand(SubCommand::with_name("test"))
                      .arg(Arg::with_name("config")
                           .long("config")
                           .value_name("FILE")
                           .multiple(true)
                           .takes_value(true))
                      .get_matches();
    println!("{:?}", matches);
}

Steps to reproduce the issue

  1. cargo run -- --config 'banana=true' test
  2. cargo run -- --config='banana=true' test
  3. Note how clap parses the last argument.

Affected Version of clap*

2.33

Actual Behavior Summary

clap will greedily consume values into space-separated --config flag.

Expected Behavior Summary

I’m not sure how common place this behaviour (--long arg1 arg2 parsing as --long arg1 --long arg2) is. Definitely was unexpected to me.

@nagisa nagisa added the C-bug Category: Updating dependencies label Mar 4, 2020
@CreepySkeleton
Copy link
Contributor

Yeah, this is a known problem, but clap simply doesn't know where the options end and where the subcommand starts. Even if there's a way to rule this out, I'm not aware of it, so I doubt this will change. Take it as limitation.

I think we should document it better though.

We also consider to improve error reporting in such cases.

@CreepySkeleton CreepySkeleton changed the title multiple(true) top-level flags do not interact well with subcommands. Document that multi-value options do not get along with trailing subcommands Mar 4, 2020
@CreepySkeleton CreepySkeleton added A-docs Area: documentation, including docs.rs, readme, examples, etc... and removed C-bug Category: Updating dependencies labels Mar 4, 2020
@pksunkara
Copy link
Member

Check this for more info, #1610 and please give us feedback if that explains your issue. I am thinking of pointing to that issue in the docs for multiple.

@nagisa
Copy link
Author

nagisa commented Mar 4, 2020

Is there a way to implement an option that would require each value for the multiple flag to be accompanied with the flag? That is to disable this implicit token consumption? I’m sure most of the users will be plenty happy with --flag val1 val2... not parsing the val2 into --flag.

EDIT: That is, I originally expected multiple to mean that it would require something along the lines of --flag val1 --flag val2... and if I wanted to supply multiple values without repeating the --flag I would be responsible for setting use_delimiter.

EDIT: Ah, I see, its require_delimiters. Cool. Definitely very difficult to discover.

@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Mar 4, 2020

The more I think about it, the more I get assured that we need two different methods for that: multiple_values and multiple_occurrences.

  • multiple_values(true): --foo val1 val2 but not -foo val1 --foo val2
  • multiple_occurrences(true): -foo val1 --foo val2 but not --foo val1 val2
  • multiple_values(true).multiple_occurrences(true): enables both

@nagisa
Copy link
Author

nagisa commented Mar 4, 2020

I got mostly the behaviour I want by setting require_delimiters(true).value_delimiter("\0"), so this is not critical by any means.

@CreepySkeleton
Copy link
Contributor

But it should be documented better nonetheless.

@davidMcneil
Copy link
Contributor

davidMcneil commented Apr 15, 2020

Yeah, this is a known problem, but clap simply doesn't know where the options end and where the subcommand starts. Even if there's a way to rule this out, I'm not aware of it, so I doubt this will change. Take it as limitation.

@CreepySkeleton could we check if the next token matches a subcommand and if it does stop parsing the argument and instead parse the subcommand? Essentially, remove this line. It looks like this used to be the behavior, but was changed due to #1031.

My personal opinion is that handling (1) "multi-value options with subcommands" is more important than (2) "argument value taking precedence over subcommand with the same name". The reasons being:

  • 1 occurs more frequently than 2. How often do arg values and subcommands with the same name occur next to each other?
  • 2 the user of the cli can easily fix 2 by simply adding an =. There is no good way for the user to get around 1 without the author of the app already taking special action.

@pksunkara
Copy link
Member

We are happy to add support for something like this with an AppSetting. But it would have to be something for later and not a priority for 3.0

@CreepySkeleton
Copy link
Contributor

If we remove that line, positional with multiple values would stop working at all; the logic over there looks far more complicated.

  1. I believe, when it comes to defaults, this kind of decisions should not be based on personal opinions, yours and mine all the same. For example, my experience is exactly contrary here.

  2. This is a good argument.

I'd go for changing the default behavior (because 2) and introducing an AppSetting or something so developer could revert it back.

@pksunkara
Copy link
Member

I wouldn't change the default but introduce an AppSetting. I feel the default should be how it is behaving now.

@CreepySkeleton
Copy link
Contributor

Maybe we should conduct a survey of sorts? Ask on users forum or something, "Here's the problem, which default would you like and why?". Our opinions... well, they are our opinions, and we are not the majority here, despite we are indeed the greatest!

@pksunkara
Copy link
Member

Adding -- after multi value options is the standard in CLI tools. We had said this many times in the past to people who came to us with this issue. And this is also the main reason we should keep it as a default.

@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Apr 16, 2020

No no, these are different problems.

  • -- is about "everything after this delimiter belongs to a single clap Arg, no matter what it starts from (subcommand name, -, whatever). It is used as the last option in CLI, no option can possibly follow it because it would be considered as a value.
    app --option subcommand --option2 -- --option3 abc subcommand
        -----------------------------    ------------------------
         normal options and subcmds      this is not parsed at all
                                         just a list of values
    
  • This problem is about "if an option taking unrestrained number of values contains something that is a valid subcommand among these values, the subcommand (and everything following) must not be considered as value". In other words, such option can be followed by something else. This is possible today via
    app --option val1 val2 val3 subcommand arg
                 -------------- ---------- ---
                      values     sub name  sub argument
    

To be honest, I personally don't like this design. I consider this design error prone and pretty non-intuitive, I would not write such a CLI and I would try to dissuade people from this sort of desing. But, even thought I'm admittedly a very wise person, I'm not the whole community. This is why I'm proposing to ask. Just to hear people out.

If we find their argument not so compelling or opinions will differ significantly, we will just pull our Dictatorship card and chant "Succumb to our will!".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation, including docs.rs, readme, examples, etc...
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants