Skip to content

Commit

Permalink
fix(Requirements): fixes an issue where conflicting args would still …
Browse files Browse the repository at this point in the history
…show up as required

Prior to this commit if one had two requirements which conflicted with each other, and an
error was generated due to a totally seperate argument, BOTH required but conflicting args
would show up in the help message as "missing but required"

This is now fixed and the *used* required arg shows up, but the conflicting required arg
has been dropped.

Closes #1158
  • Loading branch information
kbknapp committed Feb 5, 2018
1 parent 2961526 commit e06cefa
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/app/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ where
fn add_reqs(&mut self, a: &Arg<'a, 'b>) {
if a.is_set(ArgSettings::Required) {
// If the arg is required, add all it's requirements to master required list
self.required.push(a.b.name);
if let Some(ref areqs) = a.b.requires {
for name in areqs
.iter()
Expand All @@ -260,7 +261,6 @@ where
self.required.push(name);
}
}
self.required.push(a.b.name);
}
}

Expand Down
45 changes: 37 additions & 8 deletions src/app/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,14 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
self.0.add_defaults(matcher)?;
if let ParseResult::Opt(a) = needs_val_of {
debugln!("Validator::validate: needs_val_of={:?}", a);
let o = self.0
let o = {
self.0
.opts
.iter()
.find(|o| o.b.name == a)
.expect(INTERNAL_ERROR_MSG);
.expect(INTERNAL_ERROR_MSG)
.clone()
};
self.validate_required(matcher)?;
reqs_validated = true;
let should_err = if let Some(v) = matcher.0.args.get(&*o.b.name) {
Expand All @@ -50,7 +53,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
};
if should_err {
return Err(Error::empty_value(
o,
&o,
&*usage::create_error_usage(self.0, matcher, None),
self.0.color(),
));
Expand Down Expand Up @@ -438,20 +441,46 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
Ok(())
}

fn validate_required(&self, matcher: &ArgMatcher) -> ClapResult<()> {
fn validate_required(&mut self, matcher: &ArgMatcher) -> ClapResult<()> {
debugln!(
"Validator::validate_required: required={:?};",
self.0.required
);

'outer: for name in &self.0.required {
let mut should_err = false;
let mut to_rem = Vec::new();
for name in &self.0.required {
debugln!("Validator::validate_required:iter:{}:", name);
if matcher.contains(name) {
continue 'outer;
continue;
}
if let Some(a) = find_any_by_name!(self.0, *name) {
if to_rem.contains(name) {
continue;
} else if let Some(a) = find_any_by_name!(self.0, *name) {
if self.is_missing_required_ok(a, matcher) {
continue 'outer;
to_rem.push(a.name());
if let Some(reqs) = a.requires() {
for r in reqs
.iter()
.filter(|&&(val, _)| val.is_none())
.map(|&(_, name)| name)
{
to_rem.push(r);
}
}
continue;
}
}
should_err = true;
break;
}
if should_err {
for r in &to_rem {
'inner: for i in (0 .. self.0.required.len()).rev() {
if &self.0.required[i] == r {
self.0.required.swap_remove(i);
break 'inner;
}
}
}
return self.missing_required_error(matcher, None);
Expand Down

0 comments on commit e06cefa

Please sign in to comment.