Change the way parser do self override

src/build/app/settings.rs

@@ -225,7 +225,7 @@ pub enum AppSettings {
     /// [`Arg::allow_hyphen_values`]: ./struct.Arg.html#method.allow_hyphen_values
     AllowLeadingHyphen,
 
-    /// Specifies that all arguments override themselves. This is the equivolent to saying the `foo`
+    /// Specifies that all arguments override themselves. This is the equivalent to saying the `foo`
     /// arg using [`Arg::overrides_with("foo")`] for all defined arguments.
     ///
     /// [`Arg::overrides_with("foo")`]: ./struct.Arg.html#method.overrides_with

src/parse/matches/matched_arg.rs

@@ -89,6 +89,8 @@ impl MatchedArg {
         self.vals.iter().flatten().count() == 0
     }
 
+    // Will be used later
+    #[allow(dead_code)]
     pub(crate) fn remove_vals(&mut self, len: usize) {
         let mut want_remove = len;
         let mut remove_group = None;
@@ -110,6 +112,13 @@ impl MatchedArg {
         }
     }
 
+    pub(crate) fn override_vals(&mut self) {
+        let len = self.vals.len();
+        if len > 1 {
+            self.vals.drain(0..len - 1);
+        }
+    }
+
     pub(crate) fn contains_val(&self, val: &str) -> bool {
         self.vals_flatten()
             .any(|v| OsString::as_os_str(v) == OsStr::new(val))
@@ -124,6 +133,47 @@ impl MatchedArg {
 mod tests {
     use super::*;
 
+    #[test]
+    fn test_vals_override() {
+        let mut m = MatchedArg::new();
+        m.push_val("aaa".into());
+        m.new_val_group();
+        m.append_val("bbb".into());
+        m.append_val("ccc".into());
+        m.new_val_group();
+        m.append_val("ddd".into());
+        m.push_val("eee".into());
+        m.new_val_group();
+        m.append_val("fff".into());
+        m.append_val("ggg".into());
+        m.append_val("hhh".into());
+        m.append_val("iii".into());
+
+        m.override_vals();
+        let vals: Vec<&Vec<OsString>> = m.vals().collect();
+        assert_eq!(
+            vals,
+            vec![&vec![
+                OsString::from("fff"),
+                OsString::from("ggg"),
+                OsString::from("hhh"),
+                OsString::from("iii"),
+            ]]
+        );
+        m.override_vals();
+
+        let vals: Vec<&Vec<OsString>> = m.vals().collect();
+        assert_eq!(
+            vals,
+            vec![&vec![
+                OsString::from("fff"),
+                OsString::from("ggg"),
+                OsString::from("hhh"),
+                OsString::from("iii"),
+            ]]
+        );
+    }
+
     #[test]
     fn test_grouped_vals_push_and_append() {
         let mut m = MatchedArg::new();

src/parse/parser.rs

@@ -1346,34 +1346,27 @@ impl<'help, 'app> Parser<'help, 'app> {
         let mut arg_overrides = Vec::new();
         for name in matcher.arg_names() {
             debug!("Parser::remove_overrides:iter:{:?}", name);
-            if let Some(arg) = self.app.find(name) {
-                let mut handle_self_override = |o: &Id| {
-                    if (arg.is_set(ArgSettings::MultipleValues)
-                        || arg.is_set(ArgSettings::MultipleOccurrences))
-                        || !arg.has_switch()
-                    {
-                        return true;
+            if let Some(overrider) = self.app.find(name) {
+                let mut override_self = false;
+                for overridee in &overrider.overrides {
+                    debug!("Parser::remove_overrides:iter:{:?} => {:?}", name, o);
+                    if *overridee == overrider.id {
+                        override_self = true;
+                    } else {
+                        arg_overrides.push((overrider.id.clone(), overridee));
+                        arg_overrides.push((overridee.clone(), &overrider.id));
                     }
+                }
+                if (self.is_set(AS::AllArgsOverrideSelf) || override_self)
+                    && !overrider.is_set(ArgSettings::MultipleValues)
+                    && !overrider.is_set(ArgSettings::MultipleOccurrences)
+                    && overrider.has_switch()
+                {
                     debug!(
                         "Parser::remove_overrides:iter:{:?}:iter:{:?}: self override",
                         name, o
                     );
-                    self_override.push(o.clone());
-                    false
-                };
-                for o in &arg.overrides {
-                    debug!("Parser::remove_overrides:iter:{:?} => {:?}", name, o);
-                    if *o == arg.id {
-                        if handle_self_override(o) {
-                            continue;
-                        }
-                    } else {
-                        arg_overrides.push((arg.id.clone(), o));
-                        arg_overrides.push((o.clone(), &arg.id));
-                    }
-                }
-                if self.is_set(AS::AllArgsOverrideSelf) {
-                    let _ = handle_self_override(&arg.id);
+                    self_override.push(overrider.id.clone());
                 }
             }
         }
@@ -1391,13 +1384,8 @@ impl<'help, 'app> Parser<'help, 'app> {
         for name in &self_override {
             debug!("Parser::remove_overrides:iter:self:{:?}: resetting", name);
             if let Some(ma) = matcher.get_mut(name) {
-                if ma.occurs < 2 {
-                    continue;
-                }
                 ma.occurs = 1;
-
-                let len = ma.num_vals().saturating_sub(1);
-                ma.remove_vals(len);
+                ma.override_vals();
             }
         }
 

tests/grouped_values.rs

@@ -1,6 +1,6 @@
 mod utils;
 
-use clap::{App, Arg};
+use clap::{App, AppSettings, Arg};
 
 #[test]
 fn value_sets_works() {
@@ -145,3 +145,46 @@ fn value_sets_multiple_positional_arg_last_multiple() {
         vec![vec!["val2", "val3", "val4", "val5", "val6"]]
     );
 }
+
+#[test]
+fn issue_1374() {
+    let app = App::new("MyApp").arg(
+        Arg::new("input")
+            .takes_value(true)
+            .long("input")
+            .overrides_with("input")
+            .min_values(0),
+    );
+    let matches = app
+        .clone()
+        .get_matches_from(&["MyApp", "--input", "a", "b", "c", "--input", "d"]);
+    let vs = matches.values_of("input").unwrap();
+    assert_eq!(vs.collect::<Vec<_>>(), vec!["d"]);
+    let matches = app
+        .clone()
+        .get_matches_from(&["MyApp", "--input", "a", "b", "--input", "c", "d"]);
+    let vs = matches.values_of("input").unwrap();
+    assert_eq!(vs.collect::<Vec<_>>(), vec!["c", "d"]);
+}
+
+#[test]
+fn issue_2171() {
+    let schema = App::new("ripgrep#1701 reproducer")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::new("pretty").short('p').long("pretty"))
+        .arg(Arg::new("search_zip").short('z').long("search-zip"));
+
+    let test_args = &[
+        vec!["reproducer", "-pz", "-p"],
+        vec!["reproducer", "-pzp"],
+        vec!["reproducer", "-zpp"],
+        vec!["reproducer", "-pp", "-z"],
+        vec!["reproducer", "-p", "-p", "-z"],
+        vec!["reproducer", "-p", "-pz"],
+        vec!["reproducer", "-ppz"],
+    ];
+
+    for argv in test_args {
+        let _ = schema.clone().try_get_matches_from(argv).unwrap();
+    }
+}