Skip to content

Commit

Permalink
fix(MultipleValues): properly distinguishes between multiple values a…
Browse files Browse the repository at this point in the history 
…nd multiple occurrences

When using number_of_values() or value_names() you no longer have to set
.multiple(true) unless you want the argument to be allowed multiple
times (i.e. *not* the value, but the argument itself). This means
without multiple(true) set (and assuming 2 values) -o val1 val2 is only
allowed one time, but with multiple(true) set, -o val1 val2 -o val3 val4
is allowed.

Closes #99
  • Loading branch information
@kbknapp
kbknapp committed May 5, 2015
1 parent 4a23f1b commit dd2a756
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 36 deletions.
81 changes: 50 additions & 31 deletions src/app.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,11 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
max_vals: a.max_vals,
help: a.help,
};
if pb.num_vals.unwrap_or(0) > 1 && !pb.multiple {
if pb.min_vals.is_some() && !pb.multiple {
panic!("Argument \"{}\" does not allow multiple values, yet it is expecting {} \
values", pb.name, pb.num_vals.unwrap());
}
if pb.max_vals.is_some() && !pb.multiple {
panic!("Argument \"{}\" does not allow multiple values, yet it is expecting {} \
values", pb.name, pb.num_vals.unwrap());
}
Expand All @@ -332,7 +336,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
let mut rhs = HashSet::new();
// without derefing n = &&str
for n in r {
rhs.insert(*n);
rhs.insert(*n);
if pb.required {
self.required.insert(*n);
}
Expand Down Expand Up @@ -371,7 +375,11 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
if let Some(ref vec) = ob.val_names {
ob.num_vals = Some(vec.len() as u8);
}
if ob.num_vals.unwrap_or(0) > 1 && !ob.multiple {
if ob.min_vals.is_some() && !ob.multiple {
panic!("Argument \"{}\" does not allow multiple values, yet it is expecting {} \
values", ob.name, ob.num_vals.unwrap());
}
if ob.max_vals.is_some() && !ob.multiple {
panic!("Argument \"{}\" does not allow multiple values, yet it is expecting {} \
values", ob.name, ob.num_vals.unwrap());
}
Expand All @@ -387,8 +395,8 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
if let Some(ref r) = a.requires {
let mut rhs = HashSet::new();
// without derefing n = &&str
for n in r {
rhs.insert(*n);
for n in r {
rhs.insert(*n);
if ob.required {
self.required.insert(*n);
}
Expand Down Expand Up @@ -683,8 +691,8 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
}

if g_vec.is_empty() {
return args.iter().map(|s| s.to_owned()).collect::<Vec<_>>()
}
return args.iter().map(|s| s.to_owned()).collect::<Vec<_>>()
}
return g_vec.iter().map(|g| self.get_group_members(g)).fold(vec![], |acc, v| acc + &v)
}

Expand All @@ -707,8 +715,8 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
}

if g_vec.is_empty() {
return args.iter().map(|s| *s).collect::<Vec<_>>()
}
return args.iter().map(|s| *s).collect::<Vec<_>>()
}
return g_vec.iter()
.map(|g| self.get_group_members_names(g))
.fold(vec![], |acc, v| acc + &v)
Expand Down Expand Up @@ -841,7 +849,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
let mut hs = self.required.iter().map(|n| *n).collect::<HashSet<_>>();
tmp_vec.iter().map(|n| hs.insert(*n)).collect::<Vec<_>>();
let reqs = self.get_required_from(hs);

let r_string = reqs.iter().fold(String::new(), |acc, s| acc + &format!(" {}", s)[..]);

usage.push_str(&format!("{}{}",
Expand Down Expand Up @@ -1203,11 +1211,11 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
if let Some(num) = opt.num_vals {
if let Some(ref ma) = matches.args.get(opt.name) {
if let Some(ref vals) = ma.values {
if num == vals.len() as u8 {
if num == vals.len() as u8 && !opt.multiple {
self.report_error(format!("The argument \"{}\" was found, \
but '{}' only expects {} values",
arg,
opt,
but '{}' only expects {} values",
arg,
opt,
vals.len()),
true,
true,
Expand Down Expand Up @@ -1401,10 +1409,10 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
match needs_val_of {
Some(ref a) => {
if let Some(o) = self.opts.get(a) {
if o.multiple && self.required.is_empty() {
if o.multiple && self.required.is_empty() {
let should_err = match matches.values_of(o.name) {
Some(ref v) => if v.len() == 0 { true } else { false },
None => true,
None => true,
};
if should_err {
self.report_error(
Expand All @@ -1423,14 +1431,14 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
}
else {
self.report_error(format!("The following required arguments were not \
supplied:\n{}",
supplied:\n{}",
self.get_required_from(self.required.iter()
.map(|s| *s)
.collect::<HashSet<_>>())
.iter()
.fold(String::new(), |acc, s| acc + &format!("\t'{}'\n",s)[..])),
true,
true,
true,
true,
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
}
} else {
Expand All @@ -1452,14 +1460,14 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
if !self.required.is_empty() {
if self.validate_required(&matches) {
self.report_error(format!("The following required arguments were not \
supplied:\n{}",
supplied:\n{}",
self.get_required_from(self.required.iter()
.map(|s| *s)
.collect::<HashSet<_>>())
.iter()
.fold(String::new(), |acc, s| acc + &format!("\t'{}'\n",s)[..])),
true,
true,
true,
true,
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
}
}
Expand Down Expand Up @@ -1741,7 +1749,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
}

// Shouldn't reach here
self.report_error(format!("Argument --{} isn't valid", arg),
self.report_error(format!("The argument --{} isn't valid", arg),
true,
true,
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
Expand All @@ -1757,7 +1765,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
for c in arg.chars() {
self.check_for_help_and_version(c);
if !self.parse_single_short_flag(matches, c) {
self.report_error(format!("Argument -{} isn't valid",arg),
self.report_error(format!("The argument -{} isn't valid",arg),
true,
true,
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
Expand Down Expand Up @@ -1795,8 +1803,8 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{

if matches.args.contains_key(v.name) {
if !v.multiple {
self.report_error(format!("Argument -{} was supplied more than once, but does \
not support multiple values", arg),
self.report_error(format!("The argument -{} was supplied more than once, but \
does not support multiple values", arg),
true,
true,
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
Expand Down Expand Up @@ -1834,7 +1842,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
}

// Didn't match a flag or option, must be invalid
self.report_error( format!("Argument -{} isn't valid",arg_c),
self.report_error( format!("The argument -{} isn't valid",arg_c),
true,
true,
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
Expand Down Expand Up @@ -1863,7 +1871,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{

// Make sure this isn't one being added multiple times if it doesn't suppor it
if matches.args.contains_key(v.name) && !v.multiple {
self.report_error(format!("Argument -{} was supplied more than once, but does \
self.report_error(format!("The argument -{} was supplied more than once, but does \
not support multiple values", arg),
true,
true,
Expand Down Expand Up @@ -1961,13 +1969,24 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
if let Some(ref vals) = ma.values {
if let Some(f) = self.opts.get(name) {
if let Some(num) = f.num_vals {
if num != vals.len() as u8 {
let should_err = if f.multiple {
((vals.len() as u8) % num) != 0
} else {
num != (vals.len() as u8)
};
if should_err {
self.report_error(format!("The argument '{}' requires {} values, \
but {} w{} provided",
f,
num,
vals.len(),
if vals.len() == 1 {"as"}else{"ere"}),
if f.multiple {
vals.len() % num as usize
} else {
vals.len()
},
if vals.len() == 1 ||
( f.multiple &&
( vals.len() % num as usize) == 1) {"as"}else{"ere"}),
true,
true,
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
Expand Down
35 changes: 30 additions & 5 deletions src/args/arg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -654,9 +654,11 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
/// `.number_of_values(3)`, and this argument wouldn't be satisfied unless the user provided
/// 3 and only 3 values.
///
/// **NOTE:** The argument *must* have `.multiple(true)` or `...` to use this setting. Which
/// implies that `qty` must be > 1 (i.e. setting `.number_of_values(1)` would be unnecessary)
/// **NOTE:** `qty` must be > 1
///
/// **NOTE:** Does *not* require `.multiple(true)` to be set. Setting `.multiple(true)` would
/// allow `-f <file> <file> <file> -f <file> <file> <file>` where as *not* setting
/// `.multiple(true)` would only allow one occurrence of this argument.
///
/// # Example
///
Expand All @@ -668,6 +670,11 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
/// .number_of_values(3)
/// # ).get_matches();
pub fn number_of_values(mut self, qty: u8) -> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
if qty < 2 {
panic!("Arguments with number_of_values(qty) qty must be > 1. Prefer \
takes_value(true) for arguments with onyl one value, or flags for arguments \
with 0 values.");
}
self.num_vals = Some(qty);
self
}
Expand All @@ -677,8 +684,9 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
/// `.max_values(3)`, and this argument would be satisfied if the user provided, 1, 2, or 3
/// values.
///
/// **NOTE:** The argument *must* have `.multiple(true)` or `...` to use this setting. Which
/// implies that `qty` must be > 1 (i.e. setting `.max_values(1)` would be unnecessary)
/// **NOTE:** `qty` must be > 1
///
/// **NOTE:** This implicity sets `.multiple(true)`
///
/// # Example
///
Expand All @@ -690,7 +698,13 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
/// .max_values(3)
/// # ).get_matches();
pub fn max_values(mut self, qty: u8) -> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
if qty < 2 {
panic!("Arguments with max_values(qty) qty must be > 1. Prefer \
takes_value(true) for arguments with onyl one value, or flags for arguments \
with 0 values.");
}
self.max_vals = Some(qty);
self.multiple = true;
self
}

Expand All @@ -699,7 +713,9 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
/// `.min_values(2)`, and this argument would be satisfied if the user provided, 2 or more
/// values.
///
/// **NOTE:** The argument *must* have `.multiple(true)` or `...` to use this setting.
/// **NOTE:** This implicity sets `.multiple(true)`
///
/// **NOTE:** `qty` must be > 0
///
/// **NOTE:** `qty` *must* be > 0. If you wish to have an argument with 0 or more values prefer
/// two seperate arguments (a flag, and an option with multiple values).
Expand All @@ -714,7 +730,12 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
/// .min_values(2)
/// # ).get_matches();
pub fn min_values(mut self, qty: u8) -> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
if qty < 1 {
panic!("Arguments with min_values(qty) qty must be > 0. Prefer flags for arguments \
with 0 values.");
}
self.min_vals = Some(qty);
self.multiple = true;
self
}

Expand All @@ -727,6 +748,10 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
/// be aware that the number of "names" you set for the values, will be the *exact* number of
/// values required to satisfy this argument
///
/// **NOTE:** Does *not* require `.multiple(true)` to be set. Setting `.multiple(true)` would
/// allow `-f <file> <file> <file> -f <file> <file> <file>` where as *not* setting
/// `.multiple(true)` would only allow one occurrence of this argument.
///
/// # Example
///
/// ```no_run
Expand Down

0 comments on commit dd2a756

Please sign in to comment.