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

takes_value() parameters with default_value() are shown in USAGE like mandatory #1140

Closed
albel727 opened this issue Jan 5, 2018 · 7 comments
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations E-medium Call for participation: Experience needed to fix: Medium / intermediate
Milestone

Comments

@albel727
Copy link

albel727 commented Jan 5, 2018

Affected Version of clap

clap 2.29.0

Expected Behavior Summary

When command line parsing fails, a USAGE section is shown. This usage section should NOT contain non-required parameters with default values. Suppose there are two parameters, "required" and "defaulted", and the user didn't specify the required one. The output should be as follows

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

USAGE:
    prog [OPTIONS] <required>

For more information try --help

Actual Behavior Summary

Defaulted option parameters are mentioned in parse error help for no particular reason, confusingly looking like they're mandatory, even if they're required(false).

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

USAGE:
    prog <required> --defaulted <defaulted>

For more information try --help

Steps to Reproduce the issue

Utilize the following code. Note that if one comments out ".default_value()", the parse error no longer contains the mention of the defaulted parameter, as expected.

Sample Code or Link to Sample Code

    App::new("Test")
        .arg(
            Arg::with_name("required")
                .required(true)
        )
        .arg(
            Arg::with_name("defaulted")
                .takes_value(true)
                .short("d")
                .long("defaulted")
                .default_value("1")
        ).get_matches();
@albel727 albel727 changed the title takes_value() parameters with default_value() are shown in USAGE like required takes_value() parameters with default_value() are shown in USAGE like mandatory Jan 5, 2018
@albel727
Copy link
Author

albel727 commented Jan 5, 2018

It's particularly annoying that this shadows all other flags. So

    App::new("Test")
        .arg(
            Arg::with_name("required")
                .required(true)
        )
        .arg(
            Arg::with_name("defaulted")
                .takes_value(true)
                .short("d")
                .long("defaulted")
                .default_value("1")
        )
        .arg(
            Arg::with_name("another")
        )
        .get_matches_from(vec!["prog"]);

produces

USAGE:
    prog <required> --defaulted <defaulted>

instead of

USAGE:
    prog [OPTIONS] <required> [another]

@kbknapp
Copy link
Member

kbknapp commented Jan 9, 2018

The error messages show "used arguments" i.e. "this possibly what the command you just tried running should have been run"

But yes, defaults get thrown in there too. I can do some work to remove default values from error messages. This wouldn't however "turn off" the shadowing. With that fix

$ prog another
error: missing required [..snip]

USAGE:
    prog <required> --defaulted <defaulted> [another]

Would become

$ prog another
error: missing required [..snip]

USAGE:
    prog <required> [another]

I'm open to adding an option to turn off this "smart usage" so that the error would become just the default:

$ prog another
error: missing required [..snip]

USAGE:
    prog [FLAGS] [OPTIONS] <required>

(you can already kind of do this by just supplying your own usage string, which will get used no matter what.)

@kbknapp
Copy link
Member

kbknapp commented Jan 9, 2018

If someone wants to add this setting to disable the "smart usage" I'd be happy to mentor it. Removing the defaults from the usage string is probably more complicated than just adding this setting.

@albel727
Copy link
Author

albel727 commented Jan 10, 2018

I see. I'll consider the possible ways to proceed from here, after I take a look at the clap sources when I'm done with other things. For the time being, I have another alternative to posit.

How hard would it be to, at least, make the defaulted but non-required arguments be printed in square brackets, so as to convey their non-mandatory nature? I.e. something like

USAGE:
    prog <required> [--defaulted <defaulted>] [another]

Or do you think that this would violate user's expectations, making them believe that they have to put "--defaulted" in square brackets in order to use it?

@kbknapp
Copy link
Member

kbknapp commented Jan 15, 2018

I.e. something like

It would be fine, but almost as much work as simply not printing the defaults (which is a slightly better solution IMO).

In the source for either of these two fixes, when clap is printing the usage string, it doesn't know if something is a default value or not, it just sees an argument was used (used by user, or used by default). So it would require re-looping through the used arguments to see if they have a default, and if so if the value that was used was in fact the default.

@kbknapp kbknapp added C-enhancement Category: Raise on the bar on expectations P4: nice to have E-hard Call for participation: Experience needed to fix: Hard / a lot labels Jul 22, 2018
arusahni added a commit to arusahni/git-req that referenced this issue Dec 30, 2019
Workaround for clap bug: clap-rs/clap#1140
where the arg will always be displayed in the usage string.
arusahni added a commit to arusahni/git-req that referenced this issue Jan 1, 2020
Workaround for clap bug: clap-rs/clap#1140
where the arg will always be displayed in the usage string.
@pksunkara pksunkara added this to the 3.1 milestone Apr 9, 2020
@pksunkara pksunkara removed the W: 3.x label Aug 13, 2021
@epage
Copy link
Member

epage commented Dec 8, 2021

In some ways, this reminds me of #3076

@epage epage added A-help Area: documentation, including docs.rs, readme, examples, etc... and removed C: usage strings E-hard Call for participation: Experience needed to fix: Hard / a lot labels Dec 8, 2021
@epage epage removed this from the 3.1 milestone Dec 9, 2021
@epage epage added the E-medium Call for participation: Experience needed to fix: Medium / intermediate label Dec 9, 2021
@epage epage added this to the 3.x milestone Jan 4, 2022
@epage epage modified the milestones: 3.x, 3.1 Feb 2, 2022
@epage
Copy link
Member

epage commented Feb 8, 2022

This was fixed by #2609

@epage epage closed this as completed Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet
Development

No branches or pull requests

4 participants