fix(error): Don't suggest '--' when it doesn't help

examples/derive_ref/interop_tests.md

@@ -37,8 +37,6 @@ $ interop_augment_args --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'
-
 Usage: interop_augment_args[EXE] [OPTIONS]
 
 For more information try '--help'
@@ -74,8 +72,6 @@ $ interop_augment_subcommands derived --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'
-
 Usage: interop_augment_subcommands[EXE] derived [OPTIONS]
 
 For more information try '--help'
@@ -245,8 +241,6 @@ $ interop_flatten_hand_args --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'
-
 Usage: interop_flatten_hand_args[EXE] [OPTIONS]
 
 For more information try '--help'

src/error/context.rs

@@ -29,6 +29,8 @@ pub enum ContextKind {
     SuggestedValue,
     /// Trailing argument
     TrailingArg,
+    /// Potential fix for the user
+    SuggestedTrailingArg,
     /// A usage string
     Usage,
     /// An opaque message to the user
@@ -51,6 +53,7 @@ impl ContextKind {
             Self::SuggestedSubcommand => Some("Suggested Subcommand"),
             Self::SuggestedArg => Some("Suggested Argument"),
             Self::SuggestedValue => Some("Suggested Value"),
+            Self::SuggestedTrailingArg => Some("Suggested Trailing Argument"),
             Self::TrailingArg => Some("Trailing Argument"),
             Self::Usage => None,
             Self::Custom => None,

src/error/format.rs

@@ -360,7 +360,8 @@ fn write_dynamic_context(error: &crate::error::Error, styled: &mut StyledStr) ->
 
                 let invalid_arg = error.get(ContextKind::InvalidArg);
                 if let Some(ContextValue::String(invalid_arg)) = invalid_arg {
-                    if invalid_arg.starts_with('-') {
+                    let suggested_trailing_arg = error.get(ContextKind::SuggestedTrailingArg);
+                    if suggested_trailing_arg == Some(&ContextValue::Bool(true)) {
                         styled.none("\n\n");
                         styled.none(TAB);
                         styled.none("If you tried to supply '");

src/error/mod.rs

@@ -638,6 +638,7 @@ impl<F: ErrorFormatter> Error<F> {
         cmd: &Command,
         arg: String,
         did_you_mean: Option<(String, Option<String>)>,
+        suggested_trailing_arg: bool,
         usage: Option<StyledStr>,
     ) -> Self {
         let mut err = Self::new(ErrorKind::UnknownArgument).with_cmd(cmd);
@@ -662,6 +663,12 @@ impl<F: ErrorFormatter> Error<F> {
                     );
                 }
             }
+            if suggested_trailing_arg {
+                err = err.insert_context_unchecked(
+                    ContextKind::SuggestedTrailingArg,
+                    ContextValue::Bool(true),
+                );
+            }
         }
 
         err

src/parser/parser.rs

@@ -175,7 +175,12 @@ impl<'cmd> Parser<'cmd> {
                                 .remaining(&mut args_cursor)
                                 .map(|x| x.to_str().expect(INVALID_UTF8))
                                 .collect();
-                            return Err(self.did_you_mean_error(&arg, matcher, &remaining_args));
+                            return Err(self.did_you_mean_error(
+                                &arg,
+                                matcher,
+                                &remaining_args,
+                                trailing_values,
+                            ));
                         }
                         ParseResult::UnneededAttachedValue { rest, used, arg } => {
                             let _ = self.resolve_pending(matcher);
@@ -257,10 +262,14 @@ impl<'cmd> Parser<'cmd> {
                         }
                         ParseResult::NoMatchingArg { arg } => {
                             let _ = self.resolve_pending(matcher);
+                            // We already know it looks like a flag
+                            let suggested_trailing_arg =
+                                !trailing_values && self.cmd.has_positionals();
                             return Err(ClapError::unknown_argument(
                                 self.cmd,
                                 arg,
                                 None,
+                                suggested_trailing_arg,
                                 Usage::new(self.cmd).create_usage_with_title(&[]),
                             ));
                         }
@@ -374,10 +383,14 @@ impl<'cmd> Parser<'cmd> {
             if let Some(arg) = self.cmd.get_keymap().get(&pos_counter) {
                 if arg.is_last_set() && !trailing_values {
                     let _ = self.resolve_pending(matcher);
+                    // Its already considered a positional, we don't need to suggest turning it
+                    // into one
+                    let suggested_trailing_arg = false;
                     return Err(ClapError::unknown_argument(
                         self.cmd,
                         arg_os.display().to_string(),
                         None,
+                        suggested_trailing_arg,
                         Usage::new(self.cmd).create_usage_with_title(&[]),
                     ));
                 }
@@ -524,10 +537,14 @@ impl<'cmd> Parser<'cmd> {
             );
         }
 
+        let suggested_trailing_arg = !trailing_values
+            && self.cmd.has_positionals()
+            && (arg_os.is_long() || arg_os.is_short());
         ClapError::unknown_argument(
             self.cmd,
             arg_os.display().to_string(),
             None,
+            suggested_trailing_arg,
             Usage::new(self.cmd).create_usage_with_title(&[]),
         )
     }
@@ -1511,6 +1528,7 @@ impl<'cmd> Parser<'cmd> {
         arg: &str,
         matcher: &mut ArgMatcher,
         remaining_args: &[&str],
+        trailing_values: bool,
     ) -> ClapError {
         debug!("Parser::did_you_mean_error: arg={}", arg);
         // Didn't match a flag or option
@@ -1549,10 +1567,16 @@ impl<'cmd> Parser<'cmd> {
             .cloned()
             .collect();
 
+        // `did_you_mean` is a lot more likely and should cause us to skip the `--` suggestion
+        //
+        // In theory, this is only called for `--long`s, so we don't need to check
+        let suggested_trailing_arg =
+            did_you_mean.is_none() && !trailing_values && self.cmd.has_positionals();
         ClapError::unknown_argument(
             self.cmd,
             format!("--{}", arg),
             did_you_mean,
+            suggested_trailing_arg,
             Usage::new(self.cmd)
                 .required(&required)
                 .create_usage_with_title(&*used),

tests/builder/error.rs

@@ -162,8 +162,6 @@ fn trailing_already_in_use() {
     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'
-
 Usage: rg [PATTERN]
 
 For more information try '--help'
@@ -183,8 +181,6 @@ fn cant_use_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'
-
 Usage: test
 
 For more information try '--help'

tests/builder/opts.rs

@@ -451,8 +451,6 @@ error: Found argument '--optio' which wasn't expected, or isn't valid in this co
 
   Did you mean '--option'?
 
-  If you tried to supply '--optio' as a value rather than a flag, use '-- --optio'
-
 Usage: clap-test --option <opt>... [positional] [positional2] [positional3]...
 
 For more information try '--help'
@@ -551,8 +549,6 @@ error: Found argument '--files-without-matches' which wasn't expected, or isn't
 
   Did you mean '--files-without-match'?
 
-  If you tried to supply '--files-without-matches' as a value rather than a flag, use '-- --files-without-matches'
-
 Usage: ripgrep-616 --files-without-match
 
 For more information try '--help'

tests/builder/subcommands.rs

@@ -145,8 +145,6 @@ error: Found argument '--subcmarg' which wasn't expected, or isn't valid in this
 
   Did you mean to put '--subcmdarg' after the subcommand 'subcmd'?
 
-  If you tried to supply '--subcmarg' as a value rather than a flag, use '-- --subcmarg'
-
 Usage: dym [COMMAND]
 
 For more information try '--help'
@@ -165,8 +163,6 @@ fn subcmd_did_you_mean_output_arg_false_positives() {
     static EXPECTED: &str = "\
 error: Found argument '--subcmarg' which wasn't expected, or isn't valid in this context
 
-  If you tried to supply '--subcmarg' as a value rather than a flag, use '-- --subcmarg'
-
 Usage: dym [COMMAND]
 
 For more information try '--help'

tests/ui/error_stderr.toml

@@ -5,8 +5,6 @@ stdout = ""
 stderr = """
 error: Found argument '--unknown-argument' which wasn't expected, or isn't valid in this context
 
-  If you tried to supply '--unknown-argument' as a value rather than a flag, use '-- --unknown-argument'
-
 Usage: stdio-fixture[EXE] [OPTIONS] [COMMAND]
 
 For more information try '--help'