Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(error): Don't duplicate args in usage #3689

Merged
merged 3 commits into from May 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/output/usage.rs
Expand Up @@ -333,14 +333,14 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
incls: &[Id],
matcher: Option<&ArgMatcher>,
incl_last: bool,
) -> Vec<String> {
) -> IndexSet<String> {
debug!(
"Usage::get_required_usage_from: incls={:?}, matcher={:?}, incl_last={:?}",
incls,
matcher.is_some(),
incl_last
);
let mut ret_val = Vec::new();
let mut ret_val = IndexSet::new();

let mut unrolled_reqs = IndexSet::new();

Expand Down Expand Up @@ -399,7 +399,7 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
{
debug!("Usage::get_required_usage_from:iter:{:?}", a);
let arg = self.cmd.find(a).expect(INTERNAL_ERROR_MSG).to_string();
ret_val.push(arg);
ret_val.insert(arg);
}
let mut g_vec: Vec<String> = vec![];
for g in unrolled_reqs
Expand All @@ -423,7 +423,7 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
g_vec.push(elem);
}
}
ret_val.extend_from_slice(&g_vec);
ret_val.extend(g_vec);

let mut pvec = unrolled_reqs
.iter()
Expand All @@ -438,9 +438,9 @@ impl<'help, 'cmd> Usage<'help, 'cmd> {
pvec.sort_by_key(|(ind, _)| *ind); // sort by index

for (_, p) in pvec {
debug!("Usage::get_required_usage_from:iter:{:?}", p.id);
debug!("Usage::get_required_usage_from:push:{:?}", p.id);
if !args_in_groups.contains(&p.id) {
ret_val.push(p.to_string());
ret_val.insert(p.to_string());
}
}

Expand Down
13 changes: 9 additions & 4 deletions src/parse/validator.rs
Expand Up @@ -290,6 +290,10 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
let used_filtered: Vec<Id> = matcher
.arg_names()
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|n| {
// Filter out the args we don't want to specify.
self.cmd.find(n).map_or(true, |a| !a.is_hide_set())
})
.filter(|key| !conflicting_keys.contains(key))
.cloned()
.collect();
Expand Down Expand Up @@ -582,7 +586,10 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {

let usg = Usage::new(self.cmd).required(&self.required);

let req_args = usg.get_required_usage_from(&incl, Some(matcher), true);
let req_args = usg
.get_required_usage_from(&incl, Some(matcher), true)
.into_iter()
.collect::<Vec<_>>();

debug!(
"Validator::missing_required_error: req_args={:#?}",
Expand All @@ -594,9 +601,7 @@ impl<'help, 'cmd> Validator<'help, 'cmd> {
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|n| {
// Filter out the args we don't want to specify.
self.cmd
.find(n)
.map_or(true, |a| !a.is_hide_set() && !self.required.contains(&a.id))
self.cmd.find(n).map_or(true, |a| !a.is_hide_set())
})
.cloned()
.chain(incl)
Expand Down
4 changes: 2 additions & 2 deletions tests/builder/double_require.rs
Expand Up @@ -16,7 +16,7 @@ static ONLY_B_ERROR: &str = "error: The following required arguments were not pr
-c

USAGE:
prog [OPTIONS] -b -c
prog -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] -c -b
prog -c -b

For more information try --help
";
Expand Down
29 changes: 28 additions & 1 deletion tests/builder/require.rs
Expand Up @@ -170,7 +170,7 @@ static POSITIONAL_REQ_IF_VAL: &str = "error: The following required arguments we
<opt>

USAGE:
clap-test <flag> <foo> <opt> [ARGS]
clap-test <flag> <foo> <opt>

For more information try --help
";
Expand Down Expand Up @@ -1345,3 +1345,30 @@ fn required_unless_all_on_default_value() {

assert!(result.is_err(), "{:?}", result.unwrap());
}

#[test]
fn required_error_doesnt_duplicate() {
let cmd = Command::new("Clap-created-USAGE-string-bug")
.arg(Arg::new("a").required(true))
.arg(
Arg::new("b")
.short('b')
.takes_value(true)
.conflicts_with("c"),
)
.arg(
Arg::new("c")
.short('c')
.takes_value(true)
.conflicts_with("b"),
);
const EXPECTED: &str = "\
error: The argument '-b <b>' cannot be used with '-c <c>'

USAGE:
clap-test -b <b> <a>

For more information try --help
";
utils::assert_output(cmd, "clap-test aaa -b bbb -c ccc", EXPECTED, true);
}