Repository: clap-rs/clap
Pull request #1929 (2020-05-15 03:24:54)

fix(errors): incorrect missing argument error messages
correctly filter the suggestion arguments returned by the missing_required_argument error.
- a simple argument has been supplied, don't include it
- if a required group has been satisfied, don't include it
- add test for these cases.

pksunkara: Looks like github and azure connection have some issues.
pksunkara: Looks like something happened, can you please re-push? Thanks

Patch:
diff --git a/src/output/usage.rs b/src/output/usage.rs
index db6693bf..3bd0e82d 100644
--- a/src/output/usage.rs
+++ b/src/output/usage.rs
@@ -332,11 +332,17 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> {
                 for aa in self.p.app.unroll_requirements_for_arg(a, m) {
                     unrolled_reqs.push(aa);
                 }
-            } else {
-                unrolled_reqs.push(a.clone());
             }
+            // alway include the required arg itself. it will not be enumerated
+            // by unroll_requirements_for_arg.
+            unrolled_reqs.push(a.clone());
         }
 
+        debug!(
+            "Usage::get_required_usage_from: unrolled_reqs={:?}",
+            unrolled_reqs
+        );
+
         let args_in_groups = self
             .p
             .app
@@ -396,6 +402,19 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> {
             .iter()
             .filter(|n| self.p.app.groups.iter().any(|g| g.id == **n))
         {
+            // don't print requirement for required groups that have an arg.
+            if let Some(m) = matcher {
+                let have_group_entry = self
+                    .p
+                    .app
+                    .unroll_args_in_group(&g)
+                    .iter()
+                    .any(|arg| m.contains(&arg));
+                if have_group_entry {
+                    continue;
+                }
+            }
+
             let elem = self.p.app.format_group(g);
             if !g_vec.contains(&elem) {
                 g_vec.push(elem);
diff --git a/src/parse/validator.rs b/src/parse/validator.rs
index eed0cfa0..e0e3285c 100644
--- a/src/parse/validator.rs
+++ b/src/parse/validator.rs
@@ -594,7 +594,7 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
         let req_args = if let Some(x) = incl {
             usg.get_required_usage_from(&[x.clone()], Some(matcher), true)
         } else {
-            usg.get_required_usage_from(&[], None, true)
+            usg.get_required_usage_from(&[], Some(matcher), true)
         };
 
         debug!(


Test patch:
diff --git a/tests/require.rs b/tests/require.rs
index a143b333..df9871b2 100644
--- a/tests/require.rs
+++ b/tests/require.rs
@@ -10,6 +10,23 @@ USAGE:
 
 For more information try --help";
 
+static REQUIRE_EQUALS_FILTERED: &str = "error: The following required arguments were not provided:
+    --opt=<FILE>
+
+USAGE:
+    clap-test --opt=<FILE> --foo=<FILE>
+
+For more information try --help";
+
+static REQUIRE_EQUALS_FILTERED_GROUP: &str =
+    "error: The following required arguments were not provided:
+    --opt=<FILE>
+
+USAGE:
+    clap-test --opt=<FILE> --foo=<FILE> <--g1=<FILE>|--g2=<FILE>>
+
+For more information try --help";
+
 static MISSING_REQ: &str = "error: The following required arguments were not provided:
     <positional2>
     --long-option-2 <option2>
@@ -505,6 +522,40 @@ fn required_if_val_present_fail() {
     assert_eq!(res.unwrap_err().kind, ErrorKind::MissingRequiredArgument);
 }
 
+#[test]
+fn list_correct_required_args() {
+    let app = App::new("Test app")
+        .version("1.0")
+        .author("F0x06")
+        .about("Arg test")
+        .arg(
+            Arg::new("target")
+                .takes_value(true)
+                .required(true)
+                .possible_values(&["file", "stdout"])
+                .long("target"),
+        )
+        .arg(
+            Arg::new("input")
+                .takes_value(true)
+                .required(true)
+                .long("input"),
+        )
+        .arg(
+            Arg::new("output")
+                .takes_value(true)
+                .required(true)
+                .long("output"),
+        );
+
+    assert!(utils::compare_output(
+        app,
+        "test --input somepath --target file",
+        COND_REQ_IN_USAGE,
+        true
+    ));
+}
+
 #[test]
 fn required_if_val_present_fail_error_output() {
     let app = App::new("Test app")
@@ -639,6 +690,83 @@ fn require_eq() {
     ));
 }
 
+#[test]
+fn require_eq_filtered() {
+    let app = App::new("clap-test")
+        .version("v1.4.8")
+        .arg(
+            Arg::new("opt")
+                .long("opt")
+                .short('o')
+                .required(true)
+                .require_equals(true)
+                .value_name("FILE")
+                .about("some"),
+        )
+        .arg(
+            Arg::new("foo")
+                .long("foo")
+                .short('f')
+                .required(true)
+                .require_equals(true)
+                .value_name("FILE")
+                .about("some other arg"),
+        );
+    assert!(utils::compare_output(
+        app,
+        "clap-test -f=blah",
+        REQUIRE_EQUALS_FILTERED,
+        true
+    ));
+}
+
+#[test]
+fn require_eq_filtered_group() {
+    let app = App::new("clap-test")
+        .version("v1.4.8")
+        .arg(
+            Arg::new("opt")
+                .long("opt")
+                .short('o')
+                .required(true)
+                .require_equals(true)
+                .value_name("FILE")
+                .about("some"),
+        )
+        .arg(
+            Arg::new("foo")
+                .long("foo")
+                .short('f')
+                .required(true)
+                .require_equals(true)
+                .value_name("FILE")
+                .about("some other arg"),
+        )
+        .arg(
+            Arg::new("g1")
+                .long("g1")
+                .require_equals(true)
+                .value_name("FILE"),
+        )
+        .arg(
+            Arg::new("g2")
+                .long("g2")
+                .require_equals(true)
+                .value_name("FILE"),
+        )
+        .group(
+            ArgGroup::new("test_group")
+                .args(&["g1", "g2"])
+                .required(true),
+        );
+    assert!(utils::compare_output(
+        app,
+        "clap-test -f=blah --g1=blah",
+        REQUIRE_EQUALS_FILTERED_GROUP,
+        true
+    ));
+}
+
 static ISSUE_1158: &str = "error: The following required arguments were not provided:
     -x <X>
     -y <Y>