Merge pull request #2696 from ldm0/override

Make `overrides_with` working when `MultipleValues` is enabled.
This commit is contained in:
Pavan Kumar Sunkara 2021-08-14 10:41:40 +01:00 committed by GitHub
commit 203613d5e3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 56 additions and 30 deletions

View file

@ -1099,10 +1099,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
///
@ -1139,8 +1140,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};
@ -1150,8 +1153,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};
@ -1163,8 +1168,26 @@ 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::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"), 1);
/// assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["3", "4", "5"]);
/// ```
///
/// Just like flags, options with [`ArgSettings::MultipleOccurrences`] set
/// will ignore the "override self" setting.
///
/// ```
/// # use clap::{App, Arg};
@ -1176,23 +1199,6 @@ impl<'help> Arg<'help> {
/// 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 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`)
///
/// ```
/// # use clap::{App, Arg};
/// let m = App::new("posix")
/// .arg(Arg::from("--opt [val] 'some option'")
/// .overrides_with("opt"))
/// .get_matches_from(vec!["", "--opt=some,other", "--opt=one,two"]);
/// assert!(m.is_present("opt"));
/// assert_eq!(m.occurrences_of("opt"), 1);
/// assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["one,two"]);
/// ```
/// [`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

View file

@ -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()
{

View file

@ -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"]
);
}