Skip to content

Commit dd2a756

Browse files
kbknappkbknapp
committed
fix(MultipleValues): properly distinguishes between multiple values and 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
1 parent 4a23f1b commit dd2a756

File tree

2 files changed

+80
-36
lines changed

2 files changed

+80
-36
lines changed

‎src/app.rs‎

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,11 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
315315
max_vals: a.max_vals,
316316
help: a.help,
317317
};
318-
if pb.num_vals.unwrap_or(0) > 1 && !pb.multiple {
318+
if pb.min_vals.is_some() && !pb.multiple {
319+
panic!("Argument \"{}\" does not allow multiple values, yet it is expecting {} \
320+
values", pb.name, pb.num_vals.unwrap());
321+
}
322+
if pb.max_vals.is_some() && !pb.multiple {
319323
panic!("Argument \"{}\" does not allow multiple values, yet it is expecting {} \
320324
values", pb.name, pb.num_vals.unwrap());
321325
}
@@ -332,7 +336,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
332336
let mut rhs = HashSet::new();
333337
// without derefing n = &&str
334338
for n in r {
335-
rhs.insert(*n);
339+
rhs.insert(*n);
336340
if pb.required {
337341
self.required.insert(*n);
338342
}
@@ -371,7 +375,11 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
371375
if let Some(ref vec) = ob.val_names {
372376
ob.num_vals = Some(vec.len() as u8);
373377
}
374-
if ob.num_vals.unwrap_or(0) > 1 && !ob.multiple {
378+
if ob.min_vals.is_some() && !ob.multiple {
379+
panic!("Argument \"{}\" does not allow multiple values, yet it is expecting {} \
380+
values", ob.name, ob.num_vals.unwrap());
381+
}
382+
if ob.max_vals.is_some() && !ob.multiple {
375383
panic!("Argument \"{}\" does not allow multiple values, yet it is expecting {} \
376384
values", ob.name, ob.num_vals.unwrap());
377385
}
@@ -387,8 +395,8 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
387395
if let Some(ref r) = a.requires {
388396
let mut rhs = HashSet::new();
389397
// without derefing n = &&str
390-
for n in r {
391-
rhs.insert(*n);
398+
for n in r {
399+
rhs.insert(*n);
392400
if ob.required {
393401
self.required.insert(*n);
394402
}
@@ -683,8 +691,8 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
683691
}
684692

685693
if g_vec.is_empty() {
686-
return args.iter().map(|s| s.to_owned()).collect::<Vec<_>>()
687-
}
694+
return args.iter().map(|s| s.to_owned()).collect::<Vec<_>>()
695+
}
688696
return g_vec.iter().map(|g| self.get_group_members(g)).fold(vec![], |acc, v| acc + &v)
689697
}
690698

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

709717
if g_vec.is_empty() {
710-
return args.iter().map(|s| *s).collect::<Vec<_>>()
711-
}
718+
return args.iter().map(|s| *s).collect::<Vec<_>>()
719+
}
712720
return g_vec.iter()
713721
.map(|g| self.get_group_members_names(g))
714722
.fold(vec![], |acc, v| acc + &v)
@@ -841,7 +849,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
841849
let mut hs = self.required.iter().map(|n| *n).collect::<HashSet<_>>();
842850
tmp_vec.iter().map(|n| hs.insert(*n)).collect::<Vec<_>>();
843851
let reqs = self.get_required_from(hs);
844-
852+
845853
let r_string = reqs.iter().fold(String::new(), |acc, s| acc + &format!(" {}", s)[..]);
846854

847855
usage.push_str(&format!("{}{}",
@@ -1203,11 +1211,11 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
12031211
if let Some(num) = opt.num_vals {
12041212
if let Some(ref ma) = matches.args.get(opt.name) {
12051213
if let Some(ref vals) = ma.values {
1206-
if num == vals.len() as u8 {
1214+
if num == vals.len() as u8 && !opt.multiple {
12071215
self.report_error(format!("The argument \"{}\" was found, \
1208-
but '{}' only expects {} values",
1209-
arg,
1210-
opt,
1216+
but '{}' only expects {} values",
1217+
arg,
1218+
opt,
12111219
vals.len()),
12121220
true,
12131221
true,
@@ -1401,10 +1409,10 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
14011409
match needs_val_of {
14021410
Some(ref a) => {
14031411
if let Some(o) = self.opts.get(a) {
1404-
if o.multiple && self.required.is_empty() {
1412+
if o.multiple && self.required.is_empty() {
14051413
let should_err = match matches.values_of(o.name) {
14061414
Some(ref v) => if v.len() == 0 { true } else { false },
1407-
None => true,
1415+
None => true,
14081416
};
14091417
if should_err {
14101418
self.report_error(
@@ -1423,14 +1431,14 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
14231431
}
14241432
else {
14251433
self.report_error(format!("The following required arguments were not \
1426-
supplied:\n{}",
1434+
supplied:\n{}",
14271435
self.get_required_from(self.required.iter()
14281436
.map(|s| *s)
14291437
.collect::<HashSet<_>>())
14301438
.iter()
14311439
.fold(String::new(), |acc, s| acc + &format!("\t'{}'\n",s)[..])),
1432-
true,
1433-
true,
1440+
true,
1441+
true,
14341442
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
14351443
}
14361444
} else {
@@ -1452,14 +1460,14 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
14521460
if !self.required.is_empty() {
14531461
if self.validate_required(&matches) {
14541462
self.report_error(format!("The following required arguments were not \
1455-
supplied:\n{}",
1463+
supplied:\n{}",
14561464
self.get_required_from(self.required.iter()
14571465
.map(|s| *s)
14581466
.collect::<HashSet<_>>())
14591467
.iter()
14601468
.fold(String::new(), |acc, s| acc + &format!("\t'{}'\n",s)[..])),
1461-
true,
1462-
true,
1469+
true,
1470+
true,
14631471
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
14641472
}
14651473
}
@@ -1741,7 +1749,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
17411749
}
17421750

17431751
// Shouldn't reach here
1744-
self.report_error(format!("Argument --{} isn't valid", arg),
1752+
self.report_error(format!("The argument --{} isn't valid", arg),
17451753
true,
17461754
true,
17471755
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
@@ -1757,7 +1765,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
17571765
for c in arg.chars() {
17581766
self.check_for_help_and_version(c);
17591767
if !self.parse_single_short_flag(matches, c) {
1760-
self.report_error(format!("Argument -{} isn't valid",arg),
1768+
self.report_error(format!("The argument -{} isn't valid",arg),
17611769
true,
17621770
true,
17631771
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));
@@ -1795,8 +1803,8 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
17951803

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

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

18641872
// Make sure this isn't one being added multiple times if it doesn't suppor it
18651873
if matches.args.contains_key(v.name) && !v.multiple {
1866-
self.report_error(format!("Argument -{} was supplied more than once, but does \
1874+
self.report_error(format!("The argument -{} was supplied more than once, but does \
18671875
not support multiple values", arg),
18681876
true,
18691877
true,
@@ -1961,13 +1969,24 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
19611969
if let Some(ref vals) = ma.values {
19621970
if let Some(f) = self.opts.get(name) {
19631971
if let Some(num) = f.num_vals {
1964-
if num != vals.len() as u8 {
1972+
let should_err = if f.multiple {
1973+
((vals.len() as u8) % num) != 0
1974+
} else {
1975+
num != (vals.len() as u8)
1976+
};
1977+
if should_err {
19651978
self.report_error(format!("The argument '{}' requires {} values, \
19661979
but {} w{} provided",
19671980
f,
19681981
num,
1969-
vals.len(),
1970-
if vals.len() == 1 {"as"}else{"ere"}),
1982+
if f.multiple {
1983+
vals.len() % num as usize
1984+
} else {
1985+
vals.len()
1986+
},
1987+
if vals.len() == 1 ||
1988+
( f.multiple &&
1989+
( vals.len() % num as usize) == 1) {"as"}else{"ere"}),
19711990
true,
19721991
true,
19731992
Some(matches.args.keys().map(|k| *k).collect::<Vec<_>>()));

‎src/args/arg.rs‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -654,9 +654,11 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
654654
/// `.number_of_values(3)`, and this argument wouldn't be satisfied unless the user provided
655655
/// 3 and only 3 values.
656656
///
657-
/// **NOTE:** The argument *must* have `.multiple(true)` or `...` to use this setting. Which
658-
/// implies that `qty` must be > 1 (i.e. setting `.number_of_values(1)` would be unnecessary)
657+
/// **NOTE:** `qty` must be > 1
659658
///
659+
/// **NOTE:** Does *not* require `.multiple(true)` to be set. Setting `.multiple(true)` would
660+
/// allow `-f <file> <file> <file> -f <file> <file> <file>` where as *not* setting
661+
/// `.multiple(true)` would only allow one occurrence of this argument.
660662
///
661663
/// # Example
662664
///
@@ -668,6 +670,11 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
668670
/// .number_of_values(3)
669671
/// # ).get_matches();
670672
pub fn number_of_values(mut self, qty: u8) -> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
673+
if qty < 2 {
674+
panic!("Arguments with number_of_values(qty) qty must be > 1. Prefer \
675+
takes_value(true) for arguments with onyl one value, or flags for arguments \
676+
with 0 values.");
677+
}
671678
self.num_vals = Some(qty);
672679
self
673680
}
@@ -677,8 +684,9 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
677684
/// `.max_values(3)`, and this argument would be satisfied if the user provided, 1, 2, or 3
678685
/// values.
679686
///
680-
/// **NOTE:** The argument *must* have `.multiple(true)` or `...` to use this setting. Which
681-
/// implies that `qty` must be > 1 (i.e. setting `.max_values(1)` would be unnecessary)
687+
/// **NOTE:** `qty` must be > 1
688+
///
689+
/// **NOTE:** This implicity sets `.multiple(true)`
682690
///
683691
/// # Example
684692
///
@@ -690,7 +698,13 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
690698
/// .max_values(3)
691699
/// # ).get_matches();
692700
pub fn max_values(mut self, qty: u8) -> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
701+
if qty < 2 {
702+
panic!("Arguments with max_values(qty) qty must be > 1. Prefer \
703+
takes_value(true) for arguments with onyl one value, or flags for arguments \
704+
with 0 values.");
705+
}
693706
self.max_vals = Some(qty);
707+
self.multiple = true;
694708
self
695709
}
696710

@@ -699,7 +713,9 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
699713
/// `.min_values(2)`, and this argument would be satisfied if the user provided, 2 or more
700714
/// values.
701715
///
702-
/// **NOTE:** The argument *must* have `.multiple(true)` or `...` to use this setting.
716+
/// **NOTE:** This implicity sets `.multiple(true)`
717+
///
718+
/// **NOTE:** `qty` must be > 0
703719
///
704720
/// **NOTE:** `qty` *must* be > 0. If you wish to have an argument with 0 or more values prefer
705721
/// two seperate arguments (a flag, and an option with multiple values).
@@ -714,7 +730,12 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
714730
/// .min_values(2)
715731
/// # ).get_matches();
716732
pub fn min_values(mut self, qty: u8) -> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
733+
if qty < 1 {
734+
panic!("Arguments with min_values(qty) qty must be > 0. Prefer flags for arguments \
735+
with 0 values.");
736+
}
717737
self.min_vals = Some(qty);
738+
self.multiple = true;
718739
self
719740
}
720741

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

0 commit comments

Comments
 (0)