Clap: Subcommands impossible to use with required arguments with ArgsNegateSubcommands

[glfmn]

Rust Version

rustc 1.22.1 (05e2e1c41 2017-11-22)
cargo 0.23.0 (61fa02415 2017-11-22)

Affected Version of clap

clap 2.29.0 (registry+https://github.com/rust-lang/crates.io-index)

Expected Behavior Summary

When setting ArgsNegateSubcommands, subcommands should work without required arguments that do not belong to the subcommand as it will be impossible to invoke the subcommand if the required arguments negate it. The usage string reflects this with:

USAGE:
    bin [OPTIONS] <REQUIRED>
    bin <SUBCOMMAND>

Actual Behavior Summary

It becomes impossible to invoke the subcommand, as required arguments negate the subcommand, but the subcommand requires the arguments. The fix is to set SubcommandsNegateReqs in addition to ArgsNegateSubcommands, but this is very confusing at first and conflicts with the usage string which says:

USAGE:
    bin [OPTIONS] <REQUIRED>
    bin <SUBCOMMAND>

Steps to Reproduce the issue

Create an application with the macro:

        let matches = clap_app!(app =>
            (version: crate_version!())
            (author: crate_authors!())
            (about: crate_description!())
            (@arg REQUIRED: +required "required argument")
            (@setting ArgsNegateSubcommands)
            (@subcommand sub =>
                (about: "impossible to invoke subcommand")
            )
        ).get_matches();

$ cargo run -- sub

error: The following required arguments were not provided:
    <REQUIRED>

USAGE:
    app [OPTIONS] <REQUIRED>
    app <SUBCOMMAND>

For more information try --help

$ cargo run -- asdf sub

error: Found argument 'sub' which wasn't expected, or isn't valid in this context

USAGE:
    app [OPTIONS] <REQUIRED>
    app <SUBCOMMAND>

For more information try --help

Sample Code or Link to Sample Code

Here is a gist with my full code which produced the issue, and steps to reproduce with the errors:

$ cargo run -- isrepo

error: The following required arguments were not provided:
    <FORMAT>

USAGE:
    gist [OPTIONS] <FORMAT>
    gist <SUBCOMMAND>

For more information try --help

$ cargo run -- asdf isrepo

error: Found argument 'isrepo' which wasn't expected, or isn't valid in this context

USAGE:
    gist [OPTIONS] <FORMAT>
    gist <SUBCOMMAND>

For more information try --help

[kbknapp]

I agree it can be confusing. I'm going to file this a docs issue as I think that's the best way forward for the time being. Simply adding a line in the docs explaining how requirements work with those two AppSettings should work for now.

I don't want to implicitly set one or the other because there could be times when you don't want both settings applied.

[glfmn]

When would both settings _not_ be appropriate, or even usable? A simple program like mine likely lacks the complexity to require such a situation, and I haven't tried to write a more complicated command-line tool, and I agree that updating the docs is a good fix in the interim, but the biggest problem is that the behaviour contradicts the usage string generated by clap-rs. How can we address that?

[kbknapp]

When would both settings not be appropriate, or even usable?

It's only an issue if there is a required argument, correct?

I'm not against having ArgsNegateSubcommands imply SubcommandsNegateReqs, I just meant for the time being a docs fix far quicker than a code fix. This week is going to be a busy one for me, so I may not be able to get to some low hanging fruit, but am more than willing to point some people in the right direction if they'd like to fix it.

[samtay]

I think this is biting me now, but I'm not sure if it's the same issue. I think the functionality I want is currently impossible. I have an app that will take any arbitrary string (that is, a required positional arg with multiple(true)), but also has a subcommand config that would take precedence:

$ app arbitrary string fine # works fine, my required arg takes three values here
$ app config --set field value # subcommand also works fine
$ app -- config fine here too # works fine, my required arg takes four values here

Is this possible in clap-2.33.1? Using the ArgsNegateSubcommands and SubcommandsNegateReqs helps a little bit, but clearly the third scenario is very broken:

$ app -- config fine here too
error: The subcommand 'config' wasn't recognized
    Did you mean 'config'?

If you believe you received this message in error, try re-running with 'app -- config'

USAGE:
    app [FLAGS] [OPTIONS] <query>...
    app <SUBCOMMAND>

For more information try --help

The error message is trying to be helpful but clearly this is all sorts of messed up.. any help appreciated.

[samtay]

Oh wow. Oddly, this works

$ app -- -- config fine here too
# matches with positional arg = ["--","config","fine","here","too"]

I'm not familiar with the internals of clap, but this seems like an interesting detail, might help you all in the 3.0 implementation.

[pksunkara]

Subcommands are not allowed after --

[CreepySkeleton]

@samtay What you want is _optional_ positional argument with min_values(0) and setting(ArgSettings::AllowLeadingHyphen). Unfortunately, there's no way for subcommands to take precedence over options in clap 2.33, but 3.0 will have AppSettings::SubcommandPrecedenceOverArg

[samtay]

@pksunkara I think you are misreading my comment. I do not want subcommands after --, I want to use -- as intended: everything after -- should be interpreted as positional arguments. Using app -- config fine here too _should_ treat config as a positional argument, instead of as a subcommand.

@CreepySkeleton Your suggestion doesn't work for me unfortunately, the same faulty error message is printed

$ app -- config what
error: The subcommand 'config' wasn't recognized
    Did you mean 'config'?

Version 3.0 sounds promising though. For now I'll just avoid using subcommands in my interface (for my situation adding a new subcommand for the required positional args would be too much noise, it would be like if grep forced you to use grep query <QUERY>, not worth it). Will be a little clunkier but that's alright, worth it for the rest of the clap features. Thanks.

[pksunkara]

Your suggestion doesn't work for me unfortunately, the same faulty error message is printed

Have you tried it with the master branch?

I do not want subcommands after --, I want to use -- as intended: everything after -- should be interpreted as positional arguments. Using app -- config fine here too should treat config as a positional argument, instead of as a subcommand.

Yes, that is what should happen. Subcommands should not be parsed after seeing --, so all of them will be just normal arguments IIRC.

[samtay]

No, I'm not on master. I think we have some miscommunication problems though. Above, when I say _should_, I mean this is the behavior I _expect_, but not the behavior I _observe_.

Yes, that is what should happen. Subcommands should not be parsed after seeing --, so all of them will be just normal arguments IIRC.

In particular, my shell snippet above directly contradicts this:

$ app -- config fine here too
error: The subcommand 'config' wasn't recognized
    Did you mean 'config'?

This shows that clap is trying to parse subcommands after --. If you use any word other than config the above will not error. If you use config, which is the name of a subcommand, it errors. Of course, this is clap 2.33.1 so it might be irrelevant to you. It sounds like this wouldn't be a problem in 3.0.

[CreepySkeleton]

@samtay Could you please post the reproducer so I can tinker with it?

[pksunkara]

I mean this is the behavior I expect

I get that. But we need to know what the current behaviour is to understand if this is actually a bug or not. And to do that, you would need to check with master.

[samtay]

Confirmed, this behavior does not work on 2.33.1 but does work on master branch (relevant gist):

❮ clap_test -- config -r
'slops' values: Some(["config", "-r"])

❮ clap_test config -r
config --reset TRUE
'slops' values: None

[pksunkara]

So, I think we can close this issue since it's fixed. We will not be backporting it to v2.