From ef92e2b639ed305bdade4741f60fa85cb0101c5a Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Thu, 27 Sep 2018 02:20:35 +0200 Subject: [PATCH] fix: Dont mention unused subcommands --- src/app/parser.rs | 25 ++++++++++++++++------- src/suggestions.rs | 48 +++++++++++++++++++++++++------------------- tests/subcommands.rs | 37 ++++++++++++++++++++++++---------- 3 files changed, 71 insertions(+), 39 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index fb1e5c0ae18..cb367cf6c66 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -928,7 +928,7 @@ where } if arg_os.starts_with(b"--") { - needs_val_of = self.parse_long_arg(matcher, &arg_os)?; + needs_val_of = self.parse_long_arg(matcher, &arg_os, it)?; debugln!( "Parser:get_matches_with: After parse_long_arg {:?}", needs_val_of @@ -1554,11 +1554,16 @@ where } } - fn parse_long_arg( + fn parse_long_arg( &mut self, matcher: &mut ArgMatcher<'a>, full_arg: &OsStr, - ) -> ClapResult> { + it: &mut Peekable, + ) -> ClapResult> + where + I: Iterator, + T: Into + Clone, + { // maybe here lifetime should be 'a debugln!("Parser::parse_long_arg;"); @@ -1614,8 +1619,14 @@ where } debugln!("Parser::parse_long_arg: Didn't match anything"); - self.did_you_mean_error(arg.to_str().expect(INVALID_UTF8), matcher) - .map(|_| ParseResult::NotFound) + + let args_rest: Vec<_> = it.map(|x| x.clone().into()).collect(); + let args_rest2: Vec<_> = args_rest.iter().map(|x| x.to_str().expect(INVALID_UTF8)).collect(); + self.did_you_mean_error( + arg.to_str().expect(INVALID_UTF8), + matcher, + &args_rest2[..] + ).map(|_| ParseResult::NotFound) } #[cfg_attr(feature = "lints", allow(len_zero))] @@ -1872,9 +1883,9 @@ where Ok(ParseResult::Flag) } - fn did_you_mean_error(&self, arg: &str, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> { + fn did_you_mean_error(&self, arg: &str, matcher: &mut ArgMatcher<'a>, args_rest: &[&str]) -> ClapResult<()> { // Didn't match a flag or option - let suffix = suggestions::did_you_mean_flag_suffix(arg, longs!(self), &self.subcommands); + let suffix = suggestions::did_you_mean_flag_suffix(arg, &args_rest, longs!(self), &self.subcommands); // Add the arg to the matches to build a proper usage string if let Some(name) = suffix.1 { diff --git a/src/suggestions.rs b/src/suggestions.rs index 23832cb3a02..197ed4989be 100644 --- a/src/suggestions.rs +++ b/src/suggestions.rs @@ -44,6 +44,7 @@ where #[cfg_attr(feature = "lints", allow(needless_lifetimes))] pub fn did_you_mean_flag_suffix<'z, T, I>( arg: &str, + args_rest: &'z [&str], longs: I, subcommands: &'z [App], ) -> (String, Option<&'z str>) @@ -51,16 +52,18 @@ where T: AsRef + 'z, I: IntoIterator, { - match did_you_mean(arg, longs) { - Some(candidate) => { - let suffix = format!( - "\n\tDid you mean {}{}?", - Format::Good("--"), - Format::Good(candidate) + if let Some(candidate) = did_you_mean(arg, longs) { + let suffix = format!( + "\n\tDid you mean {}{}?", + Format::Good("--"), + Format::Good(candidate) ); - return (suffix, Some(candidate)); - } - None => for subcommand in subcommands { + return (suffix, Some(candidate)); + } + + subcommands + .into_iter() + .filter_map(|subcommand| { let opts = subcommand .p .flags @@ -68,18 +71,21 @@ where .filter_map(|f| f.s.long) .chain(subcommand.p.opts.iter().filter_map(|o| o.s.long)); - if let Some(candidate) = did_you_mean(arg, opts) { - let suffix = format!( - "\n\tDid you mean to put '{}{}' after the subcommand '{}'?", - Format::Good("--"), - Format::Good(candidate), - Format::Good(subcommand.get_name()) - ); - return (suffix, Some(candidate)); - } - }, - } - (String::new(), None) + let candidate = did_you_mean(arg, opts)?; + let score = args_rest.iter().position(|x| *x == subcommand.get_name())?; + + let suffix = format!( + "\n\tDid you mean to put '{}{}' after the subcommand '{}'?", + Format::Good("--"), + Format::Good(candidate), + Format::Good(subcommand.get_name()) + ); + + Some((score, (suffix, Some(candidate)))) + }) + .min_by_key(|&(score, _)| score) + .map(|(_, suggestion)| suggestion) + .unwrap_or_else(|| (String::new(), None)) } /// Returns a suffix that can be empty, or is the standard 'did you mean' phrase diff --git a/tests/subcommands.rs b/tests/subcommands.rs index de2f8b56117..6257aaf0734 100644 --- a/tests/subcommands.rs +++ b/tests/subcommands.rs @@ -42,15 +42,6 @@ USAGE: For more information try --help"; -#[cfg(feature = "suggestions")] -static DYM_ARG: &'static str = "error: Found argument '--subcm' which wasn't expected, or isn't valid in this context -\tDid you mean to put '--subcmdarg' after the subcommand 'subcmd'? - -USAGE: - dym [SUBCOMMAND] - -For more information try --help"; - #[test] fn subcommand() { let m = App::new("test") @@ -137,10 +128,34 @@ fn subcmd_did_you_mean_output() { #[test] #[cfg(feature="suggestions")] fn subcmd_did_you_mean_output_arg() { + static EXPECTED: &'static 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'? + +USAGE: + dym [SUBCOMMAND] + +For more information try --help"; + let app = App::new("dym") .subcommand(SubCommand::with_name("subcmd") .arg_from_usage("-s --subcmdarg [subcmdarg] 'tests'") ); - assert!(test::compare_output(app, "dym --subcm foo", DYM_ARG, true)); + assert!(test::compare_output(app, "dym --subcmarg subcmd", EXPECTED, true)); +} + +#[test] +#[cfg(feature="suggestions")] +fn subcmd_did_you_mean_output_arg_false_positives() { + static EXPECTED: &'static str = "error: Found argument '--subcmarg' which wasn't expected, or isn't valid in this context + +USAGE: + dym [SUBCOMMAND] + +For more information try --help"; + + let app = App::new("dym") + .subcommand(SubCommand::with_name("subcmd") + .arg_from_usage("-s --subcmdarg [subcmdarg] 'tests'") ); + assert!(test::compare_output(app, "dym --subcmarg foo", EXPECTED, true)); } #[test] @@ -198,4 +213,4 @@ fn issue_1031_args_with_same_name_no_more_vals() { let m = res.unwrap(); assert_eq!(m.value_of("ui-path"), Some("value")); assert_eq!(m.subcommand_name(), Some("signer")); -} \ No newline at end of file +}