From 1ced2a7433ea8937a1b260ea65d708f32ca7c95e Mon Sep 17 00:00:00 2001 From: Kevin K Date: Tue, 1 Nov 2016 16:10:40 -0400 Subject: [PATCH] feat(Positional Args): allows specifying the second to last positional argument as multiple(true) Now one can build CLIs that support things like `mv ... ` There are a few requirements and caveats; * The final positional argument (and all positional arguments prior) *must* be required * Only one positional argument may be `multiple(true)` * Only the second to last, or last positional argument may be `multiple(true)` Closes #725 --- src/app/macros.rs | 2 +- src/app/mod.rs | 8 +-- src/app/parser.rs | 133 +++++++++++++++++++++++++++++++++----------- src/app/settings.rs | 64 +++++++++++---------- 4 files changed, 143 insertions(+), 64 deletions(-) diff --git a/src/app/macros.rs b/src/app/macros.rs index 69be5bfcade..0f9f5ee54e0 100644 --- a/src/app/macros.rs +++ b/src/app/macros.rs @@ -131,7 +131,7 @@ macro_rules! validate_multiples { ($_self:ident, $a:ident, $m:ident) => { debugln!("macro=validate_multiples!;"); if $m.contains(&$a.name) && !$a.settings.is_set(ArgSettings::Multiple) { -// Not the first time, and we don't allow multiples + // Not the first time, and we don't allow multiples return Err(Error::unexpected_multiple_usage($a, &*$_self.create_current_usage($m), $_self.color())) diff --git a/src/app/mod.rs b/src/app/mod.rs index 6374bc57227..cb4f7818750 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1250,7 +1250,7 @@ impl<'a, 'b> App<'a, 'b> { /// [`AppSettings::NoBinaryName`]: ./enum.AppSettings.html#variant.NoBinaryName pub fn get_matches_from(mut self, itr: I) -> ArgMatches<'a> where I: IntoIterator, - T: Into + T: Into + Clone { self.get_matches_from_safe_borrow(itr).unwrap_or_else(|e| { // Otherwise, write to stderr and exit @@ -1292,7 +1292,7 @@ impl<'a, 'b> App<'a, 'b> { /// [`AppSettings::NoBinaryName`]: ./enum.AppSettings.html#variant.NoBinaryName pub fn get_matches_from_safe(mut self, itr: I) -> ClapResult> where I: IntoIterator, - T: Into + T: Into + Clone { self.get_matches_from_safe_borrow(itr) } @@ -1320,7 +1320,7 @@ impl<'a, 'b> App<'a, 'b> { /// [`AppSettings::NoBinaryName`]: ./enum.AppSettings.html#variant.NoBinaryName pub fn get_matches_from_safe_borrow(&mut self, itr: I) -> ClapResult> where I: IntoIterator, - T: Into + T: Into + Clone { // Verify all positional assertions pass self.p.verify_positionals(); @@ -1355,7 +1355,7 @@ impl<'a, 'b> App<'a, 'b> { } // do the real parsing - if let Err(e) = self.p.get_matches_with(&mut matcher, &mut it) { + if let Err(e) = self.p.get_matches_with(&mut matcher, &mut it.peekable()) { return Err(e); } diff --git a/src/app/parser.rs b/src/app/parser.rs index b0b987c13a1..60e4c75d455 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -8,6 +8,7 @@ use std::io::{self, BufWriter, Write}; use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; use std::slice::Iter; +use std::iter::Peekable; // Third Party use vec_map::{self, VecMap}; @@ -118,15 +119,14 @@ impl<'a, 'b> Parser<'a, 'b> let out_dir = PathBuf::from(od); 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), - Shell::Zsh => format!("_{}", name) + Shell::Zsh => format!("_{}", name), }; let mut file = match File::create(out_dir.join(file_name)) { - Err(why) => panic!("couldn't create completion file: {}", - why.description()), + Err(why) => panic!("couldn't create completion file: {}", why.description()), Ok(file) => file, }; self.gen_completions_to(for_shell, &mut file) @@ -266,10 +266,16 @@ impl<'a, 'b> Parser<'a, 'b> for (i, o) in self.opts.iter_mut().enumerate().filter(|&(_, ref o)| o.disp_ord == 999) { o.disp_ord = if unified { o.unified_ord } else { i }; } - for (i, f) in self.flags.iter_mut().enumerate().filter(|&(_, ref f)| f.disp_ord == 999) { + for (i, f) in self.flags + .iter_mut() + .enumerate() + .filter(|&(_, ref f)| f.disp_ord == 999) { f.disp_ord = if unified { f.unified_ord } else { i }; } - for (i, sc) in &mut self.subcommands.iter_mut().enumerate().filter(|&(_, ref sc)| sc.p.meta.disp_ord == 999) { + for (i, sc) in &mut self.subcommands + .iter_mut() + .enumerate() + .filter(|&(_, ref sc)| sc.p.meta.disp_ord == 999) { sc.p.meta.disp_ord = i; } } @@ -517,12 +523,40 @@ impl<'a, 'b> Parser<'a, 'b> } // Next we verify that only the highest index has a .multiple(true) (if any) - debug_assert!(!self.positionals - .values() - .any(|a| a.settings.is_set(ArgSettings::Multiple) && - (a.index as usize != self.positionals.len()) - ), - "Only the positional argument with the highest index may accept multiple values"); + if self.positionals() + .any(|a| { + a.settings.is_set(ArgSettings::Multiple) && + (a.index as usize != self.positionals.len()) + }) { + debug_assert!(self.positionals() + .filter(|p| p.settings.is_set(ArgSettings::Multiple) + && p.num_vals.is_none()).map(|_| 1).sum::() <= 1, + "Only one positional argument with .multiple(true) set is allowed per command"); + + debug_assert!(self.positionals() + .rev() + .next() + .unwrap() + .is_set(ArgSettings::Required), + "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)"); + + self.set(AppSettings::LowIndexMultiplePositional); + } + + debug_assert!(self.positionals() + .filter(|p| p.settings.is_set(ArgSettings::Multiple) + && p.num_vals.is_none()) + .map(|_| 1) + .sum::() <= 1, + "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 @@ -666,10 +700,10 @@ impl<'a, 'b> Parser<'a, 'b> #[cfg_attr(feature = "lints", allow(while_let_on_iterator))] pub fn get_matches_with(&mut self, matcher: &mut ArgMatcher<'a>, - it: &mut I) + it: &mut Peekable) -> ClapResult<()> where I: Iterator, - T: Into + T: Into + Clone { debugln!("fn=get_matches_with;"); // First we create the `--help` and `--version` arguments and add them if @@ -684,15 +718,7 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Begin parsing '{:?}' ({:?})", arg_os, &*arg_os.as_bytes()); // Is this a new argument, or values from a previous option? - debug!("Starts new arg..."); - let starts_new_arg = if arg_os.starts_with(b"-") { - sdebugln!("Maybe"); - // a singe '-' by itself is a value and typically means "stdin" on unix systems - !(arg_os.len_() == 1) - } else { - sdebugln!("No"); - false - }; + let starts_new_arg = is_new_arg(&arg_os); // Has the user already passed '--'? Meaning only positional args follow if !self.trailing_vals { @@ -739,7 +765,8 @@ impl<'a, 'b> Parser<'a, 'b> arg_os.to_string_lossy().parse::().is_ok())); if needs_val_of.is_none() { if self.is_set(AppSettings::AllowNegativeNumbers) { - if !(arg_os.to_string_lossy().parse::().is_ok() || arg_os.to_string_lossy().parse::().is_ok()) { + 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), @@ -747,7 +774,7 @@ impl<'a, 'b> Parser<'a, 'b> } } else if !self.is_set(AppSettings::AllowLeadingHyphen) { continue; - } + } } else { continue; } @@ -775,6 +802,26 @@ impl<'a, 'b> Parser<'a, 'b> } } + debugln!("Positional counter...{}", pos_counter); + debug!("Checking for low index multiples..."); + if self.is_set(AppSettings::LowIndexMultiplePositional) && pos_counter == (self.positionals.len() - 1) { + sdebugln!("Found"); + if let Some(na) = it.peek() { + let n = (*na).clone().into(); + if is_new_arg(&n) || self.possible_subcommand(&n) || suggestions::did_you_mean(&n.to_string_lossy(), + self.subcommands + .iter() + .map(|s| &s.p.meta.name)).is_some() { + debugln!("Bumping the positional counter..."); + pos_counter += 1; + } + } else { + debugln!("Bumping the positional counter..."); + pos_counter += 1; + } + } else { + sdebugln!("None"); + } if let Some(p) = self.positionals.get(pos_counter) { parse_positional!(self, p, arg_os, pos_counter, matcher); } else if self.settings.is_set(AppSettings::AllowExternalSubcommands) { @@ -953,10 +1000,10 @@ impl<'a, 'b> Parser<'a, 'b> fn parse_subcommand(&mut self, sc_name: String, matcher: &mut ArgMatcher<'a>, - it: &mut I) + it: &mut Peekable) -> ClapResult<()> where I: Iterator, - T: Into + T: Into + Clone { use std::fmt::Write; debugln!("fn=parse_subcommand;"); @@ -1949,10 +1996,11 @@ impl<'a, 'b> Parser<'a, 'b> } None } - + fn find_flag(&self, name: &str) -> Option<&FlagBuilder<'a, 'b>> { for f in self.flags() { - if f.name == name || f.aliases.as_ref().unwrap_or(&vec![("",false)]).iter().any(|&(n,_)| n == name) { + if f.name == name || + f.aliases.as_ref().unwrap_or(&vec![("",false)]).iter().any(|&(n, _)| n == name) { return Some(f); } } @@ -1961,7 +2009,8 @@ impl<'a, 'b> Parser<'a, 'b> fn find_option(&self, name: &str) -> Option<&OptBuilder<'a, 'b>> { for o in self.opts() { - if o.name == name || o.aliases.as_ref().unwrap_or(&vec![("",false)]).iter().any(|&(n,_)| n == name) { + if o.name == name || + o.aliases.as_ref().unwrap_or(&vec![("",false)]).iter().any(|&(n, _)| n == name) { return Some(o); } } @@ -1983,7 +2032,15 @@ impl<'a, 'b> Parser<'a, 'b> debugln!("Looking for sc...{}", sc); debugln!("Currently in Parser...{}", 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() && s.p.meta.aliases.as_ref().unwrap().iter().any(|&(s,_)| s == sc.split(' ').rev().next().expect(INTERNAL_ERROR_MSG))) { + 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))) { return Some(s); } if let Some(app) = s.p.find_subcommand(sc) { @@ -2019,3 +2076,17 @@ impl<'a, 'b> Clone for Parser<'a, 'b> } } } + +#[inline] +fn is_new_arg(arg_os: &OsStr) -> bool { + // Is this a new argument, or values from a previous option? + debug!("Starts new arg..."); + if arg_os.starts_with(b"-") { + sdebugln!("Maybe"); + // a singe '-' by itself is a value and typically means "stdin" on unix systems + !(arg_os.len_() == 1) + } else { + sdebugln!("No"); + false + } +} diff --git a/src/app/settings.rs b/src/app/settings.rs index b86eeae1eec..fa43fa73a37 100644 --- a/src/app/settings.rs +++ b/src/app/settings.rs @@ -4,34 +4,35 @@ use std::str::FromStr; bitflags! { flags Flags: u32 { - const SC_NEGATE_REQS = 0b0000000000000000000000000001, - const SC_REQUIRED = 0b0000000000000000000000000010, - const A_REQUIRED_ELSE_HELP = 0b0000000000000000000000000100, - const GLOBAL_VERSION = 0b0000000000000000000000001000, - const VERSIONLESS_SC = 0b0000000000000000000000010000, - const UNIFIED_HELP = 0b0000000000000000000000100000, - const WAIT_ON_ERROR = 0b0000000000000000000001000000, - const SC_REQUIRED_ELSE_HELP= 0b0000000000000000000010000000, - const NEEDS_LONG_HELP = 0b0000000000000000000100000000, - const NEEDS_LONG_VERSION = 0b0000000000000000001000000000, - const NEEDS_SC_HELP = 0b0000000000000000010000000000, - const DISABLE_VERSION = 0b0000000000000000100000000000, - const HIDDEN = 0b0000000000000001000000000000, - const TRAILING_VARARG = 0b0000000000000010000000000000, - const NO_BIN_NAME = 0b0000000000000100000000000000, - const ALLOW_UNK_SC = 0b0000000000001000000000000000, - const UTF8_STRICT = 0b0000000000010000000000000000, - const UTF8_NONE = 0b0000000000100000000000000000, - const LEADING_HYPHEN = 0b0000000001000000000000000000, - const NO_POS_VALUES = 0b0000000010000000000000000000, - const NEXT_LINE_HELP = 0b0000000100000000000000000000, - const DERIVE_DISP_ORDER = 0b0000001000000000000000000000, - const COLORED_HELP = 0b0000010000000000000000000000, - const COLOR_ALWAYS = 0b0000100000000000000000000000, - const COLOR_AUTO = 0b0001000000000000000000000000, - const COLOR_NEVER = 0b0010000000000000000000000000, - const DONT_DELIM_TRAIL = 0b0100000000000000000000000000, - const ALLOW_NEG_NUMS = 0b1000000000000000000000000000, + const SC_NEGATE_REQS = 0b00000000000000000000000000001, + const SC_REQUIRED = 0b00000000000000000000000000010, + const A_REQUIRED_ELSE_HELP = 0b00000000000000000000000000100, + const GLOBAL_VERSION = 0b00000000000000000000000001000, + const VERSIONLESS_SC = 0b00000000000000000000000010000, + const UNIFIED_HELP = 0b00000000000000000000000100000, + const WAIT_ON_ERROR = 0b00000000000000000000001000000, + const SC_REQUIRED_ELSE_HELP= 0b00000000000000000000010000000, + const NEEDS_LONG_HELP = 0b00000000000000000000100000000, + const NEEDS_LONG_VERSION = 0b00000000000000000001000000000, + const NEEDS_SC_HELP = 0b00000000000000000010000000000, + const DISABLE_VERSION = 0b00000000000000000100000000000, + const HIDDEN = 0b00000000000000001000000000000, + const TRAILING_VARARG = 0b00000000000000010000000000000, + const NO_BIN_NAME = 0b00000000000000100000000000000, + const ALLOW_UNK_SC = 0b00000000000001000000000000000, + const UTF8_STRICT = 0b00000000000010000000000000000, + const UTF8_NONE = 0b00000000000100000000000000000, + const LEADING_HYPHEN = 0b00000000001000000000000000000, + const NO_POS_VALUES = 0b00000000010000000000000000000, + const NEXT_LINE_HELP = 0b00000000100000000000000000000, + const DERIVE_DISP_ORDER = 0b00000001000000000000000000000, + const COLORED_HELP = 0b00000010000000000000000000000, + const COLOR_ALWAYS = 0b00000100000000000000000000000, + const COLOR_AUTO = 0b00001000000000000000000000000, + const COLOR_NEVER = 0b00010000000000000000000000000, + const DONT_DELIM_TRAIL = 0b00100000000000000000000000000, + const ALLOW_NEG_NUMS = 0b01000000000000000000000000000, + const LOW_INDEX_MUL_POS = 0b10000000000000000000000000000, } } @@ -72,6 +73,7 @@ impl AppFlags { GlobalVersion => GLOBAL_VERSION, HidePossibleValuesInHelp => NO_POS_VALUES, Hidden => HIDDEN, + LowIndexMultiplePositional => LOW_INDEX_MUL_POS, NeedsLongHelp => NEEDS_LONG_HELP, NeedsLongVersion => NEEDS_LONG_VERSION, NeedsSubcommandHelp => NEEDS_SC_HELP, @@ -663,6 +665,9 @@ pub enum AppSettings { #[doc(hidden)] NeedsSubcommandHelp, + #[doc(hidden)] + LowIndexMultiplePositional, + } impl FromStr for AppSettings { @@ -684,6 +689,7 @@ impl FromStr for AppSettings { "globalversion" => Ok(AppSettings::GlobalVersion), "hidden" => Ok(AppSettings::Hidden), "hidepossiblevaluesinhelp" => Ok(AppSettings::HidePossibleValuesInHelp), + "lowindexmultiplepositional" => Ok(AppSettings::LowIndexMultiplePositional), "nobinaryname" => Ok(AppSettings::NoBinaryName), "nextlinehelp" => Ok(AppSettings::NextLineHelp), "strictutf8" => Ok(AppSettings::StrictUtf8), @@ -735,6 +741,8 @@ mod test { AppSettings::Hidden); assert_eq!("hidepossiblevaluesinhelp".parse::().unwrap(), AppSettings::HidePossibleValuesInHelp); + assert_eq!("lowindexmultiplePositional".parse::().unwrap(), + AppSettings::LowIndexMultiplePositional); assert_eq!("nobinaryname".parse::().unwrap(), AppSettings::NoBinaryName); assert_eq!("nextlinehelp".parse::().unwrap(),