Skip to content

Commit

Permalink
fix: Specialise unrolling reqs for different scenarios
Browse files Browse the repository at this point in the history
  • Loading branch information
pksunkara committed Dec 9, 2021
1 parent 3b409ee commit 936d3e8
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 25 deletions.
55 changes: 39 additions & 16 deletions src/build/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3124,21 +3124,10 @@ impl<'help> App<'help> {
args
}

pub(crate) fn unroll_requirements_for_arg(&self, arg: &Id, matcher: &ArgMatcher) -> Vec<Id> {
let requires_if_or_not = |(val, req_arg): &(Option<&str>, Id)| -> Option<Id> {
let predicate = if let Some(val) = val {
ArgPredicate::Equals(*val)
} else {
ArgPredicate::IsPresent
};

if matcher.check_explicit(arg, predicate) {
Some(req_arg.clone())
} else {
None
}
};

pub(crate) fn unroll_requirements_for_arg<F>(&self, func: F, arg: &Id) -> Vec<Id>
where
F: Fn(&(Option<&str>, Id)) -> Option<Id>,
{
let mut processed = vec![];
let mut r_vec = vec![arg];
let mut args = vec![];
Expand All @@ -3151,7 +3140,7 @@ impl<'help> App<'help> {
processed.push(a);

if let Some(arg) = self.find(a) {
for r in arg.requires.iter().filter_map(requires_if_or_not) {
for r in arg.requires.iter().filter_map(&func) {
if let Some(req) = self.find(&r) {
if !req.requires.is_empty() {
r_vec.push(&req.id)
Expand All @@ -3165,6 +3154,40 @@ impl<'help> App<'help> {
args
}

pub(crate) fn unroll_requirements_for_arg_during_validation(
&self,
arg: &Id,
matcher: &ArgMatcher,
) -> Vec<Id> {
let requires_if_or_not = |(val, req_arg): &(Option<&str>, Id)| {
let predicate = if let Some(val) = val {
ArgPredicate::Equals(*val)
} else {
ArgPredicate::IsPresent
};

if matcher.check_explicit(arg, predicate) {
Some(req_arg.clone())
} else {
None
}
};

self.unroll_requirements_for_arg(requires_if_or_not, arg)
}

pub(crate) fn unroll_requirements_for_arg_during_usage(&self, arg: &Id) -> Vec<Id> {
let requires_if_or_not = |(val, req_arg): &(Option<&str>, Id)| {
if val.is_none() {
Some(req_arg.clone())
} else {
None
}
};

self.unroll_requirements_for_arg(requires_if_or_not, arg)
}

/// Find a flag subcommand name by short flag or an alias
pub(crate) fn find_short_subcmd(&self, c: char) -> Option<&str> {
self.get_subcommands()
Expand Down
11 changes: 5 additions & 6 deletions src/output/usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,13 +381,12 @@ impl<'help, 'app, 'parser> Usage<'help, 'app, 'parser> {
let mut unrolled_reqs = IndexSet::new();

for a in self.p.required.iter() {
if let Some(m) = matcher {
for aa in self.p.app.unroll_requirements_for_arg(a, m) {
// if we don't check for duplicates here this causes duplicate error messages
// see https://github.com/clap-rs/clap/issues/2770
unrolled_reqs.insert(aa);
}
for aa in self.p.app.unroll_requirements_for_arg_during_usage(a) {
// if we don't check for duplicates here this causes duplicate error messages
// see https://github.com/clap-rs/clap/issues/2770
unrolled_reqs.insert(aa);
}

// always include the required arg itself. it will not be enumerated
// by unroll_requirements_for_arg.
unrolled_reqs.insert(a.clone());
Expand Down
6 changes: 5 additions & 1 deletion src/parse/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,11 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
for name in matcher.arg_names() {
debug!("Validator::gather_requirements:iter:{:?}", name);
if let Some(arg) = self.p.app.find(name) {
for req in self.p.app.unroll_requirements_for_arg(&arg.id, matcher) {
for req in self
.p
.app
.unroll_requirements_for_arg_during_validation(&arg.id, matcher)
{
self.p.required.insert(req);
}
} else if let Some(g) = self.p.app.groups.iter().find(|grp| grp.id == *name) {
Expand Down
4 changes: 2 additions & 2 deletions tests/builder/double_require.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ static ONLY_B_ERROR: &str = "error: The following required arguments were not pr
-c
USAGE:
prog [OPTIONS] -c -b
prog [OPTIONS] -b -c
For more information try --help
";
Expand All @@ -25,7 +25,7 @@ static ONLY_C_ERROR: &str = "error: The following required arguments were not pr
-b
USAGE:
prog [OPTIONS] -b -c
prog [OPTIONS] -c -b
For more information try --help
";
Expand Down
75 changes: 75 additions & 0 deletions tests/builder/require.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,35 @@ USAGE:
For more information try --help
";

static POSITIONAL_REQ: &str = "error: The following required arguments were not provided:
<flag>
<opt>
USAGE:
clap-test <flag> <opt> [ARGS]
For more information try --help
";

static POSITIONAL_REQ_IF_NO_VAL: &str = "error: The following required arguments were not provided:
<flag>
USAGE:
clap-test <flag> [ARGS]
For more information try --help
";

static POSITIONAL_REQ_IF_VAL: &str = "error: The following required arguments were not provided:
<foo>
<opt>
USAGE:
clap-test <flag> <foo> <opt> [ARGS]
For more information try --help
";

static MISSING_REQ: &str = "error: The following required arguments were not provided:
--long-option-2 <option2>
<positional2>
Expand Down Expand Up @@ -112,6 +141,52 @@ fn positional_required_2() {
assert_eq!(m.value_of("flag").unwrap(), "someval");
}

#[test]
fn positional_required_with_requires() {
let app = App::new("positional_required")
.arg(Arg::new("flag").required(true).requires("opt"))
.arg(Arg::new("opt"))
.arg(Arg::new("bar"));

assert!(utils::compare_output(
app,
"clap-test",
POSITIONAL_REQ,
true
));
}

#[test]
fn positional_required_with_requires_if_no_value() {
let app = App::new("positional_required")
.arg(Arg::new("flag").required(true).requires_if("val", "opt"))
.arg(Arg::new("opt"))
.arg(Arg::new("bar"));

assert!(utils::compare_output(
app,
"clap-test",
POSITIONAL_REQ_IF_NO_VAL,
true
));
}

#[test]
fn positional_required_with_requires_if_value() {
let app = App::new("positional_required")
.arg(Arg::new("flag").required(true).requires_if("val", "opt"))
.arg(Arg::new("foo").required(true))
.arg(Arg::new("opt"))
.arg(Arg::new("bar"));

assert!(utils::compare_output(
app,
"clap-test val",
POSITIONAL_REQ_IF_VAL,
true
));
}

#[test]
fn group_required() {
let result = App::new("group_required")
Expand Down

0 comments on commit 936d3e8

Please sign in to comment.