imp(Overrides): clap now supports arguments which override with thems…

…elves

This allows properly supporting shell aliases and config files.

Before, if an option only accepted a single value and that option was
set inside an alias, the CLI could never override that value.

For instance:

```
$ alias prog='prog --option=value'
$ prog --option=other
```

This would cause an error. Now clap gracefully accepts the new value to replace the old value.

Closes #976

src/app/validator.rs

@@ -336,7 +336,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
     where
         A: AnyArg<'a, 'b> + Display,
     {
-        debugln!("Validator::validate_arg_num_vals;");
+        debugln!("Validator::validate_arg_num_vals:{}", a.name());
         if let Some(num) = a.num_vals() {
             debugln!("Validator::validate_arg_num_vals: num_vals set...{}", num);
             let should_err = if a.is_set(ArgSettings::Multiple) {

src/args/arg.rs

@@ -1203,6 +1203,10 @@ impl<'a, 'b> Arg<'a, 'b> {
     /// **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 cannot override themselves (or we would never be able
+    /// to advance to the next positional). If a positional agument lists itself as an override,
+    /// it is simply ignored.
+    ///
     /// # Examples
     ///
     /// ```rust
@@ -1222,6 +1226,74 @@ impl<'a, 'b> Arg<'a, 'b> {
     ///                                 // was never used because it was overridden with color
     /// assert!(!m.is_present("flag"));
     /// ```
+    /// Care must be taken when using this setting, and having an arg override with itself. This
+    /// is common practice when supporting things like shell aliases, config files, etc.
+    /// However, when combined with multiple values, it can get dicy.
+    /// Here is how clap handles such situations:
+    ///
+    /// When a flag overrides itself, it's as if the flag was only ever used once (essentially
+    /// preventing a "Unexpected multiple usage" error):
+    ///
+    /// ```rust
+    /// # use clap::{App, Arg};
+    /// let m = App::new("posix")
+    ///             .arg(Arg::from_usage("--flag  'some flag'").overrides_with("flag"))
+    ///             .get_matches_from(vec!["posix", "--flag", "--flag"]);
+    /// assert!(m.is_present("flag"));
+    /// assert_eq!(m.occurrences_of("flag"), 1);
+    /// ```
+    /// Making a flag `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.
+    ///
+    /// ```
+    /// # use clap::{App, Arg};
+    /// let m = App::new("posix")
+    ///             .arg(Arg::from_usage("--flag...  'some flag'").overrides_with("flag"))
+    ///             .get_matches_from(vec!["", "--flag", "--flag", "--flag", "--flag"]);
+    /// 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
+    ///
+    /// ```
+    /// # use clap::{App, Arg};
+    /// let m = App::new("posix")
+    ///             .arg(Arg::from_usage("--opt [val] 'some option'").overrides_with("opt"))
+    ///             .get_matches_from(vec!["", "--opt=some", "--opt=other"]);
+    /// assert!(m.is_present("opt"));
+    /// assert_eq!(m.occurrences_of("opt"), 1);
+    /// 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.
+    ///
+    /// ```
+    /// # use clap::{App, Arg};
+    /// let m = App::new("posix")
+    ///             .arg(Arg::from_usage("--opt [val]... 'some option'")
+    ///                 .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"]);
+    /// ```
+    ///
+    /// 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.
+    ///
+    /// ```
+    /// # use clap::{App, Arg};
+    /// let m = App::new("posix")
+    ///             .arg(Arg::from_usage("--opt [val]... 'some option'")
+    ///                 .overrides_with("opt")
+    ///                 .number_of_values(1)
+    ///                 .require_delimiter(true))
+    ///             .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"]);
+    /// ```
     pub fn overrides_with(mut self, name: &'a str) -> Self {
         if let Some(ref mut vec) = self.b.overrides {
             vec.push(name.as_ref());

src/args/arg_matcher.rs

@@ -21,13 +21,17 @@ impl<'a> Default for ArgMatcher<'a> {
 impl<'a> ArgMatcher<'a> {
     pub fn new() -> Self { ArgMatcher::default() }
 
-    pub fn process_arg_overrides<'b>(&mut self, a: Option<&AnyArg<'a, 'b>>, overrides: &mut Vec<(&'b str, &'a str)>, required: &mut Vec<&'a str>) {
+    pub fn process_arg_overrides<'b>(&mut self, a: Option<&AnyArg<'a, 'b>>, overrides: &mut Vec<(&'b str, &'a str)>, required: &mut Vec<&'a str>, check_all: bool) {
         debugln!("ArgMatcher::process_arg_overrides:{:?};", a.map_or(None, |a| Some(a.name())));
         if let Some(aa) = a {
+            let mut self_done = false;
             if let Some(a_overrides) = aa.overrides() {
                 for overr in a_overrides {
                     debugln!("ArgMatcher::process_arg_overrides:iter:{};", overr);
-                    if self.is_present(overr) {
+                    if overr == &aa.name() {
+                        self_done = true;
+                        self.handle_self_overrides(a);
+                    } else if self.is_present(overr) {
                         debugln!("ArgMatcher::process_arg_overrides:iter:{}: removing from matches;", overr);
                         self.remove(overr);
                         for i in (0 .. required.len()).rev() {
@@ -37,11 +41,38 @@ impl<'a> ArgMatcher<'a> {
                                 break;
                             }
                         }
+                        overrides.push((overr, aa.name()));
                     } else {
                         overrides.push((overr, aa.name()));
                     }
                 }
             }
+            if check_all && !self_done {
+                self.handle_self_overrides(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)) {
+                // 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 
+                // than one
+                return;
+            }
+            if let Some(ma) = self.get_mut(aa.name()) {
+                if ma.vals.len() > 1 {
+                    // swap_remove(0) would be O(1) but does not preserve order, which
+                    // we need
+                    ma.vals.remove(0);
+                    ma.occurs = 1;
+                } else if !aa.takes_value() && ma.occurs > 1 {
+                    ma.occurs = 1;
+                }
+            }
         }
     }
 
@@ -143,7 +174,6 @@ impl<'a> ArgMatcher<'a> {
             occurs: 0,
             vals: Vec::with_capacity(1),
         });
-        // let len = ma.vals.len() + 1;
         ma.vals.push(val.to_owned());
     }