Fix: Don't mention unused subcommands (Partly cherry pick ef92e2b)

Add `\t` for consistency

src/parse/errors.rs

@@ -819,7 +819,7 @@ impl Error {
         // suggest `--` for disambiguation.
         if arg.starts_with('-') {
             c.none(format!(
-                "\n\nIf you tried to supply `{}` as a value rather than a flag, use `-- {}`",
+                "\n\n\tIf you tried to supply `{}` as a value rather than a flag, use `-- {}`",
                 arg, arg
             ));
         }
@@ -842,7 +842,7 @@ impl Error {
         c.none("' which wasn't expected, or isn't valid in this context");
 
         c.none(format!(
-            "\n\nIf you tried to supply `{}` as a subcommand, remove the '--' before it.",
+            "\n\n\tIf you tried to supply `{}` as a subcommand, remove the '--' before it.",
             arg
         ));
         put_usage(&mut c, usage);

src/parse/features/suggestions.rs

@@ -35,6 +35,7 @@ where
 /// Returns a suffix that can be empty, or is the standard 'did you mean' phrase
 pub(crate) fn did_you_mean_flag<I, T>(
     arg: &str,
+    remaining_args: &[&str],
     longs: I,
     subcommands: &mut [App],
 ) -> Option<(String, Option<String>)>
@@ -46,9 +47,9 @@ where
 
     match did_you_mean(arg, longs).pop() {
         Some(candidate) => Some((candidate, None)),
-
-        None => {
-            for subcommand in subcommands {
+        None => subcommands
+            .iter_mut()
+            .filter_map(|subcommand| {
                 subcommand._build();
 
                 let longs = subcommand.args.keys.iter().map(|x| &x.key).filter_map(|a| {
@@ -59,12 +60,14 @@ where
                     }
                 });
 
-                if let Some(candidate) = did_you_mean(arg, longs).pop() {
-                    return Some((candidate, Some(subcommand.get_name().to_string())));
-                }
-            }
-            None
-        }
+                let subcommand_name = subcommand.get_name();
+
+                let candidate = did_you_mean(arg, longs).pop()?;
+                let score = remaining_args.iter().position(|x| *x == subcommand_name)?;
+                Some((score, (candidate, Some(subcommand_name.to_string()))))
+            })
+            .min_by_key(|(x, _)| *x)
+            .map(|(_, suggestion)| suggestion),
     }
 }
 

src/parse/parser.rs

