Skip to content
This repository has been archived by the owner on Jan 1, 2022. It is now read-only.

ArgRequiredElseHelp is ignored if there are *any* default values #96

Open
epage opened this issue Dec 6, 2021 · 8 comments
Open

ArgRequiredElseHelp is ignored if there are *any* default values #96

epage opened this issue Dec 6, 2021 · 8 comments
Milestone

Comments

@epage
Copy link
Owner

epage commented Dec 6, 2021

Issue by whitfin
Tuesday May 01, 2018 at 04:59 GMT
Originally opened as clap-rs/clap#1264


Rust Version

rustc 1.25.0 (84203cac6 2018-03-25)

Affected Version of clap

2.31.2

Expected Behavior Summary

If ArgRequiredElseHelp is enabled, you should see the help menu if an option is not provided correctly.

Actual Behavior Summary

If you have two arguments, and one has a default, this the help menu is not correctly printed.

Steps to Reproduce the issue

Run the code below in a test project via cargo run. It won't work (rightly so) because the "second" option is not provided - but you'll get an error instead of the help menu.

Do the same again with the default value removed from "first", and you'll then see a help menu. Regardless of the default value of "first", it should show the help menu both times.

App::new("")
    // package metadata from cargo
    .name(env!("CARGO_PKG_NAME"))
    .about(env!("CARGO_PKG_DESCRIPTION"))
    .version(env!("CARGO_PKG_VERSION"))

    // arguments and flag details
    .arg(Arg::with_name("second")
        .help("second option")
        .required(true))
    .arg(Arg::with_name("first")
        .help("first option")
        .default_value("something"))

    // settings required for parsing
    .setting(AppSettings::ArgRequiredElseHelp)
@epage epage added this to the 3.1 milestone Dec 6, 2021
@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by whitfin
Tuesday May 01, 2018 at 05:04 GMT


I think perhaps https://github.com/kbknapp/clap-rs/blob/master/src/app/validator.rs#L63 should use !reqs_validated instead of matcher.is_empty().

I dropped a PR in clap-rs/clap#1265; all tests pass still and the issue appears fixed. I'm not familiar enough to know if this is the best approach, so let me know of any feedback.

Edit: actually that PR appears to render the help menu too frequently now, so closed for the time being :)

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by kbknapp
Tuesday Jun 05, 2018 at 01:38 GMT


Sorry for the long wait, I've been away for a few weeks with my day job.

while I agree it's not optimal, a default value is a value and therefore clap treats it as such. I'd possibly consider adding some sort of setting such as UserArgRequiredElseHelp but I'm not sure it'd quite be worth it as the "fix" for this is to simply not provide a default and use something like, matches.value_of("my-arg-that-needs-a-default").unwrap_or("default_value") and just "tell" the user in the help text manually that there is a default value (but to clap there isn't).

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by Kayvlim
Tuesday Jul 14, 2020 at 14:59 GMT


Bringing this back because I found this issue when searching before opening a new one.

Versions:
rustc 1.44.1 (c7087fe00 2020-06-17)
clap 3.0.0-beta.1

@kbknapp: if the "fix" is to call unwrap_or instead of using the mechanism already in place for default_values, this either defeats the purpose of this mechanism or the purpose of ArgRequiredElseHelp.

I believe AppSettings::ArgRequiredElseHelp is only relevant when we want to display the help to a user that did not write any arguments in the command line. This is the typical interaction for "let me see how this program works".
This behavior should not be dependent on whether there are arguments with default values. The user still did not type any argument.
I should also point out that even when no arguments are .required(true), ArgRequiredElseHelp does the right thing as long as no default_values are present.

Is there any use case where ArgRequiredElseHelp makes sense when an arg with a default_value is present?

It seems to me that clap's approach to default values is equating them with the user passing them in the command line (with the exception of occurrences_of returning zero), but what I am expecting is that is_present returns true and any attempt to retrieve the value (such as calling value_of) returns the default_value instead of None, with no other behavior being affected.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Sunday Jul 19, 2020 at 10:37 GMT


@CreepySkeleton you were working on default stuff recently, right? Can you take a look at this last comment?

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by CreepySkeleton
Sunday Jul 19, 2020 at 15:38 GMT


I find myself agreeing with @Kayvlim . Default values aren't ephemeral, they are very real values. Clap treats them in almost exactly the same way as any other value (barring conflict resolution which they are exempt from, or at least should have been).

Is there any use case where ArgRequiredElseHelp makes sense when an arg with a default_value is present?

That is a good question. My answer would be no, there aren't, and in this case we should panic if ArgRequiredElseHelp is set and any of the args has default_value.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by Kayvlim
Wednesday Jul 22, 2020 at 01:06 GMT


@CreepySkeleton I suppose you are only agreeing with how I think clap works, but not with how I think it should be changed?

I don't agree that ArgRequiredElseHelp should panic if any arg has a default_value. It's a perfectly legitimate use case:

If the user runs my application without arguments, I want them to see the help, but if they run it with the single required argument, I want the application to run with a sensible set of default values for the arguments they didn't pass (or "override"). I shouldn't have to unwrap_or every non-passed argument to accomplish this when a mechanism for default values is available just because I also want to display the help when no arguments have been passed.

So, I think ArgRequiredElseHelp should do exactly the same thing it would if there were no default_values whatsoever. This doesn't make default values ephemeral. Instead, it stops pretending they were passed by the user when they really weren't.

In other words, I don't see default values as values the user positively intended to pass to the application by omission. I see them as values the application needs to assume when the user hasn't said anything about them and None is not acceptable.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by CreepySkeleton
Friday Jul 24, 2020 at 03:25 GMT


Well, let's see what we can do about that...

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by dimo414
Friday Feb 05, 2021 at 08:05 GMT


I was bitten by this issue myself while trying to add ArgRequiredElseHelp to a CLI. It's wholly unintuitive - both to the user and the developer - that the use of default values would be considered sufficient to bypass this check.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant