From 24e383922016fb525227f93a8ad021fbe3a1d7db Mon Sep 17 00:00:00 2001 From: Kevin K Date: Tue, 28 Feb 2017 12:43:24 -0500 Subject: [PATCH 01/31] perf: changes internal use of VecMap to Vec for matched values of Args This makes a very big difference for CLIs that parse a large number of values (think ripgrep over a large directory). This commit improved the ripgrep parsing of ~2,000 values, simulating giving ripgrep a bunch of files. Parsing when from ~1,200,000 ns to ~400,000 ns! This was conducted a i7-5600U 2.6GHz --- src/app/parser.rs | 23 +++++++------- src/args/arg_matcher.rs | 19 ++++-------- src/args/arg_matches.rs | 66 ++++++----------------------------------- src/args/matched_arg.rs | 7 ++--- 4 files changed, 27 insertions(+), 88 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index 1294df47..b66289a8 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -1634,7 +1634,7 @@ impl<'a, 'b> Parser<'a, 'b> where A: AnyArg<'a, 'b> + Display { debugln!("Parser::validate_values: arg={:?}", arg.name()); - for (_, val) in &ma.vals { + for val in &ma.vals { if self.is_set(AS::StrictUtf8) && val.to_str().is_none() { debugln!("Parser::validate_values: invalid UTF-8 found in val {:?}", val); @@ -1838,14 +1838,11 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Parser::validate_arg_num_vals: max_vals set...{}", num); if (ma.vals.len() as u64) > num { debugln!("Parser::validate_arg_num_vals: Sending error TooManyValues"); - return Err(Error::too_many_values(ma.vals - .get(ma.vals - .keys() - .last() - .expect(INTERNAL_ERROR_MSG)) - .expect(INTERNAL_ERROR_MSG) - .to_str() - .expect(INVALID_UTF8), + return Err(Error::too_many_values(ma.vals.iter() + .last() + .expect(INTERNAL_ERROR_MSG) + .to_str() + .expect(INVALID_UTF8), a, &*self.create_current_usage(matcher, None), self.color())); @@ -1882,7 +1879,7 @@ impl<'a, 'b> Parser<'a, 'b> if let Some(a_reqs) = a.requires() { for &(val, name) in a_reqs.iter().filter(|&&(val, _)| val.is_some()) { if ma.vals - .values() + .iter() .any(|v| v == val.expect(INTERNAL_ERROR_MSG) && !matcher.contains(name)) { return self.missing_required_error(matcher, None); } @@ -1942,8 +1939,8 @@ impl<'a, 'b> Parser<'a, 'b> // Validate the conditionally required args for &(a, v, r) in &self.r_ifs { if let Some(ma) = matcher.get(a) { - for val in ma.vals.values() { - if v == val && matcher.get(r).is_none() { + if matcher.get(r).is_none() { + if ma.vals.iter().any(|val| val == v) { return self.missing_required_error(matcher, Some(r)); } } @@ -2196,7 +2193,7 @@ impl<'a, 'b> Parser<'a, 'b> for &(arg, val, default) in vm.values() { let add = if let Some(a) = $m.get(arg) { if let Some(v) = val { - a.vals.values().any(|value| v == value) + a.vals.iter().any(|value| v == value) } else { true } diff --git a/src/args/arg_matcher.rs b/src/args/arg_matcher.rs index 3052acd9..12e58fc1 100644 --- a/src/args/arg_matcher.rs +++ b/src/args/arg_matcher.rs @@ -4,9 +4,6 @@ use std::ffi::OsStr; use std::ops::Deref; use std::mem; -// Third Party -use vec_map::VecMap; - // Internal use args::{ArgMatches, MatchedArg, SubCommand}; use args::AnyArg; @@ -25,7 +22,7 @@ impl<'a> ArgMatcher<'a> { pub fn propagate(&mut self, arg: &'a str) { debugln!("ArgMatcher::propagate: arg={}", arg); - let vals: VecMap<_> = if let Some(ma) = self.get(arg) { + let vals: Vec<_> = if let Some(ma) = self.get(arg) { ma.vals.clone() } else { debugln!("ArgMatcher::propagate: arg wasn't used"); @@ -36,15 +33,11 @@ impl<'a> ArgMatcher<'a> { let sma = (*sc).matches.args.entry(arg).or_insert_with(|| { let mut gma = MatchedArg::new(); gma.occurs += 1; - for (i, v) in &vals { - gma.vals.insert(i, v.clone()); - } + gma.vals = vals.clone(); gma }); if sma.vals.is_empty() { - for (i, v) in &vals { - sma.vals.insert(i, v.clone()); - } + sma.vals = vals.clone(); } } let mut am = ArgMatcher(mem::replace(&mut sc.matches, ArgMatches::new())); @@ -105,10 +98,10 @@ impl<'a> ArgMatcher<'a> { pub fn add_val_to(&mut self, arg: &'a str, val: &OsStr) { let ma = self.entry(arg).or_insert(MatchedArg { occurs: 0, - vals: VecMap::new(), + vals: Vec::with_capacity(1), }); - let len = ma.vals.len() + 1; - ma.vals.insert(len, val.to_owned()); + // let len = ma.vals.len() + 1; + ma.vals.push(val.to_owned()); } pub fn needs_more_vals<'b, A>(&self, o: &A) -> bool diff --git a/src/args/arg_matches.rs b/src/args/arg_matches.rs index 12364fdc..3a60c773 100644 --- a/src/args/arg_matches.rs +++ b/src/args/arg_matches.rs @@ -3,10 +3,7 @@ use std::borrow::Cow; use std::collections::HashMap; use std::ffi::{OsStr, OsString}; use std::iter::Map; -use std::slice; - -// Third Party -use vec_map; +use std::slice::Iter; // Internal use INVALID_UTF8; @@ -113,7 +110,7 @@ impl<'a> ArgMatches<'a> { /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html pub fn value_of>(&self, name: S) -> Option<&str> { if let Some(arg) = self.args.get(name.as_ref()) { - if let Some(v) = arg.vals.values().nth(0) { + if let Some(v) = arg.vals.iter().nth(0) { return Some(v.to_str().expect(INVALID_UTF8)); } } @@ -145,7 +142,7 @@ impl<'a> ArgMatches<'a> { /// [`Arg::values_of_lossy`]: ./struct.ArgMatches.html#method.values_of_lossy pub fn value_of_lossy>(&'a self, name: S) -> Option> { if let Some(arg) = self.args.get(name.as_ref()) { - if let Some(v) = arg.vals.values().nth(0) { + if let Some(v) = arg.vals.iter().nth(0) { return Some(v.to_string_lossy()); } } @@ -182,7 +179,7 @@ impl<'a> ArgMatches<'a> { pub fn value_of_os>(&self, name: S) -> Option<&OsStr> { self.args .get(name.as_ref()) - .map_or(None, |arg| arg.vals.values().nth(0).map(|v| v.as_os_str())) + .map_or(None, |arg| arg.vals.iter().nth(0).map(|v| v.as_os_str())) } /// Gets a [`Values`] struct which implements [`Iterator`] for values of a specific argument @@ -214,7 +211,7 @@ impl<'a> ArgMatches<'a> { if let Some(arg) = self.args.get(name.as_ref()) { fn to_str_slice(o: &OsString) -> &str { o.to_str().expect(INVALID_UTF8) } let to_str_slice: fn(&OsString) -> &str = to_str_slice; // coerce to fn pointer - return Some(Values { iter: arg.vals.values().map(to_str_slice) }); + return Some(Values { iter: arg.vals.iter().map(to_str_slice) }); } None } @@ -246,7 +243,7 @@ impl<'a> ArgMatches<'a> { pub fn values_of_lossy>(&'a self, name: S) -> Option> { if let Some(arg) = self.args.get(name.as_ref()) { return Some(arg.vals - .values() + .iter() .map(|v| v.to_string_lossy().into_owned()) .collect()); } @@ -288,7 +285,7 @@ impl<'a> ArgMatches<'a> { fn to_str_slice(o: &OsString) -> &OsStr { &*o } let to_str_slice: fn(&'a OsString) -> &'a OsStr = to_str_slice; // coerce to fn pointer if let Some(arg) = self.args.get(name.as_ref()) { - return Some(OsValues { iter: arg.vals.values().map(to_str_slice) }); + return Some(OsValues { iter: arg.vals.iter().map(to_str_slice) }); } None } @@ -554,7 +551,7 @@ impl<'a> ArgMatches<'a> { #[derive(Clone)] #[allow(missing_debug_implementations)] pub struct Values<'a> { - iter: Map, fn(&'a OsString) -> &'a str>, + iter: Map, fn(&'a OsString) -> &'a str>, } impl<'a> Iterator for Values<'a> { @@ -570,51 +567,6 @@ impl<'a> DoubleEndedIterator for Values<'a> { impl<'a> ExactSizeIterator for Values<'a> {} -/// An iterator over the key-value pairs of a map. -#[derive(Clone)] -pub struct Iter<'a, V: 'a> { - front: usize, - back: usize, - iter: slice::Iter<'a, Option>, -} - -impl<'a, V> Iterator for Iter<'a, V> { - type Item = &'a V; - - #[inline] - fn next(&mut self) -> Option<&'a V> { - while self.front < self.back { - if let Some(elem) = self.iter.next() { - if let Some(x) = elem.as_ref() { - self.front += 1; - return Some(x); - } - } - self.front += 1; - } - None - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { (0, Some(self.back - self.front)) } -} - -impl<'a, V> DoubleEndedIterator for Iter<'a, V> { - #[inline] - fn next_back(&mut self) -> Option<&'a V> { - while self.front < self.back { - if let Some(elem) = self.iter.next_back() { - if let Some(x) = elem.as_ref() { - self.back -= 1; - return Some(x); - } - } - self.back -= 1; - } - None - } -} - /// An iterator for getting multiple values out of an argument via the [`ArgMatches::values_of_os`] /// method. Usage of this iterator allows values which contain invalid UTF-8 code points unlike /// [`Values`]. @@ -639,7 +591,7 @@ impl<'a, V> DoubleEndedIterator for Iter<'a, V> { #[derive(Clone)] #[allow(missing_debug_implementations)] pub struct OsValues<'a> { - iter: Map, fn(&'a OsString) -> &'a OsStr>, + iter: Map, fn(&'a OsString) -> &'a OsStr>, } impl<'a> Iterator for OsValues<'a> { diff --git a/src/args/matched_arg.rs b/src/args/matched_arg.rs index e852f26c..9a73af9d 100644 --- a/src/args/matched_arg.rs +++ b/src/args/matched_arg.rs @@ -1,23 +1,20 @@ // Std use std::ffi::OsString; -// Third Party -use vec_map::VecMap; - #[doc(hidden)] #[derive(Debug, Clone)] pub struct MatchedArg { #[doc(hidden)] pub occurs: u64, #[doc(hidden)] - pub vals: VecMap, + pub vals: Vec, } impl Default for MatchedArg { fn default() -> Self { MatchedArg { occurs: 1, - vals: VecMap::new(), + vals: Vec::with_capacity(1), } } } From 432837624c711986b2562cb33215759f3db1d631 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Tue, 28 Feb 2017 21:20:46 -0500 Subject: [PATCH 02/31] chore: tags release beta --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index da02d8a2..4fe979c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "clap" -version = "2.21.0" +version = "2.21.0-beta" authors = ["Kevin K. "] exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"] repository = "https://github.com/kbknapp/clap-rs.git" From 8adf353e0b71709c183e29305ac87ea01d04ac20 Mon Sep 17 00:00:00 2001 From: Joost Yervante Damad Date: Fri, 3 Mar 2017 09:32:09 +0100 Subject: [PATCH 03/31] fix(help): don't show ARGS when there are only hidden positional args Also no need to check for Hidden inside for that already is filtered on !Hidden. Closes #882 --- src/app/help.rs | 6 ++---- src/app/parser.rs | 2 +- tests/hidden_args.rs | 9 +++++---- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/app/help.rs b/src/app/help.rs index 848671a8..a47b8053 100644 --- a/src/app/help.rs +++ b/src/app/help.rs @@ -195,9 +195,7 @@ impl<'a> Help<'a> { if arg.longest_filter() { self.longest = cmp::max(self.longest, arg.to_string().len()); } - if !arg.is_set(ArgSettings::Hidden) { - arg_v.push(arg) - } + arg_v.push(arg) } let mut first = true; for arg in arg_v { @@ -541,7 +539,7 @@ impl<'a> Help<'a> { pub fn write_all_args(&mut self, parser: &Parser) -> ClapResult<()> { debugln!("Help::write_all_args;"); let flags = parser.has_flags(); - let pos = parser.has_positionals(); + let pos = parser.positionals().filter(|arg| !arg.is_set(ArgSettings::Hidden)).count() > 0; let opts = parser.has_opts(); let subcmds = parser.has_subcommands(); diff --git a/src/app/parser.rs b/src/app/parser.rs index b66289a8..4d1b6bbb 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -542,7 +542,7 @@ impl<'a, 'b> Parser<'a, 'b> #[inline] pub fn has_positionals(&self) -> bool { !self.positionals.is_empty() } - + #[inline] pub fn has_subcommands(&self) -> bool { !self.subcommands.is_empty() } diff --git a/tests/hidden_args.rs b/tests/hidden_args.rs index f73c4986..80dab45b 100644 --- a/tests/hidden_args.rs +++ b/tests/hidden_args.rs @@ -5,12 +5,12 @@ use clap::{App, Arg}; include!("../clap-test.rs"); -static HIDDEN_ARGS: &'static str = "test 1.3 +static HIDDEN_ARGS: &'static str = "test 1.4 Kevin K. tests stuff USAGE: - test [FLAGS] [OPTIONS] + test [FLAGS] [OPTIONS] [DUMMY] FLAGS: -F, --flag2 some other flag @@ -25,9 +25,10 @@ fn hidden_args() { let app = App::new("test") .author("Kevin K.") .about("tests stuff") - .version("1.3") + .version("1.4") .args(&[Arg::from_usage("-f, --flag 'some flag'").hidden(true), Arg::from_usage("-F, --flag2 'some other flag'"), - Arg::from_usage("--option [opt] 'some option'")]); + Arg::from_usage("--option [opt] 'some option'"), + Arg::with_name("DUMMY").required(false).hidden(true)]); assert!(test::compare_output(app, "test --help", HIDDEN_ARGS, false)); } From 150756b9891e8044bacd20883bb68094a0fa9f93 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 2 Mar 2017 10:58:06 -0500 Subject: [PATCH 04/31] setting(InferSubcommands): adds a setting to allow one to infer shortened subcommands or aliases (i.e. for subcommmand "test", "t", "te", or "tes" would be allowed assuming no other ambiguities) Closes #863 --- src/app/macros.rs | 2 + src/app/parser.rs | 160 +++++++++++++++++++++++++----------------- src/app/settings.rs | 104 +++++++++++++++++---------- src/args/settings.rs | 28 ++++---- src/macros.rs | 23 +++++- src/suggestions.rs | 2 +- tests/app_settings.rs | 85 +++++++++++++++++++++- 7 files changed, 282 insertions(+), 122 deletions(-) diff --git a/src/app/macros.rs b/src/app/macros.rs index 7224fa42..c60ee342 100644 --- a/src/app/macros.rs +++ b/src/app/macros.rs @@ -149,6 +149,8 @@ macro_rules! parse_positional { let _ = $_self.groups_for_arg($p.b.name) .and_then(|vec| Some($matcher.inc_occurrences_of(&*vec))); arg_post_processing!($_self, $p, $matcher); + + $_self.set(AS::ValidArgFound); // Only increment the positional counter if it doesn't allow multiples if !$p.b.settings.is_set(ArgSettings::Multiple) { $pos_counter += 1; diff --git a/src/app/parser.rs b/src/app/parser.rs index 4d1b6bbb..8e700d07 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -534,6 +534,9 @@ impl<'a, 'b> Parser<'a, 'b> false } + #[inline] + pub fn has_args(&self) -> bool { !(self.flags.is_empty() && self.opts.is_empty() && self.positionals.is_empty()) } + #[inline] pub fn has_opts(&self) -> bool { !self.opts.is_empty() } @@ -665,25 +668,50 @@ impl<'a, 'b> Parser<'a, 'b> } // Checks if the arg matches a subcommand name, or any of it's aliases (if defined) - #[inline] - fn possible_subcommand(&self, arg_os: &OsStr) -> bool { + fn possible_subcommand(&self, arg_os: &OsStr) -> (bool, Option<&str>) { debugln!("Parser::possible_subcommand: arg={:?}", arg_os); - if self.is_set(AS::ArgsNegateSubcommands) && self.is_set(AS::ValidArgFound) { - return false; + fn starts(h: &str, n: &OsStr) -> bool { + #[cfg(not(target_os = "windows"))] + use std::os::unix::ffi::OsStrExt; + #[cfg(target_os = "windows")] + use ossstringext::OsStrExt3; + + let n_bytes = n.as_bytes(); + let h_bytes = OsStr::new(h).as_bytes(); + + h_bytes.starts_with(n_bytes) } - self.subcommands - .iter() - .any(|s| { - &s.p.meta.name[..] == &*arg_os || - (s.p.meta.aliases.is_some() && - s.p - .meta - .aliases - .as_ref() - .unwrap() - .iter() - .any(|&(a, _)| a == &*arg_os)) - }) + + if self.is_set(AS::ArgsNegateSubcommands) && self.is_set(AS::ValidArgFound) { + return (false, None); + } + if !self.is_set(AS::InferSubcommands) { + if let Some(sc) = find_subcmd!(self, arg_os) { + return (true, Some(&sc.p.meta.name)); + } + } else { + let v = self.subcommands + .iter() + .filter(|s| { + starts(&s.p.meta.name[..], &*arg_os) || + (s.p.meta.aliases.is_some() && + s.p + .meta + .aliases + .as_ref() + .unwrap() + .iter() + .filter(|&&(a, _)| starts(a, &*arg_os)) + .count() == 1) + }) + .map(|sc| &sc.p.meta.name) + .collect::>(); + + if v.len() == 1 { + return (true, Some(v[0])); + } + } + return (false, None); } fn parse_help_subcommand(&self, it: &mut I) -> ClapResult<()> @@ -786,29 +814,27 @@ impl<'a, 'b> Parser<'a, 'b> } else { false }; - debugln!("Parser::is_new_arg: Does arg allow leading hyphen...{:?}", - arg_allows_tac); + debugln!("Parser::is_new_arg: Arg::allow_leading_hyphen({:?})", arg_allows_tac); // Is this a new argument, or values from a previous option? - debug!("Parser::is_new_arg: Starts new arg..."); let mut ret = if arg_os.starts_with(b"--") { - sdebugln!("--"); + debugln!("Parser::is_new_arg: -- found"); if arg_os.len_() == 2 { return true; // We have to return true so override everything else } true } else if arg_os.starts_with(b"-") { - sdebugln!("-"); + debugln!("Parser::is_new_arg: - found"); // a singe '-' by itself is a value and typically means "stdin" on unix systems !(arg_os.len_() == 1) } else { - sdebugln!("Probable value"); + debugln!("Parser::is_new_arg: probably value"); false }; ret = ret && !arg_allows_tac; - debugln!("Parser::is_new_arg: Starts new arg...{:?}", ret); + debugln!("Parser::is_new_arg: starts_new_arg={:?}", ret); ret } @@ -824,6 +850,7 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Parser::get_matches_with;"); // Verify all positional assertions pass self.verify_positionals(); + let has_args = self.has_args(); // Next we create the `--help` and `--version` arguments and add them if // necessary @@ -845,12 +872,16 @@ impl<'a, 'b> Parser<'a, 'b> // Has the user already passed '--'? Meaning only positional args follow if !self.is_set(AS::TrailingValues) { // Does the arg match a subcommand name, or any of it's aliases (if defined) - if self.possible_subcommand(&arg_os) { - if &*arg_os == "help" && self.is_set(AS::NeedsSubcommandHelp) { - try!(self.parse_help_subcommand(it)); + { + let (is_match, sc_name) = self.possible_subcommand(&arg_os); + if is_match { + let sc_name = sc_name.expect(INTERNAL_ERROR_MSG); + if sc_name == "help" && self.is_set(AS::NeedsSubcommandHelp) { + try!(self.parse_help_subcommand(it)); + } + subcmd_name = Some(sc_name.to_owned()); + break; } - subcmd_name = Some(arg_os.to_str().expect(INVALID_UTF8).to_owned()); - break; } if !starts_new_arg { @@ -868,6 +899,7 @@ impl<'a, 'b> Parser<'a, 'b> if arg_os.len_() == 2 { // The user has passed '--' which means only positional args follow no // matter what they start with + debugln!("Parser::get_matches_with: found '--' setting TrailingVals=true"); self.set(AS::TrailingValues); continue; } @@ -900,11 +932,11 @@ impl<'a, 'b> Parser<'a, 'b> } } - if !self.is_set(AS::ArgsNegateSubcommands) { + if !(self.is_set(AS::ArgsNegateSubcommands) && + self.is_set(AS::ValidArgFound)) && + !self.is_set(AS::InferSubcommands) { if let Some(cdate) = suggestions::did_you_mean(&*arg_os.to_string_lossy(), - self.subcommands - .iter() - .map(|s| &s.p.meta.name)) { + sc_names!(self)) { return Err(Error::invalid_subcommand(arg_os.to_string_lossy() .into_owned(), cdate, @@ -920,15 +952,13 @@ impl<'a, 'b> Parser<'a, 'b> } let low_index_mults = self.is_set(AS::LowIndexMultiplePositional) && - !self.positionals.is_empty() && pos_counter == (self.positionals.len() - 1); let missing_pos = self.is_set(AS::AllowMissingPositional) && - !self.positionals.is_empty() && pos_counter == (self.positionals.len() - 1); - debugln!("Parser::get_matches_with: Low index multiples...{:?}", - low_index_mults); debugln!("Parser::get_matches_with: Positional counter...{}", pos_counter); + debugln!("Parser::get_matches_with: Low index multiples...{:?}", + low_index_mults); if low_index_mults || missing_pos { if let Some(na) = it.peek() { let n = (*na).clone().into(); @@ -941,11 +971,12 @@ impl<'a, 'b> Parser<'a, 'b> } else { None }; - if self.is_new_arg(&n, needs_val_of) || self.possible_subcommand(&n) || + let sc_match = { + self.possible_subcommand(&n).0 + }; + if self.is_new_arg(&n, needs_val_of) || sc_match || suggestions::did_you_mean(&n.to_string_lossy(), - self.subcommands - .iter() - .map(|s| &s.p.meta.name)) + sc_names!(self)) .is_some() { debugln!("Parser::get_matches_with: Bumping the positional counter..."); pos_counter += 1; @@ -988,36 +1019,33 @@ impl<'a, 'b> Parser<'a, 'b> matches: sc_m.into(), }); } else if !(self.is_set(AS::AllowLeadingHyphen) || - self.is_set(AS::AllowNegativeNumbers)) { + self.is_set(AS::AllowNegativeNumbers)) && !self.is_set(AS::InferSubcommands) { return Err(Error::unknown_argument(&*arg_os.to_string_lossy(), "", &*self.create_current_usage(matcher, None), self.color())); + } else if !(has_args) && self.has_subcommands() { + if let Some(cdate) = suggestions::did_you_mean(&*arg_os.to_string_lossy(), + sc_names!(self)) { + return Err(Error::invalid_subcommand(arg_os.to_string_lossy() + .into_owned(), + cdate, + self.meta + .bin_name + .as_ref() + .unwrap_or(&self.meta.name), + &*self.create_current_usage(matcher, + None), + self.color())); + } } } if let Some(ref pos_sc_name) = subcmd_name { - // is this is a real subcommand, or an alias - if self.subcommands.iter().any(|sc| &sc.p.meta.name == pos_sc_name) { - try!(self.parse_subcommand(&*pos_sc_name, matcher, it)); - } else { - let sc_name = &*self.subcommands - .iter() - .filter(|sc| sc.p.meta.aliases.is_some()) - .filter(|sc| { - sc.p - .meta - .aliases - .as_ref() - .expect(INTERNAL_ERROR_MSG) - .iter() - .any(|&(a, _)| &a == &&*pos_sc_name) - }) - .map(|sc| sc.p.meta.name.clone()) - .next() - .expect(INTERNAL_ERROR_MSG); - try!(self.parse_subcommand(sc_name, matcher, it)); - } + let sc_name = { + find_subcmd!(self, pos_sc_name).expect(INTERNAL_ERROR_MSG).p.meta.name.clone() + }; + try!(self.parse_subcommand(&*sc_name, matcher, it)); } else if self.is_set(AS::SubcommandRequired) { let bn = self.meta.bin_name.as_ref().unwrap_or(&self.meta.name); return Err(Error::missing_subcommand(bn, @@ -1404,6 +1432,7 @@ impl<'a, 'b> Parser<'a, 'b> opt.to_string()); self.settings.set(AS::ValidArgFound); let ret = try!(self.parse_opt(val, opt, val.is_some(), matcher)); + self.set(AS::ValidArgFound); arg_post_processing!(self, opt, matcher); return Ok(ret); @@ -1420,6 +1449,7 @@ impl<'a, 'b> Parser<'a, 'b> // Handle conflicts, requirements, etc. arg_post_processing!(self, flag, matcher); + self.set(AS::ValidArgFound); return Ok(None); } else if self.is_set(AS::AllowLeadingHyphen) { return Ok(None); @@ -1489,6 +1519,7 @@ impl<'a, 'b> Parser<'a, 'b> arg_post_processing!(self, opt, matcher); + self.set(AS::ValidArgFound); return Ok(ret); } else if let Some(flag) = find_flag_by_short!(self, c) { debugln!("Parser::parse_short_arg:iter: Found valid short flag -{}", @@ -1497,6 +1528,8 @@ impl<'a, 'b> Parser<'a, 'b> // Only flags can be help or version try!(self.check_for_help_and_version_char(c)); try!(self.parse_flag(flag, matcher)); + + self.set(AS::ValidArgFound); // Handle conflicts, requirements, overrides, etc. // Must be called here due to mutablilty arg_post_processing!(self, flag, matcher); @@ -2264,6 +2297,7 @@ impl<'a, 'b> Parser<'a, 'b> None } + // Only used for completion scripts due to bin_name messiness #[cfg_attr(feature = "lints", allow(explicit_iter_loop))] pub fn find_subcommand(&'b self, sc: &str) -> Option<&'b App<'a, 'b>> { debugln!("Parser::find_subcommand: sc={}", sc); diff --git a/src/app/settings.rs b/src/app/settings.rs index 42edc610..3b7a326b 100644 --- a/src/app/settings.rs +++ b/src/app/settings.rs @@ -5,44 +5,45 @@ use std::ops::BitOr; bitflags! { flags Flags: u64 { - const SC_NEGATE_REQS = 0b00000000000000000000000000000000000001, - const SC_REQUIRED = 0b00000000000000000000000000000000000010, - const A_REQUIRED_ELSE_HELP = 0b00000000000000000000000000000000000100, - const GLOBAL_VERSION = 0b00000000000000000000000000000000001000, - const VERSIONLESS_SC = 0b00000000000000000000000000000000010000, - const UNIFIED_HELP = 0b00000000000000000000000000000000100000, - const WAIT_ON_ERROR = 0b00000000000000000000000000000001000000, - const SC_REQUIRED_ELSE_HELP= 0b00000000000000000000000000000010000000, - const NEEDS_LONG_HELP = 0b00000000000000000000000000000100000000, - const NEEDS_LONG_VERSION = 0b00000000000000000000000000001000000000, - const NEEDS_SC_HELP = 0b00000000000000000000000000010000000000, - const DISABLE_VERSION = 0b00000000000000000000000000100000000000, - const HIDDEN = 0b00000000000000000000000001000000000000, - const TRAILING_VARARG = 0b00000000000000000000000010000000000000, - const NO_BIN_NAME = 0b00000000000000000000000100000000000000, - const ALLOW_UNK_SC = 0b00000000000000000000001000000000000000, - const UTF8_STRICT = 0b00000000000000000000010000000000000000, - const UTF8_NONE = 0b00000000000000000000100000000000000000, - const LEADING_HYPHEN = 0b00000000000000000001000000000000000000, - const NO_POS_VALUES = 0b00000000000000000010000000000000000000, - const NEXT_LINE_HELP = 0b00000000000000000100000000000000000000, - const DERIVE_DISP_ORDER = 0b00000000000000001000000000000000000000, - const COLORED_HELP = 0b00000000000000010000000000000000000000, - const COLOR_ALWAYS = 0b00000000000000100000000000000000000000, - const COLOR_AUTO = 0b00000000000001000000000000000000000000, - const COLOR_NEVER = 0b00000000000010000000000000000000000000, - const DONT_DELIM_TRAIL = 0b00000000000100000000000000000000000000, - const ALLOW_NEG_NUMS = 0b00000000001000000000000000000000000000, - const LOW_INDEX_MUL_POS = 0b00000000010000000000000000000000000000, - const DISABLE_HELP_SC = 0b00000000100000000000000000000000000000, - const DONT_COLLAPSE_ARGS = 0b00000001000000000000000000000000000000, - const ARGS_NEGATE_SCS = 0b00000010000000000000000000000000000000, - const PROPAGATE_VALS_DOWN = 0b00000100000000000000000000000000000000, - const ALLOW_MISSING_POS = 0b00001000000000000000000000000000000000, - const TRAILING_VALUES = 0b00010000000000000000000000000000000000, - const VALID_NEG_NUM_FOUND = 0b00100000000000000000000000000000000000, - const PROPOGATED = 0b01000000000000000000000000000000000000, - const VALID_ARG_FOUND = 0b10000000000000000000000000000000000000 + const SC_NEGATE_REQS = 1 << 0, + const SC_REQUIRED = 1 << 1, + const A_REQUIRED_ELSE_HELP = 1 << 2, + const GLOBAL_VERSION = 1 << 3, + const VERSIONLESS_SC = 1 << 4, + const UNIFIED_HELP = 1 << 5, + const WAIT_ON_ERROR = 1 << 6, + const SC_REQUIRED_ELSE_HELP= 1 << 7, + const NEEDS_LONG_HELP = 1 << 8, + const NEEDS_LONG_VERSION = 1 << 9, + const NEEDS_SC_HELP = 1 << 10, + const DISABLE_VERSION = 1 << 11, + const HIDDEN = 1 << 12, + const TRAILING_VARARG = 1 << 13, + const NO_BIN_NAME = 1 << 14, + const ALLOW_UNK_SC = 1 << 15, + const UTF8_STRICT = 1 << 16, + const UTF8_NONE = 1 << 17, + const LEADING_HYPHEN = 1 << 18, + const NO_POS_VALUES = 1 << 19, + const NEXT_LINE_HELP = 1 << 20, + const DERIVE_DISP_ORDER = 1 << 21, + const COLORED_HELP = 1 << 22, + const COLOR_ALWAYS = 1 << 23, + const COLOR_AUTO = 1 << 24, + const COLOR_NEVER = 1 << 25, + const DONT_DELIM_TRAIL = 1 << 26, + const ALLOW_NEG_NUMS = 1 << 27, + const LOW_INDEX_MUL_POS = 1 << 28, + const DISABLE_HELP_SC = 1 << 29, + const DONT_COLLAPSE_ARGS = 1 << 30, + const ARGS_NEGATE_SCS = 1 << 31, + const PROPAGATE_VALS_DOWN = 1 << 32, + const ALLOW_MISSING_POS = 1 << 33, + const TRAILING_VALUES = 1 << 34, + const VALID_NEG_NUM_FOUND = 1 << 35, + const PROPOGATED = 1 << 36, + const VALID_ARG_FOUND = 1 << 37, + const INFER_SUBCOMMANDS = 1 << 38, } } @@ -102,7 +103,8 @@ impl AppFlags { TrailingValues => TRAILING_VALUES, ValidNegNumFound => VALID_NEG_NUM_FOUND, Propogated => PROPOGATED, - ValidArgFound => VALID_ARG_FOUND + ValidArgFound => VALID_ARG_FOUND, + InferSubcommands => INFER_SUBCOMMANDS } } @@ -525,6 +527,27 @@ pub enum AppSettings { /// This can be useful if there are many values, or they are explained elsewhere. HidePossibleValuesInHelp, + /// Tries to match unknown args to partial [`subcommands`] or their aliases. For example to + /// match a subcommand named `test`, one could use `t`, `te`, `tes`, and `test`. + /// + /// **NOTE:** The match *must not* be ambiguous at all in order to succeed. i.e. to match `te` + /// to `test` there could not also be a subcommand or alias `temp` because both start with `te` + /// + /// # Examples + /// + /// ```no_run + /// # use clap::{App, Arg, SubCommand, AppSettings}; + /// let m = App::new("prog") + /// .setting(AppSettings::InferSubcommands) + /// .subcommand(SubCommand::with_name("test")) + /// .get_matches_from(vec![ + /// "prog", "te" + /// ]); + /// assert_eq!(m.subcommand_name(), Some("test")); + /// ``` + /// [`subcommands`]: ./struct.SubCommand.html + InferSubcommands, + /// Specifies that the parser should not assume the first argument passed is the binary name. /// This is normally the case when using a "daemon" style mode, or an interactive CLI where one /// one would not normally type the binary or program name for each command. @@ -851,6 +874,7 @@ impl FromStr for AppSettings { "globalversion" => Ok(AppSettings::GlobalVersion), "hidden" => Ok(AppSettings::Hidden), "hidepossiblevaluesinhelp" => Ok(AppSettings::HidePossibleValuesInHelp), + "infersubcommands" => Ok(AppSettings::InferSubcommands), "lowindexmultiplepositional" => Ok(AppSettings::LowIndexMultiplePositional), "nobinaryname" => Ok(AppSettings::NoBinaryName), "nextlinehelp" => Ok(AppSettings::NextLineHelp), @@ -943,6 +967,8 @@ mod test { AppSettings::Propogated); assert_eq!("trailingvalues".parse::().unwrap(), AppSettings::TrailingValues); + assert_eq!("infersubcommands".parse::().unwrap(), + AppSettings::InferSubcommands); assert!("hahahaha".parse::().is_err()); } } diff --git a/src/args/settings.rs b/src/args/settings.rs index d2a85561..28281dce 100644 --- a/src/args/settings.rs +++ b/src/args/settings.rs @@ -4,20 +4,20 @@ use std::str::FromStr; bitflags! { flags Flags: u16 { - const REQUIRED = 0b00000000000001, - const MULTIPLE = 0b00000000000010, - const EMPTY_VALS = 0b00000000000100, - const GLOBAL = 0b00000000001000, - const HIDDEN = 0b00000000010000, - const TAKES_VAL = 0b00000000100000, - const USE_DELIM = 0b00000001000000, - const NEXT_LINE_HELP = 0b00000010000000, - const R_UNLESS_ALL = 0b00000100000000, - const REQ_DELIM = 0b00001000000000, - const DELIM_NOT_SET = 0b00010000000000, - const HIDE_POS_VALS = 0b00100000000000, - const ALLOW_TAC_VALS = 0b01000000000000, - const REQUIRE_EQUALS = 0b10000000000000, + const REQUIRED = 1 << 0, + const MULTIPLE = 1 << 1, + const EMPTY_VALS = 1 << 2, + const GLOBAL = 1 << 3, + const HIDDEN = 1 << 4, + const TAKES_VAL = 1 << 5, + const USE_DELIM = 1 << 6, + const NEXT_LINE_HELP = 1 << 7, + const R_UNLESS_ALL = 1 << 8, + const REQ_DELIM = 1 << 9, + const DELIM_NOT_SET = 1 << 10, + const HIDE_POS_VALS = 1 << 11, + const ALLOW_TAC_VALS = 1 << 12, + const REQUIRE_EQUALS = 1 << 13, } } diff --git a/src/macros.rs b/src/macros.rs index 2ce6d679..e5483741 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -989,7 +989,7 @@ macro_rules! find_subcmd { $_self.subcommands .iter() .find(|s| { - s.p.meta.name == $sc || + &*s.p.meta.name == $sc || (s.p.meta.aliases.is_some() && s.p .meta @@ -1029,12 +1029,18 @@ macro_rules! _shorts_longs { macro_rules! arg_names { ($_self:ident) => {{ - _names!($_self) + _names!(@args $_self) + }}; +} + +macro_rules! sc_names { + ($_self:ident) => {{ + _names!(@sc $_self) }}; } macro_rules! _names { - ($_self:ident) => {{ + (@args $_self:ident) => {{ $_self.flags .iter() .map(|f| &*f.b.name) @@ -1043,4 +1049,15 @@ macro_rules! _names { .chain($_self.positionals.values() .map(|p| &*p.b.name))) }}; + (@sc $_self:ident) => {{ + $_self.subcommands + .iter() + .map(|s| &*s.p.meta.name) + .chain($_self.subcommands + .iter() + .filter(|s| s.p.meta.aliases.is_some()) + .flat_map(|s| s.p.meta.aliases.as_ref().unwrap().iter().map(|&(n, _)| n))) + // .map(|n| &n) + + }} } diff --git a/src/suggestions.rs b/src/suggestions.rs index d11c6646..107e9579 100644 --- a/src/suggestions.rs +++ b/src/suggestions.rs @@ -12,7 +12,7 @@ use fmt::Format; #[cfg(feature = "suggestions")] #[cfg_attr(feature = "lints", allow(needless_lifetimes))] pub fn did_you_mean<'a, T, I>(v: &str, possible_values: I) -> Option<&'a str> - where T: AsRef + 'a, + where T: AsRef + 'a + ?Sized, I: IntoIterator { diff --git a/tests/app_settings.rs b/tests/app_settings.rs index 17aa8bd8..f933b3bc 100644 --- a/tests/app_settings.rs +++ b/tests/app_settings.rs @@ -110,6 +110,83 @@ fn arg_required_else_help() { assert_eq!(err.kind, ErrorKind::MissingArgumentOrSubcommand); } +#[test] +fn infer_subcommands_fail_no_args() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(SubCommand::with_name("test")) + .subcommand(SubCommand::with_name("temp")) + .get_matches_from_safe(vec![ + "prog", "te" + ]); + assert!(m.is_err()); + assert_eq!(m.unwrap_err().kind, ErrorKind::InvalidSubcommand); +} + +#[test] +fn infer_subcommands_fail_with_args() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .arg(Arg::with_name("some")) + .subcommand(SubCommand::with_name("test")) + .subcommand(SubCommand::with_name("temp")) + .get_matches_from_safe(vec![ + "prog", "t" + ]); + assert!(m.is_ok(), "{:?}", m.unwrap_err().kind); + assert_eq!(m.unwrap().value_of("some"), Some("t")); +} + +#[test] +fn infer_subcommands_fail_with_args2() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .arg(Arg::with_name("some")) + .subcommand(SubCommand::with_name("test")) + .subcommand(SubCommand::with_name("temp")) + .get_matches_from_safe(vec![ + "prog", "te" + ]); + assert!(m.is_ok(), "{:?}", m.unwrap_err().kind); + assert_eq!(m.unwrap().value_of("some"), Some("te")); +} + +#[test] +fn infer_subcommands_pass() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(SubCommand::with_name("test")) + .get_matches_from(vec![ + "prog", "te" + ]); + assert_eq!(m.subcommand_name(), Some("test")); +} + +#[test] +fn infer_subcommands_pass_close() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(SubCommand::with_name("test")) + .subcommand(SubCommand::with_name("temp")) + .get_matches_from(vec![ + "prog", "tes" + ]); + assert_eq!(m.subcommand_name(), Some("test")); +} + +#[test] +fn infer_subcommands_fail_suggestions() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(SubCommand::with_name("test")) + .subcommand(SubCommand::with_name("temp")) + .get_matches_from_safe(vec![ + "prog", "temps" + ]); + assert!(m.is_err()); + assert_eq!(m.unwrap_err().kind, ErrorKind::InvalidSubcommand); +} + #[test] fn no_bin_name() { let result = App::new("arg_required") @@ -452,7 +529,9 @@ fn propagate_vals_down() { .setting(AppSettings::PropagateGlobalValuesDown) .arg(Arg::from_usage("[cmd] 'command to run'").global(true)) .subcommand(SubCommand::with_name("foo")) - .get_matches_from(vec!["myprog", "set", "foo"]); + .get_matches_from_safe(vec!["myprog", "set", "foo"]); + assert!(m.is_ok(), "{:?}", m.unwrap_err().kind); + let m = m.unwrap(); assert_eq!(m.value_of("cmd"), Some("set")); let sub_m = m.subcommand_matches("foo").unwrap(); assert_eq!(sub_m.value_of("cmd"), Some("set")); @@ -464,7 +543,9 @@ fn allow_missing_positional() { .setting(AppSettings::AllowMissingPositional) .arg(Arg::from_usage("[src] 'some file'").default_value("src")) .arg_from_usage(" 'some file'") - .get_matches_from(vec!["test", "file"]); + .get_matches_from_safe(vec!["test", "file"]); + assert!(m.is_ok(), "{:?}", m.unwrap_err().kind); + let m = m.unwrap(); assert_eq!(m.value_of("src"), Some("src")); assert_eq!(m.value_of("dest"), Some("file")); } \ No newline at end of file From 6f638a53c18a66979b89220c9945ef97e49571b3 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 3 Mar 2017 05:13:28 -0500 Subject: [PATCH 05/31] perf: doesn't run arg_post_processing on multiple values anymore --- src/app/macros.rs | 7 +++++-- src/app/parser.rs | 48 ++++++++++++++++++++++++++------------------- src/app/settings.rs | 11 ++++++++++- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/src/app/macros.rs b/src/app/macros.rs index c60ee342..41af48c5 100644 --- a/src/app/macros.rs +++ b/src/app/macros.rs @@ -148,9 +148,12 @@ macro_rules! parse_positional { $matcher.inc_occurrence_of($p.b.name); let _ = $_self.groups_for_arg($p.b.name) .and_then(|vec| Some($matcher.inc_occurrences_of(&*vec))); - arg_post_processing!($_self, $p, $matcher); + if $_self.cache.map_or(true, |name| name != $p.b.name) { + arg_post_processing!($_self, $p, $matcher); + $_self.cache = Some($p.b.name); + } - $_self.set(AS::ValidArgFound); + $_self.settings.set(AS::ValidArgFound); // Only increment the positional counter if it doesn't allow multiples if !$p.b.settings.is_set(ArgSettings::Multiple) { $pos_counter += 1; diff --git a/src/app/parser.rs b/src/app/parser.rs index 8e700d07..4688e85a 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -54,6 +54,7 @@ pub struct Parser<'a, 'b> overrides: Vec<&'b str>, help_short: Option, version_short: Option, + cache: Option<&'a str>, help_message: Option<&'a str>, version_message: Option<&'a str>, } @@ -81,14 +82,6 @@ impl<'a, 'b> Parser<'a, 'b> self.version_short = Some(c); } - pub fn help_message(&mut self, s: &'a str) { - self.help_message = Some(s); - } - - pub fn version_message(&mut self, s: &'a str) { - self.version_message = Some(s); - } - pub fn gen_completions_to(&mut self, for_shell: Shell, buf: &mut W) { if !self.is_set(AS::Propogated) { self.propogate_help_version(); @@ -1289,7 +1282,7 @@ impl<'a, 'b> Parser<'a, 'b> let arg = FlagBuilder { b: Base { name: "hclap_help", - help: Some(self.help_message.unwrap_or("Prints help information")), + help: Some("Prints help information"), ..Default::default() }, s: Switched { @@ -1309,7 +1302,7 @@ impl<'a, 'b> Parser<'a, 'b> let arg = FlagBuilder { b: Base { name: "vclap_version", - help: Some(self.version_message.unwrap_or("Prints version information")), + help: Some("Prints version information"), ..Default::default() }, s: Switched { @@ -1432,8 +1425,10 @@ impl<'a, 'b> Parser<'a, 'b> opt.to_string()); self.settings.set(AS::ValidArgFound); let ret = try!(self.parse_opt(val, opt, val.is_some(), matcher)); - self.set(AS::ValidArgFound); - arg_post_processing!(self, opt, matcher); + if self.cache.map_or(true, |name| name != opt.b.name) { + arg_post_processing!(self, opt, matcher); + self.cache = Some(opt.b.name); + } return Ok(ret); } else if let Some(flag) = find_flag_by_long!(@os self, &arg) { @@ -1447,9 +1442,11 @@ impl<'a, 'b> Parser<'a, 'b> try!(self.parse_flag(flag, matcher)); // Handle conflicts, requirements, etc. - arg_post_processing!(self, flag, matcher); + if self.cache.map_or(true, |name| name != flag.b.name) { + arg_post_processing!(self, flag, matcher); + self.cache = Some(flag.b.name); + } - self.set(AS::ValidArgFound); return Ok(None); } else if self.is_set(AS::AllowLeadingHyphen) { return Ok(None); @@ -1517,9 +1514,11 @@ impl<'a, 'b> Parser<'a, 'b> // Default to "we're expecting a value later" let ret = try!(self.parse_opt(val, opt, false, matcher)); - arg_post_processing!(self, opt, matcher); + if self.cache.map_or(true, |name| name != opt.b.name) { + arg_post_processing!(self, opt, matcher); + self.cache = Some(opt.b.name); + } - self.set(AS::ValidArgFound); return Ok(ret); } else if let Some(flag) = find_flag_by_short!(self, c) { debugln!("Parser::parse_short_arg:iter: Found valid short flag -{}", @@ -1529,10 +1528,12 @@ impl<'a, 'b> Parser<'a, 'b> try!(self.check_for_help_and_version_char(c)); try!(self.parse_flag(flag, matcher)); - self.set(AS::ValidArgFound); // Handle conflicts, requirements, overrides, etc. // Must be called here due to mutablilty - arg_post_processing!(self, flag, matcher); + if self.cache.map_or(true, |name| name != flag.b.name) { + arg_post_processing!(self, flag, matcher); + self.cache = Some(flag.b.name); + } } else { let arg = format!("-{}", c); return Err(Error::unknown_argument(&*arg, @@ -2215,7 +2216,11 @@ impl<'a, 'b> Parser<'a, 'b> if let Some(ref val) = $a.v.default_val { if $m.get($a.b.name).is_none() { try!($_self.add_val_to_arg($a, OsStr::new(val), $m)); - arg_post_processing!($_self, $a, $m); + + if $_self.cache.map_or(true, |name| name != $a.name()) { + arg_post_processing!($_self, $a, $m); + $_self.cache = Some($a.name()); + } } } }; @@ -2235,7 +2240,10 @@ impl<'a, 'b> Parser<'a, 'b> }; if add { try!($_self.add_val_to_arg($a, OsStr::new(default), $m)); - arg_post_processing!($_self, $a, $m); + if $_self.cache.map_or(true, |name| name != $a.name()) { + arg_post_processing!($_self, $a, $m); + $_self.cache = Some($a.name()); + } done = true; break; } diff --git a/src/app/settings.rs b/src/app/settings.rs index 3b7a326b..a092e259 100644 --- a/src/app/settings.rs +++ b/src/app/settings.rs @@ -527,12 +527,18 @@ pub enum AppSettings { /// This can be useful if there are many values, or they are explained elsewhere. HidePossibleValuesInHelp, - /// Tries to match unknown args to partial [`subcommands`] or their aliases. For example to + /// Tries to match unknown args to partial [`subcommands`] or their [aliases]. For example to /// match a subcommand named `test`, one could use `t`, `te`, `tes`, and `test`. /// /// **NOTE:** The match *must not* be ambiguous at all in order to succeed. i.e. to match `te` /// to `test` there could not also be a subcommand or alias `temp` because both start with `te` /// + /// **CAUTION:** This setting can interfere with [positional/free arguments], take care when + /// designing CLIs which allow inferred subcommands and have potential positional/free + /// arguments who's values could start with the same characters as subcommands. If this is the + /// case, it's recommended to use settings such as [`AppSeettings::ArgsNegateSubcommands`] in + /// conjuction with this setting. + /// /// # Examples /// /// ```no_run @@ -546,6 +552,9 @@ pub enum AppSettings { /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` /// [`subcommands`]: ./struct.SubCommand.html + /// [positional/free arguments]: ./struct.Arg.html#method.index + /// [aliases]: ./struct.App.html#method.alias + /// [`AppSeettings::ArgsNegateSubcommands`]: ./enum.AppSettings.html#variant.ArgsNegateSubcommands InferSubcommands, /// Specifies that the parser should not assume the first argument passed is the binary name. From 6b58efa330190bb5ec355405894ebb9aee7b6034 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 4 Mar 2017 14:53:15 -0500 Subject: [PATCH 06/31] tests: adds some minor benches for adding args by ref or moving --- benches/02_simple.rs | 65 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/benches/02_simple.rs b/benches/02_simple.rs index eab94b37..010fa866 100644 --- a/benches/02_simple.rs +++ b/benches/02_simple.rs @@ -3,7 +3,7 @@ extern crate clap; extern crate test; -use clap::App; +use clap::{App, Arg}; use test::Bencher; @@ -25,6 +25,69 @@ fn build_app(b: &mut Bencher) { b.iter(|| create_app!()); } +#[bench] +fn add_flag(b: &mut Bencher) { + fn build_app() -> App<'static, 'static> { + App::new("claptests") + } + + b.iter(|| build_app().arg(Arg::from_usage("-s, --some 'something'"))); +} + +#[bench] +fn add_flag_ref(b: &mut Bencher) { + fn build_app() -> App<'static, 'static> { + App::new("claptests") + } + + b.iter(|| { + let arg = Arg::from_usage("-s, --some 'something'"); + build_app().arg(&arg) + }); +} + +#[bench] +fn add_opt(b: &mut Bencher) { + fn build_app() -> App<'static, 'static> { + App::new("claptests") + } + + b.iter(|| build_app().arg(Arg::from_usage("-s, --some 'something'"))); +} + +#[bench] +fn add_opt_ref(b: &mut Bencher) { + fn build_app() -> App<'static, 'static> { + App::new("claptests") + } + + b.iter(|| { + let arg = Arg::from_usage("-s, --some 'something'"); + build_app().arg(&arg) + }); +} + +#[bench] +fn add_pos(b: &mut Bencher) { + fn build_app() -> App<'static, 'static> { + App::new("claptests") + } + + b.iter(|| build_app().arg(Arg::with_name("some"))); +} + +#[bench] +fn add_pos_ref(b: &mut Bencher) { + fn build_app() -> App<'static, 'static> { + App::new("claptests") + } + + b.iter(|| { + let arg = Arg::with_name("some"); + build_app().arg(&arg) + }); +} + #[bench] fn parse_clean(b: &mut Bencher) { b.iter(|| create_app!().get_matches_from(vec![""])); From 91d828032201c21095152901e68c9197443831e6 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 4 Mar 2017 15:10:17 -0500 Subject: [PATCH 07/31] chore: rustfmt run --- src/app/parser.rs | 116 +++++++++++++++++++++++++--------------------- 1 file changed, 63 insertions(+), 53 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index 4688e85a..8e51d8bd 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -345,20 +345,28 @@ impl<'a, 'b> Parser<'a, 'b> let mut new_reqs: Vec<&str> = vec![]; macro_rules! get_requires { (@group $a: ident, $v:ident, $p:ident) => {{ - if let Some(rl) = self.groups.iter().filter(|g| g.requires.is_some()).find(|g| &g.name == $a).map(|g| g.requires.as_ref().unwrap()) { + if let Some(rl) = self.groups.iter() + .filter(|g| g.requires.is_some()) + .find(|g| &g.name == $a) + .map(|g| g.requires.as_ref().unwrap()) { for r in rl { if !$p.contains(&r) { - debugln!("Parser::get_required_from:iter:{}: adding group req={:?}", $a, r); + debugln!("Parser::get_required_from:iter:{}: adding group req={:?}", + $a, r); $v.push(r); } } } }}; ($a:ident, $what:ident, $how:ident, $v:ident, $p:ident) => {{ - if let Some(rl) = self.$what.$how().filter(|a| a.b.requires.is_some()).find(|arg| &arg.b.name == $a).map(|a| a.b.requires.as_ref().unwrap()) { + if let Some(rl) = self.$what.$how() + .filter(|a| a.b.requires.is_some()) + .find(|arg| &arg.b.name == $a) + .map(|a| a.b.requires.as_ref().unwrap()) { for &(_, r) in rl.iter() { if !$p.contains(&r) { - debugln!("Parser::get_required_from:iter:{}: adding arg req={:?}",$a, r); + debugln!("Parser::get_required_from:iter:{}: adding arg req={:?}", + $a, r); $v.push(r); } } @@ -528,7 +536,9 @@ impl<'a, 'b> Parser<'a, 'b> } #[inline] - pub fn has_args(&self) -> bool { !(self.flags.is_empty() && self.opts.is_empty() && self.positionals.is_empty()) } + pub fn has_args(&self) -> bool { + !(self.flags.is_empty() && self.opts.is_empty() && self.positionals.is_empty()) + } #[inline] pub fn has_opts(&self) -> bool { !self.opts.is_empty() } @@ -561,8 +571,8 @@ impl<'a, 'b> Parser<'a, 'b> // but no 2) 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", + format!("Found positional argument \"{}\" who's index is {} but there \ + are only {} positional arguments defined", p.b.name, idx, self.positionals.len())); @@ -583,15 +593,16 @@ impl<'a, 'b> Parser<'a, 'b> // Or the second to last has a terminator set || it.next().unwrap().v.terminator.is_some() }, - "When using a positional argument with .multiple(true) that is *not the last* \ - positional argument, the last positional argument (i.e the one with the highest \ - index) *must* have .required(true) set."); + "When using a positional argument with .multiple(true) that is *not the \ + last* positional argument, the last positional argument (i.e the one \ + with the highest index) *must* have .required(true) set."); debug_assert!({ let num = self.positionals.len() - 1; self.positionals.get(num).unwrap().is_set(ArgSettings::Multiple) }, - "Only the last positional argument, or second to last positional argument may be set to .multiple(true)"); + "Only the last positional argument, or second to last positional \ + argument may be set to .multiple(true)"); self.set(AS::LowIndexMultiplePositional); } @@ -603,7 +614,8 @@ impl<'a, 'b> Parser<'a, 'b> }) .map(|_| 1) .sum::() <= 1, - "Only one positional argument with .multiple(true) set is allowed per command"); + "Only one positional argument with .multiple(true) set is allowed per \ + command"); // If it's required we also need to ensure all previous positionals are // required too @@ -615,8 +627,9 @@ impl<'a, 'b> Parser<'a, 'b> // [arg1] is Ok // [arg1] Is not debug_assert!(p.b.settings.is_set(ArgSettings::Required), - "Found positional argument which is not required with a lower index \ - than a required positional argument by two or more: {:?} index {}", + "Found positional argument which is not required with a lower \ + index than a required positional argument by two or more: {:?} \ + index {}", p.b.name, p.index); } else if p.b.settings.is_set(ArgSettings::Required) { @@ -635,8 +648,8 @@ impl<'a, 'b> Parser<'a, 'b> for p in self.positionals.values().rev() { if found { debug_assert!(p.b.settings.is_set(ArgSettings::Required), - "Found positional argument which is not required with a lower index \ - than a required positional argument: {:?} index {}", + "Found positional argument which is not required with a lower \ + index than a required positional argument: {:?} index {}", p.b.name, p.index); } else if p.b.settings.is_set(ArgSettings::Required) { @@ -688,7 +701,7 @@ impl<'a, 'b> Parser<'a, 'b> .filter(|s| { starts(&s.p.meta.name[..], &*arg_os) || (s.p.meta.aliases.is_some() && - s.p + s.p .meta .aliases .as_ref() @@ -699,7 +712,7 @@ impl<'a, 'b> Parser<'a, 'b> }) .map(|sc| &sc.p.meta.name) .collect::>(); - + if v.len() == 1 { return (true, Some(v[0])); } @@ -807,7 +820,8 @@ impl<'a, 'b> Parser<'a, 'b> } else { false }; - debugln!("Parser::is_new_arg: Arg::allow_leading_hyphen({:?})", arg_allows_tac); + debugln!("Parser::is_new_arg: Arg::allow_leading_hyphen({:?})", + arg_allows_tac); // Is this a new argument, or values from a previous option? let mut ret = if arg_os.starts_with(b"--") { @@ -867,9 +881,9 @@ impl<'a, 'b> Parser<'a, 'b> // Does the arg match a subcommand name, or any of it's aliases (if defined) { let (is_match, sc_name) = self.possible_subcommand(&arg_os); - if is_match { + if is_match { let sc_name = sc_name.expect(INTERNAL_ERROR_MSG); - if sc_name == "help" && self.is_set(AS::NeedsSubcommandHelp) { + if sc_name == "help" && self.is_set(AS::NeedsSubcommandHelp) { try!(self.parse_help_subcommand(it)); } subcmd_name = Some(sc_name.to_owned()); @@ -892,7 +906,8 @@ impl<'a, 'b> Parser<'a, 'b> if arg_os.len_() == 2 { // The user has passed '--' which means only positional args follow no // matter what they start with - debugln!("Parser::get_matches_with: found '--' setting TrailingVals=true"); + debugln!("Parser::get_matches_with: found '--'"); + debugln!("Parser::get_matches_with: setting TrailingVals=true"); self.set(AS::TrailingValues); continue; } @@ -912,9 +927,9 @@ impl<'a, 'b> Parser<'a, 'b> if !(arg_os.to_string_lossy().parse::().is_ok() || arg_os.to_string_lossy().parse::().is_ok()) { return Err(Error::unknown_argument(&*arg_os.to_string_lossy(), - "", - &*self.create_current_usage(matcher, None), - self.color())); + "", + &*self.create_current_usage(matcher, None), + self.color())); } } else if !self.is_set(AS::AllowLeadingHyphen) { continue; @@ -925,9 +940,8 @@ impl<'a, 'b> Parser<'a, 'b> } } - if !(self.is_set(AS::ArgsNegateSubcommands) && - self.is_set(AS::ValidArgFound)) && - !self.is_set(AS::InferSubcommands) { + if !(self.is_set(AS::ArgsNegateSubcommands) && self.is_set(AS::ValidArgFound)) && + !self.is_set(AS::InferSubcommands) { if let Some(cdate) = suggestions::did_you_mean(&*arg_os.to_string_lossy(), sc_names!(self)) { return Err(Error::invalid_subcommand(arg_os.to_string_lossy() @@ -968,9 +982,7 @@ impl<'a, 'b> Parser<'a, 'b> self.possible_subcommand(&n).0 }; if self.is_new_arg(&n, needs_val_of) || sc_match || - suggestions::did_you_mean(&n.to_string_lossy(), - sc_names!(self)) - .is_some() { + suggestions::did_you_mean(&n.to_string_lossy(), sc_names!(self)).is_some() { debugln!("Parser::get_matches_with: Bumping the positional counter..."); pos_counter += 1; } @@ -1012,14 +1024,15 @@ impl<'a, 'b> Parser<'a, 'b> matches: sc_m.into(), }); } else if !(self.is_set(AS::AllowLeadingHyphen) || - self.is_set(AS::AllowNegativeNumbers)) && !self.is_set(AS::InferSubcommands) { + self.is_set(AS::AllowNegativeNumbers)) && + !self.is_set(AS::InferSubcommands) { return Err(Error::unknown_argument(&*arg_os.to_string_lossy(), "", &*self.create_current_usage(matcher, None), self.color())); } else if !(has_args) && self.has_subcommands() { if let Some(cdate) = suggestions::did_you_mean(&*arg_os.to_string_lossy(), - sc_names!(self)) { + sc_names!(self)) { return Err(Error::invalid_subcommand(arg_os.to_string_lossy() .into_owned(), cdate, @@ -1462,7 +1475,7 @@ impl<'a, 'b> Parser<'a, 'b> matcher: &mut ArgMatcher<'a>, full_arg: &OsStr) -> ClapResult> { - debugln!("Parser::parse_short_arg;"); + debugln!("Parser::parse_short_arg: full_arg={:?}", full_arg); let arg_os = full_arg.trim_left_matches(b'-'); let arg = arg_os.to_string_lossy(); @@ -1470,7 +1483,7 @@ impl<'a, 'b> Parser<'a, 'b> // `-v` `-a` `-l` assuming `v` `a` and `l` are all, or mostly, valid shorts. if self.is_set(AS::AllowLeadingHyphen) { if arg.chars().any(|c| !self.contains_short(c)) { - debugln!("Parser::parse_short_arg: Leading Hyphen Allowed and -{} isn't a valid short", + debugln!("Parser::parse_short_arg: LeadingHyphenAllowed yet -{} isn't valid", arg); return Ok(None); } @@ -1482,28 +1495,24 @@ impl<'a, 'b> Parser<'a, 'b> } for c in arg.chars() { - debugln!("Parser::parse_short_arg:iter: short={}", c); + debugln!("Parser::parse_short_arg:iter:{}", c); // Check for matching short options, and return the name if there is no trailing // concatenated value: -oval // Option: -o // Value: val if let Some(opt) = find_opt_by_short!(self, c) { - debugln!("Parser::parse_short_arg:iter: Found valid short opt -{} in '{}'", - c, - arg); + debugln!("Parser::parse_short_arg:iter:{}: Found valid opt", c); self.settings.set(AS::ValidArgFound); // Check for trailing concatenated value let p: Vec<_> = arg.splitn(2, c).collect(); - debugln!("Parser::parse_short_arg:iter: arg: {:?}, arg_os: {:?}, full_arg: {:?}", - arg, - arg_os, - full_arg); - debugln!("Parser::parse_short_arg:iter: p[0]: {:?}, p[1]: {:?}", + debugln!("Parser::parse_short_arg:iter:{}: p[0]={:?}, p[1]={:?}", + c, p[0].as_bytes(), p[1].as_bytes()); let i = p[0].as_bytes().len() + 1; let val = if p[1].as_bytes().len() > 0 { - debugln!("Parser::parse_short_arg:iter: setting val: {:?} (bytes), {:?} (ascii)", + debugln!("Parser::parse_short_arg:iter:{}: val={:?} (bytes), val={:?} (ascii)", + c, arg_os.split_at(i).1.as_bytes(), arg_os.split_at(i).1); Some(arg_os.split_at(i).1) @@ -1521,8 +1530,7 @@ impl<'a, 'b> Parser<'a, 'b> return Ok(ret); } else if let Some(flag) = find_flag_by_short!(self, c) { - debugln!("Parser::parse_short_arg:iter: Found valid short flag -{}", - c); + debugln!("Parser::parse_short_arg:iter:{}: Found valid flag", c); self.settings.set(AS::ValidArgFound); // Only flags can be help or version try!(self.check_for_help_and_version_char(c)); @@ -1738,7 +1746,8 @@ impl<'a, 'b> Parser<'a, 'b> let mut c_with = find_from!($me, $name, blacklist, &$matcher); c_with = c_with.or( $me.find_any_arg($name).map_or(None, |aa| aa.blacklist()) - .map_or(None, |bl| bl.iter().find(|arg| $matcher.contains(arg))) + .map_or(None, + |bl| bl.iter().find(|arg| $matcher.contains(arg))) .map_or(None, |an| $me.find_any_arg(an)) .map_or(None, |aa| Some(format!("{}", aa))) ); @@ -1872,11 +1881,12 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Parser::validate_arg_num_vals: max_vals set...{}", num); if (ma.vals.len() as u64) > num { debugln!("Parser::validate_arg_num_vals: Sending error TooManyValues"); - return Err(Error::too_many_values(ma.vals.iter() - .last() - .expect(INTERNAL_ERROR_MSG) - .to_str() - .expect(INVALID_UTF8), + return Err(Error::too_many_values(ma.vals + .iter() + .last() + .expect(INTERNAL_ERROR_MSG) + .to_str() + .expect(INVALID_UTF8), a, &*self.create_current_usage(matcher, None), self.color())); From 97e8db23b3e225d980100553d4616089837b2c91 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 4 Mar 2017 15:14:10 -0500 Subject: [PATCH 08/31] fix: now correctly shows subcommand as required in the usage string when AppSettings::SubcommandRequiredElseHelp is used Close #883 --- src/app/parser.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index 8e51d8bd..b0e50824 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -2139,7 +2139,8 @@ impl<'a, 'b> Parser<'a, 'b> if self.has_subcommands() && !self.is_set(AS::SubcommandRequired) { usage.push_str(" [SUBCOMMAND]"); - } else if self.is_set(AS::SubcommandRequired) && self.has_subcommands() { + } else if (self.is_set(AS::SubcommandRequired) || + self.is_set(AS::SubcommandRequiredElseHelp)) && self.has_subcommands() { usage.push_str(" "); } } else { From 539ad6073f6d4afb2e1ea5a8baf7366b3c66eb07 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 4 Mar 2017 15:29:07 -0500 Subject: [PATCH 09/31] fix: doesn't include the various [ARGS] [FLAGS] or [OPTIONS] if the only ones available are hidden Closes #882 --- src/app/parser.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index b0e50824..155fba74 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -474,7 +474,8 @@ impl<'a, 'b> Parser<'a, 'b> pub fn get_args_tag(&self) -> Option { debugln!("Parser::get_args_tag;"); let mut count = 0; - 'outer: for p in self.positionals.values().filter(|p| !p.is_set(ArgSettings::Required)) { + 'outer: for p in self.positionals.values().filter(|p| !p.is_set(ArgSettings::Required) && + !p.is_set(ArgSettings::Hidden)) { debugln!("Parser::get_args_tag:iter:{}:", p.b.name); if let Some(g_vec) = self.groups_for_arg(p.b.name) { for grp_s in &g_vec { @@ -486,20 +487,21 @@ impl<'a, 'b> Parser<'a, 'b> } } count += 1; - debugln!("Parser::get_args_tag:iter: {} Args not required", count); + debugln!("Parser::get_args_tag:iter: {} Args not required or hidden", count); } - if !self.is_set(AS::DontCollapseArgsInUsage) && (count > 1 || self.positionals.len() > 1) { + if !self.is_set(AS::DontCollapseArgsInUsage) && count > 1 { return None; // [ARGS] } else if count == 1 { let p = self.positionals .values() - .find(|p| !p.is_set(ArgSettings::Required)) + .find(|p| !p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Hidden)) .expect(INTERNAL_ERROR_MSG); return Some(format!(" [{}]{}", p.name_no_brackets(), p.multiple_str())); } else if self.is_set(AS::DontCollapseArgsInUsage) && !self.positionals.is_empty() { return Some(self.positionals .values() .filter(|p| !p.is_set(ArgSettings::Required)) + .filter(|p| !p.is_set(ArgSettings::Hidden)) .map(|p| format!(" [{}]{}", p.name_no_brackets(), p.multiple_str())) .collect::>() .join("")); @@ -527,6 +529,9 @@ impl<'a, 'b> Parser<'a, 'b> } } } + if f.is_set(ArgSettings::Hidden) { + continue; + } debugln!("Parser::needs_flags_tag:iter: [FLAGS] required"); return true; } @@ -2113,7 +2118,8 @@ impl<'a, 'b> Parser<'a, 'b> usage.push_str(" [OPTIONS]"); } if !self.is_set(AS::UnifiedHelpMessage) && self.has_opts() && - self.opts.iter().any(|o| !o.b.settings.is_set(ArgSettings::Required)) { + self.opts.iter().any(|o| !o.is_set(ArgSettings::Required) && + !o.is_set!(AS::Hidden)) { usage.push_str(" [OPTIONS]"); } @@ -2122,13 +2128,14 @@ impl<'a, 'b> Parser<'a, 'b> // places a '--' in the usage string if there are args and options // supporting multiple values if self.has_positionals() && - self.opts.iter().any(|o| o.b.settings.is_set(ArgSettings::Multiple)) && - self.positionals.values().any(|p| !p.b.settings.is_set(ArgSettings::Required)) && + self.opts.iter().any(|o| o.is_set(ArgSettings::Multiple)) && + self.positionals.values().any(|p| !p.is_set(ArgSettings::Required)) && !self.has_subcommands() { usage.push_str(" [--]") } if self.has_positionals() && - self.positionals.values().any(|p| !p.b.settings.is_set(ArgSettings::Required)) { + self.positionals.values().any(|p| !p.is_set(ArgSettings::Required) && + !p.is_set(ArgSettings::Hidden)) { if let Some(args_tag) = self.get_args_tag() { usage.push_str(&*args_tag); } else { From d63d404e5ee1857e5e2ec39a33be5014731d4fee Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 4 Mar 2017 15:35:18 -0500 Subject: [PATCH 10/31] fix: doesn't print the argument sections in the help message if all args in that section are hidden --- src/app/parser.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/app/parser.rs b/src/app/parser.rs index 155fba74..f43620a3 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -557,6 +557,30 @@ impl<'a, 'b> Parser<'a, 'b> #[inline] pub fn has_subcommands(&self) -> bool { !self.subcommands.is_empty() } + #[inline] + pub fn has_visible_opts(&self) -> bool { + if self.opts.is_empty() { return false; } + self.opts.iter().any(|o| !o.is_set(ArgSettings::Hidden)) + } + + #[inline] + pub fn has_visible_flags(&self) -> bool { + if self.flags.is_empty() { return false; } + self.flags.iter().any(|f| !f.is_set(ArgSettings::Hidden)) + } + + #[inline] + pub fn has_visible_positionals(&self) -> bool { + if self.positionals.is_empty() { return false; } + self.positionals.values().any(|p| !p.is_set(ArgSettings::Hidden)) + } + + #[inline] + pub fn has_visible_subcommands(&self) -> bool { + if self.subcommands.is_empty() { return false; } + self.subcommands.iter().any(|s| !s.is_set(AppSettings::Hidden)) + } + #[inline] pub fn is_set(&self, s: AS) -> bool { self.settings.is_set(s) } From c8ab24bafa81e64fed518c7deb95645290e304cb Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 4 Mar 2017 17:37:08 -0500 Subject: [PATCH 11/31] imp: when AppSettings::SubcommandsNegateReqs and ArgsNegateSubcommands are used, a new more accurate double line usage string is shown Closes #871 --- src/app/parser.rs | 31 ++++++++++++++++++++----------- tests/positionals.rs | 2 +- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index f43620a3..d13a7f59 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -578,7 +578,7 @@ impl<'a, 'b> Parser<'a, 'b> #[inline] pub fn has_visible_subcommands(&self) -> bool { if self.subcommands.is_empty() { return false; } - self.subcommands.iter().any(|s| !s.is_set(AppSettings::Hidden)) + self.subcommands.iter().any(|s| !s.p.is_set(AS::Hidden)) } #[inline] @@ -2120,7 +2120,7 @@ impl<'a, 'b> Parser<'a, 'b> if let Some(u) = self.meta.usage_str { usage.push_str(&*u); } else if used.is_empty() { - usage.push_str(&*self.meta + let name = self.meta .usage .as_ref() .unwrap_or_else(|| { @@ -2128,7 +2128,8 @@ impl<'a, 'b> Parser<'a, 'b> .bin_name .as_ref() .unwrap_or(&self.meta.name) - })); + }); + usage.push_str(&*name); let mut reqs: Vec<&str> = self.required().map(|r| &**r).collect(); reqs.dedup(); let req_string = self.get_required_from(&reqs, None, None) @@ -2141,9 +2142,9 @@ impl<'a, 'b> Parser<'a, 'b> } else if flags { usage.push_str(" [OPTIONS]"); } - if !self.is_set(AS::UnifiedHelpMessage) && self.has_opts() && + if !self.is_set(AS::UnifiedHelpMessage) && self.opts.iter().any(|o| !o.is_set(ArgSettings::Required) && - !o.is_set!(AS::Hidden)) { + !o.is_set(ArgSettings::Hidden)) { usage.push_str(" [OPTIONS]"); } @@ -2154,7 +2155,7 @@ impl<'a, 'b> Parser<'a, 'b> if self.has_positionals() && self.opts.iter().any(|o| o.is_set(ArgSettings::Multiple)) && self.positionals.values().any(|p| !p.is_set(ArgSettings::Required)) && - !self.has_subcommands() { + !self.has_visible_subcommands() { usage.push_str(" [--]") } if self.has_positionals() && @@ -2168,11 +2169,19 @@ impl<'a, 'b> Parser<'a, 'b> } - if self.has_subcommands() && !self.is_set(AS::SubcommandRequired) { - usage.push_str(" [SUBCOMMAND]"); - } else if (self.is_set(AS::SubcommandRequired) || - self.is_set(AS::SubcommandRequiredElseHelp)) && self.has_subcommands() { - usage.push_str(" "); + if self.is_set(AS::SubcommandsNegateReqs) || self.is_set(AS::ArgsNegateSubcommands) { + if self.has_visible_subcommands() { + usage.push_str("\n "); + usage.push_str(&*name); + usage.push_str(" "); + } + } else { + if self.has_visible_subcommands() && !self.is_set(AS::SubcommandRequired) { + usage.push_str(" [SUBCOMMAND]"); + } else if (self.is_set(AS::SubcommandRequired) || + self.is_set(AS::SubcommandRequiredElseHelp)) && self.has_subcommands() { + usage.push_str(" "); + } } } else { self.smart_usage(&mut usage, used); diff --git a/tests/positionals.rs b/tests/positionals.rs index e8a0cd7b..b356d299 100644 --- a/tests/positionals.rs +++ b/tests/positionals.rs @@ -180,7 +180,7 @@ fn multiple_positional_one_required_usage_string() { .arg_from_usage(" 'some file'") .arg_from_usage("[FILES]... 'some file'") .get_matches_from(vec!["test", "file"]); - assert_eq!(m.usage(), "USAGE:\n test [ARGS]"); + assert_eq!(m.usage(), "USAGE:\n test [FILES]..."); } #[test] From 642db9b042c29fdcb5e77b0529ad01e57cb54355 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 08:52:37 -0500 Subject: [PATCH 12/31] tests: adds tests for new dual usage strings with certain subcommand settings --- tests/help.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/help.rs b/tests/help.rs index 63266442..4e5a6489 100644 --- a/tests/help.rs +++ b/tests/help.rs @@ -36,6 +36,47 @@ SUBCOMMANDS: help Prints this message or the help of the given subcommand(s) subcmd tests subcommands"; +static SC_NEGATES_REQS: &'static str = "prog 1.0 + +USAGE: + prog --opt [PATH] + prog [PATH] + +FLAGS: + -h, --help Prints help information + -V, --version Prints version information + +OPTIONS: + -o, --opt tests options + +ARGS: + + +SUBCOMMANDS: + help Prints this message or the help of the given subcommand(s) + test"; + +static ARGS_NEGATE_SC: &'static str = "prog 1.0 + +USAGE: + prog [FLAGS] [OPTIONS] [PATH] + prog + +FLAGS: + -f, --flag testing flags + -h, --help Prints help information + -V, --version Prints version information + +OPTIONS: + -o, --opt tests options + +ARGS: + + +SUBCOMMANDS: + help Prints this message or the help of the given subcommand(s) + test"; + static AFTER_HELP: &'static str = "some text that comes before the help clap-test v1.4.8 @@ -50,6 +91,19 @@ FLAGS: some text that comes after the help"; +static HIDDEN_ARGS: &'static str = "prog 1.0 + +USAGE: + prog [FLAGS] [OPTIONS] + +FLAGS: + -f, --flag testing flags + -h, --help Prints help information + -V, --version Prints version information + +OPTIONS: + -o, --opt tests options"; + static SC_HELP: &'static str = "clap-test-subcmd 0.1 Kevin K. tests subcommands @@ -531,6 +585,40 @@ fn issue_760() { .takes_value(true)); assert!(test::compare_output(app, "ctest --help", ISSUE_760, false)); } + +#[test] +fn hidden_args() { + let app = App::new("prog") + .version("1.0") + .args_from_usage("-f, --flag 'testing flags' + -o, --opt [FILE] 'tests options'") + .arg(Arg::with_name("pos").hidden(true)); + assert!(test::compare_output(app, "prog --help", HIDDEN_ARGS, false)); +} + +#[test] +fn sc_negates_reqs() { + let app = App::new("prog") + .version("1.0") + .setting(AppSettings::SubcommandsNegateReqs) + .arg_from_usage("-o, --opt 'tests options'") + .arg(Arg::with_name("PATH")) + .subcommand(SubCommand::with_name("test")); + assert!(test::compare_output(app, "prog --help", SC_NEGATES_REQS, false)); +} + +#[test] +fn args_negate_sc() { + let app = App::new("prog") + .version("1.0") + .setting(AppSettings::ArgsNegateSubcommands) + .args_from_usage("-f, --flag 'testing flags' + -o, --opt [FILE] 'tests options'") + .arg(Arg::with_name("PATH")) + .subcommand(SubCommand::with_name("test")); + assert!(test::compare_output(app, "prog --help", ARGS_NEGATE_SC, false)); +} + #[test] fn issue_777_wrap_all_things() { let app = App::new("A app with a crazy very long long long name hahaha") From 44ed8b663c92d31ceabe5136e7fc33889f8e4eaa Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 15:51:38 -0500 Subject: [PATCH 13/31] refactor: moves validation code into it's own module --- src/app/mod.rs | 1 + src/app/parser.rs | 428 ++---------------------------------------- src/app/validator.rs | 430 +++++++++++++++++++++++++++++++++++++++++++ src/macros.rs | 9 +- 4 files changed, 446 insertions(+), 422 deletions(-) create mode 100644 src/app/validator.rs diff --git a/src/app/mod.rs b/src/app/mod.rs index 5c25dc89..96e393db 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -4,6 +4,7 @@ mod macros; pub mod parser; mod meta; mod help; +mod validator; // Std use std::env; diff --git a/src/app/parser.rs b/src/app/parser.rs index d13a7f59..982fc764 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -21,17 +21,17 @@ use app::App; use app::help::Help; use app::meta::AppMeta; use app::settings::AppFlags; -use args::{AnyArg, ArgMatcher, Base, Switched, Arg, ArgGroup, FlagBuilder, OptBuilder, PosBuilder, - MatchedArg}; +use args::{AnyArg, ArgMatcher, Base, Switched, Arg, ArgGroup, FlagBuilder, OptBuilder, PosBuilder}; use args::settings::ArgSettings; use completions::ComplGen; use errors::{Error, ErrorKind}; use errors::Result as ClapResult; -use fmt::{Colorizer, ColorWhen}; +use fmt::ColorWhen; use osstringext::OsStrExt2; use completions::Shell; use suggestions; use app::settings::AppSettings as AS; +use app::validator::Validator; #[allow(missing_debug_implementations)] #[doc(hidden)] @@ -46,12 +46,12 @@ pub struct Parser<'a, 'b> pub opts: Vec>, pub positionals: VecMap>, pub subcommands: Vec>, - groups: Vec>, + pub groups: Vec>, pub global_args: Vec>, - required: Vec<&'a str>, - r_ifs: Vec<(&'a str, &'b str, &'a str)>, - blacklist: Vec<&'b str>, - overrides: Vec<&'b str>, + pub required: Vec<&'a str>, + pub r_ifs: Vec<(&'a str, &'b str, &'a str)>, + pub blacklist: Vec<&'b str>, + pub overrides: Vec<&'b str>, help_short: Option, version_short: Option, cache: Option<&'a str>, @@ -1097,54 +1097,9 @@ impl<'a, 'b> Parser<'a, 'b> }); } - self.validate(needs_val_of, subcmd_name, matcher) + Validator::new(self).validate(needs_val_of, subcmd_name, matcher) } - fn validate(&mut self, - needs_val_of: Option<&'a str>, - subcmd_name: Option, - matcher: &mut ArgMatcher<'a>) - -> ClapResult<()> { - debugln!("Parser::validate;"); - let mut reqs_validated = false; - try!(self.add_defaults(matcher)); - if let Some(a) = needs_val_of { - debugln!("Parser::validate: needs_val_of={:?}", a); - if let Some(o) = find_by_name!(self, &a, opts, iter) { - try!(self.validate_required(matcher)); - reqs_validated = true; - let should_err = if let Some(v) = matcher.0.args.get(&*o.b.name) { - v.vals.is_empty() && !(o.v.min_vals.is_some() && o.v.min_vals.unwrap() == 0) - } else { - true - }; - if should_err { - return Err(Error::empty_value(o, - &*self.create_current_usage(matcher, None), - self.color())); - } - } - } - - try!(self.validate_blacklist(matcher)); - if !(self.is_set(AS::SubcommandsNegateReqs) && subcmd_name.is_some()) && !reqs_validated { - try!(self.validate_required(matcher)); - } - try!(self.validate_matched_args(matcher)); - matcher.usage(self.create_usage(&[])); - - if matcher.is_empty() && matcher.subcommand_name().is_none() && - self.is_set(AS::ArgRequiredElseHelp) { - let mut out = vec![]; - try!(self.write_help_err(&mut out)); - return Err(Error { - message: String::from_utf8_lossy(&*out).into_owned(), - kind: ErrorKind::MissingArgumentOrSubcommand, - info: None, - }); - } - Ok(()) - } fn propogate_help_version(&mut self) { debugln!("Parser::propogate_help_version;"); @@ -1294,7 +1249,7 @@ impl<'a, 'b> Parser<'a, 'b> args.iter().map(ToOwned::to_owned).collect() } - fn arg_names_in_group(&self, group: &str) -> Vec<&'a str> { + pub fn arg_names_in_group(&self, group: &str) -> Vec<&'a str> { let mut g_vec = vec![]; let mut args = vec![]; @@ -1697,62 +1652,6 @@ impl<'a, 'b> Parser<'a, 'b> Ok(None) } - fn validate_values(&self, - arg: &A, - ma: &MatchedArg, - matcher: &ArgMatcher<'a>) - -> ClapResult<()> - where A: AnyArg<'a, 'b> + Display - { - debugln!("Parser::validate_values: arg={:?}", arg.name()); - for val in &ma.vals { - if self.is_set(AS::StrictUtf8) && val.to_str().is_none() { - debugln!("Parser::validate_values: invalid UTF-8 found in val {:?}", - val); - return Err(Error::invalid_utf8(&*self.create_current_usage(matcher, None), - self.color())); - } - if let Some(p_vals) = arg.possible_vals() { - debugln!("Parser::validate_values: possible_vals={:?}", p_vals); - let val_str = val.to_string_lossy(); - if !p_vals.contains(&&*val_str) { - return Err(Error::invalid_value(val_str, - p_vals, - arg, - &*self.create_current_usage(matcher, None), - self.color())); - } - } - if !arg.is_set(ArgSettings::EmptyValues) && val.is_empty_() && - matcher.contains(&*arg.name()) { - debugln!("Parser::validate_values: illegal empty val found"); - return Err(Error::empty_value(arg, - &*self.create_current_usage(matcher, None), - self.color())); - } - if let Some(vtor) = arg.validator() { - debug!("Parser::validate_values: checking validator..."); - if let Err(e) = vtor(val.to_string_lossy().into_owned()) { - sdebugln!("error"); - return Err(Error::value_validation(Some(arg), e, self.color())); - } else { - sdebugln!("good"); - } - } - if let Some(vtor) = arg.validator_os() { - debug!("Parser::validate_values: checking validator_os..."); - if let Err(e) = vtor(&val) { - sdebugln!("error"); - return Err(Error::value_validation(Some(arg), - (*e).to_string_lossy().to_string(), - self.color())); - } else { - sdebugln!("good"); - } - } - } - Ok(()) - } fn parse_flag(&self, flag: &FlagBuilder<'a, 'b>, @@ -1767,311 +1666,6 @@ impl<'a, 'b> Parser<'a, 'b> Ok(()) } - fn validate_blacklist(&self, matcher: &mut ArgMatcher) -> ClapResult<()> { - debugln!("Parser::validate_blacklist: blacklist={:?}", self.blacklist); - macro_rules! build_err { - ($me:ident, $name:expr, $matcher:ident) => ({ - debugln!("build_err!: name={}", $name); - let mut c_with = find_from!($me, $name, blacklist, &$matcher); - c_with = c_with.or( - $me.find_any_arg($name).map_or(None, |aa| aa.blacklist()) - .map_or(None, - |bl| bl.iter().find(|arg| $matcher.contains(arg))) - .map_or(None, |an| $me.find_any_arg(an)) - .map_or(None, |aa| Some(format!("{}", aa))) - ); - debugln!("build_err!: '{:?}' conflicts with '{}'", c_with, $name); - $matcher.remove($name); - let usg = $me.create_current_usage($matcher, None); - if let Some(f) = find_by_name!($me, $name, flags, iter) { - debugln!("build_err!: It was a flag..."); - Error::argument_conflict(f, c_with, &*usg, self.color()) - } else if let Some(o) = find_by_name!($me, $name, opts, iter) { - debugln!("build_err!: It was an option..."); - Error::argument_conflict(o, c_with, &*usg, self.color()) - } else { - match find_by_name!($me, $name, positionals, values) { - Some(p) => { - debugln!("build_err!: It was a positional..."); - Error::argument_conflict(p, c_with, &*usg, self.color()) - }, - None => panic!(INTERNAL_ERROR_MSG) - } - } - }); - } - - for name in &self.blacklist { - debugln!("Parser::validate_blacklist:iter: Checking blacklisted name: {}", - name); - if self.groups.iter().any(|g| &g.name == name) { - debugln!("Parser::validate_blacklist:iter: groups contains it..."); - for n in self.arg_names_in_group(name) { - debugln!("Parser::validate_blacklist:iter:iter: Checking arg '{}' in group...", - n); - if matcher.contains(n) { - debugln!("Parser::validate_blacklist:iter:iter: matcher contains it..."); - return Err(build_err!(self, &n, matcher)); - } - } - } else if matcher.contains(name) { - debugln!("Parser::validate_blacklist:iter: matcher contains it..."); - return Err(build_err!(self, name, matcher)); - } - } - Ok(()) - } - - fn validate_matched_args(&self, matcher: &mut ArgMatcher) -> ClapResult<()> { - debugln!("Parser::validate_matched_args;"); - for (name, ma) in matcher.iter() { - debugln!("Parser::validate_matched_args:iter:{}: vals={:#?}", - name, - ma.vals); - if let Some(opt) = find_by_name!(self, name, opts, iter) { - try!(self.validate_arg_num_vals(opt, ma, matcher)); - try!(self.validate_values(opt, ma, matcher)); - try!(self.validate_arg_requires(opt, ma, matcher)); - try!(self.validate_arg_num_occurs(opt, ma, matcher)); - } else if let Some(flag) = find_by_name!(self, name, flags, iter) { - try!(self.validate_arg_requires(flag, ma, matcher)); - try!(self.validate_arg_num_occurs(flag, ma, matcher)); - } else if let Some(pos) = find_by_name!(self, name, positionals, values) { - try!(self.validate_arg_num_vals(pos, ma, matcher)); - try!(self.validate_arg_num_occurs(pos, ma, matcher)); - try!(self.validate_values(pos, ma, matcher)); - try!(self.validate_arg_requires(pos, ma, matcher)); - } else { - let grp = self.groups.iter().find(|g| &g.name == name).expect(INTERNAL_ERROR_MSG); - if let Some(ref g_reqs) = grp.requires { - if g_reqs.iter().any(|&n| !matcher.contains(n)) { - return self.missing_required_error(matcher, None); - } - } - } - } - Ok(()) - } - - fn validate_arg_num_occurs(&self, - a: &A, - ma: &MatchedArg, - matcher: &ArgMatcher) - -> ClapResult<()> - where A: AnyArg<'a, 'b> + Display - { - debugln!("Parser::validate_arg_num_occurs: a={};", a.name()); - if ma.occurs > 1 && !a.is_set(ArgSettings::Multiple) { - // Not the first time, and we don't allow multiples - return Err(Error::unexpected_multiple_usage(a, - &*self.create_current_usage(matcher, None), - self.color())); - } - Ok(()) - } - - fn validate_arg_num_vals(&self, - a: &A, - ma: &MatchedArg, - matcher: &ArgMatcher) - -> ClapResult<()> - where A: AnyArg<'a, 'b> + Display - { - debugln!("Parser::validate_arg_num_vals;"); - if let Some(num) = a.num_vals() { - debugln!("Parser::validate_arg_num_vals: num_vals set...{}", num); - let should_err = if a.is_set(ArgSettings::Multiple) { - ((ma.vals.len() as u64) % num) != 0 - } else { - num != (ma.vals.len() as u64) - }; - if should_err { - debugln!("Parser::validate_arg_num_vals: Sending error WrongNumberOfValues"); - return Err(Error::wrong_number_of_values(a, - num, - if a.is_set(ArgSettings::Multiple) { - (ma.vals.len() % num as usize) - } else { - ma.vals.len() - }, - if ma.vals.len() == 1 || - (a.is_set(ArgSettings::Multiple) && - (ma.vals.len() % num as usize) == - 1) { - "as" - } else { - "ere" - }, - &*self.create_current_usage(matcher, None), - self.color())); - } - } - if let Some(num) = a.max_vals() { - debugln!("Parser::validate_arg_num_vals: max_vals set...{}", num); - if (ma.vals.len() as u64) > num { - debugln!("Parser::validate_arg_num_vals: Sending error TooManyValues"); - return Err(Error::too_many_values(ma.vals - .iter() - .last() - .expect(INTERNAL_ERROR_MSG) - .to_str() - .expect(INVALID_UTF8), - a, - &*self.create_current_usage(matcher, None), - self.color())); - } - } - if let Some(num) = a.min_vals() { - debugln!("Parser::validate_arg_num_vals: min_vals set: {}", num); - if (ma.vals.len() as u64) < num { - debugln!("Parser::validate_arg_num_vals: Sending error TooFewValues"); - return Err(Error::too_few_values(a, - num, - ma.vals.len(), - &*self.create_current_usage(matcher, None), - self.color())); - } - } - // Issue 665 (https://github.com/kbknapp/clap-rs/issues/665) - if a.takes_value() && !a.is_set(ArgSettings::EmptyValues) && ma.vals.is_empty() { - return Err(Error::empty_value(a, - &*self.create_current_usage(matcher, None), - self.color())); - } - Ok(()) - } - - fn validate_arg_requires(&self, - a: &A, - ma: &MatchedArg, - matcher: &ArgMatcher) - -> ClapResult<()> - where A: AnyArg<'a, 'b> + Display - { - debugln!("Parser::validate_arg_requires;"); - if let Some(a_reqs) = a.requires() { - for &(val, name) in a_reqs.iter().filter(|&&(val, _)| val.is_some()) { - if ma.vals - .iter() - .any(|v| v == val.expect(INTERNAL_ERROR_MSG) && !matcher.contains(name)) { - return self.missing_required_error(matcher, None); - } - } - } - Ok(()) - } - - #[inline] - fn missing_required_error(&self, matcher: &ArgMatcher, extra: Option<&str>) -> ClapResult<()> { - debugln!("Parser::missing_required_error: extra={:?}", extra); - let c = Colorizer { - use_stderr: true, - when: self.color(), - }; - let mut reqs = self.required.iter().map(|&r| &*r).collect::>(); - if let Some(r) = extra { - reqs.push(r); - } - reqs.retain(|n| !matcher.contains(n)); - reqs.dedup(); - debugln!("Parser::missing_required_error: reqs={:#?}", reqs); - Err(Error::missing_required_argument(&*self.get_required_from(&reqs[..], - Some(matcher), - extra) - .iter() - .fold(String::new(), |acc, s| { - acc + &format!("\n {}", c.error(s))[..] - }), - &*self.create_current_usage(matcher, extra), - self.color())) - } - - fn validate_required(&self, matcher: &ArgMatcher) -> ClapResult<()> { - debugln!("Parser::validate_required: required={:?};", self.required); - 'outer: for name in &self.required { - debugln!("Parser::validate_required:iter:{}:", name); - if matcher.contains(name) { - continue 'outer; - } - if let Some(a) = find_by_name!(self, name, flags, iter) { - if self.is_missing_required_ok(a, matcher) { - continue 'outer; - } - } else if let Some(a) = find_by_name!(self, name, opts, iter) { - if self.is_missing_required_ok(a, matcher) { - continue 'outer; - } - } else if let Some(a) = find_by_name!(self, name, positionals, values) { - if self.is_missing_required_ok(a, matcher) { - continue 'outer; - } - } - return self.missing_required_error(matcher, None); - } - - // Validate the conditionally required args - for &(a, v, r) in &self.r_ifs { - if let Some(ma) = matcher.get(a) { - if matcher.get(r).is_none() { - if ma.vals.iter().any(|val| val == v) { - return self.missing_required_error(matcher, Some(r)); - } - } - } - } - Ok(()) - } - - fn check_conflicts(&self, a: &A, matcher: &ArgMatcher) -> Option - where A: AnyArg<'a, 'b> - { - debugln!("Parser::check_conflicts: a={:?};", a.name()); - a.blacklist().map(|bl| { - bl.iter().any(|conf| { - matcher.contains(conf) || - self.groups - .iter() - .find(|g| &g.name == conf) - .map_or(false, |g| g.args.iter().any(|arg| matcher.contains(arg))) - }) - }) - } - - fn check_required_unless(&self, a: &A, matcher: &ArgMatcher) -> Option - where A: AnyArg<'a, 'b> - { - debugln!("Parser::check_required_unless: a={:?};", a.name()); - macro_rules! check { - ($how:ident, $_self:ident, $a:ident, $m:ident) => {{ - $a.required_unless().map(|ru| { - ru.iter().$how(|n| { - $m.contains(n) || { - if let Some(grp) = $_self.groups.iter().find(|g| &g.name == n) { - grp.args.iter().any(|arg| $m.contains(arg)) - } else { - false - } - } - }) - }) - }}; - } - if a.is_set(ArgSettings::RequiredUnlessAll) { - check!(all, self, a, matcher) - } else { - check!(any, self, a, matcher) - } - } - - #[inline] - fn is_missing_required_ok(&self, a: &A, matcher: &ArgMatcher) -> bool - where A: AnyArg<'a, 'b> - { - debugln!("Parser::is_missing_required_ok: a={}", a.name()); - self.check_conflicts(a, matcher).unwrap_or(false) || - self.check_required_unless(a, matcher).unwrap_or(false) - } - fn did_you_mean_error(&self, arg: &str, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> { // Didn't match a flag or option...maybe it was a typo and close to one let suffix = @@ -2261,7 +1855,7 @@ impl<'a, 'b> Parser<'a, 'b> Help::write_parser_help_to_stderr(w, self) } - fn add_defaults(&mut self, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> { + pub fn add_defaults(&mut self, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> { macro_rules! add_val { (@default $_self:ident, $a:ident, $m:ident) => { if let Some(ref val) = $a.v.default_val { diff --git a/src/app/validator.rs b/src/app/validator.rs new file mode 100644 index 00000000..afce3089 --- /dev/null +++ b/src/app/validator.rs @@ -0,0 +1,430 @@ +// std +use std::fmt::Display; + +// Internal +use INTERNAL_ERROR_MSG; +use INVALID_UTF8; +use args::{AnyArg, ArgMatcher, MatchedArg}; +use args::settings::ArgSettings; +use errors::{Error, ErrorKind}; +use errors::Result as ClapResult; +use osstringext::OsStrExt2; +use app::settings::AppSettings as AS; +use app::parser::Parser; +use fmt::Colorizer; + +pub struct Validator<'a, 'b, 'z>(&'z mut Parser<'a, 'b>) where 'a: 'b, 'b: 'z; + +impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { + pub fn new(p: &'z mut Parser<'a, 'b>) -> Self { + Validator(p) + } + + pub fn validate(&mut self, + needs_val_of: Option<&'a str>, + subcmd_name: Option, + matcher: &mut ArgMatcher<'a>) + -> ClapResult<()> { + debugln!("Validator::validate;"); + let mut reqs_validated = false; + try!(self.0.add_defaults(matcher)); + if let Some(a) = needs_val_of { + debugln!("Validator::validate: needs_val_of={:?}", a); + if let Some(o) = find_by_name!(self.0, &a, opts, iter) { + try!(self.validate_required(matcher)); + reqs_validated = true; + let should_err = if let Some(v) = matcher.0.args.get(&*o.b.name) { + v.vals.is_empty() && !(o.v.min_vals.is_some() && o.v.min_vals.unwrap() == 0) + } else { + true + }; + if should_err { + return Err(Error::empty_value(o, + &*self.0.create_current_usage(matcher, None), + self.0.color())); + } + } + } + + try!(self.validate_blacklist(matcher)); + if !(self.0.is_set(AS::SubcommandsNegateReqs) && subcmd_name.is_some()) && !reqs_validated { + try!(self.validate_required(matcher)); + } + try!(self.validate_matched_args(matcher)); + matcher.usage(self.0.create_usage(&[])); + + if matcher.is_empty() && matcher.subcommand_name().is_none() && + self.0.is_set(AS::ArgRequiredElseHelp) { + let mut out = vec![]; + try!(self.0.write_help_err(&mut out)); + return Err(Error { + message: String::from_utf8_lossy(&*out).into_owned(), + kind: ErrorKind::MissingArgumentOrSubcommand, + info: None, + }); + } + Ok(()) + } + + fn validate_values(&self, + arg: &A, + ma: &MatchedArg, + matcher: &ArgMatcher<'a>) + -> ClapResult<()> + where A: AnyArg<'a, 'b> + Display + { + debugln!("Validator::validate_values: arg={:?}", arg.name()); + for val in &ma.vals { + if self.0.is_set(AS::StrictUtf8) && val.to_str().is_none() { + debugln!("Validator::validate_values: invalid UTF-8 found in val {:?}", + val); + return Err(Error::invalid_utf8(&*self.0.create_current_usage(matcher, None), + self.0.color())); + } + if let Some(p_vals) = arg.possible_vals() { + debugln!("Validator::validate_values: possible_vals={:?}", p_vals); + let val_str = val.to_string_lossy(); + if !p_vals.contains(&&*val_str) { + return Err(Error::invalid_value(val_str, + p_vals, + arg, + &*self.0.create_current_usage(matcher, None), + self.0.color())); + } + } + if !arg.is_set(ArgSettings::EmptyValues) && val.is_empty_() && + matcher.contains(&*arg.name()) { + debugln!("Validator::validate_values: illegal empty val found"); + return Err(Error::empty_value(arg, + &*self.0.create_current_usage(matcher, None), + self.0.color())); + } + if let Some(vtor) = arg.validator() { + debug!("Validator::validate_values: checking validator..."); + if let Err(e) = vtor(val.to_string_lossy().into_owned()) { + sdebugln!("error"); + return Err(Error::value_validation(Some(arg), e, self.0.color())); + } else { + sdebugln!("good"); + } + } + if let Some(vtor) = arg.validator_os() { + debug!("Validator::validate_values: checking validator_os..."); + if let Err(e) = vtor(&val) { + sdebugln!("error"); + return Err(Error::value_validation(Some(arg), + (*e).to_string_lossy().to_string(), + self.0.color())); + } else { + sdebugln!("good"); + } + } + } + Ok(()) + } + + fn validate_blacklist(&self, matcher: &mut ArgMatcher) -> ClapResult<()> { + debugln!("Validator::validate_blacklist: blacklist={:?}", self.0.blacklist); + macro_rules! build_err { + ($p:expr, $name:expr, $matcher:ident) => ({ + debugln!("build_err!: name={}", $name); + let mut c_with = find_from!($p, $name, blacklist, &$matcher); + c_with = c_with.or( + $p.find_any_arg($name).map_or(None, |aa| aa.blacklist()) + .map_or(None, + |bl| bl.iter().find(|arg| $matcher.contains(arg))) + .map_or(None, |an| $p.find_any_arg(an)) + .map_or(None, |aa| Some(format!("{}", aa))) + ); + debugln!("build_err!: '{:?}' conflicts with '{}'", c_with, $name); + $matcher.remove($name); + let usg = $p.create_current_usage($matcher, None); + if let Some(f) = find_by_name!($p, $name, flags, iter) { + debugln!("build_err!: It was a flag..."); + Error::argument_conflict(f, c_with, &*usg, self.0.color()) + } else if let Some(o) = find_by_name!($p, $name, opts, iter) { + debugln!("build_err!: It was an option..."); + Error::argument_conflict(o, c_with, &*usg, self.0.color()) + } else { + match find_by_name!($p, $name, positionals, values) { + Some(p) => { + debugln!("build_err!: It was a positional..."); + Error::argument_conflict(p, c_with, &*usg, self.0.color()) + }, + None => panic!(INTERNAL_ERROR_MSG) + } + } + }); + } + + for name in &self.0.blacklist { + debugln!("Validator::validate_blacklist:iter: Checking blacklisted name: {}", + name); + if self.0.groups.iter().any(|g| &g.name == name) { + debugln!("Validator::validate_blacklist:iter: groups contains it..."); + for n in self.0.arg_names_in_group(name) { + debugln!("Validator::validate_blacklist:iter:iter: Checking arg '{}' in group...", + n); + if matcher.contains(n) { + debugln!("Validator::validate_blacklist:iter:iter: matcher contains it..."); + return Err(build_err!(self.0, &n, matcher)); + } + } + } else if matcher.contains(name) { + debugln!("Validator::validate_blacklist:iter: matcher contains it..."); + return Err(build_err!(self.0, name, matcher)); + } + } + Ok(()) + } + + fn validate_matched_args(&self, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> { + debugln!("Validator::validate_matched_args;"); + for (name, ma) in matcher.iter() { + debugln!("Validator::validate_matched_args:iter:{}: vals={:#?}", + name, + ma.vals); + if let Some(opt) = find_by_name!(self.0, name, opts, iter) { + try!(self.validate_arg_num_vals(opt, ma, matcher)); + try!(self.validate_values(opt, ma, matcher)); + try!(self.validate_arg_requires(opt, ma, matcher)); + try!(self.validate_arg_num_occurs(opt, ma, matcher)); + } else if let Some(flag) = find_by_name!(self.0, name, flags, iter) { + try!(self.validate_arg_requires(flag, ma, matcher)); + try!(self.validate_arg_num_occurs(flag, ma, matcher)); + } else if let Some(pos) = find_by_name!(self.0, name, positionals, values) { + try!(self.validate_arg_num_vals(pos, ma, matcher)); + try!(self.validate_arg_num_occurs(pos, ma, matcher)); + try!(self.validate_values(pos, ma, matcher)); + try!(self.validate_arg_requires(pos, ma, matcher)); + } else { + let grp = self.0.groups.iter().find(|g| &g.name == name).expect(INTERNAL_ERROR_MSG); + if let Some(ref g_reqs) = grp.requires { + if g_reqs.iter().any(|&n| !matcher.contains(n)) { + return self.missing_required_error(matcher, None); + } + } + } + } + Ok(()) + } + + fn validate_arg_num_occurs(&self, + a: &A, + ma: &MatchedArg, + matcher: &ArgMatcher) + -> ClapResult<()> + where A: AnyArg<'a, 'b> + Display + { + debugln!("Validator::validate_arg_num_occurs: a={};", a.name()); + if ma.occurs > 1 && !a.is_set(ArgSettings::Multiple) { + // Not the first time, and we don't allow multiples + return Err(Error::unexpected_multiple_usage(a, + &*self.0.create_current_usage(matcher, None), + self.0.color())); + } + Ok(()) + } + + fn validate_arg_num_vals(&self, + a: &A, + ma: &MatchedArg, + matcher: &ArgMatcher) + -> ClapResult<()> + where A: AnyArg<'a, 'b> + Display + { + debugln!("Validator::validate_arg_num_vals;"); + if let Some(num) = a.num_vals() { + debugln!("Validator::validate_arg_num_vals: num_vals set...{}", num); + let should_err = if a.is_set(ArgSettings::Multiple) { + ((ma.vals.len() as u64) % num) != 0 + } else { + num != (ma.vals.len() as u64) + }; + if should_err { + debugln!("Validator::validate_arg_num_vals: Sending error WrongNumberOfValues"); + return Err(Error::wrong_number_of_values(a, + num, + if a.is_set(ArgSettings::Multiple) { + (ma.vals.len() % num as usize) + } else { + ma.vals.len() + }, + if ma.vals.len() == 1 || + (a.is_set(ArgSettings::Multiple) && + (ma.vals.len() % num as usize) == + 1) { + "as" + } else { + "ere" + }, + &*self.0.create_current_usage(matcher, None), + self.0.color())); + } + } + if let Some(num) = a.max_vals() { + debugln!("Validator::validate_arg_num_vals: max_vals set...{}", num); + if (ma.vals.len() as u64) > num { + debugln!("Validator::validate_arg_num_vals: Sending error TooManyValues"); + return Err(Error::too_many_values(ma.vals + .iter() + .last() + .expect(INTERNAL_ERROR_MSG) + .to_str() + .expect(INVALID_UTF8), + a, + &*self.0.create_current_usage(matcher, None), + self.0.color())); + } + } + if let Some(num) = a.min_vals() { + debugln!("Validator::validate_arg_num_vals: min_vals set: {}", num); + if (ma.vals.len() as u64) < num { + debugln!("Validator::validate_arg_num_vals: Sending error TooFewValues"); + return Err(Error::too_few_values(a, + num, + ma.vals.len(), + &*self.0.create_current_usage(matcher, None), + self.0.color())); + } + } + // Issue 665 (https://github.com/kbknapp/clap-rs/issues/665) + if a.takes_value() && !a.is_set(ArgSettings::EmptyValues) && ma.vals.is_empty() { + return Err(Error::empty_value(a, + &*self.0.create_current_usage(matcher, None), + self.0.color())); + } + Ok(()) + } + + fn validate_arg_requires(&self, + a: &A, + ma: &MatchedArg, + matcher: &ArgMatcher) + -> ClapResult<()> + where A: AnyArg<'a, 'b> + Display + { + debugln!("Validator::validate_arg_requires;"); + if let Some(a_reqs) = a.requires() { + for &(val, name) in a_reqs.iter().filter(|&&(val, _)| val.is_some()) { + if ma.vals + .iter() + .any(|v| v == val.expect(INTERNAL_ERROR_MSG) && !matcher.contains(name)) { + return self.missing_required_error(matcher, None); + } + } + } + Ok(()) + } + + fn validate_required(&self, matcher: &ArgMatcher) -> ClapResult<()> { + debugln!("Validator::validate_required: required={:?};", self.0.required); + 'outer: for name in &self.0.required { + debugln!("Validator::validate_required:iter:{}:", name); + if matcher.contains(name) { + continue 'outer; + } + if let Some(a) = find_by_name!(self.0, name, flags, iter) { + if self.is_missing_required_ok(a, matcher) { + continue 'outer; + } + } else if let Some(a) = find_by_name!(self.0, name, opts, iter) { + if self.is_missing_required_ok(a, matcher) { + continue 'outer; + } + } else if let Some(a) = find_by_name!(self.0, name, positionals, values) { + if self.is_missing_required_ok(a, matcher) { + continue 'outer; + } + } + return self.missing_required_error(matcher, None); + } + + // Validate the conditionally required args + for &(a, v, r) in &self.0.r_ifs { + if let Some(ma) = matcher.get(a) { + if matcher.get(r).is_none() { + if ma.vals.iter().any(|val| val == v) { + return self.missing_required_error(matcher, Some(r)); + } + } + } + } + Ok(()) + } + + fn validate_conflicts(&self, a: &A, matcher: &ArgMatcher) -> Option + where A: AnyArg<'a, 'b> + { + debugln!("Validator::validate_conflicts: a={:?};", a.name()); + a.blacklist().map(|bl| { + bl.iter().any(|conf| { + matcher.contains(conf) || + self.0.groups + .iter() + .find(|g| &g.name == conf) + .map_or(false, |g| g.args.iter().any(|arg| matcher.contains(arg))) + }) + }) + } + + fn validate_required_unless(&self, a: &A, matcher: &ArgMatcher) -> Option + where A: AnyArg<'a, 'b> + { + debugln!("Validator::validate_required_unless: a={:?};", a.name()); + macro_rules! check { + ($how:ident, $_self:expr, $a:ident, $m:ident) => {{ + $a.required_unless().map(|ru| { + ru.iter().$how(|n| { + $m.contains(n) || { + if let Some(grp) = $_self.groups.iter().find(|g| &g.name == n) { + grp.args.iter().any(|arg| $m.contains(arg)) + } else { + false + } + } + }) + }) + }}; + } + if a.is_set(ArgSettings::RequiredUnlessAll) { + check!(all, self.0, a, matcher) + } else { + check!(any, self.0, a, matcher) + } + } + + #[inline] + fn missing_required_error(&self, matcher: &ArgMatcher, extra: Option<&str>) -> ClapResult<()> { + debugln!("Validator::missing_required_error: extra={:?}", extra); + let c = Colorizer { + use_stderr: true, + when: self.0.color(), + }; + let mut reqs = self.0.required.iter().map(|&r| &*r).collect::>(); + if let Some(r) = extra { + reqs.push(r); + } + reqs.retain(|n| !matcher.contains(n)); + reqs.dedup(); + debugln!("Validator::missing_required_error: reqs={:#?}", reqs); + Err(Error::missing_required_argument(&*self.0.get_required_from(&reqs[..], + Some(matcher), + extra) + .iter() + .fold(String::new(), |acc, s| { + acc + &format!("\n {}", c.error(s))[..] + }), + &*self.0.create_current_usage(matcher, extra), + self.0.color())) + } + + #[inline] + fn is_missing_required_ok(&self, a: &A, matcher: &ArgMatcher) -> bool + where A: AnyArg<'a, 'b> + { + debugln!("Validator::is_missing_required_ok: a={}", a.name()); + self.validate_conflicts(a, matcher).unwrap_or(false) || + self.validate_required_unless(a, matcher).unwrap_or(false) + } +} \ No newline at end of file diff --git a/src/macros.rs b/src/macros.rs index e5483741..6fc7763a 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -847,7 +847,7 @@ macro_rules! vec_remove_all { }; } macro_rules! find_from { - ($_self:ident, $arg_name:expr, $from:ident, $matcher:expr) => {{ + ($_self:expr, $arg_name:expr, $from:ident, $matcher:expr) => {{ let mut ret = None; for k in $matcher.arg_names() { if let Some(f) = find_by_name!($_self, &k, flags, iter) { @@ -877,7 +877,7 @@ macro_rules! find_from { } macro_rules! find_name_from { - ($_self:ident, $arg_name:expr, $from:ident, $matcher:expr) => {{ + ($_self:expr, $arg_name:expr, $from:ident, $matcher:expr) => {{ let mut ret = None; for k in $matcher.arg_names() { if let Some(f) = find_by_name!($_self, &k, flags, iter) { @@ -908,8 +908,8 @@ macro_rules! find_name_from { // Finds an arg by name macro_rules! find_by_name { - ($_self:ident, $name:expr, $what:ident, $how:ident) => { - $_self.$what.$how().find(|o| &o.b.name == $name) + ($p:expr, $name:expr, $what:ident, $how:ident) => { + $p.$what.$how().find(|o| &o.b.name == $name) } } @@ -1057,7 +1057,6 @@ macro_rules! _names { .iter() .filter(|s| s.p.meta.aliases.is_some()) .flat_map(|s| s.p.meta.aliases.as_ref().unwrap().iter().map(|&(n, _)| n))) - // .map(|n| &n) }} } From 622b609c57ac9610f708ce834c170f33a4f30535 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 16:09:08 -0500 Subject: [PATCH 14/31] refactor: moves usage string generation code into it's own module --- src/app/help.rs | 5 +- src/app/mod.rs | 1 + src/app/parser.rs | 483 +++++++++++++++++++------------------------ src/app/usage.rs | 153 ++++++++++++++ src/app/validator.rs | 107 ++++++---- 5 files changed, 435 insertions(+), 314 deletions(-) create mode 100644 src/app/usage.rs diff --git a/src/app/help.rs b/src/app/help.rs index a47b8053..a73c726f 100644 --- a/src/app/help.rs +++ b/src/app/help.rs @@ -11,6 +11,7 @@ use app::parser::Parser; use args::{AnyArg, ArgSettings, DispOrder}; use errors::{Error, Result as ClapResult}; use fmt::{Format, Colorizer}; +use app::usage; // Third Party use unicode_width::UnicodeWidthStr; @@ -682,7 +683,7 @@ impl<'a> Help<'a> { try!(write!(self.writer, "\n{}{}\n\n", TAB, - parser.create_usage_no_title(&[]))); + usage::create_help_usage(parser))); let flags = parser.has_flags(); let pos = parser.has_positionals(); @@ -879,7 +880,7 @@ impl<'a> Help<'a> { parser.meta.about.unwrap_or("unknown about"))); } b"usage" => { - try!(write!(self.writer, "{}", parser.create_usage_no_title(&[]))); + try!(write!(self.writer, "{}", usage::create_help_usage(parser))); } b"all-args" => { try!(self.write_all_args(&parser)); diff --git a/src/app/mod.rs b/src/app/mod.rs index 96e393db..c24b4d4d 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -5,6 +5,7 @@ pub mod parser; mod meta; mod help; mod validator; +mod usage; // Std use std::env; diff --git a/src/app/parser.rs b/src/app/parser.rs index 982fc764..6d574c6d 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -32,6 +32,7 @@ use completions::Shell; use suggestions; use app::settings::AppSettings as AS; use app::validator::Validator; +use app::usage; #[allow(missing_debug_implementations)] #[doc(hidden)] @@ -98,7 +99,11 @@ impl<'a, 'b> Parser<'a, 'b> use std::error::Error; let out_dir = PathBuf::from(od); - let name = &*self.meta.bin_name.as_ref().unwrap().clone(); + let name = &*self.meta + .bin_name + .as_ref() + .unwrap() + .clone(); let file_name = match for_shell { Shell::Bash => format!("{}.bash-completion", name), Shell::Fish => format!("{}.fish", name), @@ -249,8 +254,10 @@ impl<'a, 'b> Parser<'a, 'b> } } if self.groups.iter().any(|g| g.name == group.name) { - let grp = - self.groups.iter_mut().find(|g| g.name == group.name).expect(INTERNAL_ERROR_MSG); + let grp = self.groups + .iter_mut() + .find(|g| g.name == group.name) + .expect(INTERNAL_ERROR_MSG); grp.args.extend_from_slice(&group.args); grp.requires = group.requires.clone(); grp.conflicts = group.conflicts.clone(); @@ -290,7 +297,11 @@ impl<'a, 'b> Parser<'a, 'b> if vsc { sc.p.set(AS::DisableVersion); } - if gv && sc.p.meta.version.is_none() && self.meta.version.is_some() { + if gv && + sc.p + .meta + .version + .is_none() && self.meta.version.is_some() { sc.p.set(AS::GlobalVersion); sc.p.meta.version = Some(self.meta.version.unwrap()); } @@ -306,21 +317,21 @@ impl<'a, 'b> Parser<'a, 'b> if self.is_set(AS::DeriveDisplayOrder) { let unified = self.is_set(AS::UnifiedHelpMessage); for (i, o) in self.opts - .iter_mut() - .enumerate() - .filter(|&(_, ref o)| o.s.disp_ord == 999) { + .iter_mut() + .enumerate() + .filter(|&(_, ref o)| o.s.disp_ord == 999) { o.s.disp_ord = if unified { o.s.unified_ord } else { i }; } for (i, f) in self.flags - .iter_mut() - .enumerate() - .filter(|&(_, ref f)| f.s.disp_ord == 999) { + .iter_mut() + .enumerate() + .filter(|&(_, ref f)| f.s.disp_ord == 999) { f.s.disp_ord = if unified { f.s.unified_ord } else { i }; } for (i, sc) in &mut self.subcommands - .iter_mut() - .enumerate() - .filter(|&(_, ref sc)| sc.p.meta.disp_ord == 999) { + .iter_mut() + .enumerate() + .filter(|&(_, ref sc)| sc.p.meta.disp_ord == 999) { sc.p.meta.disp_ord = i; } } @@ -441,24 +452,25 @@ impl<'a, 'b> Parser<'a, 'b> } } for a in desc_reqs.iter() - .filter(|name| !self.positionals.values().any(|p| &&p.b.name == name)) - .filter(|name| !self.groups.iter().any(|g| &&g.name == name)) - .filter(|name| !args_in_groups.contains(name)) - .filter(|name| !(matcher.is_some() && matcher.as_ref().unwrap().contains(name))) { + .filter(|name| !self.positionals.values().any(|p| &&p.b.name == name)) + .filter(|name| !self.groups.iter().any(|g| &&g.name == name)) + .filter(|name| !args_in_groups.contains(name)) + .filter(|name| { + !(matcher.is_some() && matcher.as_ref().unwrap().contains(name)) + }) { debugln!("Parser::get_required_from:iter:{}:", a); let arg = find_by_name!(self, a, flags, iter) .map(|f| f.to_string()) .unwrap_or_else(|| { - find_by_name!(self, a, opts, iter) - .map(|o| o.to_string()) - .expect(INTERNAL_ERROR_MSG) - }); + find_by_name!(self, a, opts, iter) + .map(|o| o.to_string()) + .expect(INTERNAL_ERROR_MSG) + }); ret_val.push_back(arg); } let mut g_vec = vec![]; for g in desc_reqs.iter().filter(|n| self.groups.iter().any(|g| &&g.name == n)) { - let g_string = self.args_in_group(g) - .join("|"); + let g_string = self.args_in_group(g).join("|"); g_vec.push(format!("<{}>", &g_string[..g_string.len()])); } g_vec.sort(); @@ -474,8 +486,10 @@ impl<'a, 'b> Parser<'a, 'b> pub fn get_args_tag(&self) -> Option { debugln!("Parser::get_args_tag;"); let mut count = 0; - 'outer: for p in self.positionals.values().filter(|p| !p.is_set(ArgSettings::Required) && - !p.is_set(ArgSettings::Hidden)) { + 'outer: for p in self.positionals.values().filter(|p| { + !p.is_set(ArgSettings::Required) && + !p.is_set(ArgSettings::Hidden) + }) { debugln!("Parser::get_args_tag:iter:{}:", p.b.name); if let Some(g_vec) = self.groups_for_arg(p.b.name) { for grp_s in &g_vec { @@ -487,7 +501,8 @@ impl<'a, 'b> Parser<'a, 'b> } } count += 1; - debugln!("Parser::get_args_tag:iter: {} Args not required or hidden", count); + debugln!("Parser::get_args_tag:iter: {} Args not required or hidden", + count); } if !self.is_set(AS::DontCollapseArgsInUsage) && count > 1 { return None; // [ARGS] @@ -499,12 +514,14 @@ impl<'a, 'b> Parser<'a, 'b> return Some(format!(" [{}]{}", p.name_no_brackets(), p.multiple_str())); } else if self.is_set(AS::DontCollapseArgsInUsage) && !self.positionals.is_empty() { return Some(self.positionals - .values() - .filter(|p| !p.is_set(ArgSettings::Required)) - .filter(|p| !p.is_set(ArgSettings::Hidden)) - .map(|p| format!(" [{}]{}", p.name_no_brackets(), p.multiple_str())) - .collect::>() - .join("")); + .values() + .filter(|p| !p.is_set(ArgSettings::Required)) + .filter(|p| !p.is_set(ArgSettings::Hidden)) + .map(|p| { + format!(" [{}]{}", p.name_no_brackets(), p.multiple_str()) + }) + .collect::>() + .join("")); } Some("".into()) } @@ -558,26 +575,34 @@ impl<'a, 'b> Parser<'a, 'b> pub fn has_subcommands(&self) -> bool { !self.subcommands.is_empty() } #[inline] - pub fn has_visible_opts(&self) -> bool { - if self.opts.is_empty() { return false; } + pub fn has_visible_opts(&self) -> bool { + if self.opts.is_empty() { + return false; + } self.opts.iter().any(|o| !o.is_set(ArgSettings::Hidden)) } #[inline] - pub fn has_visible_flags(&self) -> bool { - if self.flags.is_empty() { return false; } + pub fn has_visible_flags(&self) -> bool { + if self.flags.is_empty() { + return false; + } self.flags.iter().any(|f| !f.is_set(ArgSettings::Hidden)) } #[inline] - pub fn has_visible_positionals(&self) -> bool { - if self.positionals.is_empty() { return false; } + pub fn has_visible_positionals(&self) -> bool { + if self.positionals.is_empty() { + return false; + } self.positionals.values().any(|p| !p.is_set(ArgSettings::Hidden)) } #[inline] - pub fn has_visible_subcommands(&self) -> bool { - if self.subcommands.is_empty() { return false; } + pub fn has_visible_subcommands(&self) -> bool { + if self.subcommands.is_empty() { + return false; + } self.subcommands.iter().any(|s| !s.p.is_set(AS::Hidden)) } @@ -598,7 +623,10 @@ 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, 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", @@ -608,12 +636,10 @@ impl<'a, 'b> Parser<'a, 'b> } // Next we verify that only the highest index has a .multiple(true) (if any) - if - self.positionals - .values() - .any(|a| { - a.is_set(ArgSettings::Multiple) && (a.index as usize != self.positionals.len()) - }) { + if self.positionals.values().any(|a| { + a.is_set(ArgSettings::Multiple) && + (a.index as usize != self.positionals.len()) + }) { debug_assert!({ let mut it = self.positionals.values().rev(); @@ -628,7 +654,10 @@ impl<'a, 'b> Parser<'a, 'b> debug_assert!({ let num = self.positionals.len() - 1; - self.positionals.get(num).unwrap().is_set(ArgSettings::Multiple) + self.positionals + .get(num) + .unwrap() + .is_set(ArgSettings::Multiple) }, "Only the last positional argument, or second to last positional \ argument may be set to .multiple(true)"); @@ -639,8 +668,9 @@ impl<'a, 'b> Parser<'a, 'b> debug_assert!(self.positionals .values() .filter(|p| { - p.b.settings.is_set(ArgSettings::Multiple) && p.v.num_vals.is_none() - }) + p.b.settings.is_set(ArgSettings::Multiple) && + p.v.num_vals.is_none() + }) .map(|_| 1) .sum::() <= 1, "Only one positional argument with .multiple(true) set is allowed per \ @@ -729,15 +759,18 @@ impl<'a, 'b> Parser<'a, 'b> .iter() .filter(|s| { starts(&s.p.meta.name[..], &*arg_os) || - (s.p.meta.aliases.is_some() && + (s.p + .meta + .aliases + .is_some() && s.p - .meta - .aliases - .as_ref() - .unwrap() - .iter() - .filter(|&&(a, _)| starts(a, &*arg_os)) - .count() == 1) + .meta + .aliases + .as_ref() + .unwrap() + .iter() + .filter(|&&(a, _)| starts(a, &*arg_os)) + .count() == 1) }) .map(|sc| &sc.p.meta.name) .collect::>(); @@ -769,24 +802,21 @@ impl<'a, 'b> Parser<'a, 'b> help_help = true; } if let Some(c) = sc.subcommands - .iter() - .find(|s| &*s.p.meta.name == cmd) - .map(|sc| &sc.p) { + .iter() + .find(|s| &*s.p.meta.name == cmd) + .map(|sc| &sc.p) { sc = c; if i == cmds.len() - 1 { break; } } else if let Some(c) = sc.subcommands - .iter() - .find(|s| if let Some(ref als) = s.p - .meta - .aliases { - als.iter() - .any(|&(a, _)| &a == &&*cmd.to_string_lossy()) - } else { - false - }) - .map(|sc| &sc.p) { + .iter() + .find(|s| if let Some(ref als) = s.p.meta.aliases { + als.iter().any(|&(a, _)| &a == &&*cmd.to_string_lossy()) + } else { + false + }) + .map(|sc| &sc.p) { sc = c; if i == cmds.len() - 1 { break; @@ -957,7 +987,7 @@ impl<'a, 'b> Parser<'a, 'b> arg_os.to_string_lossy().parse::().is_ok()) { return Err(Error::unknown_argument(&*arg_os.to_string_lossy(), "", - &*self.create_current_usage(matcher, None), + &*usage::create_error_usage(self, matcher, None), self.color())); } } else if !self.is_set(AS::AllowLeadingHyphen) { @@ -980,7 +1010,7 @@ impl<'a, 'b> Parser<'a, 'b> .bin_name .as_ref() .unwrap_or(&self.meta.name), - &*self.create_current_usage(matcher, + &*usage::create_error_usage(self, matcher, None), self.color())); } @@ -1029,8 +1059,9 @@ impl<'a, 'b> Parser<'a, 'b> Some(s) => s.to_string(), None => { if !self.is_set(AS::StrictUtf8) { - return Err(Error::invalid_utf8(&*self.create_current_usage(matcher, - None), + return Err(Error::invalid_utf8(&*usage::create_error_usage(self, + matcher, + None), self.color())); } arg_os.to_string_lossy().into_owned() @@ -1042,59 +1073,73 @@ impl<'a, 'b> Parser<'a, 'b> while let Some(v) = it.next() { let a = v.into(); if a.to_str().is_none() && !self.is_set(AS::StrictUtf8) { - return Err(Error::invalid_utf8(&*self.create_current_usage(matcher, None), + return Err(Error::invalid_utf8(&*usage::create_error_usage(self, + matcher, + None), self.color())); } sc_m.add_val_to("", &a); } matcher.subcommand(SubCommand { - name: sc_name, - matches: sc_m.into(), - }); + name: sc_name, + matches: sc_m.into(), + }); } else if !(self.is_set(AS::AllowLeadingHyphen) || self.is_set(AS::AllowNegativeNumbers)) && !self.is_set(AS::InferSubcommands) { return Err(Error::unknown_argument(&*arg_os.to_string_lossy(), "", - &*self.create_current_usage(matcher, None), + &*usage::create_error_usage(self, + matcher, + None), self.color())); } else if !(has_args) && self.has_subcommands() { if let Some(cdate) = suggestions::did_you_mean(&*arg_os.to_string_lossy(), sc_names!(self)) { - return Err(Error::invalid_subcommand(arg_os.to_string_lossy() - .into_owned(), - cdate, - self.meta - .bin_name - .as_ref() - .unwrap_or(&self.meta.name), - &*self.create_current_usage(matcher, - None), - self.color())); + return Err(Error::invalid_subcommand(arg_os.to_string_lossy().into_owned(), + cdate, + self.meta + .bin_name + .as_ref() + .unwrap_or(&self.meta.name), + &*usage::create_error_usage(self, + matcher, + None), + self.color())); } } } if let Some(ref pos_sc_name) = subcmd_name { let sc_name = { - find_subcmd!(self, pos_sc_name).expect(INTERNAL_ERROR_MSG).p.meta.name.clone() + find_subcmd!(self, pos_sc_name) + .expect(INTERNAL_ERROR_MSG) + .p + .meta + .name + .clone() }; try!(self.parse_subcommand(&*sc_name, matcher, it)); } else if self.is_set(AS::SubcommandRequired) { - let bn = self.meta.bin_name.as_ref().unwrap_or(&self.meta.name); + let bn = self.meta + .bin_name + .as_ref() + .unwrap_or(&self.meta.name); return Err(Error::missing_subcommand(bn, - &self.create_current_usage(matcher, None), + &usage::create_error_usage(self, + matcher, + None), self.color())); } else if self.is_set(AS::SubcommandRequiredElseHelp) { debugln!("parser::get_matches_with: SubcommandRequiredElseHelp=true"); let mut out = vec![]; try!(self.write_help_err(&mut out)); return Err(Error { - message: String::from_utf8_lossy(&*out).into_owned(), - kind: ErrorKind::MissingArgumentOrSubcommand, - info: None, - }); + message: String::from_utf8_lossy(&*out).into_owned(), + kind: ErrorKind::MissingArgumentOrSubcommand, + info: None, + }); } Validator::new(self).validate(needs_val_of, subcmd_name, matcher) @@ -1113,7 +1158,10 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Parser::build_bin_names;"); for sc in &mut self.subcommands { debug!("Parser::build_bin_names:iter: bin_name set..."); - if sc.p.meta.bin_name.is_none() { + if sc.p + .meta + .bin_name + .is_none() { sdebugln!("No"); let bin_name = format!("{}{}{}", self.meta @@ -1151,7 +1199,10 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Parser::parse_subcommand;"); let mut mid_string = String::new(); if !self.is_set(AS::SubcommandsNegateReqs) { - let mut hs: Vec<&str> = self.required.iter().map(|n| &**n).collect(); + let mut hs: Vec<&str> = self.required + .iter() + .map(|n| &**n) + .collect(); for k in matcher.arg_names() { hs.push(k); } @@ -1162,14 +1213,15 @@ impl<'a, 'b> Parser<'a, 'b> } } mid_string.push_str(" "); - if let Some(ref mut sc) = self.subcommands - .iter_mut() - .find(|s| &s.p.meta.name == &sc_name) { + if let Some(ref mut sc) = self.subcommands.iter_mut().find(|s| &s.p.meta.name == &sc_name) { let mut sc_matcher = ArgMatcher::new(); // bin_name should be parent's bin_name + [] + the sc's name separated by // a space sc.p.meta.usage = Some(format!("{}{}{}", - self.meta.bin_name.as_ref().unwrap_or(&String::new()), + self.meta + .bin_name + .as_ref() + .unwrap_or(&String::new()), if self.meta.bin_name.is_some() { &*mid_string } else { @@ -1192,9 +1244,12 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Parser::parse_subcommand: sc settings={:#?}", sc.p.settings); try!(sc.p.get_matches_with(&mut sc_matcher, it)); matcher.subcommand(SubCommand { - name: sc.p.meta.name.clone(), - matches: sc_matcher.into(), - }); + name: sc.p + .meta + .name + .clone(), + matches: sc_matcher.into(), + }); } Ok(()) } @@ -1228,14 +1283,16 @@ impl<'a, 'b> Parser<'a, 'b> let mut g_vec = vec![]; let mut args = vec![]; - for n in &self.groups.iter().find(|g| g.name == group).expect(INTERNAL_ERROR_MSG).args { + for n in &self.groups + .iter() + .find(|g| g.name == group) + .expect(INTERNAL_ERROR_MSG) + .args { if let Some(f) = self.flags.iter().find(|f| &f.b.name == n) { args.push(f.to_string()); } else if let Some(f) = self.opts.iter().find(|o| &o.b.name == n) { args.push(f.to_string()); - } else if let Some(p) = self.positionals - .values() - .find(|p| &p.b.name == n) { + } else if let Some(p) = self.positionals.values().find(|p| &p.b.name == n) { args.push(p.b.name.to_owned()); } else { g_vec.push(*n); @@ -1253,7 +1310,11 @@ impl<'a, 'b> Parser<'a, 'b> let mut g_vec = vec![]; let mut args = vec![]; - for n in &self.groups.iter().find(|g| g.name == group).expect(INTERNAL_ERROR_MSG).args { + for n in &self.groups + .iter() + .find(|g| g.name == group) + .expect(INTERNAL_ERROR_MSG) + .args { if self.groups.iter().any(|g| &g.name == &*n) { args.extend(self.arg_names_in_group(&*n)); g_vec.push(*n); @@ -1321,26 +1382,6 @@ impl<'a, 'b> Parser<'a, 'b> // Retrieves the names of all args the user has supplied thus far, except required ones // because those will be listed in self.required - pub fn create_current_usage(&self, matcher: &'b ArgMatcher<'a>, extra: Option<&str>) -> String { - let mut args: Vec<_> = matcher.arg_names() - .iter() - .filter(|n| { - if let Some(o) = find_by_name!(self, *n, opts, iter) { - !o.b.settings.is_set(ArgSettings::Required) - } else if let Some(p) = find_by_name!(self, *n, positionals, values) { - !p.b.settings.is_set(ArgSettings::Required) - } else { - true // flags can't be required, so they're always true - } - }) - .map(|&n| n) - .collect(); - if let Some(r) = extra { - args.push(r); - } - self.create_usage(&*args) - } - fn check_for_help_and_version_str(&self, arg: &OsStr) -> ClapResult<()> { debugln!("Parser::check_for_help_and_version_str;"); debug!("Parser::check_for_help_and_version_str: Checking if --{} is help or version...", @@ -1382,10 +1423,10 @@ impl<'a, 'b> Parser<'a, 'b> let mut buf = vec![]; try!(Help::write_parser_help(&mut buf, self)); Err(Error { - message: unsafe { String::from_utf8_unchecked(buf) }, - kind: ErrorKind::HelpDisplayed, - info: None, - }) + message: unsafe { String::from_utf8_unchecked(buf) }, + kind: ErrorKind::HelpDisplayed, + info: None, + }) } fn _version(&self) -> ClapResult<()> { @@ -1393,10 +1434,10 @@ impl<'a, 'b> Parser<'a, 'b> let mut buf_w = BufWriter::new(out.lock()); try!(self.print_version(&mut buf_w)); Err(Error { - message: String::new(), - kind: ErrorKind::VersionDisplayed, - info: None, - }) + message: String::new(), + kind: ErrorKind::VersionDisplayed, + info: None, + }) } fn parse_long_arg(&mut self, @@ -1530,7 +1571,9 @@ impl<'a, 'b> Parser<'a, 'b> let arg = format!("-{}", c); return Err(Error::unknown_argument(&*arg, "", - &*self.create_current_usage(matcher, None), + &*usage::create_error_usage(self, + matcher, + None), self.color())); } } @@ -1555,7 +1598,7 @@ impl<'a, 'b> Parser<'a, 'b> (v.len_() == 0 || (opt.is_set(ArgSettings::RequireEquals) && !has_eq)) { sdebugln!("Found Empty - Error"); return Err(Error::empty_value(opt, - &*self.create_current_usage(matcher, None), + &*usage::create_error_usage(self, matcher, None), self.color())); } sdebugln!("Found - {:?}, len: {}", v, v.len_()); @@ -1566,7 +1609,7 @@ impl<'a, 'b> Parser<'a, 'b> } else if opt.is_set(ArgSettings::RequireEquals) && !opt.is_set(ArgSettings::EmptyValues) { sdebugln!("None, but requires equals...Error"); return Err(Error::empty_value(opt, - &*self.create_current_usage(matcher, None), + &*usage::create_error_usage(self, matcher, None), self.color())); } else { @@ -1689,130 +1732,10 @@ impl<'a, 'b> Parser<'a, 'b> let used_arg = format!("--{}", arg); Err(Error::unknown_argument(&*used_arg, &*suffix.0, - &*self.create_current_usage(matcher, None), + &*usage::create_error_usage(self, matcher, None), self.color())) } - // Creates a usage string if one was not provided by the user manually. This happens just - // after all arguments were parsed, but before any subcommands have been parsed - // (so as to give subcommands their own usage recursively) - pub fn create_usage(&self, used: &[&str]) -> String { - debugln!("Parser::create_usage;"); - let mut usage = String::with_capacity(75); - usage.push_str("USAGE:\n "); - usage.push_str(&self.create_usage_no_title(used)); - usage - } - - // Creates a usage string (*without title*) if one was not provided by the user - // manually. This happens just - // after all arguments were parsed, but before any subcommands have been parsed - // (so as to give subcommands their own usage recursively) - pub fn create_usage_no_title(&self, used: &[&str]) -> String { - debugln!("Parser::create_usage_no_title;"); - let mut usage = String::with_capacity(75); - if let Some(u) = self.meta.usage_str { - usage.push_str(&*u); - } else if used.is_empty() { - let name = self.meta - .usage - .as_ref() - .unwrap_or_else(|| { - self.meta - .bin_name - .as_ref() - .unwrap_or(&self.meta.name) - }); - usage.push_str(&*name); - let mut reqs: Vec<&str> = self.required().map(|r| &**r).collect(); - reqs.dedup(); - let req_string = self.get_required_from(&reqs, None, None) - .iter() - .fold(String::new(), |a, s| a + &format!(" {}", s)[..]); - - let flags = self.needs_flags_tag(); - if flags && !self.is_set(AS::UnifiedHelpMessage) { - usage.push_str(" [FLAGS]"); - } else if flags { - usage.push_str(" [OPTIONS]"); - } - if !self.is_set(AS::UnifiedHelpMessage) && - self.opts.iter().any(|o| !o.is_set(ArgSettings::Required) && - !o.is_set(ArgSettings::Hidden)) { - usage.push_str(" [OPTIONS]"); - } - - usage.push_str(&req_string[..]); - - // places a '--' in the usage string if there are args and options - // supporting multiple values - if self.has_positionals() && - self.opts.iter().any(|o| o.is_set(ArgSettings::Multiple)) && - self.positionals.values().any(|p| !p.is_set(ArgSettings::Required)) && - !self.has_visible_subcommands() { - usage.push_str(" [--]") - } - if self.has_positionals() && - self.positionals.values().any(|p| !p.is_set(ArgSettings::Required) && - !p.is_set(ArgSettings::Hidden)) { - if let Some(args_tag) = self.get_args_tag() { - usage.push_str(&*args_tag); - } else { - usage.push_str(" [ARGS]"); - } - } - - - if self.is_set(AS::SubcommandsNegateReqs) || self.is_set(AS::ArgsNegateSubcommands) { - if self.has_visible_subcommands() { - usage.push_str("\n "); - usage.push_str(&*name); - usage.push_str(" "); - } - } else { - if self.has_visible_subcommands() && !self.is_set(AS::SubcommandRequired) { - usage.push_str(" [SUBCOMMAND]"); - } else if (self.is_set(AS::SubcommandRequired) || - self.is_set(AS::SubcommandRequiredElseHelp)) && self.has_subcommands() { - usage.push_str(" "); - } - } - } else { - self.smart_usage(&mut usage, used); - } - - usage.shrink_to_fit(); - usage - } - - // Creates a context aware usage string, or "smart usage" from currently used - // args, and requirements - fn smart_usage(&self, usage: &mut String, used: &[&str]) { - debugln!("Parser::smart_usage;"); - let mut hs: Vec<&str> = self.required().map(|s| &**s).collect(); - hs.extend_from_slice(used); - - let r_string = self.get_required_from(&hs, None, None) - .iter() - .fold(String::new(), |acc, s| acc + &format!(" {}", s)[..]); - - usage.push_str(&self.meta - .usage - .as_ref() - .unwrap_or_else(|| { - self.meta - .bin_name - .as_ref() - .unwrap_or(&self.meta - .name) - }) - [..]); - usage.push_str(&*r_string); - if self.is_set(AS::SubcommandRequired) { - usage.push_str(" "); - } - } - // Prints the version to the user and exits if quit=true fn print_version(&self, w: &mut W) -> ClapResult<()> { try!(self.write_version(w)); @@ -1955,17 +1878,33 @@ impl<'a, 'b> Parser<'a, 'b> pub fn find_subcommand(&'b self, sc: &str) -> Option<&'b App<'a, 'b>> { debugln!("Parser::find_subcommand: sc={}", sc); debugln!("Parser::find_subcommand: Currently in Parser...{}", - self.meta.bin_name.as_ref().unwrap()); + self.meta + .bin_name + .as_ref() + .unwrap()); for s in self.subcommands.iter() { - if s.p.meta.bin_name.as_ref().unwrap_or(&String::new()) == sc || - (s.p.meta.aliases.is_some() && + if s.p + .meta + .bin_name + .as_ref() + .unwrap_or(&String::new()) == sc || + (s.p + .meta + .aliases + .is_some() && s.p - .meta - .aliases - .as_ref() - .unwrap() - .iter() - .any(|&(s, _)| s == sc.split(' ').rev().next().expect(INTERNAL_ERROR_MSG))) { + .meta + .aliases + .as_ref() + .unwrap() + .iter() + .any(|&(s, _)| { + s == + sc.split(' ') + .rev() + .next() + .expect(INTERNAL_ERROR_MSG) + })) { return Some(s); } if let Some(app) = s.p.find_subcommand(sc) { diff --git a/src/app/usage.rs b/src/app/usage.rs new file mode 100644 index 00000000..cfd42119 --- /dev/null +++ b/src/app/usage.rs @@ -0,0 +1,153 @@ +use args::{AnyArg, ArgMatcher}; +use args::settings::ArgSettings; +use app::settings::AppSettings as AS; +use app::parser::Parser; + +// Creates a usage string for display. This happens just after all arguments were parsed, but before +// any subcommands have been parsed (so as to give subcommands their own usage recursively) +pub fn create_usage_with_title(p: &Parser, used: &[&str]) -> String { + debugln!("Parser::create_usage_with_title;"); + let mut usage = String::with_capacity(75); + usage.push_str("USAGE:\n "); + usage.push_str(&*create_usage_no_title(p, used)); + usage +} + +// Creates a usage string to be used in error message (i.e. one with currently used args) +pub fn create_error_usage<'a, 'b>(p: &Parser<'a, 'b>, + matcher: &'b ArgMatcher<'a>, + extra: Option<&str>) + -> String { + let mut args: Vec<_> = matcher.arg_names() + .iter() + .filter(|n| { + if let Some(o) = find_by_name!(p, *n, opts, iter) { + !o.b.is_set(ArgSettings::Required) && !o.b.is_set(ArgSettings::Hidden) + } else if let Some(p) = find_by_name!(p, *n, positionals, values) { + !p.b.is_set(ArgSettings::Required) && p.b.is_set(ArgSettings::Hidden) + } else { + true // flags can't be required, so they're always true + } + }) + .map(|&n| n) + .collect(); + if let Some(r) = extra { + args.push(r); + } + create_usage_with_title(p, &*args) +} + +// Creates a usage string (*without title*) if one was not provided by the user manually. +fn create_usage_no_title(p: &Parser, used: &[&str]) -> String { + debugln!("Parser::create_usage_no_title;"); + if let Some(u) = p.meta.usage_str { + String::from(&*u) + } else if used.is_empty() { + create_help_usage(p) + } else { + create_smart_usage(p, used) + } +} + +// Creates a usage string for display in help messages (i.e. not for errors) +pub fn create_help_usage(p: &Parser) -> String { + let mut usage = String::with_capacity(75); + let name = p.meta + .usage + .as_ref() + .unwrap_or_else(|| { + p.meta + .bin_name + .as_ref() + .unwrap_or(&p.meta.name) + }); + usage.push_str(&*name); + let mut reqs: Vec<&str> = p.required().map(|r| &**r).collect(); + reqs.dedup(); + let req_string = + p.get_required_from(&reqs, None, None).iter().fold(String::new(), + |a, s| a + &format!(" {}", s)[..]); + + let flags = p.needs_flags_tag(); + if flags && !p.is_set(AS::UnifiedHelpMessage) { + usage.push_str(" [FLAGS]"); + } else if flags { + usage.push_str(" [OPTIONS]"); + } + if !p.is_set(AS::UnifiedHelpMessage) && + p.opts.iter().any(|o| { + !o.is_set(ArgSettings::Required) && !o.is_set(ArgSettings::Hidden) + }) { + usage.push_str(" [OPTIONS]"); + } + + usage.push_str(&req_string[..]); + + // places a '--' in the usage string if there are args and options + // supporting multiple values + if p.has_positionals() && p.opts.iter().any(|o| o.is_set(ArgSettings::Multiple)) && + p.positionals.values().any(|p| !p.is_set(ArgSettings::Required)) && + !p.has_visible_subcommands() { + usage.push_str(" [--]") + } + if p.has_positionals() && + p.positionals.values().any(|p| { + !p.is_set(ArgSettings::Required) && + !p.is_set(ArgSettings::Hidden) + }) { + if let Some(args_tag) = p.get_args_tag() { + usage.push_str(&*args_tag); + } else { + usage.push_str(" [ARGS]"); + } + } + + + if p.is_set(AS::SubcommandsNegateReqs) || p.is_set(AS::ArgsNegateSubcommands) { + if p.has_visible_subcommands() { + usage.push_str("\n "); + usage.push_str(&*name); + usage.push_str(" "); + } + } else { + if p.has_visible_subcommands() && !p.is_set(AS::SubcommandRequired) { + usage.push_str(" [SUBCOMMAND]"); + } else if (p.is_set(AS::SubcommandRequired) || + p.is_set(AS::SubcommandRequiredElseHelp)) && + p.has_subcommands() { + usage.push_str(" "); + } + } + usage.shrink_to_fit(); + usage +} + +// Creates a context aware usage string, or "smart usage" from currently used +// args, and requirements +fn create_smart_usage(p: &Parser, used: &[&str]) -> String { + debugln!("Parser::smart_usage;"); + let mut usage = String::with_capacity(75); + let mut hs: Vec<&str> = p.required().map(|s| &**s).collect(); + hs.extend_from_slice(used); + + let r_string = p.get_required_from(&hs, None, None).iter().fold(String::new(), |acc, s| { + acc + &format!(" {}", s)[..] + }); + + usage.push_str(&p.meta + .usage + .as_ref() + .unwrap_or_else(|| { + p.meta + .bin_name + .as_ref() + .unwrap_or(&p.meta.name) + }) + [..]); + usage.push_str(&*r_string); + if p.is_set(AS::SubcommandRequired) { + usage.push_str(" "); + } + usage.shrink_to_fit(); + usage +} diff --git a/src/app/validator.rs b/src/app/validator.rs index afce3089..2b60e823 100644 --- a/src/app/validator.rs +++ b/src/app/validator.rs @@ -12,19 +12,20 @@ use osstringext::OsStrExt2; use app::settings::AppSettings as AS; use app::parser::Parser; use fmt::Colorizer; +use app::usage; -pub struct Validator<'a, 'b, 'z>(&'z mut Parser<'a, 'b>) where 'a: 'b, 'b: 'z; +pub struct Validator<'a, 'b, 'z>(&'z mut Parser<'a, 'b>) + where 'a: 'b, + 'b: 'z; impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { - pub fn new(p: &'z mut Parser<'a, 'b>) -> Self { - Validator(p) - } + pub fn new(p: &'z mut Parser<'a, 'b>) -> Self { Validator(p) } pub fn validate(&mut self, - needs_val_of: Option<&'a str>, - subcmd_name: Option, - matcher: &mut ArgMatcher<'a>) - -> ClapResult<()> { + needs_val_of: Option<&'a str>, + subcmd_name: Option, + matcher: &mut ArgMatcher<'a>) + -> ClapResult<()> { debugln!("Validator::validate;"); let mut reqs_validated = false; try!(self.0.add_defaults(matcher)); @@ -40,7 +41,9 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { }; if should_err { return Err(Error::empty_value(o, - &*self.0.create_current_usage(matcher, None), + &*usage::create_error_usage(self.0, + matcher, + None), self.0.color())); } } @@ -51,17 +54,17 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { try!(self.validate_required(matcher)); } try!(self.validate_matched_args(matcher)); - matcher.usage(self.0.create_usage(&[])); + matcher.usage(usage::create_help_usage(self.0)); if matcher.is_empty() && matcher.subcommand_name().is_none() && self.0.is_set(AS::ArgRequiredElseHelp) { let mut out = vec![]; try!(self.0.write_help_err(&mut out)); return Err(Error { - message: String::from_utf8_lossy(&*out).into_owned(), - kind: ErrorKind::MissingArgumentOrSubcommand, - info: None, - }); + message: String::from_utf8_lossy(&*out).into_owned(), + kind: ErrorKind::MissingArgumentOrSubcommand, + info: None, + }); } Ok(()) } @@ -78,7 +81,9 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { if self.0.is_set(AS::StrictUtf8) && val.to_str().is_none() { debugln!("Validator::validate_values: invalid UTF-8 found in val {:?}", val); - return Err(Error::invalid_utf8(&*self.0.create_current_usage(matcher, None), + return Err(Error::invalid_utf8(&*usage::create_error_usage(self.0, + matcher, + None), self.0.color())); } if let Some(p_vals) = arg.possible_vals() { @@ -88,7 +93,9 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { return Err(Error::invalid_value(val_str, p_vals, arg, - &*self.0.create_current_usage(matcher, None), + &*usage::create_error_usage(self.0, + matcher, + None), self.0.color())); } } @@ -96,7 +103,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { matcher.contains(&*arg.name()) { debugln!("Validator::validate_values: illegal empty val found"); return Err(Error::empty_value(arg, - &*self.0.create_current_usage(matcher, None), + &*usage::create_error_usage(self.0, matcher, None), self.0.color())); } if let Some(vtor) = arg.validator() { @@ -124,7 +131,8 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { } fn validate_blacklist(&self, matcher: &mut ArgMatcher) -> ClapResult<()> { - debugln!("Validator::validate_blacklist: blacklist={:?}", self.0.blacklist); + debugln!("Validator::validate_blacklist: blacklist={:?}", + self.0.blacklist); macro_rules! build_err { ($p:expr, $name:expr, $matcher:ident) => ({ debugln!("build_err!: name={}", $name); @@ -138,7 +146,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { ); debugln!("build_err!: '{:?}' conflicts with '{}'", c_with, $name); $matcher.remove($name); - let usg = $p.create_current_usage($matcher, None); + let usg = usage::create_error_usage($p, $matcher, None); if let Some(f) = find_by_name!($p, $name, flags, iter) { debugln!("build_err!: It was a flag..."); Error::argument_conflict(f, c_with, &*usg, self.0.color()) @@ -160,7 +168,10 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { for name in &self.0.blacklist { debugln!("Validator::validate_blacklist:iter: Checking blacklisted name: {}", name); - if self.0.groups.iter().any(|g| &g.name == name) { + if self.0 + .groups + .iter() + .any(|g| &g.name == name) { debugln!("Validator::validate_blacklist:iter: groups contains it..."); for n in self.0.arg_names_in_group(name) { debugln!("Validator::validate_blacklist:iter:iter: Checking arg '{}' in group...", @@ -198,7 +209,11 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { try!(self.validate_values(pos, ma, matcher)); try!(self.validate_arg_requires(pos, ma, matcher)); } else { - let grp = self.0.groups.iter().find(|g| &g.name == name).expect(INTERNAL_ERROR_MSG); + let grp = self.0 + .groups + .iter() + .find(|g| &g.name == name) + .expect(INTERNAL_ERROR_MSG); if let Some(ref g_reqs) = grp.requires { if g_reqs.iter().any(|&n| !matcher.contains(n)) { return self.missing_required_error(matcher, None); @@ -220,7 +235,9 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { if ma.occurs > 1 && !a.is_set(ArgSettings::Multiple) { // Not the first time, and we don't allow multiples return Err(Error::unexpected_multiple_usage(a, - &*self.0.create_current_usage(matcher, None), + &*usage::create_error_usage(self.0, + matcher, + None), self.0.color())); } Ok(()) @@ -258,7 +275,9 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { } else { "ere" }, - &*self.0.create_current_usage(matcher, None), + &*usage::create_error_usage(self.0, + matcher, + None), self.0.color())); } } @@ -273,7 +292,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { .to_str() .expect(INVALID_UTF8), a, - &*self.0.create_current_usage(matcher, None), + &*usage::create_error_usage(self.0, matcher, None), self.0.color())); } } @@ -284,14 +303,14 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { return Err(Error::too_few_values(a, num, ma.vals.len(), - &*self.0.create_current_usage(matcher, None), + &*usage::create_error_usage(self.0, matcher, None), self.0.color())); } } // Issue 665 (https://github.com/kbknapp/clap-rs/issues/665) if a.takes_value() && !a.is_set(ArgSettings::EmptyValues) && ma.vals.is_empty() { return Err(Error::empty_value(a, - &*self.0.create_current_usage(matcher, None), + &*usage::create_error_usage(self.0, matcher, None), self.0.color())); } Ok(()) @@ -307,9 +326,10 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { debugln!("Validator::validate_arg_requires;"); if let Some(a_reqs) = a.requires() { for &(val, name) in a_reqs.iter().filter(|&&(val, _)| val.is_some()) { - if ma.vals - .iter() - .any(|v| v == val.expect(INTERNAL_ERROR_MSG) && !matcher.contains(name)) { + if ma.vals.iter().any(|v| { + v == val.expect(INTERNAL_ERROR_MSG) && + !matcher.contains(name) + }) { return self.missing_required_error(matcher, None); } } @@ -318,7 +338,8 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { } fn validate_required(&self, matcher: &ArgMatcher) -> ClapResult<()> { - debugln!("Validator::validate_required: required={:?};", self.0.required); + debugln!("Validator::validate_required: required={:?};", + self.0.required); 'outer: for name in &self.0.required { debugln!("Validator::validate_required:iter:{}:", name); if matcher.contains(name) { @@ -360,7 +381,8 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { a.blacklist().map(|bl| { bl.iter().any(|conf| { matcher.contains(conf) || - self.0.groups + self.0 + .groups .iter() .find(|g| &g.name == conf) .map_or(false, |g| g.args.iter().any(|arg| matcher.contains(arg))) @@ -401,21 +423,26 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { use_stderr: true, when: self.0.color(), }; - let mut reqs = self.0.required.iter().map(|&r| &*r).collect::>(); + let mut reqs = self.0 + .required + .iter() + .map(|&r| &*r) + .collect::>(); if let Some(r) = extra { reqs.push(r); } reqs.retain(|n| !matcher.contains(n)); reqs.dedup(); debugln!("Validator::missing_required_error: reqs={:#?}", reqs); - Err(Error::missing_required_argument(&*self.0.get_required_from(&reqs[..], - Some(matcher), - extra) - .iter() - .fold(String::new(), |acc, s| { - acc + &format!("\n {}", c.error(s))[..] - }), - &*self.0.create_current_usage(matcher, extra), + Err(Error::missing_required_argument(&*self.0 + .get_required_from(&reqs[..], + Some(matcher), + extra) + .iter() + .fold(String::new(), |acc, s| { + acc + &format!("\n {}", c.error(s))[..] + }), + &*usage::create_error_usage(self.0, matcher, extra), self.0.color())) } From 33eb5a6db0371a509dbcc7ff58047973274b62f4 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 16:51:20 -0500 Subject: [PATCH 15/31] tests: fixes failing dual usage string help test --- src/app/help.rs | 4 +-- src/app/usage.rs | 64 ++++++++++++++++++++++++-------------------- src/app/validator.rs | 2 +- 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/app/help.rs b/src/app/help.rs index a73c726f..dab618e9 100644 --- a/src/app/help.rs +++ b/src/app/help.rs @@ -683,7 +683,7 @@ impl<'a> Help<'a> { try!(write!(self.writer, "\n{}{}\n\n", TAB, - usage::create_help_usage(parser))); + usage::create_help_usage(parser, true))); let flags = parser.has_flags(); let pos = parser.has_positionals(); @@ -880,7 +880,7 @@ impl<'a> Help<'a> { parser.meta.about.unwrap_or("unknown about"))); } b"usage" => { - try!(write!(self.writer, "{}", usage::create_help_usage(parser))); + try!(write!(self.writer, "{}", usage::create_help_usage(parser, true))); } b"all-args" => { try!(self.write_all_args(&parser)); diff --git a/src/app/usage.rs b/src/app/usage.rs index cfd42119..bb66bcee 100644 --- a/src/app/usage.rs +++ b/src/app/usage.rs @@ -43,14 +43,14 @@ fn create_usage_no_title(p: &Parser, used: &[&str]) -> String { if let Some(u) = p.meta.usage_str { String::from(&*u) } else if used.is_empty() { - create_help_usage(p) + create_help_usage(p, true) } else { create_smart_usage(p, used) } } // Creates a usage string for display in help messages (i.e. not for errors) -pub fn create_help_usage(p: &Parser) -> String { +pub fn create_help_usage(p: &Parser, incl_reqs: bool) -> String { let mut usage = String::with_capacity(75); let name = p.meta .usage @@ -62,11 +62,15 @@ pub fn create_help_usage(p: &Parser) -> String { .unwrap_or(&p.meta.name) }); usage.push_str(&*name); - let mut reqs: Vec<&str> = p.required().map(|r| &**r).collect(); - reqs.dedup(); - let req_string = + let req_string = if incl_reqs { + let mut reqs: Vec<&str> = p.required().map(|r| &**r).collect(); + reqs.sort(); + reqs.dedup(); p.get_required_from(&reqs, None, None).iter().fold(String::new(), - |a, s| a + &format!(" {}", s)[..]); + |a, s| a + &format!(" {}", s)[..]) + } else { + String::new() + }; let flags = p.needs_flags_tag(); if flags && !p.is_set(AS::UnifiedHelpMessage) { @@ -75,9 +79,7 @@ pub fn create_help_usage(p: &Parser) -> String { usage.push_str(" [OPTIONS]"); } if !p.is_set(AS::UnifiedHelpMessage) && - p.opts.iter().any(|o| { - !o.is_set(ArgSettings::Required) && !o.is_set(ArgSettings::Hidden) - }) { + p.opts.iter().any(|o| !o.is_set(ArgSettings::Required) && !o.is_set(ArgSettings::Hidden)) { usage.push_str(" [OPTIONS]"); } @@ -86,15 +88,15 @@ pub fn create_help_usage(p: &Parser) -> String { // places a '--' in the usage string if there are args and options // supporting multiple values if p.has_positionals() && p.opts.iter().any(|o| o.is_set(ArgSettings::Multiple)) && - p.positionals.values().any(|p| !p.is_set(ArgSettings::Required)) && - !p.has_visible_subcommands() { + p.positionals.values().any(|p| !p.is_set(ArgSettings::Required)) && + !p.has_visible_subcommands() { usage.push_str(" [--]") } if p.has_positionals() && - p.positionals.values().any(|p| { - !p.is_set(ArgSettings::Required) && - !p.is_set(ArgSettings::Hidden) - }) { + p.positionals.values().any(|p| { + !p.is_set(ArgSettings::Required) && + !p.is_set(ArgSettings::Hidden) + }) { if let Some(args_tag) = p.get_args_tag() { usage.push_str(&*args_tag); } else { @@ -102,20 +104,24 @@ pub fn create_help_usage(p: &Parser) -> String { } } - - if p.is_set(AS::SubcommandsNegateReqs) || p.is_set(AS::ArgsNegateSubcommands) { - if p.has_visible_subcommands() { - usage.push_str("\n "); - usage.push_str(&*name); - usage.push_str(" "); - } - } else { - if p.has_visible_subcommands() && !p.is_set(AS::SubcommandRequired) { - usage.push_str(" [SUBCOMMAND]"); - } else if (p.is_set(AS::SubcommandRequired) || - p.is_set(AS::SubcommandRequiredElseHelp)) && - p.has_subcommands() { - usage.push_str(" "); + // incl_reqs is only false when this function is called recursively + if p.has_visible_subcommands() && incl_reqs { + if p.is_set(AS::SubcommandsNegateReqs) || p.is_set(AS::ArgsNegateSubcommands) { + if !p.is_set(AS::ArgsNegateSubcommands) { + usage.push_str("\n "); + usage.push_str(&*create_help_usage(p, false)); + usage.push_str(" "); + } else { + usage.push_str("\n "); + usage.push_str(&*name); + usage.push_str(" "); + } + } else { + if p.is_set(AS::SubcommandRequired) || p.is_set(AS::SubcommandRequiredElseHelp) { + usage.push_str(" "); + } else { + usage.push_str(" [SUBCOMMAND]"); + } } } usage.shrink_to_fit(); diff --git a/src/app/validator.rs b/src/app/validator.rs index 2b60e823..341b549e 100644 --- a/src/app/validator.rs +++ b/src/app/validator.rs @@ -54,7 +54,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { try!(self.validate_required(matcher)); } try!(self.validate_matched_args(matcher)); - matcher.usage(usage::create_help_usage(self.0)); + matcher.usage(usage::create_usage_with_title(self.0, &[])); if matcher.is_empty() && matcher.subcommand_name().is_none() && self.0.is_set(AS::ArgRequiredElseHelp) { From a2e31b27b0c28c61fb649b0cdd9c1350df962f1a Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 17:12:01 -0500 Subject: [PATCH 16/31] chore: increase version --- CHANGELOG.md | 41 +++++++++++++++++++++++++++++++++++++++++ Cargo.toml | 2 +- README.md | 15 ++++++++++++++- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43844f33..8d12fff0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,44 @@ + +## v2.21.0 (2017-03-09) + +#### Performance + +* doesn't run `arg_post_processing` on multiple values anymore ([ec516182](https://github.com/kbknapp/clap-rs/commit/ec5161828729f6a53f0fccec8648f71697f01f78)) +* changes internal use of `VecMap` to `Vec` for matched values of `Arg`s ([22bf137a](https://github.com/kbknapp/clap-rs/commit/22bf137ac581684c6ed460d2c3c640c503d62621)) +* vastly reduces the amount of cloning when adding non-global args minus when they're added from `App::args` which is forced to clone ([8da0303b](https://github.com/kbknapp/clap-rs/commit/8da0303bc02db5fe047cfc0631a9da41d9dc60f7)) +* refactor to remove unneeded vectors and allocations and checks for significant performance increases ([0efa4119](https://github.com/kbknapp/clap-rs/commit/0efa4119632f134fc5b8b9695b007dd94b76735d)) + +#### Documentation + +* Fix examples link in CONTRIBUTING.md ([60cf875d](https://github.com/kbknapp/clap-rs/commit/60cf875d67a252e19bb85054be57696fac2c57a1)) + +#### Improvements + +* when `AppSettings::SubcommandsNegateReqs` and `ArgsNegateSubcommands` are used, a new more accurate double line usage string is shown ([50f02300](https://github.com/kbknapp/clap-rs/commit/50f02300d81788817acefef0697e157e01b6ca32), closes [#871](https://github.com/kbknapp/clap-rs/issues/871)) + +#### API Additions + +* provides `default_value_os` and `default_value_if[s]_os` ([0f2a3782](https://github.com/kbknapp/clap-rs/commit/0f2a378219a6930748d178ba350fe5925be5dad5), closes [#849](https://github.com/kbknapp/clap-rs/issues/849)) + +#### New Settings + +* **InferSubcommands:** adds a setting to allow one to infer shortened subcommands or aliases (i.e. for subcommmand "test", "t", "te", or "tes" would be allowed assuming no other ambiguities) ([11602032](https://github.com/kbknapp/clap-rs/commit/11602032f6ff05881e3adf130356e37d5e66e8f9), closes [#863](https://github.com/kbknapp/clap-rs/issues/863)) + +#### Bug Fixes + +* doesn't print the argument sections in the help message if all args in that section are hidden ([ce5ee5f5](https://github.com/kbknapp/clap-rs/commit/ce5ee5f5a76f838104aeddd01c8ec956dd347f50)) +* doesn't include the various [ARGS] [FLAGS] or [OPTIONS] if the only ones available are hidden ([7b4000af](https://github.com/kbknapp/clap-rs/commit/7b4000af97637703645c5fb2ac8bb65bd546b95b), closes [#882](https://github.com/kbknapp/clap-rs/issues/882)) +* now correctly shows subcommand as required in the usage string when AppSettings::SubcommandRequiredElseHelp is used ([8f0884c1](https://github.com/kbknapp/clap-rs/commit/8f0884c1764983a49b45de52a1eddf8d721564d8)) +* fixes some memory leaks when an error is detected and clap exits ([8c2dd287](https://github.com/kbknapp/clap-rs/commit/8c2dd28718262ace4ae0db98563809548e02a86b)) +* fixes a trait that's marked private accidentlly, but should be crate internal public ([1ae21108](https://github.com/kbknapp/clap-rs/commit/1ae21108015cea87e5360402e1747025116c7878)) +* **Completions:** fixes a bug that tried to propogate global args multiple times when generating multiple completion scripts ([5e9b9cf4](https://github.com/kbknapp/clap-rs/commit/5e9b9cf4dd80fa66a624374fd04e6545635c1f94), closes [#846](https://github.com/kbknapp/clap-rs/issues/846)) + +#### Features + +* **Options:** adds the ability to require the equals syntax with options --opt=val ([f002693d](https://github.com/kbknapp/clap-rs/commit/f002693dec6a6959c4e9590cb7b7bfffd6d6e5bc), closes [#833](https://github.com/kbknapp/clap-rs/issues/833)) + + + ### v2.20.5 (2017-02-18) diff --git a/Cargo.toml b/Cargo.toml index 4fe979c0..da02d8a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "clap" -version = "2.21.0-beta" +version = "2.21.0" authors = ["Kevin K. "] exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"] repository = "https://github.com/kbknapp/clap-rs.git" diff --git a/README.md b/README.md index 7b232784..23aadb3b 100644 --- a/README.md +++ b/README.md @@ -45,9 +45,22 @@ Created by [gh-md-toc](https://github.com/ekalinin/github-markdown-toc) ## What's New -Here's the highlights for v2.20.5 +Here's the highlights for v2.21.0 +* Some performance improvements by reducing the ammount of duplicate work, cloning, and allocations in all cases. +* Some massive perfomance gains when using many args (i.e. things like shell glob expansions) +* adds a setting to allow one to infer shortened subcommands or aliases (i.e. for subcommmand "test", "t", "te", or "tes" would be allowed assuming no other ambiguities) +* when `AppSettings::SubcommandsNegateReqs` and `ArgsNegateSubcommands` are used, a new more accurate double line usage string is shown +* provides `default_value_os` and `default_value_if[s]_os` +* adds the ability to require the equals syntax with options `--opt=val` +* doesn't print the argument sections in the help message if all args in that section are hidden +* doesn't include the various `[ARGS]` `[FLAGS]` or `[OPTIONS]` if the only ones available are hidden +* now correctly shows subcommand as required in the usage string when AppSettings::SubcommandRequiredElseHelp is used +* fixes some "memory leaks" when an error is detected and clap exits +* fixes a trait that's marked private accidentlly, but should be crate internal public +* fixes a bug that tried to propogate global args multiple times when generating multiple completion scripts * Fixes a critical bug in the `clap_app!` macro of a missing fragment specifier when using `!property` style tags. +* Fix examples link in CONTRIBUTING.md Here's the highlights from v2.0.0 to v2.20.4 From 06e07ad8e6aadde6d76c41be6250051e01682912 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 17:13:52 -0500 Subject: [PATCH 17/31] docs(README.md): removes the old updates from previous versions as it's getting very long and hard to read --- README.md | 122 ------------------------------------------------------ 1 file changed, 122 deletions(-) diff --git a/README.md b/README.md index 23aadb3b..6bb52190 100644 --- a/README.md +++ b/README.md @@ -62,128 +62,6 @@ Here's the highlights for v2.21.0 * Fixes a critical bug in the `clap_app!` macro of a missing fragment specifier when using `!property` style tags. * Fix examples link in CONTRIBUTING.md - -Here's the highlights from v2.0.0 to v2.20.4 - -* Fixes a bug that tried to propogate global args multiple times when generating multiple completion scripts -* Fix examples link in CONTRIBUTING.md -* **Completions**: fixes bash completions for commands that have an underscore in the name -* **Completions**: fixes a bug where ZSH completions would panic if the binary name had an underscore in it -* allow final word to be wrapped in wrap_help -* **Completions**: fixes a bug where global args weren't included in the generated completion scripts -* **Macros Documentation:** adds a warning about changing values in Cargo.toml not triggering a rebuild automatically -* Fixes a critical bug where subcommand settings were being propogated too far -* Adds ArgGroup::multiple to the supported YAML fields for building ArgGroups from YAML -* Fixes a bug where the final word wasn't wrapped in help messages -* Fixes finding required arguments in group arguments -* **ArgsNegateSubcommands:** disables args being allowed between subcommands -* **DontCollapseArgsInUsage:** disables the collapsing of positional args into `[ARGS]` in the usage string -* **DisableHelpSubcommand:** disables building the `help` subcommand -* **AllowMissingPositional:** allows one to implement `$ prog [optional] ` style CLIs where the second postional argument is required, but the first is optional -* **PropagateGlobalValuesDown:** automatically propagats global arg's values down through *used* subcommands -* **Arg::value_terminator:** adds the ability to terminate multiple values with a given string or char -* **Arg::default_value_if[s]:** adds new methods for *conditional* default values (such as a particular value from another argument was used) -* **Arg::requires_if[s]:** adds the ability to *conditionally* require additional args (such as if a particular value was used) -* **Arg::required_if[s]:** adds the ability for an arg to be *conditionally* required (i.e. "arg X is only required if arg Y was used with value Z") -* **Arg::validator_os:** adds ability to validate values which may contain invalid UTF-8 -* **crate_description!:** Uses the `Cargo.toml` description field to fill in the `App::about` method at compile time -* **crate_name!:** Uses the `Cargo.toml` name field to fill in the `App::new` method at compile time -* **app_from_crate!:** Combines `crate_version!`, `crate_name!`, `crate_description!`, and `crate_authors!` into a single macro call to build a default `App` instance from the `Cargo.toml` fields -* **no_cargo:** adds a `no_cargo` feature to disable Cargo-env-var-dependent macros for those *not* using `cargo` to build their crates -* **Options:** fixes a critical bug where options weren't forced to have a value -* fixes a bug where calling the help of a subcommand wasn't ignoring required args of parent commands -* **Help Subcommand:** fixes a bug where the help subcommand couldn't be overriden -* **Low Index Multiples:** fixes a bug which caused combinations of LowIndexMultiples and `Arg::allow_hyphen_values` to fail parsing -* **Default Values:** improves the error message when default values are involved -* **YAML:** adds conditional requirements and conditional default values to YAML -* Support `--("some-arg-name")` syntax for defining long arg names when using `clap_app!` macro -* Support `("some app name")` syntax for defining app names when using `clap_app!` macro -* **Help Wrapping:** long app names (with spaces), authors, and descriptions are now wrapped appropriately -* **Conditional Default Values:** fixes the failing doc tests of Arg::default_value_ifs -* **Conditional Requirements:** adds docs for Arg::requires_ifs -* Fixes a bug where calling the help of a subcommand wasn't ignoring required args of parent commands -* Fixes a bug by escaping square brackets in ZSH completions which were causing conflicts and errors. -* **Bash Completion:** allows bash completion to fall back to traidtional bash completion upon no matching completing function -* **Arg Setting**: Allows specifying an `AllowLeadingHyphen` style setting for values only for specific args, vice command wide -* **Validators:** improves the error messages for validators -* **Required Unless:** fixes a bug where having required_unless set doesn't work when conflicts are also set -* **ZSH Completions:** fixes an issue where zsh completions caused panics if there were no subcommands -* **Completions:** Adds completion support for Microsoft PowerShell! (Thanks to @Arnavion) -* Allows specifying the second to last positional argument as `multiple(true)` (i.e. things such as `mv ... `) -* Adds an `App::get_name` and `App::get_bin_name` -* Conflicting argument errors are now symetrical, meaning more consistent and better usage suggestions -* **Completions:** adds automatic ZSH completion script generation support! :tada: :tada: -* **AppSettings:** adds new setting `AppSettings::AllowNegativeNumbers` which functions like `AllowLeadingHyphen` except only allows undefined negative numbers to pass parsing. -* Stabilize `clap_app!` macro (i.e. no longer need to use `unstable` feature) -* Deprecate `App::with_defaults` -* One can now alias arguments either visibly (which appears in the help text) or invisibly just like subcommands! -* The `from_usage` parser now correctly handles non-ascii names / options and help! -* **Value Delimiters:** fixes the confusion around implicitly setting value delimiters. (The default is to *not* use a delimiter unless explicitly set) -* Changes the default value delimiter rules (i.e. the default is `use_delimiter(false)` *unless* a setting/method that implies multiple values was used) **[Bugfix that *may* "break" code]** - * If code breaks, simply add `Arg::use_delimiter(true)` to the affected args -* Adds ability to hide the possible values from the help text on a per argument basis, instead of command wide -* Allows for limiting detected terminal width (i.e. wrap at `x` length, unless the terminal width is *smaller*) -* `clap` now ignores hard newlines in help messages and properly re-aligns text, but still wraps if the term width is too small -* Adds support for the setting `Arg::require_delimiter` from YAML -* `clap` no longer requires one to use `{n}` inside help text to insert a newline that is properly aligned. One can now use the normal `\n`. -* `clap` now ignores hard newlines in help messages and properly re-aligns text, but still wraps if the term width is too small -* Errors can now have custom description -* Uses `term_size` instead of home-grown solution on Windows -* Adds the ability to wrap help text intelligently on Windows! -* Moves docs to [docs.rs!](https://docs.rs/clap/)! -* Automatically moves help text to the next line and wraps when term width is determined to be too small, or help text is too long -* Vastly improves *development* error messages when using YAML -* Adds a shorthand way to ignore help text wrapping and use source formatting (i.e. `App::set_term_width(0)`) -* **Help Subcommand:** fixes misleading usage string when using multi-level subcommmands such as `myprog help subcmd1 subcmd2` -* **YAML:** allows using lists or single values with certain arg declarations for increased ergonomics -* **Fish Shell Completions:** one can generate a basic fish completions script at compile time! -* Adds the ability to generate completions to an `io::Write` object -* Adds an `App::unset_setting` and `App::unset_settings` -* **Completions:** one can now [generate a bash completions](https://docs.rs/clap/2.9.0/clap/struct.App.html#method.gen_completions) script at compile time! These completions work with options using [possible values](https://docs.rs/clap/2.9.0/clap/struct.Arg.html#method.possible_values), [subcommand aliases](https://docs.rs/clap/2.9.0/clap/struct.App.html#method.aliases), and even multiple levels of subcommands -* **Arg:** adds new optional setting [`Arg::require_delimiter`](https://docs.rs/clap/2.8.0/clap/struct.Arg.html#method.require_delimiter) which requires val delimiter to parse multiple values -* The terminal sizing portion has been factored out into a separate crate, [term_size](https://crates.io/crates/term_size) -* Options using multiple values and delimiters no longer parse additional values after a trailing space (i.e. `prog -o 1,2 file.txt` parses as `1,2` for `-o` and `file.txt` for a positional arg) -* Using options using multiple values and with an `=` no longer parse args after the trailing space as values (i.e. `prog -o=1 file.txt` parses as `1` for `-o` and `file.txt` for a positional arg) -* **Usage Strings:** `[FLAGS]` and `[ARGS]` are no longer blindly added to usage strings, instead only when applicable -* `arg_enum!`: allows using more than one meta item, or things like `#[repr(C)]` with `arg_enum!`s -* `App::print_help`: now prints the same as would have been printed by `--help` or the like -* Prevents invoking ` help help` and displaying incorrect help message -* Subcommand help messages requested via ` help ` now correctly match ` --help` -* One can now specify groups which require AT LEAST one of the args -* Allows adding multiple ArgGroups per Arg -* **Global Settings:** One can now set an `AppSetting` which is propogated down through child subcommands -* **Terminal Wrapping:** Allows wrapping at specified term width (Even on Windows!) (can now set an absolute width to "smart" wrap at) -* **SubCommands/Aliases:** adds support for visible aliases for subcommands (i.e. aliases that are dipslayed in the help message) -* **Subcommands/Aliases:** when viewing the help of an alias, it now display help of the aliased subcommand -* Adds new setting to stop delimiting values with `--` or `AppSettings::TrailingVarArg` -* Subcommands now support aliases - think of them as hidden subcommands that dispatch to said subcommand automatically -* Fixed times when `ArgGroup`s are duplicated in usage strings -* **Before Help:** adds support for displaying info before help message -* **Required Unless:** adds support for allowing args that are required unless certain other args are present -* **New Help Template Engine!**: Now you have full control over the layout of your help message. Major thanks to @hgrecco -* **Pull crate Authors from Cargo.toml**: One can now use the `crate_authors!` macro to automatically pull the crate authors from their Cargo.toml file -* **Colored Help Messages**: Help messages can now be optionally colored (See the `AppSettings::ColoredHelp` setting). Screenshot below. -* **Help text auto wraps and aligns at for subcommands too!** - Long help strings of subcommands will now properly wrap and align to term width on Linux and OS X. This can be turned off as well. -* **Help text auto wraps and aligns at term width!** - Long help strings will now properly wrap and align to term width on Linux and OS X (and presumably Unix too). This can be turned off as well. -* **Can customize the order of opts, flags, and subcommands in help messages** - Instead of using the default alphabetical order, you can now re-arrange the order of your args and subcommands in help message. This helps to emphasize more popular or important options. -* **Can auto-derive the order from declaration order** - Have a bunch of args or subcommmands to re-order? You can now just derive the order from the declaration order! -* **Help subcommand now accepts other subcommands as arguments!** - Similar to other CLI precedents, the `help` subcommand can now accept other subcommands as arguments to display their help message. i.e. `$ myprog help mysubcmd` (*Note* these can even be nested heavily such as `$ myprog help subcmd1 subcmd2 subcmd3` etc.) -* **Default Values**: Args can now specify default values -* **Next Line Help**: Args can have help strings on the line following the argument (useful for long arguments, or those with many values). This can be set command-wide or for individual args - -Here's a gif of them in action! - -![zsh-comppletions](http://i.imgur.com/rwlMbAv.gif) - -An example of the help text wrapping at term width: - -![screenshot](http://i.imgur.com/PAJzJJG.png) - -An example of the optional colored help: - -![screenshot](http://i.imgur.com/7fs2h5j.png) - - For full details, see [CHANGELOG.md](https://github.com/kbknapp/clap-rs/blob/master/CHANGELOG.md) ## About From d484da0d287f7ea4fcde7d1e9174162463f93624 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 17:28:32 -0500 Subject: [PATCH 18/31] fix: fixes a misspelled import for Windows --- src/app/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index 6d574c6d..d845da03 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -739,7 +739,7 @@ impl<'a, 'b> Parser<'a, 'b> #[cfg(not(target_os = "windows"))] use std::os::unix::ffi::OsStrExt; #[cfg(target_os = "windows")] - use ossstringext::OsStrExt3; + use osstringext::OsStrExt3; let n_bytes = n.as_bytes(); let h_bytes = OsStr::new(h).as_bytes(); From 5c76350f77cee45c9ff2c6e37baf482d985f1ecc Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 17:53:36 -0500 Subject: [PATCH 19/31] tests: fixes failing hidden args test --- tests/hidden_args.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/hidden_args.rs b/tests/hidden_args.rs index 80dab45b..cbc9225b 100644 --- a/tests/hidden_args.rs +++ b/tests/hidden_args.rs @@ -10,7 +10,7 @@ Kevin K. tests stuff USAGE: - test [FLAGS] [OPTIONS] [DUMMY] + test [FLAGS] [OPTIONS] FLAGS: -F, --flag2 some other flag From b841f3743fb6b52f2777ca39f8852de794e99bd4 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 20:23:45 -0500 Subject: [PATCH 20/31] fix: fixes a failing build with no-default-features --- src/suggestions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/suggestions.rs b/src/suggestions.rs index 107e9579..70e14aad 100644 --- a/src/suggestions.rs +++ b/src/suggestions.rs @@ -32,7 +32,7 @@ pub fn did_you_mean<'a, T, I>(v: &str, possible_values: I) -> Option<&'a str> #[cfg(not(feature = "suggestions"))] pub fn did_you_mean<'a, T, I>(_: &str, _: I) -> Option<&'a str> - where T: AsRef + 'a, + where T: AsRef + 'a + ?Sized, I: IntoIterator { None From d2b4c2c61b61942137c8a307eed4ac1f81564481 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 21:20:19 -0500 Subject: [PATCH 21/31] fix: fixes false positive clean parse when the suggestions feature is disabled and InferSubcommands is enabled --- src/app/parser.rs | 12 ++++++++++-- tests/app_settings.rs | 34 ++++++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index d845da03..a5cfd6ee 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -940,6 +940,7 @@ impl<'a, 'b> Parser<'a, 'b> // Does the arg match a subcommand name, or any of it's aliases (if defined) { let (is_match, sc_name) = self.possible_subcommand(&arg_os); + debugln!("Parser::get_matches_with: possible_sc={:?}, sc={:?}", is_match, sc_name); if is_match { let sc_name = sc_name.expect(INTERNAL_ERROR_MSG); if sc_name == "help" && self.is_set(AS::NeedsSubcommandHelp) { @@ -1094,7 +1095,7 @@ impl<'a, 'b> Parser<'a, 'b> matcher, None), self.color())); - } else if !(has_args) && self.has_subcommands() { + } else if !has_args || self.is_set(AS::InferSubcommands) && self.has_subcommands() { if let Some(cdate) = suggestions::did_you_mean(&*arg_os.to_string_lossy(), sc_names!(self)) { return Err(Error::invalid_subcommand(arg_os.to_string_lossy().into_owned(), @@ -1107,6 +1108,13 @@ impl<'a, 'b> Parser<'a, 'b> matcher, None), self.color())); + } else { + return Err(Error::unrecognized_subcommand(arg_os.to_string_lossy().into_owned(), + self.meta + .bin_name + .as_ref() + .unwrap_or(&self.meta.name), + self.color())); } } } @@ -1132,7 +1140,7 @@ impl<'a, 'b> Parser<'a, 'b> None), self.color())); } else if self.is_set(AS::SubcommandRequiredElseHelp) { - debugln!("parser::get_matches_with: SubcommandRequiredElseHelp=true"); + debugln!("Parser::get_matches_with: SubcommandRequiredElseHelp=true"); let mut out = vec![]; try!(self.write_help_err(&mut out)); return Err(Error { diff --git a/tests/app_settings.rs b/tests/app_settings.rs index f933b3bc..644a863b 100644 --- a/tests/app_settings.rs +++ b/tests/app_settings.rs @@ -110,6 +110,7 @@ fn arg_required_else_help() { assert_eq!(err.kind, ErrorKind::MissingArgumentOrSubcommand); } +#[cfg(not(feature = "suggestions"))] #[test] fn infer_subcommands_fail_no_args() { let m = App::new("prog") @@ -119,7 +120,21 @@ fn infer_subcommands_fail_no_args() { .get_matches_from_safe(vec![ "prog", "te" ]); - assert!(m.is_err()); + assert!(m.is_err(), "{:#?}", m.unwrap()); + assert_eq!(m.unwrap_err().kind, ErrorKind::UnrecognizedSubcommand); +} + +#[cfg(feature = "suggestions")] +#[test] +fn infer_subcommands_fail_no_args() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(SubCommand::with_name("test")) + .subcommand(SubCommand::with_name("temp")) + .get_matches_from_safe(vec![ + "prog", "te" + ]); + assert!(m.is_err(), "{:#?}", m.unwrap()); assert_eq!(m.unwrap_err().kind, ErrorKind::InvalidSubcommand); } @@ -174,6 +189,7 @@ fn infer_subcommands_pass_close() { assert_eq!(m.subcommand_name(), Some("test")); } +#[cfg(feature = "suggestions")] #[test] fn infer_subcommands_fail_suggestions() { let m = App::new("prog") @@ -183,10 +199,24 @@ fn infer_subcommands_fail_suggestions() { .get_matches_from_safe(vec![ "prog", "temps" ]); - assert!(m.is_err()); + assert!(m.is_err(), "{:#?}", m.unwrap()); assert_eq!(m.unwrap_err().kind, ErrorKind::InvalidSubcommand); } +#[cfg(not(feature = "suggestions"))] +#[test] +fn infer_subcommands_fail_suggestions() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(SubCommand::with_name("test")) + .subcommand(SubCommand::with_name("temp")) + .get_matches_from_safe(vec![ + "prog", "temps" + ]); + assert!(m.is_err(), "{:#?}", m.unwrap()); + assert_eq!(m.unwrap_err().kind, ErrorKind::UnrecognizedSubcommand); +} + #[test] fn no_bin_name() { let result = App::new("arg_required") From 1e4cce0291782c6697c54f11bcf7f0cb97170c49 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 21:21:19 -0500 Subject: [PATCH 22/31] tests: corrects the debug messages from the usage module --- src/app/usage.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/usage.rs b/src/app/usage.rs index bb66bcee..3d79bf38 100644 --- a/src/app/usage.rs +++ b/src/app/usage.rs @@ -6,7 +6,7 @@ use app::parser::Parser; // Creates a usage string for display. This happens just after all arguments were parsed, but before // any subcommands have been parsed (so as to give subcommands their own usage recursively) pub fn create_usage_with_title(p: &Parser, used: &[&str]) -> String { - debugln!("Parser::create_usage_with_title;"); + debugln!("usage::create_usage_with_title;"); let mut usage = String::with_capacity(75); usage.push_str("USAGE:\n "); usage.push_str(&*create_usage_no_title(p, used)); @@ -39,7 +39,7 @@ pub fn create_error_usage<'a, 'b>(p: &Parser<'a, 'b>, // Creates a usage string (*without title*) if one was not provided by the user manually. fn create_usage_no_title(p: &Parser, used: &[&str]) -> String { - debugln!("Parser::create_usage_no_title;"); + debugln!("usage::create_usage_no_title;"); if let Some(u) = p.meta.usage_str { String::from(&*u) } else if used.is_empty() { @@ -131,7 +131,7 @@ pub fn create_help_usage(p: &Parser, incl_reqs: bool) -> String { // Creates a context aware usage string, or "smart usage" from currently used // args, and requirements fn create_smart_usage(p: &Parser, used: &[&str]) -> String { - debugln!("Parser::smart_usage;"); + debugln!("usage::smart_usage;"); let mut usage = String::with_capacity(75); let mut hs: Vec<&str> = p.required().map(|s| &**s).collect(); hs.extend_from_slice(used); From 075036c28de3405cb4f0728b29339d06e25e1627 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 21:47:53 -0500 Subject: [PATCH 23/31] fix: fixes a regression 1.11.0 feature --- src/suggestions.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/suggestions.rs b/src/suggestions.rs index 70e14aad..9a62be13 100644 --- a/src/suggestions.rs +++ b/src/suggestions.rs @@ -11,8 +11,8 @@ use fmt::Format; /// `Some("foo")`, whereas "blark" would yield `None`. #[cfg(feature = "suggestions")] #[cfg_attr(feature = "lints", allow(needless_lifetimes))] -pub fn did_you_mean<'a, T, I>(v: &str, possible_values: I) -> Option<&'a str> - where T: AsRef + 'a + ?Sized, +pub fn did_you_mean<'a, T: ?Sized, I>(v: &str, possible_values: I) -> Option<&'a str> + where T: AsRef + 'a, I: IntoIterator { @@ -31,8 +31,8 @@ pub fn did_you_mean<'a, T, I>(v: &str, possible_values: I) -> Option<&'a str> } #[cfg(not(feature = "suggestions"))] -pub fn did_you_mean<'a, T, I>(_: &str, _: I) -> Option<&'a str> - where T: AsRef + 'a + ?Sized, +pub fn did_you_mean<'a, T: ?Sized, I>(_: &str, _: I) -> Option<&'a str> + where T: AsRef + 'a, I: IntoIterator { None From 9d4535e1c3c659216ba8f76bd48a654e23344a82 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 9 Mar 2017 22:32:32 -0500 Subject: [PATCH 24/31] chore: clippy run --- Cargo.toml | 4 ++-- src/app/parser.rs | 15 +++++++------- src/app/usage.rs | 17 ++++++---------- src/app/validator.rs | 43 ++++++++++++++++++++--------------------- src/args/arg_matches.rs | 6 +++--- src/args/group.rs | 1 + src/lib.rs | 1 + src/suggestions.rs | 1 + 8 files changed, 43 insertions(+), 45 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index da02d8a2..dd44b388 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,9 +22,9 @@ unicode-width = "0.1.4" unicode-segmentation = "1.0.1" strsim = { version = "0.6.0", optional = true } ansi_term = { version = "0.9.0", optional = true } -term_size = { version = "0.2.2", optional = true } +term_size = { version = "0.2.3", optional = true } yaml-rust = { version = "0.3.5", optional = true } -clippy = { version = "~0.0.112", optional = true } +clippy = { version = "~0.0.118", optional = true } atty = { version = "0.2.2", optional = true } [dev-dependencies] diff --git a/src/app/parser.rs b/src/app/parser.rs index a5cfd6ee..28ad47c0 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -217,10 +217,10 @@ impl<'a, 'b> Parser<'a, 'b> } // actually adds the arguments but from a borrow (which means we have to do some clonine) pub fn add_arg_ref(&mut self, a: &Arg<'a, 'b>) { - self.debug_asserts(&a); - self.add_conditional_reqs(&a); - self.add_arg_groups(&a); - self.add_reqs(&a); + self.debug_asserts(a); + self.add_conditional_reqs(a); + self.add_arg_groups(a); + self.add_reqs(a); if a.index.is_some() || (a.s.short.is_none() && a.s.long.is_none()) { let i = if a.index.is_none() { (self.positionals.len() + 1) @@ -779,7 +779,7 @@ impl<'a, 'b> Parser<'a, 'b> return (true, Some(v[0])); } } - return (false, None); + (false, None) } fn parse_help_subcommand(&self, it: &mut I) -> ClapResult<()> @@ -1324,7 +1324,7 @@ impl<'a, 'b> Parser<'a, 'b> .expect(INTERNAL_ERROR_MSG) .args { if self.groups.iter().any(|g| &g.name == &*n) { - args.extend(self.arg_names_in_group(&*n)); + args.extend(self.arg_names_in_group(n)); g_vec.push(*n); } else { args.push(*n); @@ -1504,6 +1504,7 @@ impl<'a, 'b> Parser<'a, 'b> self.did_you_mean_error(arg.to_str().expect(INVALID_UTF8), matcher).map(|_| None) } + #[cfg_attr(feature = "lints", allow(len_zero))] fn parse_short_arg(&mut self, matcher: &mut ArgMatcher<'a>, full_arg: &OsStr) @@ -1882,7 +1883,7 @@ impl<'a, 'b> Parser<'a, 'b> } // Only used for completion scripts due to bin_name messiness - #[cfg_attr(feature = "lints", allow(explicit_iter_loop))] + #[cfg_attr(feature = "lints", allow(block_in_if_condition_stmt))] pub fn find_subcommand(&'b self, sc: &str) -> Option<&'b App<'a, 'b>> { debugln!("Parser::find_subcommand: sc={}", sc); debugln!("Parser::find_subcommand: Currently in Parser...{}", diff --git a/src/app/usage.rs b/src/app/usage.rs index 3d79bf38..72631cd9 100644 --- a/src/app/usage.rs +++ b/src/app/usage.rs @@ -1,4 +1,4 @@ -use args::{AnyArg, ArgMatcher}; +use args::{AnyArg, ArgMatcher, PosBuilder}; use args::settings::ArgSettings; use app::settings::AppSettings as AS; use app::parser::Parser; @@ -92,11 +92,8 @@ pub fn create_help_usage(p: &Parser, incl_reqs: bool) -> String { !p.has_visible_subcommands() { usage.push_str(" [--]") } - if p.has_positionals() && - p.positionals.values().any(|p| { - !p.is_set(ArgSettings::Required) && - !p.is_set(ArgSettings::Hidden) - }) { + let not_req_or_hidden= |p: &PosBuilder| !p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Hidden); + if p.has_positionals() && p.positionals.values().any(not_req_or_hidden) { if let Some(args_tag) = p.get_args_tag() { usage.push_str(&*args_tag); } else { @@ -116,12 +113,10 @@ pub fn create_help_usage(p: &Parser, incl_reqs: bool) -> String { usage.push_str(&*name); usage.push_str(" "); } + } else if p.is_set(AS::SubcommandRequired) || p.is_set(AS::SubcommandRequiredElseHelp) { + usage.push_str(" "); } else { - if p.is_set(AS::SubcommandRequired) || p.is_set(AS::SubcommandRequiredElseHelp) { - usage.push_str(" "); - } else { - usage.push_str(" [SUBCOMMAND]"); - } + usage.push_str(" [SUBCOMMAND]"); } } usage.shrink_to_fit(); diff --git a/src/app/validator.rs b/src/app/validator.rs index 341b549e..9130751a 100644 --- a/src/app/validator.rs +++ b/src/app/validator.rs @@ -42,8 +42,8 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { if should_err { return Err(Error::empty_value(o, &*usage::create_error_usage(self.0, - matcher, - None), + matcher, + None), self.0.color())); } } @@ -81,9 +81,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { if self.0.is_set(AS::StrictUtf8) && val.to_str().is_none() { debugln!("Validator::validate_values: invalid UTF-8 found in val {:?}", val); - return Err(Error::invalid_utf8(&*usage::create_error_usage(self.0, - matcher, - None), + return Err(Error::invalid_utf8(&*usage::create_error_usage(self.0, matcher, None), self.0.color())); } if let Some(p_vals) = arg.possible_vals() { @@ -94,8 +92,8 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { p_vals, arg, &*usage::create_error_usage(self.0, - matcher, - None), + matcher, + None), self.0.color())); } } @@ -117,7 +115,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { } if let Some(vtor) = arg.validator_os() { debug!("Validator::validate_values: checking validator_os..."); - if let Err(e) = vtor(&val) { + if let Err(e) = vtor(val) { sdebugln!("error"); return Err(Error::value_validation(Some(arg), (*e).to_string_lossy().to_string(), @@ -236,8 +234,8 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { // Not the first time, and we don't allow multiples return Err(Error::unexpected_multiple_usage(a, &*usage::create_error_usage(self.0, - matcher, - None), + matcher, + None), self.0.color())); } Ok(()) @@ -276,8 +274,8 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { "ere" }, &*usage::create_error_usage(self.0, - matcher, - None), + matcher, + None), self.0.color())); } } @@ -292,7 +290,9 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { .to_str() .expect(INVALID_UTF8), a, - &*usage::create_error_usage(self.0, matcher, None), + &*usage::create_error_usage(self.0, + matcher, + None), self.0.color())); } } @@ -303,7 +303,9 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { return Err(Error::too_few_values(a, num, ma.vals.len(), - &*usage::create_error_usage(self.0, matcher, None), + &*usage::create_error_usage(self.0, + matcher, + None), self.0.color())); } } @@ -326,10 +328,9 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { debugln!("Validator::validate_arg_requires;"); if let Some(a_reqs) = a.requires() { for &(val, name) in a_reqs.iter().filter(|&&(val, _)| val.is_some()) { - if ma.vals.iter().any(|v| { - v == val.expect(INTERNAL_ERROR_MSG) && - !matcher.contains(name) - }) { + let missing_req = + |v| v == val.expect(INTERNAL_ERROR_MSG) && !matcher.contains(name); + if ma.vals.iter().any(missing_req) { return self.missing_required_error(matcher, None); } } @@ -364,10 +365,8 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { // Validate the conditionally required args for &(a, v, r) in &self.0.r_ifs { if let Some(ma) = matcher.get(a) { - if matcher.get(r).is_none() { - if ma.vals.iter().any(|val| val == v) { - return self.missing_required_error(matcher, Some(r)); - } + if matcher.get(r).is_none() && ma.vals.iter().any(|val| val == v) { + return self.missing_required_error(matcher, Some(r)); } } } diff --git a/src/args/arg_matches.rs b/src/args/arg_matches.rs index 3a60c773..aec7f4da 100644 --- a/src/args/arg_matches.rs +++ b/src/args/arg_matches.rs @@ -110,7 +110,7 @@ impl<'a> ArgMatches<'a> { /// [`panic!`]: https://doc.rust-lang.org/std/macro.panic!.html pub fn value_of>(&self, name: S) -> Option<&str> { if let Some(arg) = self.args.get(name.as_ref()) { - if let Some(v) = arg.vals.iter().nth(0) { + if let Some(v) = arg.vals.get(0) { return Some(v.to_str().expect(INVALID_UTF8)); } } @@ -142,7 +142,7 @@ impl<'a> ArgMatches<'a> { /// [`Arg::values_of_lossy`]: ./struct.ArgMatches.html#method.values_of_lossy pub fn value_of_lossy>(&'a self, name: S) -> Option> { if let Some(arg) = self.args.get(name.as_ref()) { - if let Some(v) = arg.vals.iter().nth(0) { + if let Some(v) = arg.vals.get(0) { return Some(v.to_string_lossy()); } } @@ -179,7 +179,7 @@ impl<'a> ArgMatches<'a> { pub fn value_of_os>(&self, name: S) -> Option<&OsStr> { self.args .get(name.as_ref()) - .map_or(None, |arg| arg.vals.iter().nth(0).map(|v| v.as_os_str())) + .map_or(None, |arg| arg.vals.get(0).map(|v| v.as_os_str())) } /// Gets a [`Values`] struct which implements [`Iterator`] for values of a specific argument diff --git a/src/args/group.rs b/src/args/group.rs index d6cb9eb4..4ff2638a 100644 --- a/src/args/group.rs +++ b/src/args/group.rs @@ -152,6 +152,7 @@ impl<'a> ArgGroup<'a> { /// assert!(m.is_present("flag")); /// ``` /// [argument]: ./struct.Arg.html + #[cfg_attr(feature = "lints", allow(should_assert_eq))] pub fn arg(mut self, n: &'a str) -> Self { assert!(self.name != n, "ArgGroup '{}' can not have same name as arg inside it", diff --git a/src/lib.rs b/src/lib.rs index c39f232e..256c1420 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -527,6 +527,7 @@ #![cfg_attr(feature = "lints", deny(warnings))] #![cfg_attr(feature = "lints", allow(cyclomatic_complexity))] #![cfg_attr(feature = "lints", allow(doc_markdown))] +#![cfg_attr(feature = "lints", allow(explicit_iter_loop))] #[cfg(feature = "suggestions")] extern crate strsim; diff --git a/src/suggestions.rs b/src/suggestions.rs index 9a62be13..5c7cade5 100644 --- a/src/suggestions.rs +++ b/src/suggestions.rs @@ -68,6 +68,7 @@ pub fn did_you_mean_suffix<'z, T, I>(arg: &str, } /// A helper to determine message formatting +#[derive(Copy, Clone, Debug)] pub enum DidYouMeanMessageStyle { /// Suggested value is a long flag LongFlag, From e802a472dec8a2e671cc493fb8d006c0accae1f7 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 10 Mar 2017 08:29:57 -0500 Subject: [PATCH 25/31] docs(CHANGELOG.md): adds the details about ability to change the help message for the auto-generated help/version --- CHANGELOG.md | 1 + README.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d12fff0..58e499fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ #### API Additions * provides `default_value_os` and `default_value_if[s]_os` ([0f2a3782](https://github.com/kbknapp/clap-rs/commit/0f2a378219a6930748d178ba350fe5925be5dad5), closes [#849](https://github.com/kbknapp/clap-rs/issues/849)) +* provides `App::help_message` and `App::version_message` which allows one to override the auto-generated help/version flag associated help ([389c413](https://github.com/kbknapp/clap-rs/commit/389c413b7023dccab8c76aa00577ea1d048e7a99), closes [#889](https://github.com/kbknapp/clap-rs/issues/889)) #### New Settings diff --git a/README.md b/README.md index 6bb52190..e5797765 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,7 @@ Here's the highlights for v2.21.0 * adds a setting to allow one to infer shortened subcommands or aliases (i.e. for subcommmand "test", "t", "te", or "tes" would be allowed assuming no other ambiguities) * when `AppSettings::SubcommandsNegateReqs` and `ArgsNegateSubcommands` are used, a new more accurate double line usage string is shown * provides `default_value_os` and `default_value_if[s]_os` +* provides `App::help_message` and `App::version_message` which allows one to override the auto-generated help/version flag associated help * adds the ability to require the equals syntax with options `--opt=val` * doesn't print the argument sections in the help message if all args in that section are hidden * doesn't include the various `[ARGS]` `[FLAGS]` or `[OPTIONS]` if the only ones available are hidden From 989862d2cbae1a1b3a1b9f9cca24385de834ecfe Mon Sep 17 00:00:00 2001 From: Kevin K Date: Fri, 10 Mar 2017 15:27:24 -0500 Subject: [PATCH 26/31] chore: fixes small rebase regression --- src/app/mod.rs | 4 ++-- src/app/parser.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index c24b4d4d..9fa181ee 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -407,7 +407,7 @@ impl<'a, 'b> App<'a, 'b> { /// # ; /// ``` pub fn help_message>(mut self, s: S) -> Self { - self.p.help_message(s.into()); + self.p.help_message = Some(s.into()); self } @@ -425,7 +425,7 @@ impl<'a, 'b> App<'a, 'b> { /// # ; /// ``` pub fn version_message>(mut self, s: S) -> Self { - self.p.version_message(s.into()); + self.p.version_message = Some(s.into()); self } diff --git a/src/app/parser.rs b/src/app/parser.rs index 28ad47c0..6153e274 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -56,8 +56,8 @@ pub struct Parser<'a, 'b> help_short: Option, version_short: Option, cache: Option<&'a str>, - help_message: Option<&'a str>, - version_message: Option<&'a str>, + pub help_message: Option<&'a str>, + pub version_message: Option<&'a str>, } impl<'a, 'b> Parser<'a, 'b> @@ -1348,7 +1348,7 @@ impl<'a, 'b> Parser<'a, 'b> let arg = FlagBuilder { b: Base { name: "hclap_help", - help: Some("Prints help information"), + help: self.help_message.or(Some("Prints help information")), ..Default::default() }, s: Switched { @@ -1368,7 +1368,7 @@ impl<'a, 'b> Parser<'a, 'b> let arg = FlagBuilder { b: Base { name: "vclap_version", - help: Some("Prints version information"), + help: self.version_message.or(Some("Prints version information")), ..Default::default() }, s: Switched { From f9668297a4fa3601f5202cea702ffe964d2ab2cd Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 11 Mar 2017 00:14:46 -0500 Subject: [PATCH 27/31] api(Arg::last): adds the ability to mark a positional argument as 'last' which means it should be used with `--` syntax and can be accessed early Marking a positional argument `.last(true)` will allow accessing this argument earlier if the `--` syntax is used (i.e. skipping other positional args) and also change the usage string to one of the following: * `$ prog [arg1] [-- ]` * `$ prog [arg1] -- ` (if the arg marked `.last(true)` is also marked `.required(true)`) Closes #888 --- src/app/parser.rs | 449 ++++++++++++++----------------------------- src/app/settings.rs | 7 +- src/app/usage.rs | 304 +++++++++++++++++++++++++++-- src/app/validator.rs | 16 +- src/args/arg.rs | 80 ++++++++ src/args/settings.rs | 10 +- 6 files changed, 540 insertions(+), 326 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index 6153e274..1288f7e1 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -1,5 +1,4 @@ // Std -use std::collections::{BTreeMap, VecDeque}; use std::ffi::{OsStr, OsString}; use std::fmt::Display; use std::fs::File; @@ -119,33 +118,44 @@ impl<'a, 'b> Parser<'a, 'b> } #[inline] - fn debug_asserts(&self, a: &Arg) { - debug_assert!(!arg_names!(self).any(|name| name == a.b.name), - format!("Non-unique argument name: {} is already in use", a.b.name)); + fn debug_asserts(&self, a: &Arg) -> bool { + assert!(!arg_names!(self).any(|name| name == a.b.name), + format!("Non-unique argument name: {} is already in use", a.b.name)); if let Some(l) = a.s.long { - debug_assert!(!self.contains_long(l), - format!("Argument long must be unique\n\n\t--{} is already in use", - l)); + assert!(!self.contains_long(l), + "Argument long must be unique\n\n\t--{} is already in use", + l); } if let Some(s) = a.s.short { - debug_assert!(!self.contains_short(s), - format!("Argument short must be unique\n\n\t-{} is already in use", - s)); + assert!(!self.contains_short(s), + "Argument short must be unique\n\n\t-{} is already in use", + s); } let i = if a.index.is_none() { (self.positionals.len() + 1) } else { a.index.unwrap() as usize }; - debug_assert!(!self.positionals.contains_key(i), - format!("Argument \"{}\" has the same index as another positional \ + assert!(!self.positionals.contains_key(i), + "Argument \"{}\" has the same index as another positional \ argument\n\n\tPerhaps try .multiple(true) to allow one positional argument \ to take multiple values", - a.b.name)); - debug_assert!(!(a.is_set(ArgSettings::Required) && a.is_set(ArgSettings::Global)), - format!("Global arguments cannot be required.\n\n\t'{}' is marked as \ + a.b.name); + assert!(!(a.is_set(ArgSettings::Required) && a.is_set(ArgSettings::Global)), + "Global arguments cannot be required.\n\n\t'{}' is marked as \ global and required", - a.b.name)); + a.b.name); + if a.b.is_set(ArgSettings::Last) { + assert!(!self.positionals.values().any(|p| p.b.is_set(ArgSettings::Last)), + "Only one positional argument may have last(true) set. Found two."); + assert!(a.s.long.is_none(), + "Flags or Options may not have last(true) set. {} has both a long and last(true) set.", + a.b.name); + assert!(a.s.short.is_none(), + "Flags or Options may not have last(true) set. {} has both a short and last(true) set.", + a.b.name); + } + true } #[inline] @@ -188,16 +198,27 @@ impl<'a, 'b> Parser<'a, 'b> } } + #[inline] + fn implied_settings(&mut self, a: &Arg<'a, 'b>) { + if a.is_set(ArgSettings::Last) { + // if an arg has `Last` set, we need to imply DontCollapseArgsInUsage so that args + // in the usage string don't get confused or left out. + self.set(AS::DontCollapseArgsInUsage); + self.set(AS::ContainsLast); + } + } + // actually adds the arguments pub fn add_arg(&mut self, a: Arg<'a, 'b>) { // if it's global we have to clone anyways if a.is_set(ArgSettings::Global) { return self.add_arg_ref(&a); } - self.debug_asserts(&a); + debug_assert!(self.debug_asserts(&a)); self.add_conditional_reqs(&a); self.add_arg_groups(&a); self.add_reqs(&a); + self.implied_settings(&a); if a.index.is_some() || (a.s.short.is_none() && a.s.long.is_none()) { let i = if a.index.is_none() { (self.positionals.len() + 1) @@ -217,10 +238,11 @@ impl<'a, 'b> Parser<'a, 'b> } // actually adds the arguments but from a borrow (which means we have to do some clonine) pub fn add_arg_ref(&mut self, a: &Arg<'a, 'b>) { - self.debug_asserts(a); + debug_assert!(self.debug_asserts(&a)); self.add_conditional_reqs(a); self.add_arg_groups(a); self.add_reqs(a); + self.implied_settings(&a); if a.index.is_some() || (a.s.short.is_none() && a.s.long.is_none()) { let i = if a.index.is_none() { (self.positionals.len() + 1) @@ -343,220 +365,6 @@ impl<'a, 'b> Parser<'a, 'b> pub fn required(&self) -> Iter<&str> { self.required.iter() } #[cfg_attr(feature = "lints", allow(needless_borrow))] - pub fn get_required_from(&self, - reqs: &[&'a str], - matcher: Option<&ArgMatcher<'a>>, - extra: Option<&str>) - -> VecDeque { - debugln!("Parser::get_required_from: reqs={:?}, extra={:?}", - reqs, - extra); - let mut desc_reqs: Vec<&str> = vec![]; - desc_reqs.extend(extra); - let mut new_reqs: Vec<&str> = vec![]; - macro_rules! get_requires { - (@group $a: ident, $v:ident, $p:ident) => {{ - if let Some(rl) = self.groups.iter() - .filter(|g| g.requires.is_some()) - .find(|g| &g.name == $a) - .map(|g| g.requires.as_ref().unwrap()) { - for r in rl { - if !$p.contains(&r) { - debugln!("Parser::get_required_from:iter:{}: adding group req={:?}", - $a, r); - $v.push(r); - } - } - } - }}; - ($a:ident, $what:ident, $how:ident, $v:ident, $p:ident) => {{ - if let Some(rl) = self.$what.$how() - .filter(|a| a.b.requires.is_some()) - .find(|arg| &arg.b.name == $a) - .map(|a| a.b.requires.as_ref().unwrap()) { - for &(_, r) in rl.iter() { - if !$p.contains(&r) { - debugln!("Parser::get_required_from:iter:{}: adding arg req={:?}", - $a, r); - $v.push(r); - } - } - } - }}; - } - // initialize new_reqs - for a in reqs { - get_requires!(a, flags, iter, new_reqs, reqs); - get_requires!(a, opts, iter, new_reqs, reqs); - get_requires!(a, positionals, values, new_reqs, reqs); - get_requires!(@group a, new_reqs, reqs); - } - desc_reqs.extend_from_slice(&*new_reqs); - debugln!("Parser::get_required_from: after init desc_reqs={:?}", - desc_reqs); - loop { - let mut tmp = vec![]; - for a in &new_reqs { - get_requires!(a, flags, iter, tmp, desc_reqs); - get_requires!(a, opts, iter, tmp, desc_reqs); - get_requires!(a, positionals, values, tmp, desc_reqs); - get_requires!(@group a, tmp, desc_reqs); - } - if tmp.is_empty() { - debugln!("Parser::get_required_from: no more children"); - break; - } else { - debugln!("Parser::get_required_from: after iter tmp={:?}", tmp); - debugln!("Parser::get_required_from: after iter new_reqs={:?}", - new_reqs); - desc_reqs.extend_from_slice(&*new_reqs); - new_reqs.clear(); - new_reqs.extend_from_slice(&*tmp); - debugln!("Parser::get_required_from: after iter desc_reqs={:?}", - desc_reqs); - } - } - desc_reqs.extend_from_slice(reqs); - desc_reqs.sort(); - desc_reqs.dedup(); - debugln!("Parser::get_required_from: final desc_reqs={:?}", desc_reqs); - let mut ret_val = VecDeque::new(); - let args_in_groups = self.groups - .iter() - .filter(|gn| desc_reqs.contains(&gn.name)) - .flat_map(|g| self.arg_names_in_group(&g.name)) - .collect::>(); - - let pmap = if let Some(ref m) = matcher { - desc_reqs.iter() - .filter(|a| self.positionals.values().any(|p| &&p.b.name == a)) - .filter(|&p| !m.contains(p)) - .filter_map(|p| self.positionals.values().find(|x| &x.b.name == p)) - .filter(|p| !args_in_groups.contains(&p.b.name)) - .map(|p| (p.index, p)) - .collect::>() // sort by index - } else { - desc_reqs.iter() - .filter(|a| self.positionals.values().any(|p| &&p.b.name == a)) - .filter_map(|p| self.positionals.values().find(|x| &x.b.name == p)) - .filter(|p| !args_in_groups.contains(&p.b.name)) - .map(|p| (p.index, p)) - .collect::>() // sort by index - }; - debugln!("Parser::get_required_from: args_in_groups={:?}", - args_in_groups); - for &p in pmap.values() { - let s = p.to_string(); - if args_in_groups.is_empty() || !args_in_groups.contains(&&*s) { - ret_val.push_back(s); - } - } - for a in desc_reqs.iter() - .filter(|name| !self.positionals.values().any(|p| &&p.b.name == name)) - .filter(|name| !self.groups.iter().any(|g| &&g.name == name)) - .filter(|name| !args_in_groups.contains(name)) - .filter(|name| { - !(matcher.is_some() && matcher.as_ref().unwrap().contains(name)) - }) { - debugln!("Parser::get_required_from:iter:{}:", a); - let arg = find_by_name!(self, a, flags, iter) - .map(|f| f.to_string()) - .unwrap_or_else(|| { - find_by_name!(self, a, opts, iter) - .map(|o| o.to_string()) - .expect(INTERNAL_ERROR_MSG) - }); - ret_val.push_back(arg); - } - let mut g_vec = vec![]; - for g in desc_reqs.iter().filter(|n| self.groups.iter().any(|g| &&g.name == n)) { - let g_string = self.args_in_group(g).join("|"); - g_vec.push(format!("<{}>", &g_string[..g_string.len()])); - } - g_vec.sort(); - g_vec.dedup(); - for g in g_vec { - ret_val.push_back(g); - } - - ret_val - } - - // Gets the `[ARGS]` tag for the usage string - pub fn get_args_tag(&self) -> Option { - debugln!("Parser::get_args_tag;"); - let mut count = 0; - 'outer: for p in self.positionals.values().filter(|p| { - !p.is_set(ArgSettings::Required) && - !p.is_set(ArgSettings::Hidden) - }) { - debugln!("Parser::get_args_tag:iter:{}:", p.b.name); - if let Some(g_vec) = self.groups_for_arg(p.b.name) { - for grp_s in &g_vec { - debugln!("Parser::get_args_tag:iter:{}:iter:{};", p.b.name, grp_s); - // if it's part of a required group we don't want to count it - if self.groups.iter().any(|g| g.required && (&g.name == grp_s)) { - continue 'outer; - } - } - } - count += 1; - debugln!("Parser::get_args_tag:iter: {} Args not required or hidden", - count); - } - if !self.is_set(AS::DontCollapseArgsInUsage) && count > 1 { - return None; // [ARGS] - } else if count == 1 { - let p = self.positionals - .values() - .find(|p| !p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Hidden)) - .expect(INTERNAL_ERROR_MSG); - return Some(format!(" [{}]{}", p.name_no_brackets(), p.multiple_str())); - } else if self.is_set(AS::DontCollapseArgsInUsage) && !self.positionals.is_empty() { - return Some(self.positionals - .values() - .filter(|p| !p.is_set(ArgSettings::Required)) - .filter(|p| !p.is_set(ArgSettings::Hidden)) - .map(|p| { - format!(" [{}]{}", p.name_no_brackets(), p.multiple_str()) - }) - .collect::>() - .join("")); - } - Some("".into()) - } - - // Determines if we need the `[FLAGS]` tag in the usage string - pub fn needs_flags_tag(&self) -> bool { - debugln!("Parser::needs_flags_tag;"); - 'outer: for f in &self.flags { - debugln!("Parser::needs_flags_tag:iter: f={};", f.b.name); - if let Some(l) = f.s.long { - if l == "help" || l == "version" { - // Don't print `[FLAGS]` just for help or version - continue; - } - } - if let Some(g_vec) = self.groups_for_arg(f.b.name) { - for grp_s in &g_vec { - debugln!("Parser::needs_flags_tag:iter:iter: grp_s={};", grp_s); - if self.groups.iter().any(|g| &g.name == grp_s && g.required) { - debug!("Parser::needs_flags_tag:iter:iter: Group is required"); - continue 'outer; - } - } - } - if f.is_set(ArgSettings::Hidden) { - continue; - } - debugln!("Parser::needs_flags_tag:iter: [FLAGS] required"); - return true; - } - - debugln!("Parser::needs_flags_tag: [FLAGS] not required"); - false - } - #[inline] pub fn has_args(&self) -> bool { !(self.flags.is_empty() && self.opts.is_empty() && self.positionals.is_empty()) @@ -570,7 +378,7 @@ impl<'a, 'b> Parser<'a, 'b> #[inline] pub fn has_positionals(&self) -> bool { !self.positionals.is_empty() } - + #[inline] pub fn has_subcommands(&self) -> bool { !self.subcommands.is_empty() } @@ -616,7 +424,7 @@ impl<'a, 'b> Parser<'a, 'b> pub fn unset(&mut self, s: AS) { self.settings.unset(s) } #[cfg_attr(feature = "lints", allow(block_in_if_condition_stmt))] - pub fn verify_positionals(&mut self) { + pub fn verify_positionals(&mut self) -> bool { // Because you must wait until all arguments have been supplied, this is the first chance // to make assertions on positional argument indexes // @@ -627,71 +435,72 @@ impl<'a, 'b> Parser<'a, 'b> .iter() .rev() .next() { - debug_assert!(!(idx != self.positionals.len()), - format!("Found positional argument \"{}\" who's index is {} but there \ + assert!(!(idx != self.positionals.len()), + "Found positional argument \"{}\" who's index is {} but there \ are only {} positional arguments defined", - p.b.name, - idx, - self.positionals.len())); + p.b.name, + idx, + self.positionals.len()); } // Next we verify that only the highest index has a .multiple(true) (if any) if self.positionals.values().any(|a| { - a.is_set(ArgSettings::Multiple) && + a.b.is_set(ArgSettings::Multiple) && (a.index as usize != self.positionals.len()) }) { - - debug_assert!({ - let mut it = self.positionals.values().rev(); - // Either the final positional is required - it.next().unwrap().is_set(ArgSettings::Required) - // Or the second to last has a terminator set - || it.next().unwrap().v.terminator.is_some() - }, - "When using a positional argument with .multiple(true) that is *not the \ + let mut it = self.positionals.values().rev(); + let last = it.next().unwrap(); + let second_to_last = it.next().unwrap(); + // Either the final positional is required + // Or the second to last has a terminator or .last(true) set + let ok = last.is_set(ArgSettings::Required) || + (second_to_last.v.terminator.is_some() || + second_to_last.b.is_set(ArgSettings::Last)); + assert!(ok, + "When using a positional argument with .multiple(true) that is *not the \ last* positional argument, the last positional argument (i.e the one \ - with the highest index) *must* have .required(true) set."); - - debug_assert!({ - let num = self.positionals.len() - 1; - self.positionals - .get(num) - .unwrap() - .is_set(ArgSettings::Multiple) - }, - "Only the last positional argument, or second to last positional \ + with the highest index) *must* have .required(true) or .last(true) set."); + let num = self.positionals.len() - 1; + let ok = self.positionals + .get(num) + .unwrap() + .is_set(ArgSettings::Multiple); + assert!(ok, + "Only the last positional argument, or second to last positional \ argument may be set to .multiple(true)"); - self.set(AS::LowIndexMultiplePositional); + self.settings.set(AS::LowIndexMultiplePositional); } - debug_assert!(self.positionals - .values() - .filter(|p| { - p.b.settings.is_set(ArgSettings::Multiple) && - p.v.num_vals.is_none() - }) - .map(|_| 1) - .sum::() <= 1, - "Only one positional argument with .multiple(true) set is allowed per \ + let ok = self.positionals + .values() + .filter(|p| p.b.settings.is_set(ArgSettings::Multiple) && p.v.num_vals.is_none()) + .map(|_| 1) + .sum::() <= 1; + assert!(ok, + "Only one positional argument with .multiple(true) set is allowed per \ command"); - // If it's required we also need to ensure all previous positionals are - // required too if self.is_set(AS::AllowMissingPositional) { + // Check that if a required positional argument is found, all positions with a lower + // index are also required. let mut found = false; let mut foundx2 = false; for p in self.positionals.values().rev() { if foundx2 && !p.b.settings.is_set(ArgSettings::Required) { - // [arg1] is Ok - // [arg1] Is not - debug_assert!(p.b.settings.is_set(ArgSettings::Required), - "Found positional argument which is not required with a lower \ + assert!(p.b.is_set(ArgSettings::Required), + "Found positional argument which is not required with a lower \ index than a required positional argument by two or more: {:?} \ index {}", - p.b.name, - p.index); - } else if p.b.settings.is_set(ArgSettings::Required) { + p.b.name, + p.index); + } else if p.b.is_set(ArgSettings::Required) && !p.b.is_set(ArgSettings::Last) { + // Args that .last(true) don't count since they can be required and have + // positionals with a lower index that aren't required + // Imagine: prog [opt1] -- + // Both of these are valid invocations: + // $ prog r1 -- r2 + // $ prog r1 o1 -- r2 if found { foundx2 = true; continue; @@ -703,20 +512,38 @@ impl<'a, 'b> Parser<'a, 'b> } } } else { + // Check that if a required positional argument is found, all positions with a lower + // index are also required let mut found = false; for p in self.positionals.values().rev() { if found { - debug_assert!(p.b.settings.is_set(ArgSettings::Required), - "Found positional argument which is not required with a lower \ + assert!(p.b.is_set(ArgSettings::Required), + "Found positional argument which is not required with a lower \ index than a required positional argument: {:?} index {}", - p.b.name, - p.index); - } else if p.b.settings.is_set(ArgSettings::Required) { + p.b.name, + p.index); + } else if p.b.is_set(ArgSettings::Required) && !p.b.is_set(ArgSettings::Last) { + // Args that .last(true) don't count since they can be required and have + // positionals with a lower index that aren't required + // Imagine: prog [opt1] -- + // Both of these are valid invocations: + // $ prog r1 -- r2 + // $ prog r1 o1 -- r2 found = true; continue; } } } + if self.positionals.values().any(|p| { + p.b.is_set(ArgSettings::Last) && + p.b.is_set(ArgSettings::Required) + }) && self.has_subcommands() && + !self.is_set(AS::SubcommandsNegateReqs) { + panic!("Having a required positional argument with .last(true) set *and* child \ + subcommands without setting SubcommandsNegateReqs isn't compatible."); + } + + true } pub fn propogate_globals(&mut self) { @@ -915,7 +742,7 @@ impl<'a, 'b> Parser<'a, 'b> { debugln!("Parser::get_matches_with;"); // Verify all positional assertions pass - self.verify_positionals(); + debug_assert!(self.verify_positionals()); let has_args = self.has_args(); // Next we create the `--help` and `--version` arguments and add them if @@ -940,7 +767,9 @@ impl<'a, 'b> Parser<'a, 'b> // Does the arg match a subcommand name, or any of it's aliases (if defined) { let (is_match, sc_name) = self.possible_subcommand(&arg_os); - debugln!("Parser::get_matches_with: possible_sc={:?}, sc={:?}", is_match, sc_name); + debugln!("Parser::get_matches_with: possible_sc={:?}, sc={:?}", + is_match, + sc_name); if is_match { let sc_name = sc_name.expect(INTERNAL_ERROR_MSG); if sc_name == "help" && self.is_set(AS::NeedsSubcommandHelp) { @@ -1011,7 +840,8 @@ impl<'a, 'b> Parser<'a, 'b> .bin_name .as_ref() .unwrap_or(&self.meta.name), - &*usage::create_error_usage(self, matcher, + &*usage::create_error_usage(self, + matcher, None), self.color())); } @@ -1050,8 +880,21 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Parser::get_matches_with: Bumping the positional counter..."); pos_counter += 1; } + } else if self.is_set(AS::ContainsLast) && self.is_set(AS::TrailingValues) { + // Came to -- and one postional has .last(true) set, so we go immediately + // to the last (highest index) positional + debugln!("Parser::get_matches_with: .last(true) and --, setting last pos"); + pos_counter = self.positionals.len(); } if let Some(p) = self.positionals.get(pos_counter) { + if p.is_set(ArgSettings::Last) && !self.is_set(AS::TrailingValues) { + return Err(Error::unknown_argument(&*arg_os.to_string_lossy(), + "", + &*usage::create_error_usage(self, + matcher, + None), + self.color())); + } parse_positional!(self, p, arg_os, pos_counter, matcher); self.settings.set(AS::ValidArgFound); } else if self.is_set(AS::AllowExternalSubcommands) { @@ -1061,8 +904,8 @@ impl<'a, 'b> Parser<'a, 'b> None => { if !self.is_set(AS::StrictUtf8) { return Err(Error::invalid_utf8(&*usage::create_error_usage(self, - matcher, - None), + matcher, + None), self.color())); } arg_os.to_string_lossy().into_owned() @@ -1075,8 +918,8 @@ impl<'a, 'b> Parser<'a, 'b> let a = v.into(); if a.to_str().is_none() && !self.is_set(AS::StrictUtf8) { return Err(Error::invalid_utf8(&*usage::create_error_usage(self, - matcher, - None), + matcher, + None), self.color())); } sc_m.add_val_to("", &a); @@ -1092,8 +935,8 @@ impl<'a, 'b> Parser<'a, 'b> return Err(Error::unknown_argument(&*arg_os.to_string_lossy(), "", &*usage::create_error_usage(self, - matcher, - None), + matcher, + None), self.color())); } else if !has_args || self.is_set(AS::InferSubcommands) && self.has_subcommands() { if let Some(cdate) = suggestions::did_you_mean(&*arg_os.to_string_lossy(), @@ -1105,11 +948,12 @@ impl<'a, 'b> Parser<'a, 'b> .as_ref() .unwrap_or(&self.meta.name), &*usage::create_error_usage(self, - matcher, - None), + matcher, + None), self.color())); } else { - return Err(Error::unrecognized_subcommand(arg_os.to_string_lossy().into_owned(), + return Err(Error::unrecognized_subcommand(arg_os.to_string_lossy() + .into_owned(), self.meta .bin_name .as_ref() @@ -1135,9 +979,7 @@ impl<'a, 'b> Parser<'a, 'b> .as_ref() .unwrap_or(&self.meta.name); return Err(Error::missing_subcommand(bn, - &usage::create_error_usage(self, - matcher, - None), + &usage::create_error_usage(self, matcher, None), self.color())); } else if self.is_set(AS::SubcommandRequiredElseHelp) { debugln!("Parser::get_matches_with: SubcommandRequiredElseHelp=true"); @@ -1214,7 +1056,7 @@ impl<'a, 'b> Parser<'a, 'b> for k in matcher.arg_names() { hs.push(k); } - let reqs = self.get_required_from(&hs, Some(matcher), None); + let reqs = usage::get_required_usage_from(self, &hs, Some(matcher), None, false); for s in &reqs { write!(&mut mid_string, " {}", s).expect(INTERNAL_ERROR_MSG); @@ -1262,8 +1104,7 @@ impl<'a, 'b> Parser<'a, 'b> Ok(()) } - - fn groups_for_arg(&self, name: &str) -> Option> { + pub fn groups_for_arg(&self, name: &str) -> Option> { debugln!("Parser::groups_for_arg: name={}", name); if self.groups.is_empty() { @@ -1287,7 +1128,7 @@ impl<'a, 'b> Parser<'a, 'b> Some(res) } - fn args_in_group(&self, group: &str) -> Vec { + pub fn args_in_group(&self, group: &str) -> Vec { let mut g_vec = vec![]; let mut args = vec![]; @@ -1581,8 +1422,8 @@ impl<'a, 'b> Parser<'a, 'b> return Err(Error::unknown_argument(&*arg, "", &*usage::create_error_usage(self, - matcher, - None), + matcher, + None), self.color())); } } diff --git a/src/app/settings.rs b/src/app/settings.rs index a092e259..abc544a7 100644 --- a/src/app/settings.rs +++ b/src/app/settings.rs @@ -44,6 +44,7 @@ bitflags! { const PROPOGATED = 1 << 36, const VALID_ARG_FOUND = 1 << 37, const INFER_SUBCOMMANDS = 1 << 38, + const CONTAINS_LAST = 1 << 39, } } @@ -104,7 +105,8 @@ impl AppFlags { ValidNegNumFound => VALID_NEG_NUM_FOUND, Propogated => PROPOGATED, ValidArgFound => VALID_ARG_FOUND, - InferSubcommands => INFER_SUBCOMMANDS + InferSubcommands => INFER_SUBCOMMANDS, + ContainsLast => CONTAINS_LAST } } @@ -859,6 +861,9 @@ pub enum AppSettings { #[doc(hidden)] ValidArgFound, + + #[doc(hidden)] + ContainsLast, } impl FromStr for AppSettings { diff --git a/src/app/usage.rs b/src/app/usage.rs index 72631cd9..19938b40 100644 --- a/src/app/usage.rs +++ b/src/app/usage.rs @@ -1,3 +1,8 @@ +// std +use std::collections::{BTreeMap, VecDeque}; + +// Internal +use INTERNAL_ERROR_MSG; use args::{AnyArg, ArgMatcher, PosBuilder}; use args::settings::ArgSettings; use app::settings::AppSettings as AS; @@ -66,13 +71,14 @@ pub fn create_help_usage(p: &Parser, incl_reqs: bool) -> String { let mut reqs: Vec<&str> = p.required().map(|r| &**r).collect(); reqs.sort(); reqs.dedup(); - p.get_required_from(&reqs, None, None).iter().fold(String::new(), - |a, s| a + &format!(" {}", s)[..]) + get_required_usage_from(p, &reqs, None, None, false).iter().fold(String::new(), |a, s| { + a + &format!(" {}", s)[..] + }) } else { String::new() }; - let flags = p.needs_flags_tag(); + let flags = needs_flags_tag(p); if flags && !p.is_set(AS::UnifiedHelpMessage) { usage.push_str(" [FLAGS]"); } else if flags { @@ -85,20 +91,41 @@ pub fn create_help_usage(p: &Parser, incl_reqs: bool) -> String { usage.push_str(&req_string[..]); + let has_last = p.positionals.values().any(|p| p.is_set(ArgSettings::Last)); // places a '--' in the usage string if there are args and options // supporting multiple values - if p.has_positionals() && p.opts.iter().any(|o| o.is_set(ArgSettings::Multiple)) && + if p.opts.iter().any(|o| o.is_set(ArgSettings::Multiple)) && p.positionals.values().any(|p| !p.is_set(ArgSettings::Required)) && - !p.has_visible_subcommands() { - usage.push_str(" [--]") + !p.has_visible_subcommands() && !has_last { + usage.push_str(" [--]"); } - let not_req_or_hidden= |p: &PosBuilder| !p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Hidden); + let not_req_or_hidden = + |p: &PosBuilder| !p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Hidden); if p.has_positionals() && p.positionals.values().any(not_req_or_hidden) { - if let Some(args_tag) = p.get_args_tag() { + if let Some(args_tag) = get_args_tag(p, incl_reqs) { usage.push_str(&*args_tag); } else { usage.push_str(" [ARGS]"); } + if has_last && incl_reqs { + let pos = p.positionals + .values() + .find(|p| p.b.is_set(ArgSettings::Last)) + .expect(INTERNAL_ERROR_MSG); + debugln!("usage::create_help_usage: {} has .last(true)", pos.name()); + let req = pos.is_set(ArgSettings::Required); + if req { + usage.push_str(" -- <"); + } else { + usage.push_str(" [-- <"); + } + usage.push_str(pos.name()); + usage.push_str(">"); + usage.push_str(pos.multiple_str()); + if !req { + usage.push_str("]"); + } + } } // incl_reqs is only false when this function is called recursively @@ -120,6 +147,7 @@ pub fn create_help_usage(p: &Parser, incl_reqs: bool) -> String { } } usage.shrink_to_fit(); + debugln!("usage::create_help_usage: usage={}", usage); usage } @@ -131,9 +159,10 @@ fn create_smart_usage(p: &Parser, used: &[&str]) -> String { let mut hs: Vec<&str> = p.required().map(|s| &**s).collect(); hs.extend_from_slice(used); - let r_string = p.get_required_from(&hs, None, None).iter().fold(String::new(), |acc, s| { - acc + &format!(" {}", s)[..] - }); + let r_string = + get_required_usage_from(p, &hs, None, None, false).iter().fold(String::new(), |acc, s| { + acc + &format!(" {}", s)[..] + }); usage.push_str(&p.meta .usage @@ -152,3 +181,256 @@ fn create_smart_usage(p: &Parser, used: &[&str]) -> String { usage.shrink_to_fit(); usage } + +// Gets the `[ARGS]` tag for the usage string +fn get_args_tag(p: &Parser, incl_reqs: bool) -> Option { + debugln!("usage::get_args_tag;"); + let mut count = 0; + 'outer: for pos in p.positionals + .values() + .filter(|pos| !pos.is_set(ArgSettings::Required)) + .filter(|pos| !pos.is_set(ArgSettings::Hidden)) + .filter(|pos| !pos.is_set(ArgSettings::Last)) { + debugln!("usage::get_args_tag:iter:{}:", pos.b.name); + if let Some(g_vec) = p.groups_for_arg(pos.b.name) { + for grp_s in &g_vec { + debugln!("usage::get_args_tag:iter:{}:iter:{};", pos.b.name, grp_s); + // if it's part of a required group we don't want to count it + if p.groups.iter().any(|g| g.required && (&g.name == grp_s)) { + continue 'outer; + } + } + } + count += 1; + debugln!("usage::get_args_tag:iter: {} Args not required or hidden", + count); + } + if !p.is_set(AS::DontCollapseArgsInUsage) && count > 1 { + debugln!("usage::get_args_tag:iter: More than one, returning [ARGS]"); + return None; // [ARGS] + } else if count == 1 && incl_reqs { + let pos = p.positionals + .values() + .find(|pos| { + !pos.is_set(ArgSettings::Required) && !pos.is_set(ArgSettings::Hidden) && + !pos.is_set(ArgSettings::Last) + }) + .expect(INTERNAL_ERROR_MSG); + debugln!("usage::get_args_tag:iter: Exactly one, returning {}", + pos.name()); + return Some(format!(" [{}]{}", pos.name_no_brackets(), pos.multiple_str())); + } else if p.is_set(AS::DontCollapseArgsInUsage) && !p.positionals.is_empty() && incl_reqs { + debugln!("usage::get_args_tag:iter: Don't collapse returning all"); + return Some(p.positionals + .values() + .filter(|pos| !pos.is_set(ArgSettings::Required)) + .filter(|pos| !pos.is_set(ArgSettings::Hidden)) + .filter(|pos| !pos.is_set(ArgSettings::Last)) + .map(|pos| { + format!(" [{}]{}", pos.name_no_brackets(), pos.multiple_str()) + }) + .collect::>() + .join("")); + } else if !incl_reqs { + debugln!("usage::get_args_tag:iter: incl_reqs=false, building secondary usage string"); + let highest_req_pos = p.positionals + .iter() + .filter_map(|(idx, pos)| if pos.b.is_set(ArgSettings::Required) && + !pos.b.is_set(ArgSettings::Last) { + Some(idx) + } else { + None + }) + .max() + .unwrap_or(p.positionals.len()); + return Some(p.positionals + .iter() + .filter_map(|(idx, pos)| if idx <= highest_req_pos { + Some(pos) + } else { + None + }) + .filter(|pos| !pos.is_set(ArgSettings::Required)) + .filter(|pos| !pos.is_set(ArgSettings::Hidden)) + .filter(|pos| !pos.is_set(ArgSettings::Last)) + .map(|pos| { + format!(" [{}]{}", pos.name_no_brackets(), pos.multiple_str()) + }) + .collect::>() + .join("")); + } + Some("".into()) +} + +// Determines if we need the `[FLAGS]` tag in the usage string +fn needs_flags_tag(p: &Parser) -> bool { + debugln!("usage::needs_flags_tag;"); + 'outer: for f in &p.flags { + debugln!("usage::needs_flags_tag:iter: f={};", f.b.name); + if let Some(l) = f.s.long { + if l == "help" || l == "version" { + // Don't print `[FLAGS]` just for help or version + continue; + } + } + if let Some(g_vec) = p.groups_for_arg(f.b.name) { + for grp_s in &g_vec { + debugln!("usage::needs_flags_tag:iter:iter: grp_s={};", grp_s); + if p.groups.iter().any(|g| &g.name == grp_s && g.required) { + debugln!("usage::needs_flags_tag:iter:iter: Group is required"); + continue 'outer; + } + } + } + if f.is_set(ArgSettings::Hidden) { + continue; + } + debugln!("usage::needs_flags_tag:iter: [FLAGS] required"); + return true; + } + + debugln!("usage::needs_flags_tag: [FLAGS] not required"); + false +} + +// Returns the required args in usage string form by fully unrolling all groups +pub fn get_required_usage_from<'a, 'b>(p: &Parser<'a, 'b>, + reqs: &[&'a str], + matcher: Option<&ArgMatcher<'a>>, + extra: Option<&str>, + incl_last: bool) + -> VecDeque { + debugln!("usage::get_required_usage_from: reqs={:?}, extra={:?}", + reqs, + extra); + let mut desc_reqs: Vec<&str> = vec![]; + desc_reqs.extend(extra); + let mut new_reqs: Vec<&str> = vec![]; + macro_rules! get_requires { + (@group $a: ident, $v:ident, $p:ident) => {{ + if let Some(rl) = p.groups.iter() + .filter(|g| g.requires.is_some()) + .find(|g| &g.name == $a) + .map(|g| g.requires.as_ref().unwrap()) { + for r in rl { + if !$p.contains(&r) { + debugln!("usage::get_required_usage_from:iter:{}: adding group req={:?}", + $a, r); + $v.push(r); + } + } + } + }}; + ($a:ident, $what:ident, $how:ident, $v:ident, $p:ident) => {{ + if let Some(rl) = p.$what.$how() + .filter(|a| a.b.requires.is_some()) + .find(|arg| &arg.b.name == $a) + .map(|a| a.b.requires.as_ref().unwrap()) { + for &(_, r) in rl.iter() { + if !$p.contains(&r) { + debugln!("usage::get_required_usage_from:iter:{}: adding arg req={:?}", + $a, r); + $v.push(r); + } + } + } + }}; + } + // initialize new_reqs + for a in reqs { + get_requires!(a, flags, iter, new_reqs, reqs); + get_requires!(a, opts, iter, new_reqs, reqs); + get_requires!(a, positionals, values, new_reqs, reqs); + get_requires!(@group a, new_reqs, reqs); + } + desc_reqs.extend_from_slice(&*new_reqs); + debugln!("usage::get_required_usage_from: after init desc_reqs={:?}", + desc_reqs); + loop { + let mut tmp = vec![]; + for a in &new_reqs { + get_requires!(a, flags, iter, tmp, desc_reqs); + get_requires!(a, opts, iter, tmp, desc_reqs); + get_requires!(a, positionals, values, tmp, desc_reqs); + get_requires!(@group a, tmp, desc_reqs); + } + if tmp.is_empty() { + debugln!("usage::get_required_usage_from: no more children"); + break; + } else { + debugln!("usage::get_required_usage_from: after iter tmp={:?}", tmp); + debugln!("usage::get_required_usage_from: after iter new_reqs={:?}", + new_reqs); + desc_reqs.extend_from_slice(&*new_reqs); + new_reqs.clear(); + new_reqs.extend_from_slice(&*tmp); + debugln!("usage::get_required_usage_from: after iter desc_reqs={:?}", + desc_reqs); + } + } + desc_reqs.extend_from_slice(reqs); + desc_reqs.sort(); + desc_reqs.dedup(); + debugln!("usage::get_required_usage_from: final desc_reqs={:?}", + desc_reqs); + let mut ret_val = VecDeque::new(); + let args_in_groups = p.groups + .iter() + .filter(|gn| desc_reqs.contains(&gn.name)) + .flat_map(|g| p.arg_names_in_group(&g.name)) + .collect::>(); + + let pmap = if let Some(ref m) = matcher { + desc_reqs.iter() + .filter(|a| p.positionals.values().any(|p| &&p.b.name == a)) + .filter(|&pos| !m.contains(pos)) + .filter_map(|pos| p.positionals.values().find(|x| &x.b.name == pos)) + .filter(|&pos| incl_last || !pos.is_set(ArgSettings::Last)) + .filter(|pos| !args_in_groups.contains(&pos.b.name)) + .map(|pos| (pos.index, pos)) + .collect::>() // sort by index + } else { + desc_reqs.iter() + .filter(|a| p.positionals.values().any(|pos| &&pos.b.name == a)) + .filter_map(|pos| p.positionals.values().find(|x| &x.b.name == pos)) + .filter(|&pos| incl_last || !pos.is_set(ArgSettings::Last)) + .filter(|pos| !args_in_groups.contains(&pos.b.name)) + .map(|pos| (pos.index, pos)) + .collect::>() // sort by index + }; + debugln!("usage::get_required_usage_from: args_in_groups={:?}", + args_in_groups); + for &p in pmap.values() { + let s = p.to_string(); + if args_in_groups.is_empty() || !args_in_groups.contains(&&*s) { + ret_val.push_back(s); + } + } + for a in desc_reqs.iter() + .filter(|name| !p.positionals.values().any(|p| &&p.b.name == name)) + .filter(|name| !p.groups.iter().any(|g| &&g.name == name)) + .filter(|name| !args_in_groups.contains(name)) + .filter(|name| !(matcher.is_some() && matcher.as_ref().unwrap().contains(name))) { + debugln!("usage::get_required_usage_from:iter:{}:", a); + let arg = find_by_name!(p, a, flags, iter) + .map(|f| f.to_string()) + .unwrap_or_else(|| { + find_by_name!(p, a, opts, iter) + .map(|o| o.to_string()) + .expect(INTERNAL_ERROR_MSG) + }); + ret_val.push_back(arg); + } + let mut g_vec = vec![]; + for g in desc_reqs.iter().filter(|n| p.groups.iter().any(|g| &&g.name == n)) { + let g_string = p.args_in_group(g).join("|"); + g_vec.push(format!("<{}>", &g_string[..g_string.len()])); + } + g_vec.sort(); + g_vec.dedup(); + for g in g_vec { + ret_val.push_back(g); + } + + ret_val +} diff --git a/src/app/validator.rs b/src/app/validator.rs index 9130751a..f874a8dc 100644 --- a/src/app/validator.rs +++ b/src/app/validator.rs @@ -415,7 +415,6 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { } } - #[inline] fn missing_required_error(&self, matcher: &ArgMatcher, extra: Option<&str>) -> ClapResult<()> { debugln!("Validator::missing_required_error: extra={:?}", extra); let c = Colorizer { @@ -433,14 +432,13 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> { reqs.retain(|n| !matcher.contains(n)); reqs.dedup(); debugln!("Validator::missing_required_error: reqs={:#?}", reqs); - Err(Error::missing_required_argument(&*self.0 - .get_required_from(&reqs[..], - Some(matcher), - extra) - .iter() - .fold(String::new(), |acc, s| { - acc + &format!("\n {}", c.error(s))[..] - }), + let req_args = + usage::get_required_usage_from(self.0, &reqs[..], Some(matcher), extra, true) + .iter() + .fold(String::new(), + |acc, s| acc + &format!("\n {}", c.error(s))[..]); + debugln!("Validator::missing_required_error: req_args={:#?}", req_args); + Err(Error::missing_required_argument(&*req_args, &*usage::create_error_usage(self.0, matcher, extra), self.0.color())) } diff --git a/src/args/arg.rs b/src/args/arg.rs index 53cd262f..2fd93a66 100644 --- a/src/args/arg.rs +++ b/src/args/arg.rs @@ -537,6 +537,86 @@ impl<'a, 'b> Arg<'a, 'b> { self } + /// Specifies that this arg is the last, or final, positional argument (i.e. has the highest + /// index) and is *only* able to be accessed via the `--` syntax (i.e. `$ prog args -- + /// last_arg`). Even, if no other arguments are left to parse, if the user omits the `--` syntax + /// they will receive an [`UnknownArgument`] error. Setting an argument to `.last(true)` also + /// allows one to access this arg early using the `--` syntax. Accessing an arg early, even with + /// the `--` syntax is otherwise not possible. + /// + /// **NOTE:** This will change the usage string to look like `$ prog [FLAGS] [-- ]` if + /// `ARG` is marked as `.last(true)`. + /// + /// **NOTE:** This setting will imply [`AppSettings::DontCollapseArgsInUsage`] because failing + /// to set this can make the usage string very confusing. + /// + /// **NOTE**: This setting only applies to positional arguments, and has no affect on FLAGS / + /// OPTIONS + /// + /// **CAUTION:** Setting an argument to `.last(true)` *and* having child subcommands is not + /// recommended with the exception of *also* using [`AppSettings::ArgsNegateSubcommands`] + /// (or [`AppSettings::SubcommandsNegateReqs`] if the argument marked `.last(true)` is also + /// marked [`.required(true)`]) + /// + /// # Examples + /// + /// ```rust + /// # use clap::Arg; + /// Arg::with_name("args") + /// .last(true) + /// # ; + /// ``` + /// + /// Setting [`Arg::last(true)`] ensures the arg has the highest [index] of all positional args + /// and requires that the `--` syntax be used to access it early. + /// + /// ```rust + /// # use clap::{App, Arg}; + /// let res = App::new("prog") + /// .arg(Arg::with_name("first")) + /// .arg(Arg::with_name("second")) + /// .arg(Arg::with_name("third").last(true)) + /// .get_matches_from_safe(vec![ + /// "prog", "one", "--", "three" + /// ]); + /// + /// assert!(res.is_ok()); + /// let m = res.unwrap(); + /// assert_eq!(m.value_of("third"), Some("three")); + /// assert!(m.value_of("second").is_none()); + /// ``` + /// + /// Even if the positional argument marked `.last(true)` is the only argument left to parse, + /// failing to use the `--` syntax results in an error. + /// + /// ```rust + /// # use clap::{App, Arg, ErrorKind}; + /// let res = App::new("prog") + /// .arg(Arg::with_name("first")) + /// .arg(Arg::with_name("second")) + /// .arg(Arg::with_name("third").last(true)) + /// .get_matches_from_safe(vec![ + /// "prog", "one", "two", "three" + /// ]); + /// + /// assert!(res.is_err()); + /// assert_eq!(res.unwrap_err().kind, ErrorKind::UnknownArgument); + /// ``` + /// [`Arg::last(true)`]: ./struct.Arg.html#method.last + /// [index]: ./struct.Arg.html#method.index + /// [`AppSettings::DontCollapseArgsInUsage`]: ./enum.AppSettings.html#variant.DontCollapseArgsInUsage + /// [`AppSettings::ArgsNegateSubcommands`]: ./enum.AppSettings.html#variant.ArgsNegateSubcommands + /// [`AppSettings::SubcommandsNegateReqs`]: ./enum.AppSettings.html#variant.SubcommandsNegateReqs + /// [`.required(true)`]: ./struct.Arg.html#method.required + /// [`UnknownArgument`]: ./enum.ErrorKind.html#variant.UnknownArgument + pub fn last(self, l: bool) -> Self { + if l { + self.set(ArgSettings::Last) + } else { + self.unset(ArgSettings::Last) + } + } + /// Sets whether or not the argument is required by default. Required by default means it is /// required, when no other conflicting rules have been evaluated. Conflicting rules take /// precedence over being required. **Default:** `false` diff --git a/src/args/settings.rs b/src/args/settings.rs index 28281dce..82d36f57 100644 --- a/src/args/settings.rs +++ b/src/args/settings.rs @@ -18,6 +18,7 @@ bitflags! { const HIDE_POS_VALS = 1 << 11, const ALLOW_TAC_VALS = 1 << 12, const REQUIRE_EQUALS = 1 << 13, + const LAST = 1 << 14, } } @@ -42,7 +43,8 @@ impl ArgFlags { ValueDelimiterNotSet => DELIM_NOT_SET, HidePossibleValues => HIDE_POS_VALS, AllowLeadingHyphen => ALLOW_TAC_VALS, - RequireEquals => REQUIRE_EQUALS + RequireEquals => REQUIRE_EQUALS, + Last => LAST } } @@ -82,6 +84,9 @@ pub enum ArgSettings { AllowLeadingHyphen, /// Require options use `--option=val` syntax RequireEquals, + /// Specifies that the arg is the last positional argument and may be accessed early via `--` + /// syntax + Last, #[doc(hidden)] RequiredUnlessAll, #[doc(hidden)] @@ -106,6 +111,7 @@ impl FromStr for ArgSettings { "hidepossiblevalues" => Ok(ArgSettings::HidePossibleValues), "allowleadinghyphen" => Ok(ArgSettings::AllowLeadingHyphen), "requireequals" => Ok(ArgSettings::RequireEquals), + "last" => Ok(ArgSettings::Last), _ => Err("unknown ArgSetting, cannot convert from str".to_owned()), } } @@ -145,6 +151,8 @@ mod test { ArgSettings::ValueDelimiterNotSet); assert_eq!("requireequals".parse::().unwrap(), ArgSettings::RequireEquals); + assert_eq!("last".parse::().unwrap(), + ArgSettings::Last); assert!("hahahaha".parse::().is_err()); } } From 3a10353f4b478c0185ce353946725c5ba1af24f2 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 11 Mar 2017 00:17:57 -0500 Subject: [PATCH 28/31] tests: adds tests for .last(true) args --- tests/help.rs | 110 +++++++++++++++++++++++++++++++++++++++++++ tests/positionals.rs | 23 +++++++++ 2 files changed, 133 insertions(+) diff --git a/tests/help.rs b/tests/help.rs index 4e5a6489..cd49ecf9 100644 --- a/tests/help.rs +++ b/tests/help.rs @@ -290,6 +290,72 @@ FLAGS: -H, --help Print help information -v, --version Print version information"; +static LAST_ARG: &'static str = "last 0.1 + +USAGE: + last [CORPUS] [-- ...] + +FLAGS: + -h, --help Prints help information + -V, --version Prints version information + +ARGS: + some + some + ... some"; + +static LAST_ARG_SC: &'static str = "last 0.1 + +USAGE: + last [CORPUS] [-- ...] + last + +FLAGS: + -h, --help Prints help information + -V, --version Prints version information + +ARGS: + some + some + ... some + +SUBCOMMANDS: + help Prints this message or the help of the given subcommand(s) + test some"; + +static LAST_ARG_REQ: &'static str = "last 0.1 + +USAGE: + last [CORPUS] -- ... + +FLAGS: + -h, --help Prints help information + -V, --version Prints version information + +ARGS: + some + some + ... some"; + +static LAST_ARG_REQ_SC: &'static str = "last 0.1 + +USAGE: + last [CORPUS] -- ... + last + +FLAGS: + -h, --help Prints help information + -V, --version Prints version information + +ARGS: + some + some + ... some + +SUBCOMMANDS: + help Prints this message or the help of the given subcommand(s) + test some"; + #[test] fn help_short() { let m = App::new("test") @@ -641,3 +707,47 @@ fn customize_version_and_help() { .version_message("Print version information"); assert!(test::compare_output(app, "customize --help", CUSTOM_VERSION_AND_HELP, false)); } + +#[test] +fn last_arg_mult_usage() { + let app = App::new("last") + .version("0.1") + .arg(Arg::with_name("TARGET").required(true).help("some")) + .arg(Arg::with_name("CORPUS").help("some")) + .arg(Arg::with_name("ARGS").multiple(true).last(true).help("some")); + assert!(test::compare_output(app, "last --help", LAST_ARG, false)); +} + +#[test] +fn last_arg_mult_usage_req() { + let app = App::new("last") + .version("0.1") + .arg(Arg::with_name("TARGET").required(true).help("some")) + .arg(Arg::with_name("CORPUS").help("some")) + .arg(Arg::with_name("ARGS").multiple(true).last(true).required(true).help("some")); + assert!(test::compare_output(app, "last --help", LAST_ARG_REQ, false)); +} + +#[test] +fn last_arg_mult_usage_req_with_sc() { + let app = App::new("last") + .version("0.1") + .setting(AppSettings::SubcommandsNegateReqs) + .arg(Arg::with_name("TARGET").required(true).help("some")) + .arg(Arg::with_name("CORPUS").help("some")) + .arg(Arg::with_name("ARGS").multiple(true).last(true).required(true).help("some")) + .subcommand(SubCommand::with_name("test").about("some")); + assert!(test::compare_output(app, "last --help", LAST_ARG_REQ_SC, false)); +} + +#[test] +fn last_arg_mult_usage_with_sc() { + let app = App::new("last") + .version("0.1") + .setting(AppSettings::ArgsNegateSubcommands) + .arg(Arg::with_name("TARGET").required(true).help("some")) + .arg(Arg::with_name("CORPUS").help("some")) + .arg(Arg::with_name("ARGS").multiple(true).last(true).help("some")) + .subcommand(SubCommand::with_name("test").about("some")); + assert!(test::compare_output(app, "last --help", LAST_ARG_SC, false)); +} \ No newline at end of file diff --git a/tests/positionals.rs b/tests/positionals.rs index b356d299..cf7cf468 100644 --- a/tests/positionals.rs +++ b/tests/positionals.rs @@ -211,3 +211,26 @@ fn missing_required_2() { assert!(r.is_err()); assert_eq!(r.unwrap_err().kind, ErrorKind::MissingRequiredArgument); } + +#[test] +fn last_positional() { + let r = App::new("test") + .arg_from_usage(" 'some target'") + .arg_from_usage("[CORPUS] 'some corpus'") + .arg(Arg::from_usage("[ARGS]... 'some file'").last(true)) + .get_matches_from_safe(vec!["test", "tgt", "--", "arg"]); + assert!(r.is_ok()); + let m = r.unwrap(); + assert_eq!(m.values_of("ARGS").unwrap().collect::>(), &["arg"]); +} + +#[test] +fn last_positional_no_double_dash() { + let r = App::new("test") + .arg_from_usage(" 'some target'") + .arg_from_usage("[CORPUS] 'some corpus'") + .arg(Arg::from_usage("[ARGS]... 'some file'").last(true)) + .get_matches_from_safe(vec!["test", "tgt", "crp", "arg"]); + assert!(r.is_err()); + assert_eq!(r.unwrap_err().kind, ErrorKind::UnknownArgument); +} \ No newline at end of file From 10f1ebce7f20bc1fbb1244bebede1acdf4a9e39e Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 11 Mar 2017 00:18:13 -0500 Subject: [PATCH 29/31] chore: ups the codegen units for test builds --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index dd44b388..d196268b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,7 +65,7 @@ debug = true rpath = false lto = false debug-assertions = true -codegen-units = 2 +codegen-units = 4 [profile.bench] opt-level = 3 From c53e273c7c84baa2be163ef46c40b7e56017dc6d Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 11 Mar 2017 00:20:14 -0500 Subject: [PATCH 30/31] chore: updates the changelog and readme with .last(true) details --- CHANGELOG.md | 1 + README.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58e499fa..01db9df7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ #### API Additions +* **Arg::last:** adds the ability to mark a positional argument as 'last' which means it should be used with `--` syntax and can be accessed early ([6a7aea90](https://github.com/kbknapp/clap-rs/commit/6a7aea9043b83badd9ab038b4ecc4c787716147e), closes [#888](https://github.com/kbknapp/clap-rs/issues/888)) * provides `default_value_os` and `default_value_if[s]_os` ([0f2a3782](https://github.com/kbknapp/clap-rs/commit/0f2a378219a6930748d178ba350fe5925be5dad5), closes [#849](https://github.com/kbknapp/clap-rs/issues/849)) * provides `App::help_message` and `App::version_message` which allows one to override the auto-generated help/version flag associated help ([389c413](https://github.com/kbknapp/clap-rs/commit/389c413b7023dccab8c76aa00577ea1d048e7a99), closes [#889](https://github.com/kbknapp/clap-rs/issues/889)) diff --git a/README.md b/README.md index e5797765..6d21ae61 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ Created by [gh-md-toc](https://github.com/ekalinin/github-markdown-toc) Here's the highlights for v2.21.0 +* adds the ability to mark a positional argument as 'last' which means it should be used with `--` syntax and can be accessed early to effectivly skip other positional args * Some performance improvements by reducing the ammount of duplicate work, cloning, and allocations in all cases. * Some massive perfomance gains when using many args (i.e. things like shell glob expansions) * adds a setting to allow one to infer shortened subcommands or aliases (i.e. for subcommmand "test", "t", "te", or "tes" would be allowed assuming no other ambiguities) From 96884aa1b2820dd7ec3fd0c8b5cb5827f1e61544 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 11 Mar 2017 12:38:24 -0500 Subject: [PATCH 31/31] refactor: implements PartialEq for some of the base structs --- src/app/parser.rs | 7 +++---- src/app/usage.rs | 9 +++++---- src/args/arg.rs | 6 ++++++ src/args/arg_builder/base.rs | 6 ++++++ src/args/arg_builder/flag.rs | 6 ++++++ src/args/arg_builder/option.rs | 6 ++++++ src/args/arg_builder/positional.rs | 6 ++++++ 7 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/app/parser.rs b/src/app/parser.rs index 1288f7e1..751ec189 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -1168,13 +1168,12 @@ impl<'a, 'b> Parser<'a, 'b> args.extend(self.arg_names_in_group(n)); g_vec.push(*n); } else { - args.push(*n); + if !args.contains(n) { + args.push(*n); + } } } - // TODO: faster way to sort/dedup? - args.sort(); - args.dedup(); args.iter().map(|s| *s).collect() } diff --git a/src/app/usage.rs b/src/app/usage.rs index 19938b40..b083ef4f 100644 --- a/src/app/usage.rs +++ b/src/app/usage.rs @@ -421,13 +421,14 @@ pub fn get_required_usage_from<'a, 'b>(p: &Parser<'a, 'b>, }); ret_val.push_back(arg); } - let mut g_vec = vec![]; + let mut g_vec: Vec = vec![]; for g in desc_reqs.iter().filter(|n| p.groups.iter().any(|g| &&g.name == n)) { let g_string = p.args_in_group(g).join("|"); - g_vec.push(format!("<{}>", &g_string[..g_string.len()])); + let elem = format!("<{}>", &g_string[..g_string.len()]); + if !g_vec.contains(&elem) { + g_vec.push(elem); + } } - g_vec.sort(); - g_vec.dedup(); for g in g_vec { ret_val.push_back(g); } diff --git a/src/args/arg.rs b/src/args/arg.rs index 2fd93a66..8e3734a0 100644 --- a/src/args/arg.rs +++ b/src/args/arg.rs @@ -3327,3 +3327,9 @@ impl<'a, 'b, 'z> From<&'z Arg<'a, 'b>> for Arg<'a, 'b> { } } } + +impl<'n, 'e> PartialEq for Arg<'n, 'e> { + fn eq(&self, other: &Arg<'n, 'e>) -> bool { + self.b == other.b + } +} \ No newline at end of file diff --git a/src/args/arg_builder/base.rs b/src/args/arg_builder/base.rs index 1b5e534c..8b566e38 100644 --- a/src/args/arg_builder/base.rs +++ b/src/args/arg_builder/base.rs @@ -26,3 +26,9 @@ impl<'n, 'e> Base<'n, 'e> { impl<'n, 'e, 'z> From<&'z Arg<'n, 'e>> for Base<'n, 'e> { fn from(a: &'z Arg<'n, 'e>) -> Self { a.b.clone() } } + +impl<'n, 'e> PartialEq for Base<'n, 'e> { + fn eq(&self, other: &Base<'n, 'e>) -> bool { + self.name == other.name + } +} \ No newline at end of file diff --git a/src/args/arg_builder/flag.rs b/src/args/arg_builder/flag.rs index 6dc37243..d1c16690 100644 --- a/src/args/arg_builder/flag.rs +++ b/src/args/arg_builder/flag.rs @@ -105,6 +105,12 @@ impl<'n, 'e> DispOrder for FlagBuilder<'n, 'e> { fn disp_ord(&self) -> usize { self.s.disp_ord } } +impl<'n, 'e> PartialEq for FlagBuilder<'n, 'e> { + fn eq(&self, other: &FlagBuilder<'n, 'e>) -> bool { + self.b == other.b + } +} + #[cfg(test)] mod test { use args::settings::ArgSettings; diff --git a/src/args/arg_builder/option.rs b/src/args/arg_builder/option.rs index 75f14a73..91c2ee6f 100644 --- a/src/args/arg_builder/option.rs +++ b/src/args/arg_builder/option.rs @@ -149,6 +149,12 @@ impl<'n, 'e> DispOrder for OptBuilder<'n, 'e> { fn disp_ord(&self) -> usize { self.s.disp_ord } } +impl<'n, 'e> PartialEq for OptBuilder<'n, 'e> { + fn eq(&self, other: &OptBuilder<'n, 'e>) -> bool { + self.b == other.b + } +} + #[cfg(test)] mod test { use args::settings::ArgSettings; diff --git a/src/args/arg_builder/positional.rs b/src/args/arg_builder/positional.rs index 73fde4c1..ddbd8964 100644 --- a/src/args/arg_builder/positional.rs +++ b/src/args/arg_builder/positional.rs @@ -138,6 +138,12 @@ impl<'n, 'e> DispOrder for PosBuilder<'n, 'e> { fn disp_ord(&self) -> usize { self.index as usize } } +impl<'n, 'e> PartialEq for PosBuilder<'n, 'e> { + fn eq(&self, other: &PosBuilder<'n, 'e>) -> bool { + self.b == other.b + } +} + #[cfg(test)] mod test { use args::settings::ArgSettings;