Skip to content

Commit

Permalink
fix(parser): Exclusive overrides required
Browse files Browse the repository at this point in the history
`Arg::exclusive` is just another way of defining conflicts, so a
present-exclusive arg should override required like other conflicts.

Instead of going through the message of enumerating all other arguments
as exclusive, I shortcutted it and special case exclusive in the
required check like we do with conflicts.  The big downside is the
implicit coupling between the code paths rather than having a consistent
abstraction for covering conflicts.

Fixes clap-rs#3595
  • Loading branch information
epage committed May 4, 2022
1 parent 8847222 commit c2a5b7c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
13 changes: 12 additions & 1 deletion src/parse/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,11 +479,22 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
debug!("Validator::validate_required: required={:?}", self.required);
self.gather_requires(matcher);

let is_exclusive_present = matcher.arg_names().any(|name| {
self.cmd
.find(name)
.map(|arg| arg.is_exclusive_set())
.unwrap_or_default()
});
debug!(
"Validator::validate_required: is_exclusive_present={}",
is_exclusive_present
);

for arg_or_group in self.required.iter().filter(|r| !matcher.contains(r)) {
debug!("Validator::validate_required:iter:aog={:?}", arg_or_group);
if let Some(arg) = self.cmd.find(arg_or_group) {
debug!("Validator::validate_required:iter: This is an arg");
if !self.is_missing_required_ok(arg, matcher, conflicts) {
if !is_exclusive_present && !self.is_missing_required_ok(arg, matcher, conflicts) {
return self.missing_required_error(matcher, vec![]);
}
} else if let Some(group) = self.cmd.find_group(arg_or_group) {
Expand Down
17 changes: 17 additions & 0 deletions tests/builder/conflicts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,3 +490,20 @@ fn group_conflicts_with_default_arg() {
assert_eq!(m.value_of("opt"), Some("default"));
assert!(m.is_present("flag"));
}

#[test]
fn exclusive_with_required() {
let cmd = Command::new("bug")
.arg(Arg::new("test").long("test").exclusive(true))
.arg(Arg::new("input").takes_value(true).required(true));

cmd.clone()
.try_get_matches_from(["bug", "--test", "required"])
.unwrap_err();

cmd.clone()
.try_get_matches_from(["bug", "required"])
.unwrap();

cmd.clone().try_get_matches_from(["bug", "--test"]).unwrap();
}

0 comments on commit c2a5b7c

Please sign in to comment.