Skip to content

Commit

Permalink
fix: Dont mention unused subcommands
Browse files Browse the repository at this point in the history
  • Loading branch information
untitaker committed Sep 27, 2018
1 parent f77d3b1 commit ef92e2b
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 39 deletions.
25 changes: 18 additions & 7 deletions src/app/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1554,11 +1554,16 @@ where
}
}

fn parse_long_arg(
fn parse_long_arg<I, T>(
&mut self,
matcher: &mut ArgMatcher<'a>,
full_arg: &OsStr,
) -> ClapResult<ParseResult<'a>> {
it: &mut Peekable<I>,
) -> ClapResult<ParseResult<'a>>
where
I: Iterator<Item = T>,
T: Into<OsString> + Clone,
{
// maybe here lifetime should be 'a
debugln!("Parser::parse_long_arg;");

Expand Down Expand Up @@ -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))]
Expand Down Expand Up @@ -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 {
Expand Down
48 changes: 27 additions & 21 deletions src/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,42 +44,48 @@ 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>)
where
T: AsRef<str> + 'z,
I: IntoIterator<Item = &'z T>,
{
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
.iter()
.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
Expand Down
37 changes: 26 additions & 11 deletions tests/subcommands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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"));
}
}

0 comments on commit ef92e2b

Please sign in to comment.