Repository: clap-rs/clap
Pull request #4385 (2022-10-13 19:04:00)

fix(error): Be consistent with rustc diagnostic guidelines
From
https://rustc-dev-guide.rust-lang.org/diagnostics.html#suggestion-style-guide

> Suggestions should not be a question. In particular, language like
> "did you mean" should be avoided. Sometimes, it's unclear why a
> particular suggestion is being made. In these cases, it's better to be
> upfront about what the suggestion is.
>
> The message may contain further instruction such as "to do xyz, use"
> or "to do xyz, use abc".

Inspired by #2766


Patch:
diff --git a/src/error/format.rs b/src/error/format.rs
index 2e0f8e9f..65a4e285 100644
--- a/src/error/format.rs
+++ b/src/error/format.rs
@@ -64,23 +64,40 @@ impl ErrorFormatter for RichFormatter {
             }
         }
 
+        let mut suggested = false;
         if let Some(valid) = error.get(ContextKind::SuggestedSubcommand) {
-            styled.none("\n\n");
-            did_you_mean(&mut styled, valid);
+            styled.none("\n");
+            if !suggested {
+                styled.none("\n");
+                suggested = true;
+            }
+            did_you_mean(&mut styled, "subcommand", valid);
         }
         if let Some(valid) = error.get(ContextKind::SuggestedArg) {
-            styled.none("\n\n");
-            did_you_mean(&mut styled, valid);
+            styled.none("\n");
+            if !suggested {
+                styled.none("\n");
+                suggested = true;
+            }
+            did_you_mean(&mut styled, "argument", valid);
         }
         if let Some(valid) = error.get(ContextKind::SuggestedValue) {
-            styled.none("\n\n");
-            did_you_mean(&mut styled, valid);
+            styled.none("\n");
+            if !suggested {
+                styled.none("\n");
+                suggested = true;
+            }
+            did_you_mean(&mut styled, "value", valid);
         }
         let suggestions = error.get(ContextKind::Suggested);
         if let Some(ContextValue::StyledStrs(suggestions)) = suggestions {
+            if !suggested {
+                styled.none("\n");
+            }
             for suggestion in suggestions {
-                styled.none("\n\n");
+                styled.none("\n");
                 styled.none(TAB);
+                styled.good("note: ");
                 styled.extend(suggestion.iter());
             }
         }
