Skip to content

Commit

Permalink
fix(Required Unless): fixes a bug where having required_unless set do…
Browse files Browse the repository at this point in the history
…esn't work when conflicts are also set

Closes #753
  • Loading branch information
kbknapp committed Nov 20, 2016
1 parent eb51316 commit d20331b
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 7 deletions.
22 changes: 15 additions & 7 deletions src/app/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1608,13 +1608,10 @@ impl<'a, 'b> Parser<'a, 'b>
fn validate_num_args(&self, matcher: &mut ArgMatcher) -> ClapResult<()> {
debugln!("fn=validate_num_args;");
for (name, ma) in matcher.iter() {
if self.groups.contains_key(&**name) {
continue;
} else if let Some(opt) = find_by_name!(self, name, opts, iter) {
debugln!("iter;name={}", name);
if let Some(opt) = find_by_name!(self, name, opts, iter) {
try!(self._validate_num_vals(opt, ma, matcher));
} else if let Some(pos) = self.positionals
.values()
.find(|p| &p.b.name == name) {
} else if let Some(pos) = find_by_name!(self, name, positionals, values) {
try!(self._validate_num_vals(pos, ma, matcher));
}
}
Expand Down Expand Up @@ -1685,7 +1682,9 @@ impl<'a, 'b> Parser<'a, 'b>
}

fn validate_required(&self, matcher: &ArgMatcher) -> ClapResult<()> {
debugln!("fn=validate_required;required={:?};", self.required);
'outer: for name in &self.required {
debugln!("iter;name={}", name);
if matcher.contains(name) {
continue 'outer;
}
Expand Down Expand Up @@ -1735,27 +1734,36 @@ impl<'a, 'b> Parser<'a, 'b>
fn is_missing_required_ok<A>(&self, a: &A, matcher: &ArgMatcher) -> bool
where A: AnyArg<'a, 'b>
{
debugln!("fn=is_missing_required_ok;a={}", a.name());
if let Some(bl) = a.blacklist() {
debugln!("Conflicts found...{:?}", bl);
for n in bl.iter() {
debugln!("iter;conflict={}", n);
if matcher.contains(n) ||
self.groups
.get(n)
.map_or(false, |g| g.args.iter().any(|an| matcher.contains(an))) {
return true;
}
}
} else if let Some(ru) = a.required_unless() {
}
if let Some(ru) = a.required_unless() {
debugln!("Required unless found...{:?}", ru);
let mut found_any = false;
for n in ru.iter() {
debugln!("iter;ru={}", n);
if matcher.contains(n) ||
self.groups
.get(n)
.map_or(false, |g| g.args.iter().any(|an| matcher.contains(an))) {
if !a.is_set(ArgSettings::RequiredUnlessAll) {
debugln!("Doesn't require all...returning true");
return true;
}
debugln!("Requires all...next");
found_any = true;
} else if a.is_set(ArgSettings::RequiredUnlessAll) {
debugln!("Not in matcher, or group and requires all...returning false");
return false;
}
}
Expand Down
17 changes: 17 additions & 0 deletions tests/require.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,23 @@ fn arg_require_group_3() {

// REQUIRED_UNLESS

#[test]
fn issue_753() {
let m = App::new("test")
.arg(Arg::from_usage("-l, --list 'List available interfaces (and stop there)'"))
.arg(Arg::from_usage("-i, --iface=[INTERFACE] 'Ethernet interface for fetching NTP packets'")
.required_unless("list"))
.arg(Arg::from_usage("-f, --file=[TESTFILE] 'Fetch NTP packets from pcap file'")
.conflicts_with("iface")
.required_unless("list"))
.arg(Arg::from_usage("-s, --server=[SERVER_IP] 'NTP server IP address'")
.required_unless("list"))
.arg(Arg::from_usage("-p, --port=[SERVER_PORT] 'NTP server port'")
.default_value("123"))
.get_matches_from_safe(vec!["test", "--list"]);
assert!(m.is_ok());
}

#[test]
fn required_unless() {
let res = App::new("unlesstest")
Expand Down

0 comments on commit d20331b

Please sign in to comment.