Skip to content

Commit

Permalink
fix(Overrides Self): fixes a bug where options with multiple values c…
Browse files Browse the repository at this point in the history
…ouldnt ever have multiple values

Prior to this commit, an option with multiple values and that also overrode itself (either manually
or via AppSettings::AllArgsOverrideSelf) could never have more than one value. This is fixed as
options with multiple values now ignore the override of self.

If one wants to have options with overrides of self, *and* multiple values you should set
use_delimiter(false) and *not* set multiple(true). Then tell users to use a comma to seperate
their values, i.e. `val1,val2`.
  • Loading branch information
kbknapp committed Feb 6, 2018
1 parent 4bf6d5b commit d95907c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
22 changes: 11 additions & 11 deletions src/args/arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,7 @@ impl<'a, 'b> Arg<'a, 'b> {
/// assert!(m.is_present("flag"));
/// assert_eq!(m.occurrences_of("flag"), 1);
/// ```
/// Making a flag `multiple(true)` and override itself is essentially meaningless. Therefore
/// Making a arg `multiple(true)` and override itself is essentially meaningless. Therefore
/// clap ignores an override of self if it's a flag and it already accepts multiple occurrences.
///
/// ```
Expand All @@ -1253,7 +1253,8 @@ impl<'a, 'b> Arg<'a, 'b> {
/// assert!(m.is_present("flag"));
/// assert_eq!(m.occurrences_of("flag"), 4);
/// ```
/// Now notice with options, it's as if only the last occurrence mattered
/// Now notice with options (which *do not* set `multiple(true)`), it's as if only the last
/// occurrence happened.
///
/// ```
/// # use clap::{App, Arg};
Expand All @@ -1265,8 +1266,7 @@ impl<'a, 'b> Arg<'a, 'b> {
/// assert_eq!(m.value_of("opt"), Some("other"));
/// ```
///
/// Here is where it gets interesting. If an option is declared as `multiple(true)` and it also
/// overrides with itself, only the last *set* of values will be saved.
/// Just like flags, options with `multiple(true)` set, will ignore the "override self" setting.
///
/// ```
/// # use clap::{App, Arg};
Expand All @@ -1275,20 +1275,20 @@ impl<'a, 'b> Arg<'a, 'b> {
/// .overrides_with("opt"))
/// .get_matches_from(vec!["", "--opt", "first", "over", "--opt", "other", "val"]);
/// assert!(m.is_present("opt"));
/// assert_eq!(m.occurrences_of("opt"), 1);
/// assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["other", "val"]);
/// assert_eq!(m.occurrences_of("opt"), 2);
/// assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["first", "over", "other", "val"]);
/// ```
///
/// A safe thing to do, to ensure there is no confusion is to require an argument delimiter and
/// and only one "value set" per instance of the option.
/// A safe thing to do if you'd like to support an option which supports multiple values, but
/// also is "overridable" by itself, is to use `use_delimiter(false)` and *not* use
/// `multiple(true)` while telling users to seperate values with a comma (i.e. `val1,val2`)
///
/// ```
/// # use clap::{App, Arg};
/// let m = App::new("posix")
/// .arg(Arg::from_usage("--opt [val]... 'some option'")
/// .arg(Arg::from_usage("--opt [val] 'some option'")
/// .overrides_with("opt")
/// .number_of_values(1)
/// .require_delimiter(true))
/// .use_delimiter(false))
/// .get_matches_from(vec!["", "--opt=some,other", "--opt=one,two"]);
/// assert!(m.is_present("opt"));
/// assert_eq!(m.occurrences_of("opt"), 1);
Expand Down
2 changes: 1 addition & 1 deletion src/args/arg_matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<'a> ArgMatcher<'a> {
pub fn handle_self_overrides<'b>(&mut self, a: Option<&AnyArg<'a, 'b>>) {
debugln!("ArgMatcher::handle_self_overrides:{:?};", a.map_or(None, |a| Some(a.name())));
if let Some(aa) = a {
if !aa.has_switch() || (!aa.takes_value() && aa.is_set(ArgSettings::Multiple)) {
if !aa.has_switch() || aa.is_set(ArgSettings::Multiple) {
// positional args can't override self or else we would never advance to the next

// Also flags with --multiple set are ignored otherwise we could never have more
Expand Down

0 comments on commit d95907c

Please sign in to comment.