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

Distinguish between not present and not defined Args #604

Closed
SuperFluffy opened this issue Jul 26, 2016 · 5 comments · Fixed by #2000
Closed

Distinguish between not present and not defined Args #604

SuperFluffy opened this issue Jul 26, 2016 · 5 comments · Fixed by #2000
Assignees
Labels
C-enhancement Category: Raise on the bar on expectations E-hard Call for participation: Experience needed to fix: Hard / a lot
Milestone

Comments

@SuperFluffy
Copy link
Contributor

SuperFluffy commented Jul 26, 2016

Right now ArgMatches::is_present(s) checks whether the option with name s is present in the list of arguments or not. What about Args that have not been defined, and which I erroneously check for? Maybe is_present() could return an Option<bool> instead, with Some(bool) in cases where a defined Arg was set/not set, and None in cases where the option does not appear in the list of defined Args.

Assume the following example:

let m = App::new("myprog")
    .arg(Arg::with_name("debug")
        .long("debug")
        .short("d"))
    .get_matches_from(vec![
        "myprog", "-d"
    ]);

// Check with typo returns false
let is_present = m.is_present("dbeug");

This function will of course always return false. However, dbeug was never actually defined in the first place, so it would be great if it complained about that.

@SuperFluffy SuperFluffy changed the title Distinguish between not present and not defined flags/options Distinguish between not present and not defined Args Jul 26, 2016
@kbknapp
Copy link
Member

kbknapp commented Jul 28, 2016

I agree this is an issue, and something I'm hoping to solve by proxy with the ability to use enums instead of strings (see #510). But this is all a breaking change and will be made on the 3.x branch which I'm hoping to get some time to work on in the next few weeks and actually be able to put out soon.

Having is_present return an Option<bool> could also be one of the breaking 3.x changes. I'm not opposed to it.

@kbknapp
Copy link
Member

kbknapp commented Jul 22, 2018

I think I'd prefer to panic! on an undefined arg. This will mean keeping a list of all defined args in the matches, which increases the memory usage quite a bit but perhaps only doing this in debug mode or behind a feature flag would be a good way to go about this.

@kbknapp kbknapp added C-enhancement Category: Raise on the bar on expectations D: easy E-medium Call for participation: Experience needed to fix: Medium / intermediate E-easy Call for participation: Experience needed to fix: Easy / not much and removed T: RFC / question labels Jul 22, 2018
@spwitt
Copy link

spwitt commented Oct 16, 2018

@kbknapp I've started working on this. I should have a pull request tomorrow after I update according to contribution guide

@vijfhoek
Copy link

vijfhoek commented Jun 8, 2019

For the record, I agree with using panic! for this. I think it is a really fair assumption that calling this on an undefined argument is a programming error, and not just a runtime error.

@pksunkara pksunkara modified the milestones: v3-alpha.2, v3.0 Feb 1, 2020
@pksunkara pksunkara added C: asserts and removed E-easy Call for participation: Experience needed to fix: Easy / not much E-medium Call for participation: Experience needed to fix: Medium / intermediate labels Apr 9, 2020
@pksunkara pksunkara modified the milestones: 3.0, 3.1 Apr 10, 2020
@pksunkara pksunkara modified the milestones: 3.1, 3.0 Apr 10, 2020
@pksunkara pksunkara added E-hard Call for participation: Experience needed to fix: Hard / a lot and removed D: easy labels Apr 10, 2020
@CreepySkeleton
Copy link
Contributor

This will mean keeping a list of all defined args in the matches, which increases the memory usage quite a bit but perhaps only doing this in debug mode or behind a feature flag would be a good way to go about this.

Emphasis on:

... only doing this in debug mode ...

This is pretty much the same as what we do for Id. Piece o'cake, I'll take it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Raise on the bar on expectations E-hard Call for participation: Experience needed to fix: Hard / a lot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants