Feature: add Arg::required_unless_all #85

epage · 2021-12-06T16:39:21Z

Issue by gbear605
Friday Dec 22, 2017 at 10:43 GMT
Originally opened as clap-rs/clap#1135

Rust Version

rustc 1.24.0-nightly (7eb64b86c 2017-12-20)

Affected Version of clap

clap 2.29.0

Expected Behavior Summary

With the given sample app, I should be able to have the arguments:
-a -c -e
-a -c -f
-a -d -e
-a -d -f
-b -c -e
-b -c -f
-b -d -e
-b -d -f

The overall goal of the program is that I should have to call -x or I should have to call one from each of (-a, -b), (-c, -d), and (-e, -f). I should also never have both arguments from each argument group.

Actual Behavior Summary

For all of those combinations above, it says:
error: The following required arguments were not provided: -x

Steps to Reproduce the issue

Create a new binary Cargo project with Clap and the provided code as main.rs, run cargo run -- -a -c -e (or similar with any of the above combinations)

Sample Code or Link to Sample Code

extern crate clap;
use clap::{App, Arg};

fn main() {
    App::new("Sample App")
        .arg(Arg::with_name("a").conflicts_with("b").short("a"))
        .arg(Arg::with_name("b").short("b"))
        .arg(Arg::with_name("c").conflicts_with("d").short("c"))
        .arg(Arg::with_name("d").short("d"))
        .arg(Arg::with_name("e").conflicts_with("f").short("e"))
        .arg(Arg::with_name("f").short("f"))
        .arg(
            Arg::with_name("x")
                .required_unless_one(&["a", "b"])
                .required_unless_one(&["c", "d"])
                .required_unless_one(&["e", "f"])
                .short("x"),
        )
        .get_matches();
}

Debug output

kevin@beefcake: ~/Projects/fake 
➜ ./target/debug/fake -a -c -e                                                                        ✓ [git: master]
DEBUG:clap:Parser::propagate_settings: self=Sample App, 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:Parser::get_matches_with: Begin parsing '"-a"' ([45, 97])
DEBUG:clap:Parser::is_new_arg:"-a":NotFound
DEBUG:clap:Parser::is_new_arg: arg_allows_tac=false
DEBUG:clap:Parser::is_new_arg: - found
DEBUG:clap:Parser::is_new_arg: starts_new_arg=true
DEBUG:clap:Parser::possible_subcommand: arg="-a"
DEBUG:clap:Parser::get_matches_with: possible_sc=false, sc=None
DEBUG:clap:ArgMatcher::process_arg_overrides:None;
DEBUG:clap:Parser::parse_short_arg: full_arg="-a"
DEBUG:clap:Parser::parse_short_arg:iter:a
DEBUG:clap:Parser::parse_short_arg:iter:a: Found valid flag
DEBUG:clap:Parser::check_for_help_and_version_char;
DEBUG:clap:Parser::check_for_help_and_version_char: Checking if -a is help or version...Neither
DEBUG:clap:Parser::parse_flag;
DEBUG:clap:ArgMatcher::inc_occurrence_of: arg=a
DEBUG:clap:ArgMatcher::inc_occurrence_of: first instance
DEBUG:clap:Parser::groups_for_arg: name=a
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:Parser:get_matches_with: After parse_short_arg Flag
DEBUG:clap:Parser::get_matches_with: Begin parsing '"-c"' ([45, 99])
DEBUG:clap:Parser::is_new_arg:"-c":Flag
DEBUG:clap:Parser::is_new_arg: arg_allows_tac=false
DEBUG:clap:Parser::is_new_arg: - found
DEBUG:clap:Parser::is_new_arg: starts_new_arg=true
DEBUG:clap:Parser::possible_subcommand: arg="-c"
DEBUG:clap:Parser::get_matches_with: possible_sc=false, sc=None
DEBUG:clap:ArgMatcher::process_arg_overrides:Some("a");
DEBUG:clap:Parser::parse_short_arg: full_arg="-c"
DEBUG:clap:Parser::parse_short_arg:iter:c
DEBUG:clap:Parser::parse_short_arg:iter:c: Found valid flag
DEBUG:clap:Parser::check_for_help_and_version_char;
DEBUG:clap:Parser::check_for_help_and_version_char: Checking if -c is help or version...Neither
DEBUG:clap:Parser::parse_flag;
DEBUG:clap:ArgMatcher::inc_occurrence_of: arg=c
DEBUG:clap:ArgMatcher::inc_occurrence_of: first instance
DEBUG:clap:Parser::groups_for_arg: name=c
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:Parser:get_matches_with: After parse_short_arg Flag
DEBUG:clap:Parser::get_matches_with: Begin parsing '"-e"' ([45, 101])
DEBUG:clap:Parser::is_new_arg:"-e":Flag
DEBUG:clap:Parser::is_new_arg: arg_allows_tac=false
DEBUG:clap:Parser::is_new_arg: - found
DEBUG:clap:Parser::is_new_arg: starts_new_arg=true
DEBUG:clap:Parser::possible_subcommand: arg="-e"
DEBUG:clap:Parser::get_matches_with: possible_sc=false, sc=None
DEBUG:clap:ArgMatcher::process_arg_overrides:Some("c");
DEBUG:clap:Parser::parse_short_arg: full_arg="-e"
DEBUG:clap:Parser::parse_short_arg:iter:e
DEBUG:clap:Parser::parse_short_arg:iter:e: Found valid flag
DEBUG:clap:Parser::check_for_help_and_version_char;
DEBUG:clap:Parser::check_for_help_and_version_char: Checking if -e is help or version...Neither
DEBUG:clap:Parser::parse_flag;
DEBUG:clap:ArgMatcher::inc_occurrence_of: arg=e
DEBUG:clap:ArgMatcher::inc_occurrence_of: first instance
DEBUG:clap:Parser::groups_for_arg: name=e
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:Parser:get_matches_with: After parse_short_arg Flag
DEBUG:clap:ArgMatcher::process_arg_overrides:Some("e");
DEBUG:clap:Parser::remove_overrides:[];
DEBUG:clap:Validator::validate;
DEBUG:clap:Parser::add_defaults;
DEBUG:clap:Validator::validate_blacklist;
DEBUG:clap:Validator::validate_blacklist:iter:c;
DEBUG:clap:Parser::groups_for_arg: name=c
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:Validator::validate_blacklist:iter:e;
DEBUG:clap:Parser::groups_for_arg: name=e
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:Validator::validate_blacklist:iter:a;
DEBUG:clap:Parser::groups_for_arg: name=a
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:Validator::validate_required: required=["x"];
DEBUG:clap:Validator::validate_required:iter:x:
DEBUG:clap:Validator::is_missing_required_ok: a=x
DEBUG:clap:Validator::validate_arg_conflicts: a="x";
DEBUG:clap:Validator::validate_required_unless: a="x";
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=[
    "x"
]
DEBUG:clap:usage::get_required_usage_from: reqs=["x"], 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=["x"]
DEBUG:clap:usage::get_required_usage_from: args_in_groups=[]
DEBUG:clap:usage::get_required_usage_from:iter:x:
DEBUG:clap:Colorizer::error;
DEBUG:clap:Validator::missing_required_error: req_args="\n    \u{1b}[1;31m-x\u{1b}[0m"
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=["x", "c", "e", "a"], 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=["a", "c", "e", "x"]
DEBUG:clap:usage::get_required_usage_from: args_in_groups=[]
DEBUG:clap:usage::get_required_usage_from:iter:a:
DEBUG:clap:usage::get_required_usage_from:iter:c:
DEBUG:clap:usage::get_required_usage_from:iter:e:
DEBUG:clap:usage::get_required_usage_from:iter:x:
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:
    -x

USAGE:
    fake -a -c -e -x

For more information try --help

The text was updated successfully, but these errors were encountered:

epage · 2021-12-06T16:39:22Z

Comment by willmurphyscode
Tuesday Dec 26, 2017 at 16:20 GMT

I believe this is an issue where short() conflicts with required_unless_one(). In the original code sample, removing short("x") will produce the expected behavior. Here's a PR with a failing unit test: clap-rs/clap#1136 @kbknapp Do you have any thoughts on how/where to address this issue?

epage · 2021-12-06T16:39:24Z

Comment by gbear605
Sunday Jan 07, 2018 at 04:52 GMT

From testing, this also occurs with long().

epage · 2021-12-06T16:39:24Z

Comment by gbear605
Sunday Jan 07, 2018 at 04:59 GMT

Also, having multiple required_unless_one() doesn't work as I hoped it would - it combines them so that it just requires one of a, b, c, d, e, f, when I wanted it to required one of a, b, one of c, d, and one of e, f. Is there any way to do this with clap?

epage · 2021-12-06T16:39:25Z

Comment by kbknapp
Tuesday Jan 09, 2018 at 17:21 GMT

Thanks for filing! I'll start to take a look. I've been away for the holidays and traveling. @willmurphyscode I saw your test cases in the PR. Do you want to continue with this?

epage · 2021-12-06T16:39:26Z

Comment by willmurphyscode
Wednesday Jan 10, 2018 at 11:25 GMT

@kbknapp I will start digging today. I'll ping you if I get stuck.

epage · 2021-12-06T16:39:27Z

Comment by willmurphyscode
Thursday Jan 18, 2018 at 00:21 GMT

@kbknapp I've been experimenting with the tests, trying to discover what is different between the passing and failing test runs, and the differences begin at this line:
https://github.com/kbknapp/clap-rs/blob/b6f36f55a04f8ed963a524cf1b87c7e7869964c2/src/app/parser.rs#L295

When there are no short or long forms of the arg that is "required_unless_one," then the arg is added as a positional, but if there are short or long forms, the arg is added as a flag. I believe that the correct behavior is accidentally dependent on the required_unless_one arg being treated as a positional and not as a flag, but I haven't figured out why yet. Does it seem like I'm on the right track? Do you have any insight as to where I might keep looking?

epage · 2021-12-06T16:39:28Z

Comment by kbknapp
Thursday Jan 18, 2018 at 20:22 GMT

@willmurphyscode You're exactly correct about that line of code, but I'm not sure if that's the issue or not. You're right, the tests don't use an arg with a short as the one that defines (required_unless_one), but that shouldn't be the issue (at least it's not supposed to be).

There was recently a rework to how clap validates, so I've updated the debug output in the OP to v2.29.2 (where the error still occurs).

The error seems to say Validator::validate_required_unless is the one to blame. I'd start there and work backwards to see where it's getting the info from.

epage · 2021-12-06T16:39:29Z

Comment by kbknapp
Monday Jan 22, 2018 at 19:09 GMT

@gbear605 can you test this against the master branch and see if it fixes it for you? @willmurphyscode has come up with a fix, and if it works I'll release the new version.

epage · 2021-12-06T16:39:30Z

Comment by gbear605
Tuesday Jan 23, 2018 at 03:56 GMT

My issue now is somewhat the opposite of before, although this may be intended behavior:

It now allows me to do all the arguments I want, but it also allows me to have just one or two of them. For instance, I can pass in "-a -c" or "-a" and it works, but I instead want it to require "-x" unless all three of the other argument pairs are provided.

That's what I was trying to achieve with the three calls to .required_unless_one(), but it seems to combine all three of them into the equivalent of a single required_unless_one call.

I would say that overall this is an improvement over before, as my issue now is not clearly a bug and the clear bug I was seeing before is now gone.

My debug output now with passing in just "-a":

DEBUG:clap:Parser::propagate_settings: self=Sample App, 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:Parser::get_matches_with: Begin parsing '"-a"' ([45, 97])
DEBUG:clap:Parser::is_new_arg:"-a":NotFound
DEBUG:clap:Parser::is_new_arg: arg_allows_tac=false
DEBUG:clap:Parser::is_new_arg: - found
DEBUG:clap:Parser::is_new_arg: starts_new_arg=true
DEBUG:clap:Parser::possible_subcommand: arg="-a"
DEBUG:clap:Parser::get_matches_with: possible_sc=false, sc=None
DEBUG:clap:ArgMatcher::process_arg_overrides:None;
DEBUG:clap:Parser::parse_short_arg: full_arg="-a"
DEBUG:clap:Parser::parse_short_arg:iter:a
DEBUG:clap:Parser::parse_short_arg:iter:a: Found valid flag
DEBUG:clap:Parser::check_for_help_and_version_char;
DEBUG:clap:Parser::check_for_help_and_version_char: Checking if -a is help or version...Neither
DEBUG:clap:Parser::parse_flag;
DEBUG:clap:ArgMatcher::inc_occurrence_of: arg=a
DEBUG:clap:ArgMatcher::inc_occurrence_of: first instance
DEBUG:clap:Parser::groups_for_arg: name=a
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:Parser:get_matches_with: After parse_short_arg Flag
DEBUG:clap:ArgMatcher::process_arg_overrides:Some("a");
DEBUG:clap:Parser::remove_overrides:[];
DEBUG:clap:Validator::validate;
DEBUG:clap:Parser::add_defaults;
DEBUG:clap:Validator::validate_blacklist;
DEBUG:clap:Validator::validate_blacklist:iter:a;
DEBUG:clap:Parser::groups_for_arg: name=a
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:Validator::validate_required: required=["x"];
DEBUG:clap:Validator::validate_required:iter:x:
DEBUG:clap:Validator::is_missing_required_ok: a=x
DEBUG:clap:Validator::validate_arg_conflicts: a="x";
DEBUG:clap:Validator::validate_required_unless: a="x";
DEBUG:clap:Validator::validate_matched_args;
DEBUG:clap:Validator::validate_matched_args:iter:a: vals=[]
DEBUG:clap:Validator::validate_arg_requires:a;
DEBUG:clap:Validator::validate_arg_num_occurs: a=a;
DEBUG:clap:usage::create_usage_with_title;
DEBUG:clap:usage::create_usage_no_title;
DEBUG:clap:usage::get_required_usage_from: reqs=["x"], 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=["x"]
DEBUG:clap:usage::get_required_usage_from: args_in_groups=[]
DEBUG:clap:usage::get_required_usage_from:iter:x:
DEBUG:clap:usage::needs_flags_tag;
DEBUG:clap:usage::needs_flags_tag:iter: f=a;
DEBUG:clap:Parser::groups_for_arg: name=a
DEBUG:clap:Parser::groups_for_arg: No groups defined
DEBUG:clap:usage::needs_flags_tag:iter: [FLAGS] required
DEBUG:clap:usage::create_help_usage: usage=test_project [FLAGS] -x
DEBUG:clap:ArgMatcher::get_global_values: global_arg_vec=[]

epage · 2021-12-06T16:39:31Z

Comment by willmurphyscode
Thursday Jan 25, 2018 at 13:29 GMT

@gbear605 Let me see if I understand this issue. You basically want this behavior (in some pseudocode here):

if (x.passed?) || ((a.passed? || b.passed?) && (c.passed? || d.passed?) && (e.passed? || f.passed?)):
   valid
else:
   invalid

Is that correct? Right now, required_unless_one simply builds up an array of arguments and treats the argument it modifies as optional unless one of them is passed. This seems like a new feature, basically making required_unless_one able to build up multiple predicates if it is called more than once on the same arg.

epage · 2021-12-06T16:39:32Z

Comment by gbear605
Thursday Jan 25, 2018 at 13:32 GMT

That's correct.

So, it seems that the actual bug involved here was fixed by @willmurphyscode's fix and now this is just a feature request for that behavior.

epage · 2021-12-06T16:39:33Z

Comment by kbknapp
Monday Feb 12, 2018 at 20:08 GMT

Thanks for the work @willmurphyscode ! I'm going to remove the bug and 2.x labels. However, I'll leave this feature request open for when I'm opening the feature request window for 3.x.

I have to keep blinders on or I'll never finish 3.x.

epage added C: parsing D: hard P4: nice to have T: new feature Z: mentored labels Dec 6, 2021

epage added this to the 3.1 milestone Dec 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: add Arg::required_unless_all #85

Feature: add Arg::required_unless_all #85

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

Feature: add Arg::required_unless_all #85

Feature: add Arg::required_unless_all #85

Comments

epage commented Dec 6, 2021

Rust Version

Affected Version of clap

Expected Behavior Summary

Actual Behavior Summary

Steps to Reproduce the issue

Sample Code or Link to Sample Code

Debug output

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021

epage commented Dec 6, 2021