Skip to content

Commit

Permalink
Make overrides_with working when MultipleValues is enabled.
Browse files Browse the repository at this point in the history
  • Loading branch information
ldm0 committed Aug 14, 2021
1 parent 24bfd2b commit a85857d
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 27 deletions.
54 changes: 30 additions & 24 deletions src/build/arg/mod.rs
Expand Up @@ -1096,10 +1096,11 @@ impl<'help> Arg<'help> {
/// **NOTE:** When an argument is overridden it is essentially as if it never was used, any
/// conflicts, requirements, etc. are evaluated **after** all "overrides" have been removed
///
/// **WARNING:** Positional arguments and options which accept [`Multiple*`] cannot override
/// themselves (or we would never be able to advance to the next positional). If a positional
/// argument or option with one of the [`Multiple*`] settings lists itself as an override, it is
/// simply ignored.
/// **WARNING:** Positional arguments and options which accept
/// [`ArgSettings::MultipleOccurrences`] cannot override themselves (or we
/// would never be able to advance to the next positional). If a positional
/// argument or option with one of the [`ArgSettings::MultipleOccurrences`]
/// settings lists itself as an override, it is simply ignored.
///
/// # Examples
///
Expand Down Expand Up @@ -1136,8 +1137,10 @@ impl<'help> Arg<'help> {
/// assert!(m.is_present("flag"));
/// assert_eq!(m.occurrences_of("flag"), 1);
/// ```
/// Making an arg [`Multiple*`] and override itself is essentially meaningless. Therefore
/// clap ignores an override of self if it's a flag and it already accepts multiple occurrences.
///
/// Making an arg [`ArgSettings::MultipleOccurrences`] and override itself
/// is essentially meaningless. Therefore clap ignores an override of self
/// if it's a flag and it already accepts multiple occurrences.
///
/// ```
/// # use clap::{App, Arg};
Expand All @@ -1147,8 +1150,10 @@ impl<'help> Arg<'help> {
/// assert!(m.is_present("flag"));
/// assert_eq!(m.occurrences_of("flag"), 4);
/// ```
/// Now notice with options (which *do not* set one of the [`Multiple*`]), it's as if only the
/// last occurrence happened.
///
/// Now notice with options (which *do not* set
/// [`ArgSettings::MultipleOccurrences`]), it's as if only the last
/// occurrence happened.
///
/// ```
/// # use clap::{App, Arg};
Expand All @@ -1160,36 +1165,37 @@ impl<'help> Arg<'help> {
/// assert_eq!(m.value_of("opt"), Some("other"));
/// ```
///
/// Just like flags, options with one of the [`Multiple*`] set, will ignore the "override self"
/// setting.
/// This will also work when [`ArgSettings::MultipleValues`] is enabled:
///
/// ```
/// # use clap::{App, Arg};
/// let m = App::new("posix")
/// .arg(Arg::from("--opt [val]... 'some option'")
/// .overrides_with("opt"))
/// .get_matches_from(vec!["", "--opt", "first", "over", "--opt", "other", "val"]);
/// .arg(
/// Arg::new("opt")
/// .long("opt")
/// .takes_value(true)
/// .multiple_values(true)
/// .overrides_with("opt")
/// )
/// .get_matches_from(vec!["", "--opt", "1", "2", "--opt", "3", "4", "5"]);
/// assert!(m.is_present("opt"));
/// assert_eq!(m.occurrences_of("opt"), 2);
/// assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["first", "over", "other", "val"]);
/// assert_eq!(m.occurrences_of("opt"), 1);
/// assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["3", "4", "5"]);
/// ```
///
/// 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 not use [`UseValueDelimiter`] and *not* use
/// `MultipleValues` while telling users to separate values with a comma (i.e. `val1,val2`)
/// Just like flags, options with [`ArgSettings::MultipleOccurrences`] set
/// will ignore the "override self" setting.
///
/// ```
/// # use clap::{App, Arg};
/// let m = App::new("posix")
/// .arg(Arg::from("--opt [val] 'some option'")
/// .arg(Arg::from("--opt [val]... 'some option'")
/// .overrides_with("opt"))
/// .get_matches_from(vec!["", "--opt=some,other", "--opt=one,two"]);
/// .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<_>>(), &["one,two"]);
/// assert_eq!(m.occurrences_of("opt"), 2);
/// assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["first", "over", "other", "val"]);
/// ```
/// [`Multiple*`]: crate::ArgSettings::MultipleValues
/// [`UseValueDelimiter`]: ArgSettings::UseValueDelimiter
pub fn overrides_with<T: Key>(mut self, arg_id: T) -> Self {
self.overrides.push(arg_id.into());
self
Expand Down
4 changes: 1 addition & 3 deletions src/parse/parser.rs
Expand Up @@ -1608,10 +1608,8 @@ impl<'help, 'app> Parser<'help, 'app> {
}
}
// Only do self override for argument that is not positional
// argument or flag with one of the Multiple* setting
// enabled(which is a feature).
// argument or flag with MultipleOccurrences setting enabled.
if (self.is_set(AS::AllArgsOverrideSelf) || override_self)
&& !overrider.is_set(ArgSettings::MultipleValues)
&& !overrider.is_set(ArgSettings::MultipleOccurrences)
&& !overrider.is_positional()
{
Expand Down
22 changes: 22 additions & 0 deletions tests/posix_compatible.rs
Expand Up @@ -306,3 +306,25 @@ fn require_overridden_4() {
let err = result.err().unwrap();
assert_eq!(err.kind, ErrorKind::MissingRequiredArgument);
}

#[test]
fn issue_1374_overrides_self_with_multiple_values() {
let app = App::new("test").arg(
Arg::new("input")
.long("input")
.takes_value(true)
.overrides_with("input")
.min_values(0),
);
let m = app
.clone()
.get_matches_from(&["test", "--input", "a", "b", "c", "--input", "d"]);
assert_eq!(m.values_of("input").unwrap().collect::<Vec<_>>(), &["d"]);
let m = app
.clone()
.get_matches_from(&["test", "--input", "a", "b", "--input", "c", "d"]);
assert_eq!(
m.values_of("input").unwrap().collect::<Vec<_>>(),
&["c", "d"]
);
}

0 comments on commit a85857d

Please sign in to comment.