@@ -149,7 +166,7 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
             if let Some(ContextValue::String(invalid_arg)) = invalid_arg {
                 styled.none("Equal sign is needed when assigning values to '");
                 styled.warning(invalid_arg);
-                styled.none("'.");
+                styled.none("'");
                 true
             } else {
                 false
@@ -391,15 +408,19 @@ fn try_help(styled: &mut StyledStr, help: Option<&str>) {
 }
 
 #[cfg(feature = "error-context")]
-fn did_you_mean(styled: &mut StyledStr, valid: &ContextValue) {
+fn did_you_mean(styled: &mut StyledStr, context: &str, valid: &ContextValue) {
     if let ContextValue::String(valid) = valid {
         styled.none(TAB);
-        styled.none("Did you mean '");
+        styled.good("note: ");
+        styled.none(context);
+        styled.none(" '");
         styled.good(valid);
-        styled.none("'?");
+        styled.none("' exists");
     } else if let ContextValue::Strings(valid) = valid {
         styled.none(TAB);
-        styled.none("Did you mean ");
+        styled.good("note: ");
+        styled.none(context);
+        styled.none(" ");
         for (i, valid) in valid.iter().enumerate() {
             if i != 0 {
                 styled.none(", ");
@@ -408,7 +429,11 @@ fn did_you_mean(styled: &mut StyledStr, valid: &ContextValue) {
             styled.good(valid);
             styled.none("'");
         }
-        styled.none("?");
+        if valid.len() == 1 {
+            styled.none(" exists");
+        } else {
+            styled.none(" exist");
+        }
     }
 }
 
diff --git a/src/error/mod.rs b/src/error/mod.rs
index af65529f..a83f9608 100644
--- a/src/error/mod.rs
+++ b/src/error/mod.rs
@@ -431,8 +431,9 @@ impl<F: ErrorFormatter> Error<F> {
         #[cfg(feature = "error-context")]
         {
             let mut styled_suggestion = StyledStr::new();
-            styled_suggestion
-                .none("If you believe you received this message in error, try re-running with '");
+            styled_suggestion.none("to pass '");
+            styled_suggestion.warning(&subcmd);
+            styled_suggestion.none("' as a value, use '");
             styled_suggestion.good(name);
             styled_suggestion.good(" -- ");
             styled_suggestion.good(&subcmd);
@@ -655,9 +656,9 @@ impl<F: ErrorFormatter> Error<F> {
             let mut suggestions = vec![];
             if suggested_trailing_arg {
                 let mut styled_suggestion = StyledStr::new();
-                styled_suggestion.none("If you tried to supply '");
+                styled_suggestion.none("to pass '");
                 styled_suggestion.warning(&arg);
-                styled_suggestion.none("' as a value rather than a flag, use '");
+                styled_suggestion.none("' as a value, use '");
                 styled_suggestion.good("-- ");
                 styled_suggestion.good(&arg);
                 styled_suggestion.none("'");
@@ -673,12 +674,12 @@ impl<F: ErrorFormatter> Error<F> {
             match did_you_mean {
                 Some((flag, Some(sub))) => {
                     let mut styled_suggestion = StyledStr::new();
-                    styled_suggestion.none("Did you mean to put '");
+                    styled_suggestion.none("'");
+                    styled_suggestion.good(sub);
+                    styled_suggestion.none(" ");
                     styled_suggestion.good("--");
                     styled_suggestion.good(flag);
-                    styled_suggestion.none("' after the subcommand '");
-                    styled_suggestion.good(sub);
-                    styled_suggestion.none("'?");
+                    styled_suggestion.none("' exists");
                     suggestions.push(styled_suggestion);
                 }
                 Some((flag, None)) => {
@@ -710,11 +711,11 @@ impl<F: ErrorFormatter> Error<F> {
         #[cfg(feature = "error-context")]
         {
             let mut styled_suggestion = StyledStr::new();
-            styled_suggestion.none("If you tried to supply '");
-            styled_suggestion.warning(&arg);
-            styled_suggestion.none("' as a subcommand, remove the '");
+            styled_suggestion.none("subcommand '");
+            styled_suggestion.good(&arg);
+            styled_suggestion.none("' exists; to use it, remove the '");
             styled_suggestion.warning("--");
-            styled_suggestion.none("' before it.");
+            styled_suggestion.none("' before it");
 
             err = err.extend_context_unchecked([
                 (ContextKind::InvalidArg, ContextValue::String(arg)),


Test patch:
diff --git a/examples/derive_ref/interop_tests.md b/examples/derive_ref/interop_tests.md
index b013b0c8..64c8d537 100644
--- a/examples/derive_ref/interop_tests.md
+++ b/examples/derive_ref/interop_tests.md
@@ -142,7 +142,7 @@ $ interop_hand_subcommand add --unknown
 ? failed
 error: Found argument '--unknown' which wasn't expected, or isn't valid in this context
 
-  If you tried to supply '--unknown' as a value rather than a flag, use '-- --unknown'
+  note: to pass '--unknown' as a value, use '-- --unknown'
 
 Usage: interop_hand_subcommand[EXE] add [NAME]...
 
diff --git a/tests/builder/empty_values.rs b/tests/builder/empty_values.rs
index 02aa7cf2..d818c0b9 100644
--- a/tests/builder/empty_values.rs
+++ b/tests/builder/empty_values.rs
@@ -118,7 +118,7 @@ fn no_empty_values_without_equals_but_requires_equals() {
     assert_eq!(m.unwrap_err().kind(), ErrorKind::NoEquals);
 
     static NO_EUQALS_ERROR: &str =
-        "error: Equal sign is needed when assigning values to '--config=<config>'.
+        "error: Equal sign is needed when assigning values to '--config=<config>'
 
 Usage: config [OPTIONS]
 
diff --git a/tests/builder/error.rs b/tests/builder/error.rs
index 59f034ac..123b0f22 100644
--- a/tests/builder/error.rs
+++ b/tests/builder/error.rs
@@ -141,7 +141,7 @@ fn suggest_trailing() {
     static MESSAGE: &str = "\
 error: Found argument '--foo' which wasn't expected, or isn't valid in this context
 
-  If you tried to supply '--foo' as a value rather than a flag, use '-- --foo'
+  note: to pass '--foo' as a value, use '-- --foo'
 
 Usage: rg [PATTERN]
 
diff --git a/tests/builder/flags.rs b/tests/builder/flags.rs
index 05804410..e2c5c84a 100644
--- a/tests/builder/flags.rs
+++ b/tests/builder/flags.rs
@@ -160,7 +160,7 @@ fn issue_1284_argument_in_flag_style() {
     const USE_FLAG_AS_ARGUMENT: &str = "\
 error: Found argument '--another-flag' which wasn't expected, or isn't valid in this context
 
-  If you tried to supply '--another-flag' as a value rather than a flag, use '-- --another-flag'
+  note: to pass '--another-flag' as a value, use '-- --another-flag'
 
 Usage: mycat [OPTIONS] [filename]
 
@@ -204,7 +204,7 @@ fn issue_2308_multiple_dashes() {
     static MULTIPLE_DASHES: &str = "\
 error: Found argument '-----' which wasn't expected, or isn't valid in this context
 
-  If you tried to supply '-----' as a value rather than a flag, use '-- -----'
+  note: to pass '-----' as a value, use '-- -----'
 
 Usage: test <arg>
 
diff --git a/tests/builder/opts.rs b/tests/builder/opts.rs
index cccd1146..2a671e7c 100644
--- a/tests/builder/opts.rs
+++ b/tests/builder/opts.rs
@@ -21,8 +21,7 @@ fn require_equals_fail() {
 #[test]
 #[cfg(feature = "error-context")]
 fn require_equals_fail_message() {
-    static NO_EQUALS: &str =
-        "error: Equal sign is needed when assigning values to '--config=<cfg>'.
+    static NO_EQUALS: &str = "error: Equal sign is needed when assigning values to '--config=<cfg>'
 
 Usage: prog [OPTIONS]
 
@@ -449,7 +448,7 @@ fn did_you_mean() {
     static DYM: &str = "\
 error: Found argument '--optio' which wasn't expected, or isn't valid in this context
 
-  Did you mean '--option'?
+  note: argument '--option' exists
 
 Usage: clap-test --option <opt>... [positional] [positional2] [positional3]...
 
@@ -547,7 +546,7 @@ fn issue_1073_suboptimal_flag_suggestion() {
     static DYM_ISSUE_1073: &str = "\
 error: Found argument '--files-without-matches' which wasn't expected, or isn't valid in this context
 
-  Did you mean '--files-without-match'?
+  note: argument '--files-without-match' exists
 
 Usage: ripgrep-616 --files-without-match
 
diff --git a/tests/builder/possible_values.rs b/tests/builder/possible_values.rs
index d7d6a6d5..7ab2e666 100644
--- a/tests/builder/possible_values.rs
+++ b/tests/builder/possible_values.rs
@@ -181,7 +181,7 @@ fn possible_values_output() {
 error: 'slo' isn't a valid value for '-O <option>'
   [possible values: slow, fast, \"ludicrous speed\"]
 
-  Did you mean 'slow'?
+  note: value 'slow' exists
 
 For more information try '--help'
 ";
@@ -215,7 +215,7 @@ fn possible_values_alias_output() {
 error: 'slo' isn't a valid value for '-O <option>'
   [possible values: slow, fast, \"ludicrous speed\"]
 
-  Did you mean 'slow'?
+  note: value 'slow' exists
 
 For more information try '--help'
 ";
@@ -253,7 +253,7 @@ fn possible_values_hidden_output() {
 error: 'slo' isn't a valid value for '-O <option>'
   [possible values: slow, fast, \"ludicrous speed\"]
 
-  Did you mean 'slow'?
+  note: value 'slow' exists
 
 For more information try '--help'
 ";
@@ -292,7 +292,7 @@ fn escaped_possible_values_output() {
 error: 'ludicrous' isn't a valid value for '-O <option>'
   [possible values: slow, fast, \"ludicrous speed\"]
 
-  Did you mean 'ludicrous speed'?
+  note: value 'ludicrous speed' exists
 
 For more information try '--help'
 ";
diff --git a/tests/builder/subcommands.rs b/tests/builder/subcommands.rs
index 8437aef0..e8de8156 100644
--- a/tests/builder/subcommands.rs
+++ b/tests/builder/subcommands.rs
@@ -100,9 +100,8 @@ fn subcmd_did_you_mean_output() {
     static DYM_SUBCMD: &str = "\
 error: The subcommand 'subcm' wasn't recognized
 
-  Did you mean 'subcmd'?
-
-  If you believe you received this message in error, try re-running with 'dym -- subcm'
+  note: subcommand 'subcmd' exists
+  note: to pass 'subcm' as a value, use 'dym -- subcm'
 
 Usage: dym [COMMAND]
 
@@ -121,9 +120,8 @@ fn subcmd_did_you_mean_output_ambiguous() {
     static DYM_SUBCMD_AMBIGUOUS: &str = "\
 error: The subcommand 'te' wasn't recognized
 
-  Did you mean 'test', 'temp'?
-
-  If you believe you received this message in error, try re-running with 'dym -- te'
+  note: subcommand 'test', 'temp' exist
+  note: to pass 'te' as a value, use 'dym -- te'
 
 Usage: dym [COMMAND]
 
@@ -143,7 +141,7 @@ fn subcmd_did_you_mean_output_arg() {
     static EXPECTED: &str = "\
 error: Found argument '--subcmarg' which wasn't expected, or isn't valid in this context
 
-  Did you mean to put '--subcmdarg' after the subcommand 'subcmd'?
+  note: 'subcmd --subcmdarg' exists
 
 Usage: dym [COMMAND]
 
@@ -355,7 +353,7 @@ fn subcommand_used_after_double_dash() {
     static SUBCMD_AFTER_DOUBLE_DASH: &str = "\
 error: Found argument 'subcmd' which wasn't expected, or isn't valid in this context
 
-  If you tried to supply 'subcmd' as a subcommand, remove the '--' before it.
+  note: subcommand 'subcmd' exists; to use it, remove the '--' before it
 
 Usage: cmd [COMMAND]
 
@@ -522,9 +520,8 @@ For more information try 'help'
         static BAZ_EXPECTED: &str = "\
 error: The subcommand 'baz' wasn't recognized
 
-  Did you mean 'bar'?
-
-  If you believe you received this message in error, try re-running with ' -- baz'
+  note: subcommand 'bar' exists
+  note: to pass 'baz' as a value, use ' -- baz'
 
 Usage: <COMMAND>