@@ -55,7 +55,7 @@ where
 }
 
 impl Input {
-    pub(crate) fn next(&mut self, new: Option<&[&str]>) -> Option<(&OsStr, Option<&OsStr>)> {
+    pub(crate) fn next(&mut self, new: Option<&[&str]>) -> Option<(&OsStr, &[OsString])> {
         if let Some(new) = new {
             let mut new_items: Vec<OsString> = new.iter().map(OsString::from).collect();
 
@@ -70,19 +70,12 @@ impl Input {
         if self.cursor >= self.items.len() {
             None
         } else {
+            let current = &self.items[self.cursor];
             self.cursor += 1;
-
-            if self.cursor >= self.items.len() {
-                Some((&self.items[self.cursor - 1], None))
-            } else {
-                Some((&self.items[self.cursor - 1], Some(&self.items[self.cursor])))
-            }
+            let remaining = &self.items[self.cursor..];
+            Some((current, remaining))
         }
     }
-
-    pub(crate) fn remaining(&self) -> &[OsString] {
-        &self.items[self.cursor..]
-    }
 }
 
 pub(crate) struct Parser<'help, 'app> {
@@ -357,7 +350,7 @@ impl<'help, 'app> Parser<'help, 'app> {
         let mut pos_counter = 1;
         let mut replace: Option<&[&str]> = None;
 
-        while let Some((arg_os, next_arg)) = it.next(replace) {
+        while let Some((arg_os, remaining_args)) = it.next(replace) {
             replace = None;
 
             let arg_os = ArgStr::new(arg_os);
@@ -408,7 +401,7 @@ impl<'help, 'app> Parser<'help, 'app> {
 
                         if let Some(sc_name) = sc_name {
                             if sc_name == "help" && !self.is_set(AS::NoAutoHelp) {
-                                self.parse_help_subcommand(it.remaining())?;
+                                self.parse_help_subcommand(remaining_args)?;
                             }
 
                             subcmd_name = Some(sc_name.to_owned());
@@ -419,7 +412,7 @@ impl<'help, 'app> Parser<'help, 'app> {
 
                 if starts_new_arg {
                     if arg_os.starts_with("--") {
-                        needs_val_of = self.parse_long_arg(matcher, &arg_os)?;
+                        needs_val_of = self.parse_long_arg(matcher, &arg_os, remaining_args)?;
                         debug!(
                             "Parser::get_matches_with: After parse_long_arg {:?}",
                             needs_val_of
@@ -548,7 +541,7 @@ impl<'help, 'app> Parser<'help, 'app> {
             );
 
             if low_index_mults || missing_pos {
-                if let Some(n) = next_arg {
+                if let Some(n) = remaining_args.get(0) {
                     needs_val_of = match needs_val_of {
                         ParseResult::ValuesDone(id) => ParseResult::ValuesDone(id),
 
@@ -1190,6 +1183,7 @@ impl<'help, 'app> Parser<'help, 'app> {
         &mut self,
         matcher: &mut ArgMatcher,
         full_arg: &ArgStr,
+        remaining_args: &[OsString],
     ) -> ClapResult<ParseResult> {
         // maybe here lifetime should be 'a
         debug!("Parser::parse_long_arg");
@@ -1233,7 +1227,11 @@ impl<'help, 'app> Parser<'help, 'app> {
         }
 
         debug!("Parser::parse_long_arg: Didn't match anything");
-        self.did_you_mean_error(arg.to_str().expect(INVALID_UTF8), matcher)
+        let remaining_args: Vec<_> = remaining_args
+            .iter()
+            .map(|x| x.to_str().expect(INVALID_UTF8))
+            .collect();
+        self.did_you_mean_error(arg.to_str().expect(INVALID_UTF8), matcher, &remaining_args)
             .map(|_| ParseResult::NotFound)
     }
 
@@ -1668,7 +1666,12 @@ impl<'help, 'app> Parser<'help, 'app> {
 
 // Error, Help, and Version Methods
 impl<'help, 'app> Parser<'help, 'app> {
-    fn did_you_mean_error(&mut self, arg: &str, matcher: &mut ArgMatcher) -> ClapResult<()> {
+    fn did_you_mean_error(
+        &mut self,
+        arg: &str,
+        matcher: &mut ArgMatcher,
+        remaining_args: &[&str],
+    ) -> ClapResult<()> {
         debug!("Parser::did_you_mean_error: arg={}", arg);
         // Didn't match a flag or option
         let longs = self
@@ -1686,6 +1689,7 @@ impl<'help, 'app> Parser<'help, 'app> {
 
         let did_you_mean = suggestions::did_you_mean_flag(
             arg,
+            remaining_args,
             longs.iter().map(|ref x| &x[..]),
             self.app.subcommands.as_mut_slice(),
         );

tests/flags.rs

@@ -4,7 +4,7 @@ use clap::{App, Arg};
 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`
+\tIf you tried to supply `--another-flag` as a value rather than a flag, use `-- --another-flag`
 
 USAGE:
     mycat [FLAGS] [filename]

tests/groups.rs

@@ -29,7 +29,7 @@ For more information try --help";
 static REQ_GROUP_CONFLICT_ONLY_OPTIONS: &str =
     "error: Found argument '--all' which wasn't expected, or isn't valid in this context
 
-If you tried to supply `--all` as a value rather than a flag, use `-- --all`
+\tIf you tried to supply `--all` as a value rather than a flag, use `-- --all`
 
 USAGE:
     clap-test <-a|--delete>

tests/opts.rs

@@ -8,7 +8,7 @@ static DYM: &str =
 
 \tDid you mean '--option'?
 
-If you tried to supply `--optio` as a value rather than a flag, use `-- --optio`
+\tIf you tried to supply `--optio` as a value rather than a flag, use `-- --optio`
 
 USAGE:
     clap-test --option <opt>...
@@ -21,7 +21,7 @@ static DYM_ISSUE_1073: &str =
 
 \tDid you mean '--files-without-match'?
 
-If you tried to supply `--files-without-matches` as a value rather than a flag, use `-- --files-without-matches`
+\tIf you tried to supply `--files-without-matches` as a value rather than a flag, use `-- --files-without-matches`
 
 USAGE:
     ripgrep-616 --files-without-match

tests/subcommands.rs

@@ -80,23 +80,10 @@ USAGE:
 
 For more information try --help";
 
-#[cfg(feature = "suggestions")]
-static DYM_ARG: &str =
-    "error: Found argument '--subcm' which wasn't expected, or isn't valid in this context
-
-	Did you mean to put '--subcmdarg' after the subcommand 'subcmd'?
-
-If you tried to supply `--subcm` as a value rather than a flag, use `-- --subcm`
-
-USAGE:
-    dym [SUBCOMMAND]
-
-For more information try --help";
-
 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.
+\tIf you tried to supply `subcmd` as a subcommand, remove the '--' before it.
 
 USAGE:
     app [SUBCOMMAND]
@@ -244,9 +231,51 @@ fn subcmd_did_you_mean_output_ambiguous() {
 #[test]
 #[cfg(feature = "suggestions")]
 fn subcmd_did_you_mean_output_arg() {
-    let app =
-        App::new("dym").subcommand(App::new("subcmd").arg("-s --subcmdarg [subcmdarg] 'tests'"));
-    assert!(utils::compare_output(app, "dym --subcm foo", DYM_ARG, true));
+    static EXPECTED: &str =
+        "error: Found argument '--subcmarg' which wasn't expected, or isn't valid in this context
+
+\tDid you mean to put '--subcmdarg' after the subcommand 'subcmd'?
+
+\tIf you tried to supply `--subcmarg` as a value rather than a flag, use `-- --subcmarg`
+
+USAGE:
+    dym [SUBCOMMAND]
+
+For more information try --help";
+
+    let app = App::new("dym")
+        .subcommand(App::new("subcmd").arg(Arg::from("-s --subcmdarg [subcmdarg] 'tests'")));
+
+    assert!(utils::compare_output(
+        app,
+        "dym --subcmarg subcmd",
+        EXPECTED,
+        true
+    ));
+}
+
+#[test]
+#[cfg(feature = "suggestions")]
+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
+
+\tIf you tried to supply `--subcmarg` as a value rather than a flag, use `-- --subcmarg`
+
+USAGE:
+    dym [SUBCOMMAND]
+
+For more information try --help";
+
+    let app = App::new("dym")
+        .subcommand(App::new("subcmd").arg(Arg::from("-s --subcmdarg [subcmdarg] 'tests'")));
+
+    assert!(utils::compare_output(
+        app,
+        "dym --subcmarg foo",
+        EXPECTED,
+        true
+    ));
 }
 
 #[test]