Repository: kbknapp/clap-rs
Pull request #1172 (2018-02-10 20:45:35)

V3 dev


<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/kbknapp/clap-rs/1172)
<!-- Reviewable:end -->



Patch:
diff --git a/src/app/parser.rs b/src/app/parser.rs
index 6f53f736..ce483b22 100644
--- a/src/app/parser.rs
+++ b/src/app/parser.rs
@@ -54,6 +54,7 @@ where
     num_opts: usize,
     num_flags: usize,
     pub positionals: VecMap<&'a str>,
+    seen: Vec<&'a str>,
 }
 
 // Standalone split borrow functions
@@ -104,6 +105,7 @@ where
             num_opts: 0,
             num_flags: 0,
             positionals: VecMap::new(),
+            seen: Vec::new(),
         }
     }
 
@@ -399,6 +401,7 @@ where
                 }
 
                 if starts_new_arg {
+                    self.seen.extend(self.cache);
                     if arg_os.starts_with(b"--") {
                         needs_val_of = self.parse_long_arg(matcher, &arg_os)?;
                         debugln!(
@@ -612,6 +615,9 @@ where
             }
         }
 
+        // Make sure we get the last one too
+        self.seen.extend(self.cache);
+
         if let Some(ref pos_sc_name) = subcmd_name {
             let sc_name = {
                 find_subcmd!(self.app, *pos_sc_name)
@@ -1199,47 +1205,61 @@ where
     fn remove_overrides(&mut self, matcher: &mut ArgMatcher) {
         debugln!("Parser::remove_overrides;");
         let mut to_rem: Vec<&str> = Vec::new();
-        let mut seen: Vec<&str> = Vec::new();
         let mut self_override: Vec<&str> = Vec::new();
+        let mut arg_overrides = Vec::new();
         for name in matcher.arg_names() {
             debugln!("Parser::remove_overrides:iter:{};", name);
             if let Some(arg) = find!(self.app, name) {
+                let mut handle_self_override = |o| {
+                    if (arg.is_set(ArgSettings::MultipleValues)
+                        || arg.is_set(ArgSettings::MultipleOccurrences))
+                        || !arg.has_switch()
+                    {
+                        return true;
+                    }
+                    debugln!(
+                        "Parser::remove_overrides:iter:{}:iter:{}: self override;",
+                        name,
+                        o
+                    );
+                    self_override.push(o);
+                    false
+                };
                 if let Some(ref overrides) = arg.overrides {
                     debugln!("Parser::remove_overrides:iter:{}:{:?};", name, overrides);
                     for o in overrides {
                         if o == &arg.name {
-                            if (arg.is_set(ArgSettings::MultipleValues)
-                                || arg.is_set(ArgSettings::MultipleOccurrences))
-                                || !arg.has_switch()
-                            {
+                            if handle_self_override(o) {
                                 continue;
                             }
-                            debugln!(
-                                "Parser::remove_overrides:iter:{}:iter:{}: self override;",
-                                name,
-                                o
-                            );
-                            self_override.push(o);
-                        } else if matcher.is_present(o) && !seen.contains(o) {
-                            debugln!("Parser::remove_overrides:iter:{}:iter:{}: self;", name, o);
-                            to_rem.push(arg.name);
                         } else {
-                            debugln!("Parser::remove_overrides:iter:{}:iter:{}: other;", name, o);
-                            to_rem.push(o);
+                            arg_overrides.push((&arg.name, o));
+                            arg_overrides.push((o, &arg.name));
                         }
                     }
                 }
-                seen.push(arg.name);
+                if self.is_set(AS::AllArgsOverrideSelf) {
+                    let _ = handle_self_override(arg.name);
+                }
             }
         }
-        for name in &to_rem {
-            debugln!("Parser::remove_overrides:iter:{}: removing;", name);
-            matcher.remove(name);
-            self.overriden.push(name);
+
+        // remove future overrides in reverse seen order
+        for arg in self.seen.iter().rev() {
+            for &(a, overr) in arg_overrides.iter().filter(|&&(a, _)| a == arg) {
+                if !to_rem.contains(a) {
+                    to_rem.push(overr);
+                }
+            }
         }
+
+        // Do self overrides
         for name in &self_override {
             debugln!("Parser::remove_overrides:iter:self:{}: resetting;", name);
             if let Some(ma) = matcher.get_mut(name) {
+                if ma.occurs < 2 {
+                    continue;
+                }
                 ma.occurs = 1;
                 if !ma.vals.is_empty() {
                     // This avoids a clone
@@ -1248,6 +1268,13 @@ where
                 }
             }
         }
+
+        // Finally remove conflicts
+        for name in &to_rem {
+            debugln!("Parser::remove_overrides:iter:{}: removing;", name);
+            matcher.remove(name);
+            self.overriden.push(name);
+        }
     }
 
     pub(crate) fn add_defaults(&mut self, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> {
diff --git a/src/app/settings.rs b/src/app/settings.rs
index 1b304225..4e0147e4 100644
--- a/src/app/settings.rs
+++ b/src/app/settings.rs
@@ -46,6 +46,7 @@ bitflags! {
         const VALID_ARG_FOUND      = 1 << 37;
         const INFER_SUBCOMMANDS    = 1 << 38;
         const CONTAINS_LAST        = 1 << 39;
+        const ARGS_OVERRIDE_SELF   = 1 << 40;
     }
 }
 
@@ -112,6 +113,7 @@ impl AppFlags {
         Propagated => Flags::PROPAGATED,
         ValidArgFound => Flags::VALID_ARG_FOUND,
         InferSubcommands => Flags::INFER_SUBCOMMANDS,
+        AllArgsOverrideSelf => Flags::ARGS_OVERRIDE_SELF,
         ContainsLast => Flags::CONTAINS_LAST
     }
 }
@@ -190,6 +192,11 @@ 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`
+    /// arg using [`Arg::overrides_with("foo")`] for all defined arguments.
+    /// [`Arg::overrides_with("foo")`]: ./struct.Arg.html#method.overrides_with
+    AllArgsOverrideSelf,
+
     /// Allows negative numbers to pass as values. This is similar to
     /// `AllowLeadingHyphen` except that it only allows numbers, all
     /// other undefined leading hyphens will fail to parse.
diff --git a/src/args/arg_matcher.rs b/src/args/arg_matcher.rs
index a73205be..57a9db86 100644
--- a/src/args/arg_matcher.rs
+++ b/src/args/arg_matcher.rs
@@ -21,6 +21,7 @@ impl<'a> Default for ArgMatcher<'a> {
 impl<'a> ArgMatcher<'a> {
     pub fn new() -> Self { ArgMatcher::default() }
 
+    #[allow(dead_code)]
     pub fn is_present(&self, name: &str) -> bool { self.0.is_present(name) }
 
     pub fn propagate_globals(&mut self, global_arg_vec: &[&'a str]) {


Test patch:
diff --git a/tests/app_settings.rs b/tests/app_settings.rs
index f885b9b9..d2dad0aa 100644
--- a/tests/app_settings.rs
+++ b/tests/app_settings.rs
@@ -676,3 +676,176 @@ fn issue_1093_allow_ext_sc() {
         false
     ));
 }
+
+#[test]
+fn aaos_flags() {
+    // flags
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("--flag  'some flag'"))
+        .get_matches_from_safe(vec!["", "--flag", "--flag"]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(m.is_present("flag"));
+    assert_eq!(m.occurrences_of("flag"), 1);
+}
+
+#[test]
+fn aaos_flags_mult() {
+    // flags with multiple
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("--flag...  'some flag'"))
+        .get_matches_from_safe(vec!["", "--flag", "--flag", "--flag", "--flag"]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(m.is_present("flag"));
+    assert_eq!(m.occurrences_of("flag"), 4);
+}
+
+#[test]
+fn aaos_opts() {
+    // opts
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("--opt [val] 'some option'"))
+        .get_matches_from_safe(vec!["", "--opt=some", "--opt=other"]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(m.is_present("opt"));
+    assert_eq!(m.occurrences_of("opt"), 1);
+    assert_eq!(m.value_of("opt"), Some("other"));
+}
+
+#[test]
+fn aaos_opts_w_other_overrides() {
+    // opts with other overrides
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("--opt [val] 'some option'"))
+        .arg(Arg::from_usage("--other [val] 'some other option'").overrides_with("opt"))
+        .get_matches_from_safe(vec!["", "--opt=some", "--other=test", "--opt=other"]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(m.is_present("opt"));
+    assert!(!m.is_present("other"));
+    assert_eq!(m.occurrences_of("opt"), 1);
+    assert_eq!(m.value_of("opt"), Some("other"));
+}
+
+#[test]
+fn aaos_opts_w_other_overrides_rev() {
+    // opts with other overrides, rev
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("--opt [val] 'some option'"))
+        .arg(Arg::from_usage("--other [val] 'some other option'").overrides_with("opt"))
+        .get_matches_from_safe(vec!["", "--opt=some", "--opt=other", "--other=val"]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(!m.is_present("opt"));
+    assert!(m.is_present("other"));
+    assert_eq!(m.value_of("other"), Some("val"));
+}
+
+#[test]
+fn aaos_opts_w_other_overrides_2() {
+    // opts with other overrides
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("--opt [val] 'some option'").overrides_with("other"))
+        .arg(Arg::from_usage("--other [val] 'some other option'"))
+        .get_matches_from_safe(vec!["", "--opt=some", "--other=test", "--opt=other"]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(m.is_present("opt"));
+    assert!(!m.is_present("other"));
+    assert_eq!(m.occurrences_of("opt"), 1);
+    assert_eq!(m.value_of("opt"), Some("other"));
+}
+
+#[test]
+fn aaos_opts_w_other_overrides_rev_2() {
+    // opts with other overrides, rev
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("--opt [val] 'some option'").overrides_with("other"))
+        .arg(Arg::from_usage("--other [val] 'some other option'"))
+        .get_matches_from_safe(vec!["", "--opt=some", "--opt=other", "--other=val"]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(!m.is_present("opt"));
+    assert!(m.is_present("other"));
+    assert_eq!(m.value_of("other"), Some("val"));
+}
+
+#[test]
+fn aaos_opts_mult() {
+    // opts with multiple
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(
+            Arg::from_usage("--opt [val]... 'some option'")
+                .number_of_values(1)
+                .require_delimiter(true),
+        )
+        .get_matches_from_safe(vec!["", "--opt=some", "--opt=other", "--opt=one,two"]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(m.is_present("opt"));
+    assert_eq!(m.occurrences_of("opt"), 3);
+    assert_eq!(
+        m.values_of("opt").unwrap().collect::<Vec<_>>(),
+        &["some", "other", "one", "two"]
+    );
+}
+
+#[test]
+fn aaos_opts_mult_req_delims() {
+    // opts with multiple and require delims
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("--opt [val]... 'some option'"))
+        .get_matches_from_safe(vec![
+            "", "--opt", "first", "overides", "--opt", "some", "other", "val"
+        ]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(m.is_present("opt"));
+    assert_eq!(m.occurrences_of("opt"), 2);
+    assert_eq!(
+        m.values_of("opt").unwrap().collect::<Vec<_>>(),
+        &["first", "overides", "some", "other", "val"]
+    );
+}
+
+#[test]
+fn aaos_pos_mult() {
+    // opts with multiple
+    let res = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("[val]... 'some pos'"))
+        .get_matches_from_safe(vec!["", "some", "other", "value"]);
+    assert!(res.is_ok());
+    let m = res.unwrap();
+    assert!(m.is_present("val"));
+    assert_eq!(m.occurrences_of("val"), 3);
+    assert_eq!(
+        m.values_of("val").unwrap().collect::<Vec<_>>(),
+        &["some", "other", "value"]
+    );
+}
+
+#[test]
+fn aaos_option_use_delim_false() {
+    let m = App::new("posix")
+        .setting(AppSettings::AllArgsOverrideSelf)
+        .arg(Arg::from_usage("--opt [val] 'some option'").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);
+    assert_eq!(
+        m.values_of("opt").unwrap().collect::<Vec<_>>(),
+        &["one,two"]
+    );
+}