Skip to content

Commit

Permalink
fix(error): Don't duplicate args in usage
Browse files Browse the repository at this point in the history
Gave up trying to decipher the existing logic for safe ways to
de-duplicate manually and switched to an `IndexSet` to enforce only one
of each argument exists.

Fixes #3556
  • Loading branch information
epage committed May 4, 2022
1 parent 7deb724 commit 024cdc0
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 7 deletions.
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
5 changes: 4 additions & 1 deletion src/parse/validator.rs
Expand Up @@ -566,7 +566,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 Down
27 changes: 27 additions & 0 deletions tests/builder/require.rs
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);
}

0 comments on commit 024cdc0

Please sign in to comment.