Merge pull request #2245 from ldm0/msg

Fix issue 1794 error message
clap-rs · Dec 8, 2020 · 5847303 · 5847303
2 parents 041bfda + d7e2fd6
commit 5847303
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 12 deletions.
diff --git a/src/output/usage.rs b/src/output/usage.rs
@@ -214,7 +214,7 @@ impl<'help, 'app, 'parser> Usage<'help, 'app, 'parser> {
     fn get_args_tag(&self, incl_reqs: bool) -> Option<String> {
         debug!("Usage::get_args_tag; incl_reqs = {:?}", incl_reqs);
         let mut count = 0;
-        'outer: for pos in self
+        for pos in self
             .p
             .app
             .get_positionals()
@@ -223,24 +223,22 @@ impl<'help, 'app, 'parser> Usage<'help, 'app, 'parser> {
             .filter(|pos| !pos.is_set(ArgSettings::Last))
         {
             debug!("Usage::get_args_tag:iter:{}", pos.name);
-            for grp_s in self.p.app.groups_for_arg(&pos.id) {
+            let in_required_group = self.p.app.groups_for_arg(&pos.id).any(|grp_s| {
                 debug!("Usage::get_args_tag:iter:{:?}:iter:{:?}", pos.name, grp_s);
                 // if it's part of a required group we don't want to count it
-                if self
-                    .p
+                self.p
                     .app
                     .groups
                     .iter()
                     .any(|g| g.required && (g.id == grp_s))
-                {
-                    continue 'outer;
-                }
+            });
+            if !in_required_group {
+                count += 1;
+                debug!(
+                    "Usage::get_args_tag:iter: {} Args not required or hidden",
+                    count
+                );
             }
-            count += 1;
-            debug!(
-                "Usage::get_args_tag:iter: {} Args not required or hidden",
-                count
-            );
         }
 
         if !self.p.is_set(AS::DontCollapseArgsInUsage) && count > 1 {
@@ -257,6 +255,15 @@ impl<'help, 'app, 'parser> Usage<'help, 'app, 'parser> {
                     !pos.is_set(ArgSettings::Required)
                         && !pos.is_set(ArgSettings::Hidden)
                         && !pos.is_set(ArgSettings::Last)
+                        && !self.p.app.groups_for_arg(&pos.id).any(|grp_s| {
+                            debug!("Usage::get_args_tag:iter:{:?}:iter:{:?}", pos.name, grp_s);
+                            // if it's part of a required group we don't want to count it
+                            self.p
+                                .app
+                                .groups
+                                .iter()
+                                .any(|g| g.required && (g.id == grp_s))
+                        })
                 })
                 .expect(INTERNAL_ERROR_MSG);
 

diff --git a/tests/help.rs b/tests/help.rs
@@ -2360,3 +2360,38 @@ fn option_usage_order() {
         false
     ));
 }
+
+#[test]
+fn issue_1794_usage() {
+    static USAGE_WITH_GROUP: &'static str = "hello 
+
+USAGE:
+    deno <pos1|--option1> [pos2]
+
+ARGS:
+    <pos1>    
+    <pos2>    
+
+FLAGS:
+    -h, --help       Prints help information
+        --option1    
+    -V, --version    Prints version information";
+
+    let app = clap::App::new("hello")
+        .bin_name("deno")
+        .arg(Arg::new("option1").long("option1").takes_value(false))
+        .arg(Arg::new("pos1").takes_value(true))
+        .group(
+            ArgGroup::new("arg1")
+                .args(&["pos1", "option1"])
+                .required(true),
+        )
+        .arg(Arg::new("pos2").takes_value(true));
+
+    assert!(utils::compare_output(
+        app,
+        "deno --help",
+        USAGE_WITH_GROUP,
+        false
+    ));
+}