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

requires_all ArgGroup with a default, causes the entire ArgGroup to be required #1586

Closed
Tracked by #2897
leeola opened this issue Oct 26, 2019 · 4 comments · Fixed by #2950
Closed
Tracked by #2897

requires_all ArgGroup with a default, causes the entire ArgGroup to be required #1586

leeola opened this issue Oct 26, 2019 · 4 comments · Fixed by #2950
Labels
C-bug Category: Updating dependencies E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@leeola
Copy link

leeola commented Oct 26, 2019

Rust Version

rustc 1.40.0-nightly (10a52c25c 2019-10-24)

Affected Version of clap

clap 2.33.0

Expected Behavior Summary

  1. Using ArgGroup to make 3 flags required to use together, if at all. Eg, --foo and --bar are optional, but if specified at all --foo and --bar must both have a value.
  2. Additionally, --baz has a default value, in the same group of --foo and --bar.

Actual Behavior Summary

The default value of --baz seems to make the ArgGroup behave as if the user is always attempting to use it. Such that the ArgGroup becomes required. Example:

> cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `target/debug/test-clap-arggroup`
error: The following required arguments were not provided:

USAGE:
    test-clap-arggroup <--foo <FOO>|--bar <BAR>>

For more information try --help

Steps to Reproduce the issue

Run the sample code, with no flags specified.

Sample Code or Link to Sample Code

/// A bug report test binary
use clap::*;

fn main() {
  let _matches = App::new("bin")
    .arg(
      Arg::with_name("foo")
        .required(false)
        .long("foo")
        .value_name("FOO")
        .takes_value(true),
    )
    .arg(
      Arg::with_name("bar")
        .required(false)
        .long("bar")
        .value_name("BAR")
        .takes_value(true),
    )
    .arg(
      Arg::with_name("baz")
        .required(false)
        .long("baz")
        .value_name("BAZ")
        .default_value("default value")
        .takes_value(true),
    )
    .group(
      ArgGroup::with_name("test_flags")
        .multiple(true)
        .args(&["foo", "bar", "baz"])
        .requires_all(&["foo", "bar", "baz"]),
    )
    .get_matches();
}

Debug output

Debug Output

Finished dev [unoptimized + debuginfo] target(s) in 0.01s
 Running `target/debug/test-clap-arggroup`

DEBUG:clap:Parser::propagate_settings: self=bin, g_settings=AppFlags(
(empty),
)
DEBUG:clap:Parser::get_matches_with;
DEBUG:clap:Parser::create_help_and_version;
DEBUG:clap:Parser::create_help_and_version: Building --help
DEBUG:clap:Parser::create_help_and_version: Building --version
DEBUG:clap:ArgMatcher::process_arg_overrides:None;
DEBUG:clap:Parser::remove_overrides:[];
DEBUG:clap:Validator::validate;
DEBUG:clap:Parser::add_defaults;
DEBUG:clap:Parser::add_defaults:iter:foo: doesn't have conditional defaults
DEBUG:clap:Parser::add_defaults:iter:foo: doesn't have default vals
DEBUG:clap:Parser::add_defaults:iter:bar: doesn't have conditional defaults
DEBUG:clap:Parser::add_defaults:iter:bar: doesn't have default vals
DEBUG:clap:Parser::add_defaults:iter:baz: doesn't have conditional defaults
DEBUG:clap:Parser::add_defaults:iter:baz: has default vals
DEBUG:clap:Parser::add_defaults:iter:baz: wasn't used
DEBUG:clap:Parser::add_val_to_arg; arg=baz, val="default value"
DEBUG:clap:Parser::add_val_to_arg; trailing_vals=false, DontDelimTrailingVals=false
DEBUG:clap:Parser::add_single_val_to_arg;
DEBUG:clap:Parser::add_single_val_to_arg: adding val..."default value"
DEBUG:clap:Parser::groups_for_arg: name=baz
DEBUG:clap:Parser::groups_for_arg: Searching through groups...
Found 'test_flags'
DEBUG:clap:ArgMatcher::needs_more_vals: o=baz
DEBUG:clap:Validator::validate_blacklist;
DEBUG:clap:Validator::validate_blacklist:iter:baz;
DEBUG:clap:Parser::groups_for_arg: name=baz
DEBUG:clap:Parser::groups_for_arg: Searching through groups...
Found 'test_flags'
DEBUG:clap:Validator::validate_blacklist:iter:test_flags;
DEBUG:clap:Parser::groups_for_arg: name=test_flags
DEBUG:clap:Parser::groups_for_arg: Searching through groups...
DEBUG:clap:Validator::validate_blacklist:iter:test_flags:group;
DEBUG:clap:Validator::validate_blacklist:iter:test_flags:group:iter:foo;
DEBUG:clap:Validator::validate_blacklist:iter:test_flags:group:iter:bar;
DEBUG:clap:Validator::validate_blacklist:iter:test_flags:group:iter:baz;
DEBUG:clap:Validator::validate_required: required=[];
DEBUG:clap:Validator::validate_matched_args;
DEBUG:clap:Validator::validate_matched_args:iter:baz: vals=[
"default value",
]
DEBUG:clap:Validator::validate_arg_num_vals:baz
DEBUG:clap:Validator::validate_arg_values: arg="baz"
DEBUG:clap:Validator::validate_arg_requires:baz;
DEBUG:clap:Validator::validate_arg_num_occurs: a=baz;
DEBUG:clap:Validator::validate_matched_args:iter:test_flags: vals=[
"default value",
]
DEBUG:clap:Validator::missing_required_error: extra=None
DEBUG:clap:Parser::color;
DEBUG:clap:Parser::color: Color setting...Auto
DEBUG:clap:is_a_tty: stderr=true
DEBUG:clap:Validator::missing_required_error: reqs=[]
DEBUG:clap:usage::get_required_usage_from: reqs=[], extra=None
DEBUG:clap:usage::get_required_usage_from: after init desc_reqs=[]
DEBUG:clap:usage::get_required_usage_from: no more children
DEBUG:clap:usage::get_required_usage_from: final desc_reqs=[]
DEBUG:clap:usage::get_required_usage_from: args_in_groups=[]
DEBUG:clap:Validator::missing_required_error: req_args=""
DEBUG:clap:usage::create_usage_with_title;
DEBUG:clap:usage::create_usage_no_title;
DEBUG:clap:usage::smart_usage;
DEBUG:clap:usage::get_required_usage_from: reqs=["baz", "test_flags"], extra=None
DEBUG:clap:usage::get_required_usage_from:iter:test_flags: adding group req="foo"
DEBUG:clap:usage::get_required_usage_from:iter:test_flags: adding group req="bar"
DEBUG:clap:usage::get_required_usage_from: after init desc_reqs=["foo", "bar"]
DEBUG:clap:usage::get_required_usage_from: no more children
DEBUG:clap:usage::get_required_usage_from: final desc_reqs=["bar", "baz", "foo", "test_flags"]
DEBUG:clap:usage::get_required_usage_from: args_in_groups=["foo", "bar", "baz"]
DEBUG:clap:OptBuilder::fmt:foo
DEBUG:clap:OptBuilder::fmt:bar
DEBUG:clap:OptBuilder::fmt:baz
DEBUG:clap:Parser::color;
DEBUG:clap:Parser::color: Color setting...Auto
DEBUG:clap:is_a_tty: stderr=true
DEBUG:clap:Colorizer::error;
DEBUG:clap:Colorizer::good;
error: The following required arguments were not provided:

