fix(error): Be more accurate in smart usage

For some errors, we use the unroll logic to get the list of required
arguments.  The usage then does the same, but without a matcher.  This
was causing the lists to not match.

As a side effect, this fixed an ordering issue where we were putting the
present arg after the not-present arg.  I assume its because we ended up
reporting the items twice but the first time is correctly ordered and
gets precedence.

This was split out of #3020
This commit is contained in:
Ed Page 2022-02-01 19:04:53 -06:00
parent d318752cac
commit 06aa418fa6
5 changed files with 101 additions and 16 deletions

View file

@ -3238,16 +3238,24 @@ impl<'help> App<'help> {
args
}
pub(crate) fn unroll_requirements_for_arg(&self, arg: &Id, matcher: &ArgMatcher) -> Vec<Id> {
pub(crate) fn unroll_requirements_for_arg(
&self,
arg: &Id,
matcher: Option<&ArgMatcher>,
) -> Vec<Id> {
let requires_if_or_not = |(val, req_arg): &(ArgPredicate<'_>, Id)| -> Option<Id> {
match val {
ArgPredicate::Equals(v) => {
if matcher
.get(arg)
.map(|ma| ma.contains_val_os(v))
.unwrap_or(false)
{
Some(req_arg.clone())
if let Some(matcher) = matcher {
if matcher
.get(arg)
.map(|ma| ma.contains_val_os(v))
.unwrap_or(false)
{
Some(req_arg.clone())
} else {
None
}
} else {
None
}

View file

@ -360,12 +360,10 @@ impl<'help, 'app> Usage<'help, 'app> {
let mut unrolled_reqs = IndexSet::new();
for a in self.required.iter() {
if let Some(m) = matcher {
for aa in self.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.app.unroll_requirements_for_arg(a, matcher) {
// 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.

View file

@ -272,7 +272,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(&arg.id, Some(matcher))
{
self.p.required.insert(req);
}
} else if let Some(g) = self.p.app.find_group(name) {

View file

@ -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
";
@ -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
";

View file

@ -115,6 +115,81 @@ 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
));
}
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
";
#[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
));
}
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
";
#[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
));
}
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
";
#[test]
fn group_required() {
let result = App::new("group_required")