Skip to content

Commit

Permalink
feat(Positional Args): allows specifying the second to last positiona…
Browse files Browse the repository at this point in the history 
…l argument as multiple(true)

Now one can build CLIs that support things like `mv <files>... <target>`

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
  • Loading branch information
@kbknapp
kbknapp committed Nov 1, 2016
1 parent 1d6f8fd commit 1ced2a7
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 64 deletions.
2 changes: 1 addition & 1 deletion src/app/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Expand Down
8 changes: 4 additions & 4 deletions src/app/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1250,7 +1250,7 @@ impl<'a, 'b> App<'a, 'b> {
/// [`AppSettings::NoBinaryName`]: ./enum.AppSettings.html#variant.NoBinaryName
pub fn get_matches_from<I, T>(mut self, itr: I) -> ArgMatches<'a>
where I: IntoIterator<Item = T>,
T: Into<OsString>
T: Into<OsString> + Clone
{
self.get_matches_from_safe_borrow(itr).unwrap_or_else(|e| {
// Otherwise, write to stderr and exit
Expand Down Expand Up @@ -1292,7 +1292,7 @@ impl<'a, 'b> App<'a, 'b> {
/// [`AppSettings::NoBinaryName`]: ./enum.AppSettings.html#variant.NoBinaryName
pub fn get_matches_from_safe<I, T>(mut self, itr: I) -> ClapResult<ArgMatches<'a>>
where I: IntoIterator<Item = T>,
T: Into<OsString>
T: Into<OsString> + Clone

This comment has been minimized.

Sign in to view

Copy link
@jrvanwhy

jrvanwhy Nov 12, 2016

FYI, adding Clone here is technically a breaking change (it broke my build). Is there a way to remove this bound without breaking the multiple positional arguments feature?

This comment has been minimized.

Sign in to view

Copy link
@kbknapp

kbknapp Nov 12, 2016

Author Member

@jrvanwhy Thank you for posting this! I apologize it broke your build, could you file an issue for tracking and I'll look into a way to remove this bound.

This comment has been minimized.

Sign in to view

Copy link
@jrvanwhy

jrvanwhy Nov 12, 2016

It's not hard for me to fix, so it's no big deal. It's awfully easy to make backwards-incompatible changes in Rust libraries ;).

I've filed issue #738. I may take a look and see if I can find a solution, but I'm tired and don't feel like reading code right now 8p.
{
self.get_matches_from_safe_borrow(itr)
}
Expand Down Expand Up @@ -1320,7 +1320,7 @@ impl<'a, 'b> App<'a, 'b> {
/// [`AppSettings::NoBinaryName`]: ./enum.AppSettings.html#variant.NoBinaryName
pub fn get_matches_from_safe_borrow<I, T>(&mut self, itr: I) -> ClapResult<ArgMatches<'a>>
where I: IntoIterator<Item = T>,
T: Into<OsString>
T: Into<OsString> + Clone
{
// Verify all positional assertions pass
self.p.verify_positionals();
Expand Down Expand Up @@ -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);
}

Expand Down
133 changes: 102 additions & 31 deletions src/app/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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::<u64>() <= 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::<u64>() <= 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
Expand Down Expand Up @@ -666,10 +700,10 @@ impl<'a, 'b> Parser<'a, 'b>
#[cfg_attr(feature = "lints", allow(while_let_on_iterator))]
pub fn get_matches_with<I, T>(&mut self,
matcher: &mut ArgMatcher<'a>,
it: &mut I)
it: &mut Peekable<I>)
-> ClapResult<()>
where I: Iterator<Item = T>,
T: Into<OsString>
T: Into<OsString> + Clone
{
debugln!("fn=get_matches_with;");
// First we create the `--help` and `--version` arguments and add them if
Expand All @@ -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 {
Expand Down Expand Up @@ -739,15 +765,16 @@ impl<'a, 'b> Parser<'a, 'b>
arg_os.to_string_lossy().parse::<f64>().is_ok()));
if needs_val_of.is_none() {
if self.is_set(AppSettings::AllowNegativeNumbers) {
if !(arg_os.to_string_lossy().parse::<i64>().is_ok() || arg_os.to_string_lossy().parse::<f64>().is_ok()) {
if !(arg_os.to_string_lossy().parse::<i64>().is_ok() ||
arg_os.to_string_lossy().parse::<f64>().is_ok()) {
return Err(Error::unknown_argument(&*arg_os.to_string_lossy(),
"",
&*self.create_current_usage(matcher),
self.color()));
}
} else if !self.is_set(AppSettings::AllowLeadingHyphen) {
continue;
}
}
} else {
continue;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -953,10 +1000,10 @@ impl<'a, 'b> Parser<'a, 'b>
fn parse_subcommand<I, T>(&mut self,
sc_name: String,
matcher: &mut ArgMatcher<'a>,
it: &mut I)
it: &mut Peekable<I>)
-> ClapResult<()>
where I: Iterator<Item = T>,
T: Into<OsString>
T: Into<OsString> + Clone
{
use std::fmt::Write;
debugln!("fn=parse_subcommand;");
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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
}
}
64 changes: 36 additions & 28 deletions src/app/settings.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -663,6 +665,9 @@ pub enum AppSettings {
#[doc(hidden)]
NeedsSubcommandHelp,

#[doc(hidden)]
LowIndexMultiplePositional,

}

impl FromStr for AppSettings {
Expand All @@ -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),
Expand Down Expand Up @@ -735,6 +741,8 @@ mod test {
AppSettings::Hidden);
assert_eq!("hidepossiblevaluesinhelp".parse::<AppSettings>().unwrap(),
AppSettings::HidePossibleValuesInHelp);
assert_eq!("lowindexmultiplePositional".parse::<AppSettings>().unwrap(),
AppSettings::LowIndexMultiplePositional);
assert_eq!("nobinaryname".parse::<AppSettings>().unwrap(),
AppSettings::NoBinaryName);
assert_eq!("nextlinehelp".parse::<AppSettings>().unwrap(),
Expand Down

0 comments on commit 1ced2a7

Please sign in to comment.