USAGE:
test-clap-arggroup <--foo |--bar |--baz >

For more information try --help

@leeola leeola changed the title ArgGroup flag with a default, causes the entire ArgGroup to be required requires_all ArgGroup with a default, causes the entire ArgGroup to be required Oct 26, 2019
@CreepySkeleton CreepySkeleton self-assigned this Feb 1, 2020
@CreepySkeleton CreepySkeleton added not-sure E-help-wanted Call for participation: Help is requested to fix this issue. C-bug Category: Updating dependencies labels Feb 1, 2020
@CreepySkeleton CreepySkeleton removed their assignment Feb 6, 2020
@pksunkara pksunkara added this to the 3.1 milestone Apr 9, 2020
@LordMZTE
Copy link

any progress on this?

@pksunkara pksunkara modified the milestones: 3.1, 3.0 Sep 13, 2020
@ldm0 ldm0 self-assigned this Mar 13, 2021
@ldm0
Copy link
Member

ldm0 commented Aug 8, 2021

I think this is the expected behaviour. Default value just behaves as if the user is always using it. If clap special case it, you will see an ArgGroup requires non present argument, but still successfully gets a value.

use clap::*;

fn main() {
  let _matches = App::new("bin")
    .arg(
      Arg::new("foo")
        .required(false)
        .long("foo")
        .value_name("FOO")
        .takes_value(true),
    )
    .arg(
      Arg::new("bar")
        .required(false)
        .long("bar")
        .value_name("BAR")
        .takes_value(true),
    )
    .arg(
      Arg::new("baz")
        .required(false)
        .long("baz")
        .value_name("BAZ")
        .default_value("default value")
        .takes_value(true),
    )
    .group(
      ArgGroup::new("test_flags")
        .multiple(true)
        .args(&["foo", "bar", "baz"])
        .requires_all(&["baz"]),
    )
    .get_matches();

    assert_eq!(_matches.value_of("test_flags"), Some("default value"));
}

@ldm0 ldm0 closed this as completed Aug 8, 2021
@pksunkara
Copy link
Member

pksunkara commented Aug 9, 2021

I wouldn't consider this fixed if we need to change how default values interact with parsing. This is fully related to #1906. We had similar issues in #1605. Maybe we need to rethink this?

@pksunkara pksunkara reopened this Aug 9, 2021
@ldm0 ldm0 removed their assignment Aug 15, 2021
epage added a commit to epage/clap that referenced this issue Oct 16, 2021
This is meant to lower the chance of confusion with cases like clap-rs#2714 and clap-rs#1586.

This is not meant to be exhaustive, looked at the mentioned cases in
that issue and pattern matched on other ones mentioning "is present".
@epage epage removed this from the 3.0 milestone Oct 16, 2021
@epage
Copy link
Member

epage commented Oct 16, 2021

This is related to #2714 and I am reducing the impact through documentation in #2896. I assume @pksunkara will want this to remain open as part of future work to try to resolve this,

epage added a commit to epage/clap that referenced this issue Oct 18, 2021
This is meant to lower the chance of confusion with cases like clap-rs#2714 and clap-rs#1586.

This is not meant to be exhaustive, looked at the mentioned cases in
that issue and pattern matched on other ones mentioning "is present".
epage added a commit to epage/clap that referenced this issue Oct 23, 2021
This is meant to lower the chance of confusion with cases like clap-rs#2714 and clap-rs#1586.

This is not meant to be exhaustive, looked at the mentioned cases in
that issue and pattern matched on other ones mentioning "is present".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Updating dependencies E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants