Merge pull request #3689 from epage/usage

fix(error): Don't duplicate args in usage
clap-rs · May 6, 2022 · dcda237 · dcda237
2 parents 2375a8e + e23c786
commit dcda237
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 13 deletions.
diff --git a/src/output/usage.rs b/src/output/usage.rs
@@ -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();
 
@@ -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
@@ -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()
@@ -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());
             }
         }
 

diff --git a/src/parse/validator.rs b/src/parse/validator.rs
@@ -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();
@@ -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={:#?}",
@@ -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)

diff --git a/tests/builder/double_require.rs b/tests/builder/double_require.rs
@@ -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
 ";
@@ -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
 ";

diff --git a/tests/builder/require.rs b/tests/builder/require.rs
@@ -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
 ";
@@ -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);
+}