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.

Sign up for GitHub

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

Jump to bottom

Argument cannot require a particular value of positional argument #764

Closed
nagisa opened this issue Dec 4, 2016 · 4 comments
Closed

Argument cannot require a particular value of positional argument #764

nagisa opened this issue Dec 4, 2016 · 4 comments
Assignees
@kbknapp
Milestone
2.20.0

Comments

@nagisa
Copy link

nagisa commented Dec 4, 2016

Affected Version of clap

  • 2.19.1

Expected Behavior Summary

The application works or clap reports a misconfiguration error.

Actual Behavior Summary

Instead clap complains about some required argument not being provided

     Running `.../kt --error 0.25 channel`
error: The following required arguments were not provided:

USAGE:
    kt <ELEMENT> --error <error>

and doesn’t accept either of --error 0.25 or channel positional argument in isolation:

     Running `.../kt --error 0.25`
error: The following required arguments were not provided:

USAGE:
    kt --error <error>

     Running `.../kt channel`
error: The following required arguments were not provided:

USAGE:
    kt <ELEMENT> --error <error>

Steps to Reproduce the issue

        .arg(Arg::with_name("error")
             .long("error")
             .requires("channel") // NB: particular value of a positional argument
             .default_value("0.25")
        )
        .arg(Arg::with_name("element")
             .value_name("ELEMENT")
             .possible_value("channel")
             .min_values(1)
        );
@kbknapp
Copy link
Member

kbknapp commented Dec 5, 2016 •
edited
Loading

Arg::requires takes an argument name, not an argument value. Try:

       .arg(Arg::with_name("error")
             .long("error")
             .requires("element") // Use the name, not the value
             .default_value("0.25")
        )
        .arg(Arg::with_name("element")
             .value_name("ELEMENT")
             .possible_value("channel")
             .min_values(1)
        );

Let me know if that works as expected. 😉

@nagisa
Copy link
Author

nagisa commented Dec 5, 2016 via email

@kbknapp
Copy link
Member

kbknapp commented Dec 5, 2016 •
edited
Loading

The point of this issue is that I want to require specific value of (there
might be more valid values added eventually) the positional argument.

Ahhh Ok, my mistake! There seems to be two version of this issue in my mind.

  1. Upon receiving the value channel, --error <error> is now required.
  • Implemented via Arg::requires_if(arg, value) (set on "element" as in element.requires_if("error", "channel"))
  1. Using --error <error> requires that <element> be set to channel.
  • Perhaps implemented via Arg::default_if(arg, default) (set on "element" as in element.default_if("error", "channel"))
  • An alternative would be Arg::default_if(arg, value, default) (i.e. only use this default is arg is used, and with value value such as element.default_if("error", "some_val", "channel"))

Both seem doable, although I haven't tried implementing them yet.

With number 2, the only issue I can see is should this be a new default (i.e. only set <element> to channel if the user didn't <element> directly), or a mandatory setting (i.e. override the user's manual value)? The silently override user input seems dangerous to me and is something I'm not interested in doing. The new default value seems legitimate to me.

but I would very much like if clap at very least figured out the argument config was
malformed

I had a branch which was the base of 3.x which used enum variants as argument keys instead of strings (although was still backwards compatible with strings) which solved exactly this. Although I've been waiting for Macros 1.1 in order to finish this.

Edit: more thoughts on the matter

@kbknapp kbknapp added this to the 2.20.0 milestone Dec 5, 2016
@kbknapp
Copy link
Member

kbknapp commented Dec 10, 2016 •
edited
Loading

I'm working on this in a local branch, here's the TODO:

  • Conditional default values (Arg::default_value_if[s], This arg has a default value of ____ in the following conditions)
    • Implmement default_value_if
    • Implmeent default_value_ifs
    • default_value_if docs
    • default_value_ifs docs
    • default_value_if tests
    • default_value_ifs tests
    • default_value_if YAML
    • default_value_ifs YAML
  • Conditional requirements (Arg::required_if[s], or "This arg becomes required in the following conditions...)
    • Implement required_if
    • Implement required_ifs
    • required_if docs
    • required_ifs docs
    • required_if tests
    • required_ifs tests
    • required_if YAML
    • required_ifs YAML
  • Conditional external requirements (Arg::requires_if[s], This arg has additional requirements in the following conditions...)
    • requires_if docs
    • requires_ifs docs
    • Implement requires_if
    • Implement requires_ifs
    • requires_if tests
    • requies_ifs tests
    • requires_if YAML
    • requires_ifs YAML

@kbknapp kbknapp self-assigned this Dec 27, 2016
kbknapp added a commit that referenced this issue Dec 28, 2016
@kbknapp
feat(Conditional Default Values): adds new Arg::default_value_if[s] m…
5eb06d9 
…ethods for conditional default values

One can now implement conditional default values. I.e. a default value that is only applied in
certain conditions, such as another arg being present, or another arg being present *and*
containing a specific value.

Now it's possible to say, "Only apply this default value if arg X is present" or "Only apply this
value if arg X is present, but also only if arg X's value is equal to Y"

This new method is fully compatible with the current `Arg::default_value`, which gets set only if
the arg wasn't used at runtime *and* none of the specified conditions were met.

Releates to #764
kbknapp added a commit that referenced this issue Dec 28, 2016
@kbknapp
feat(Conditional Requirements): adds the ability to conditionally req…
7583825 
…uire additional args

An arg can now conditionally require additional arguments if it's value matches a specific value.

For example, arg A can state that it only requires arg B if the value X was used with arg A. If any
other value is used with A, arg B isn't required.

Relates to #764
kbknapp added a commit that referenced this issue Dec 29, 2016
@kbknapp
feat(Conditional Default Values): adds new Arg::default_value_if[s] m…
eb4010e 
…ethods for conditional default values

One can now implement conditional default values. I.e. a default value that is only applied in
certain conditions, such as another arg being present, or another arg being present *and*
containing a specific value.

Now it's possible to say, "Only apply this default value if arg X is present" or "Only apply this
value if arg X is present, but also only if arg X's value is equal to Y"

This new method is fully compatible with the current `Arg::default_value`, which gets set only if
the arg wasn't used at runtime *and* none of the specified conditions were met.

Releates to #764
kbknapp added a commit that referenced this issue Dec 29, 2016
@kbknapp
feat(Conditional Requirements): adds the ability to conditionally req…
198449d 
…uire additional args

An arg can now conditionally require additional arguments if it's value matches a specific value.

For example, arg A can state that it only requires arg B if the value X was used with arg A. If any
other value is used with A, arg B isn't required.

Relates to #764
kbknapp added a commit that referenced this issue Dec 29, 2016
@kbknapp
feat(Conditionally Required): adds the ability for an arg to be condi…
ee9cfdd 
…tionally required

An arg can now be conditionally required (i.e. it's only required if arg
A is used with a value of V).

For example:

```rust
let res = App::new("ri")
	.arg(Arg::with_name("cfg")
	    .required_if("extra", "val")
	    .takes_value(true)
	    .long("config"))
	.arg(Arg::with_name("extra")
	    .takes_value(true)
	    .long("extra"))
	.get_matches_from_safe(vec![
	    "ri", "--extra", "val"
	]);

assert!(res.is_err());
assert_eq!(res.unwrap_err().kind, ErrorKind::MissingRequiredArgument);
```

Relates to #764
@homu homu closed this as completed in 9a4df32 Dec 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kbknapp kbknapp

Labels
None yet
Projects
None yet
Milestone
2.20.0
Development

No branches or pull requests
2 participants
@nagisa @kbknapp