From bba598931a15f066604baf121899e13c8144bf8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Sat, 6 Aug 2016 00:08:31 +0200 Subject: [PATCH 1/5] Fix issue #607: Remove already matched parameters on errors of required --- src/app/parser.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/parser.rs b/src/app/parser.rs index 389cfbfff50..3e9ca555954 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -1591,6 +1591,7 @@ impl<'a, 'b> Parser<'a, 'b> self._help().unwrap_err() } else { let mut reqs = self.required.iter().map(|&r| &*r).collect::>(); + reqs.retain(|n| !matcher.contains(n)); reqs.dedup(); Error::missing_required_argument( &*self.get_required_from(&*reqs, Some(matcher)) From 9e8e350b95e99d8882e569191f49f3b74354abf8 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 20 Aug 2016 17:14:18 -0400 Subject: [PATCH 2/5] tests: adds tests for multi level help subcommands --- tests/help.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/help.rs b/tests/help.rs index 0c73498dca5..85b8a3d7153 100644 --- a/tests/help.rs +++ b/tests/help.rs @@ -66,6 +66,21 @@ OPTIONS: ARGS: tests positionals"; +static MULTI_SC_HELP: &'static str = "ctest-subcmd-multi 0.1 +Kevin K. +tests subcommands + +USAGE: + ctest subcmd multi [FLAGS] [OPTIONS] + +FLAGS: + -f, --flag tests flags + -h, --help Prints help information + -V, --version Prints version information + +OPTIONS: + -o, --option ... tests options"; + #[test] fn help_short() { let m = App::new("test") @@ -159,6 +174,21 @@ fn after_and_before_help_output() { test::check_help(app, AFTER_HELP); } +#[test] +fn multi_level_sc_help() { + let app = App::new("ctest") + .subcommand(SubCommand::with_name("subcmd") + .subcommand(SubCommand::with_name("multi") + .about("tests subcommands") + .author("Kevin K. ") + .version("0.1") + .args_from_usage(" + -f, --flag 'tests flags' + -o, --option [scoption]... 'tests options' + "))); + test::check_err_output(app, "ctest help subcmd multi", MULTI_SC_HELP, false); +} + #[test] fn complex_subcommand_help_output() { let mut a = test::complex_app(); From e203515e3ac495b405dbba4f78fb6af148fd282e Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 20 Aug 2016 17:14:24 -0400 Subject: [PATCH 3/5] fix(Help Subcommand): fixes misleading usage string when using multi-level subcommmands For example, doing `myprog help subcmd1 subcmd2` would have incorrectly produced the usage string, `myprog subcmd2 [options]` but now correctly prints `myprog subcmd1 subcmd2 [options]` Closes #618 --- src/app/parser.rs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index 3e9ca555954..54da41382c2 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -235,6 +235,7 @@ impl<'a, 'b> Parser<'a, 'b> } else { sdebugln!("No"); } + debug!("Using Setting VersionlessSubcommands..."); if self.settings.is_set(AppSettings::VersionlessSubcommands) { sdebugln!("Yes"); @@ -620,6 +621,10 @@ impl<'a, 'b> Parser<'a, 'b> self.settings.is_set(AppSettings::NeedsSubcommandHelp) { let cmds: Vec = it.map(|c| c.into()).collect(); let mut help_help = false; + let mut bin_name = format!("{}", self.meta + .bin_name + .as_ref() + .unwrap_or(&self.meta.name.clone())); let mut sc = { let mut sc: &Parser = self; for (i, cmd) in cmds.iter().enumerate() { @@ -663,6 +668,9 @@ impl<'a, 'b> Parser<'a, 'b> .unwrap_or(&self.meta.name), self.color())); } + bin_name = format!("{} {}", + bin_name, + &*sc.meta.name); } sc.clone() }; @@ -678,17 +686,7 @@ impl<'a, 'b> Parser<'a, 'b> sc.create_help_and_version(); } if sc.meta.bin_name != self.meta.bin_name { - sc.meta.bin_name = Some(format!("{}{}{}", - self.meta - .bin_name - .as_ref() - .unwrap_or(&self.meta.name.clone()), - if self.meta.bin_name.is_some() { - " " - } else { - "" - }, - &*sc.meta.name)); + sc.meta.bin_name = Some(format!("{} {}", bin_name, sc.meta.name)); } return sc._help(); } @@ -1154,9 +1152,13 @@ impl<'a, 'b> Parser<'a, 'b> } fn _help(&self) -> ClapResult<()> { - try!(self.print_help()); + let mut buf = vec![]; + try!(Help::write_parser_help(&mut buf, self)); + let out = io::stdout(); + let mut out_buf = BufWriter::new(out.lock()); + try!(out_buf.write(&*buf)); Err(Error { - message: String::new(), + message: unsafe { String::from_utf8_unchecked(buf) }, kind: ErrorKind::HelpDisplayed, info: None, }) From 565d21f71c213726e6e031533ca2b7c8173a7e87 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 20 Aug 2016 17:22:08 -0400 Subject: [PATCH 4/5] chore: allows failing beta until vec_map/#20 is fixed --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 2207e5f1a88..042c26ed2ce 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ rust: matrix: allow_failures: - rust: nightly + - rust: beta before_script: - | pip install 'travis-cargo<0.2' --user && From 7bc1b4b6c77f6fb33563f216e7a5dbe90ac2f04c Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 20 Aug 2016 17:33:58 -0400 Subject: [PATCH 5/5] chore: clippy run --- src/app/help.rs | 10 +++++----- src/app/parser.rs | 16 ++++++++-------- src/args/arg_matches.rs | 8 ++++---- src/completions.rs | 12 ++++++------ 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/app/help.rs b/src/app/help.rs index 7891b084b4d..9b9fe82a409 100644 --- a/src/app/help.rs +++ b/src/app/help.rs @@ -150,7 +150,7 @@ impl<'a> Help<'a> { debugln!("fn=Help::write_help;"); if let Some(h) = parser.meta.help_str { try!(write!(self.writer, "{}", h).map_err(Error::from)); - } else if let Some(ref tmpl) = parser.meta.template { + } else if let Some(tmpl) = parser.meta.template { try!(self.write_templated_help(&parser, tmpl)); } else { try!(self.write_default_help(&parser)); @@ -282,7 +282,7 @@ impl<'a> Help<'a> { if !arg.takes_value() { return Ok(()); } - if let Some(ref vec) = arg.val_names() { + if let Some(vec) = arg.val_names() { let mut it = vec.iter().peekable(); while let Some((_, val)) = it.next() { try!(color!(self, "<{}>", val, good)); @@ -524,7 +524,7 @@ impl<'a> Help<'a> { fn spec_vals(&self, a: &ArgWithDisplay) -> String { debugln!("fn=spec_vals;"); - if let Some(ref pv) = a.default_val() { + if let Some(pv) = a.default_val() { debugln!("Writing defaults"); return format!(" [default: {}] {}", if self.color { @@ -562,7 +562,7 @@ impl<'a> Help<'a> { }); } else if !self.hide_pv { debugln!("Writing values"); - if let Some(ref pv) = a.possible_vals() { + if let Some(pv) = a.possible_vals() { debugln!("Possible vals...{:?}", pv); return if self.color { format!(" [values: {}]", @@ -943,7 +943,7 @@ impl<'a> Help<'a> { parser.meta.pre_help.unwrap_or("unknown before-help"))); } // Unknown tag, write it back. - ref r => { + r => { try!(self.writer.write(b"{")); try!(self.writer.write(r)); try!(self.writer.write(b"}")); diff --git a/src/app/parser.rs b/src/app/parser.rs index 54da41382c2..b4303898689 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -496,7 +496,7 @@ impl<'a, 'b> Parser<'a, 'b> // Firt we verify that the index highest supplied index, is equal to the number of // positional arguments to verify there are no gaps (i.e. supplying an index of 1 and 3 // but no 2) - if let Some((idx, ref p)) = self.positionals.iter().rev().next() { + if let Some((idx, p)) = self.positionals.iter().rev().next() { debug_assert!(!(idx != self.positionals.len()), format!("Found positional argument \"{}\" who's index is {} but there are \ only {} positional arguments defined", @@ -621,10 +621,10 @@ impl<'a, 'b> Parser<'a, 'b> self.settings.is_set(AppSettings::NeedsSubcommandHelp) { let cmds: Vec = it.map(|c| c.into()).collect(); let mut help_help = false; - let mut bin_name = format!("{}", self.meta - .bin_name - .as_ref() - .unwrap_or(&self.meta.name.clone())); + let mut bin_name = self.meta + .bin_name + .as_ref() + .unwrap_or(&self.meta.name).clone(); let mut sc = { let mut sc: &Parser = self; for (i, cmd) in cmds.iter().enumerate() { @@ -754,7 +754,7 @@ impl<'a, 'b> Parser<'a, 'b> if let Some(o) = self.opts.iter().filter(|o| &o.name == &a).next() { try!(self.validate_required(matcher)); reqs_validated = true; - let should_err = if let Some(ref v) = matcher.0.args.get(&*o.name) { + let should_err = if let Some(v) = matcher.0.args.get(&*o.name) { v.vals.is_empty() && !(o.min_vals.is_some() && o.min_vals.unwrap() == 0) } else { true @@ -1389,7 +1389,7 @@ impl<'a, 'b> Parser<'a, 'b> if self.is_set(AppSettings::StrictUtf8) && val.to_str().is_none() { return Err(Error::invalid_utf8(&*self.create_current_usage(matcher), self.color())); } - if let Some(ref p_vals) = arg.possible_vals() { + if let Some(p_vals) = arg.possible_vals() { let val_str = val.to_string_lossy(); if !p_vals.contains(&&*val_str) { return Err(Error::invalid_value(val_str, @@ -1403,7 +1403,7 @@ impl<'a, 'b> Parser<'a, 'b> matcher.contains(&*arg.name()) { return Err(Error::empty_value(arg, &*self.create_current_usage(matcher), self.color())); } - if let Some(ref vtor) = arg.validator() { + if let Some(vtor) = arg.validator() { if let Err(e) = vtor(val.to_string_lossy().into_owned()) { return Err(Error::value_validation(e, self.color())); } diff --git a/src/args/arg_matches.rs b/src/args/arg_matches.rs index aa06340b114..945817208c4 100644 --- a/src/args/arg_matches.rs +++ b/src/args/arg_matches.rs @@ -111,7 +111,7 @@ impl<'a> ArgMatches<'a> { /// [`ArgMatches::values_of`]: ./struct.ArgMatches.html#method.values_of /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html pub fn value_of>(&self, name: S) -> Option<&str> { - if let Some(ref arg) = self.args.get(name.as_ref()) { + if let Some(arg) = self.args.get(name.as_ref()) { if let Some(v) = arg.vals.values().nth(0) { return Some(v.to_str().expect(INVALID_UTF8)); } @@ -208,7 +208,7 @@ impl<'a> ArgMatches<'a> { /// [`Values`]: ./struct.Values.html /// [`Iterator`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html pub fn values_of>(&'a self, name: S) -> Option> { - if let Some(ref arg) = self.args.get(name.as_ref()) { + if let Some(arg) = self.args.get(name.as_ref()) { fn to_str_slice(o: &OsString) -> &str { o.to_str().expect(INVALID_UTF8) } @@ -240,7 +240,7 @@ impl<'a> ArgMatches<'a> { /// assert_eq!(itr.next(), None); /// ``` pub fn values_of_lossy>(&'a self, name: S) -> Option> { - if let Some(ref arg) = self.args.get(name.as_ref()) { + if let Some(arg) = self.args.get(name.as_ref()) { return Some(arg.vals .values() .map(|v| v.to_string_lossy().into_owned()) @@ -284,7 +284,7 @@ impl<'a> ArgMatches<'a> { &*o } let to_str_slice: fn(&'a OsString) -> &'a OsStr = to_str_slice; // coerce to fn pointer - if let Some(ref arg) = self.args.get(name.as_ref()) { + if let Some(arg) = self.args.get(name.as_ref()) { return Some(OsValues { iter: arg.vals.values().map(to_str_slice) }); } None diff --git a/src/completions.rs b/src/completions.rs index f0294c2aa5e..c20ae535145 100644 --- a/src/completions.rs +++ b/src/completions.rs @@ -211,7 +211,7 @@ end }; let mut buffer = detect_subcommand_function; - gen_fish_inner(command, &self, vec![], &mut buffer, has_subcommands); + gen_fish_inner(command, self, vec![], &mut buffer, has_subcommands); w!(buf, buffer.as_bytes()); } } @@ -235,7 +235,7 @@ pub fn get_all_subcommands(p: &Parser) -> Vec { } subcmds.push(sc.p.meta.name.clone()); } - for sc_v in p.subcommands.iter().map(|ref s| get_all_subcommands(&s.p)) { + for sc_v in p.subcommands.iter().map(|s| get_all_subcommands(&s.p)) { subcmds.extend(sc_v); } subcmds.sort(); @@ -269,7 +269,7 @@ pub fn get_all_subcommand_paths(p: &Parser, first: bool) -> Vec { } } } - for sc_v in p.subcommands.iter().map(|ref s| get_all_subcommand_paths(&s.p, false)) { + for sc_v in p.subcommands.iter().map(|s| get_all_subcommand_paths(&s.p, false)) { subcmds.extend(sc_v); } subcmds @@ -279,10 +279,10 @@ fn vals_for(o: &OptBuilder) -> String { use args::AnyArg; let mut ret = String::new(); let mut needs_quotes = true; - if let Some(ref vals) = o.possible_vals() { + if let Some(vals) = o.possible_vals() { needs_quotes = false; ret = format!("$(compgen -W \"{}\" -- ${{cur}})", vals.join(" ")); - } else if let Some(ref vec) = o.val_names() { + } else if let Some(vec) = o.val_names() { let mut it = vec.iter().peekable(); while let Some((_, val)) = it.next() { ret = format!("{}<{}>{}", ret, val, @@ -321,7 +321,7 @@ fn vals_for(o: &OptBuilder) -> String { ret } -fn gen_fish_inner(root_command: &String, +fn gen_fish_inner(root_command: &str, comp_gen: &ComplGen, parent_cmds: Vec<&String>, buffer: &mut String,