Repository: kbknapp/clap-rs
Pull request #1171 (2018-02-10 04:42:38)

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/1171)
<!-- Reviewable:end -->



Patch:
diff --git a/.gitignore b/.gitignore
index 34253e90..2fc226f4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,3 +25,4 @@ Cargo.lock
 # Project files
 .vscode/*
 .idea/*
+clap-rs.iml
diff --git a/Cargo.toml b/Cargo.toml
index 7b926c37..3aa321ea 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -3,21 +3,49 @@
 name = "clap"
 version = "3.0.0-alpha1"
 authors = ["Kevin K. <kbknapp@gmail.com>"]
-exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"]
+exclude = [
+	".github/*",
+	"examples/*", 
+	"tests/*", 
+	"benches/*", 
+	"clap-perf/*", 
+	"etc/*",
+	"*.png", 
+	"*.dot",
+	"*.yml",
+	"*.toml",
+	"*.md",
+	"clap-test.rs"
+]
+include = [
+	"src/**/*", 
+	"Cargo.toml", 
+	"README.md"
+]
 repository = "https://github.com/kbknapp/clap-rs"
 documentation = "https://docs.rs/clap/"
 homepage = "https://clap.rs/"
 readme = "README.md"
 license = "MIT"
-keywords = ["argument", "cli", "arg", "parser", "parse"]
+keywords = [
+	"argument", 
+	"cli", 
+	"arg", 
+	"parser", 
+	"parse"
+]
 categories = ["command-line-interface"]
 description = """
-A simple to use, efficient, and full featured  Command Line Argument Parser
+A simple to use, efficient, and full featured Command Line Argument Parser
 """
 
 [badges]
 travis-ci = { repository = "kbknapp/clap-rs" }
 appveyor = { repository = "kbknapp/clap-rs" }
+coveralls = { repostiory = "kbknapp/clap-rs", branch = "master" }
+is-it-maintained-issue-resolution = { repository = "kbknapp/clap-rs" }
+is-it-maintained-open-issues = { repository = "kbknapp/clap-rs" }
+maintenance = {status = "actively-developed"}
 
 [dependencies]
 bitflags              = "1.0"
diff --git a/README.md b/README.md
index 1864dc29..21acec56 100644
--- a/README.md
+++ b/README.md
@@ -418,7 +418,7 @@ To test out `clap`'s default auto-generated help/version follow these steps:
 *
 ```toml
 [dependencies]
-clap = "2"
+clap = "3.0.0-alpha1"
 ```
 
 * Add the following to your `src/main.rs`
@@ -441,7 +441,7 @@ For full usage, add `clap` as a dependency in your `Cargo.toml` () to use from c
 
 ```toml
 [dependencies]
-clap = "~3.0"
+clap = "~3.0.0-alpha1"
 ```
 
 (**note**: If you are concerned with supporting a minimum version of Rust that is *older* than the current stable Rust minus 2 stable releases, it's recommended to use the `~major.minor.patch` style versions in your `Cargo.toml` which will only update the patch version automatically. For more information see the [Compatibility Policy](#compatibility-policy))
@@ -464,7 +464,7 @@ To disable these, add this to your `Cargo.toml`:
 
 ```toml
 [dependencies.clap]
-version = "3.0"
+version = "3.0.0-alpha1"
 default-features = false
 ```
 
@@ -472,7 +472,7 @@ You can also selectively enable only the features you'd like to include, by addi
 
 ```toml
 [dependencies.clap]
-version = "3.0"
+version = "3.0.0-alpha1"
 default-features = false
 
 # Cherry-pick the features you'd like to use
@@ -520,7 +520,7 @@ In order to keep from being surprised of breaking changes, it is **highly** reco
 
 ```toml
 [dependencies]
-clap = "~3.0"
+clap = "~3.0.0-alpha1"
 ```
 
 This will cause *only* the patch version to be updated upon a `cargo update` call, and therefore cannot break due to new features, or bumped minimum versions of Rust.
@@ -537,11 +537,11 @@ Right now Cargo's version resolution is pretty naive, it's just a brute-force se
 
 # In one Cargo.toml
 [dependencies]
-clap = "~3.0.0"
+clap = "~3.0.0-alpha1"
 
 # In another Cargo.toml
 [dependencies]
-clap = "3.0"
+clap = "3.0.0-alpha1"
 ```
 
 This is inherently an unresolvable crate graph in Cargo right now. Cargo requires there's only one major version of a crate, and being in the same workspace these two crates must share a version. This is impossible in this location, though, as these version constraints cannot be met.
diff --git a/src/app/parser.rs b/src/app/parser.rs
index 4b528b3c..6f53f736 100644
--- a/src/app/parser.rs
+++ b/src/app/parser.rs
@@ -5,6 +5,7 @@ use std::io::{self, BufWriter, Write};
 use std::os::unix::ffi::OsStrExt;
 use std::slice::Iter;
 use std::iter::Peekable;
+use std::mem;
 
 // Third party
 use vec_map::VecMap;
@@ -1199,13 +1200,27 @@ where
         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();
         for name in matcher.arg_names() {
             debugln!("Parser::remove_overrides:iter:{};", name);
             if let Some(arg) = find!(self.app, name) {
                 if let Some(ref overrides) = arg.overrides {
                     debugln!("Parser::remove_overrides:iter:{}:{:?};", name, overrides);
                     for o in overrides {
-                        if matcher.is_present(o) && !seen.contains(o) {
+                        if o == &arg.name {
+                            if (arg.is_set(ArgSettings::MultipleValues)
+                                || arg.is_set(ArgSettings::MultipleOccurrences))
+                                || !arg.has_switch()
+                            {
+                                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 {
@@ -1222,6 +1237,17 @@ where
             matcher.remove(name);
             self.overriden.push(name);
         }
+        for name in &self_override {
+            debugln!("Parser::remove_overrides:iter:self:{}: resetting;", name);
+            if let Some(ma) = matcher.get_mut(name) {
+                ma.occurs = 1;
+                if !ma.vals.is_empty() {
+                    // This avoids a clone
+                    let mut v = vec![ma.vals.pop().expect(INTERNAL_ERROR_MSG)];
+                    mem::swap(&mut v, &mut ma.vals);
+                }
+            }
+        }
     }
 
     pub(crate) fn add_defaults(&mut self, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> {
diff --git a/src/args/arg.rs b/src/args/arg.rs
index 506c6312..0561517e 100644
--- a/src/args/arg.rs
+++ b/src/args/arg.rs
@@ -857,6 +857,11 @@ 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 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.
+    ///
     /// # Examples
     ///
     /// ```rust
@@ -876,6 +881,76 @@ 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 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.
+    ///
+    /// ```
+    /// # 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 (which *do not* set one of the [`Multiple*`]), it's as if only the
+    /// last occurrence happened.
+    ///
+    /// ```
+    /// # 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"));
+    /// ```
+    ///
+    /// Just like flags, options with one of the [`Multiple*``] set, will ignore the "override self"
+    /// setting.
+    ///
+    /// ```
+    /// # 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"), 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_usage("--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*`]: ./enum.ArgSettings.html#variant.MultipleValues
+    /// [`UseValueDelimiter`]: ./enum.ArgSettings.html#variant.UseValueDelimiter
     pub fn overrides_with(mut self, name: &'a str) -> Self {
         if let Some(ref mut vec) = self.overrides {
             vec.push(name.as_ref());


Test patch:
diff --git a/clap_test/Cargo.toml b/clap_test/Cargo.toml
new file mode 100644
index 00000000..76b08334
--- /dev/null
+++ b/clap_test/Cargo.toml
@@ -0,0 +1,8 @@
+[package]
+name = "clap_test"
+version = "0.1.0"
+authors = ["Kevin K. <kbknapp@gmail.com>"]
+
+[dependencies]
+clap = {path = "../"}
+regex = "0.2"
\ No newline at end of file
diff --git a/clap_test/src/lib.rs b/clap_test/src/lib.rs
new file mode 100644
index 00000000..749d35c8
--- /dev/null
+++ b/clap_test/src/lib.rs
@@ -0,0 +1,60 @@
+use std::str;
+use std::io::Cursor;
+
+extern crate clap;
+extern crate regex;
+
+use regex::Regex;
+
+use clap::App;
+
+fn compare<S, S2>(l: S, r: S2) -> bool
+where
+    S: AsRef<str>,
+    S2: AsRef<str>,
+{
+    let re = Regex::new("\x1b[^m]*m").unwrap();
+    // Strip out any mismatching \r character on windows that might sneak in on either side
+    let ls = l.as_ref().trim().replace("\r", "");
+    let rs = r.as_ref().trim().replace("\r", "");
+    let left = re.replace_all(&*ls, "");
+    let right = re.replace_all(&*rs, "");
+    let b = left == right;
+    if !b {
+        println!("");
+        println!("--> left");
+        println!("{}", left);
+        println!("--> right");
+        println!("{}", right);
+        println!("--")
+    }
+    b
+}
+
+pub fn compare_output(l: App, args: &str, right: &str, stderr: bool) -> bool {
+    let mut buf = Cursor::new(Vec::with_capacity(50));
+    let res = l.get_matches_from_safe(args.split(' ').collect::<Vec<_>>());
+    let err = res.unwrap_err();
+    err.write_to(&mut buf).unwrap();
+    let content = buf.into_inner();
+    let left = String::from_utf8(content).unwrap();
+    assert_eq!(
+        stderr,
+        err.use_stderr(),
+        "Should Use STDERR failed. Should be {} but is {}",
+        stderr,
+        err.use_stderr()
+    );
+    compare(left, right)
+}
+
+pub fn compare_output2(l: App, args: &str, right1: &str, right2: &str, stderr: bool) -> bool {
+    let mut buf = Cursor::new(Vec::with_capacity(50));
+    let res = l.get_matches_from_safe(args.split(' ').collect::<Vec<_>>());
+    let err = res.unwrap_err();
+    err.write_to(&mut buf).unwrap();
+    let content = buf.into_inner();
+    let left = String::from_utf8(content).unwrap();
+    assert_eq!(stderr, err.use_stderr());
+    compare(&*left, right1) || compare(&*left, right2)
+}
diff --git a/tests/posix_compatible.rs b/tests/posix_compatible.rs
index 90191a1a..6416a428 100644
--- a/tests/posix_compatible.rs
+++ b/tests/posix_compatible.rs
@@ -1,7 +1,92 @@
 extern crate clap;
 
 use clap::{App, Arg, ErrorKind};
+#[test]
+fn flag_overrides_itself() {
+    let res = App::new("posix")
+                .arg(Arg::from_usage("--flag  'some flag'").overrides_with("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 mult_flag_overrides_itself() {
+    let res = App::new("posix")
+                .arg(Arg::from_usage("--flag...  'some flag'").overrides_with("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 option_overrides_itself() {
+    let res = App::new("posix")
+                .arg(Arg::from_usage("--opt [val] 'some option'").overrides_with("opt"))
+                .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 mult_option_require_delim_overrides_itself() {
+    let res = App::new("posix")
+                .arg(Arg::from_usage("--opt [val]... 'some option'")
+                    .overrides_with("opt")
+                    .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 mult_option_overrides_itself() {
+    let res = App::new("posix")
+                .arg(Arg::from_usage("--opt [val]... 'some option'")
+                    .overrides_with("opt"))
+                .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 option_use_delim_false_override_itself() {
+
+    let m = App::new("posix")
+                .arg(Arg::from_usage("--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"]);
+}
+
+#[test]
+fn pos_mult_overrides_itself() {
+    // opts with multiple
+    let res = App::new("posix")
+                .arg(Arg::from_usage("[val]... 'some pos'").overrides_with("val"))
+                .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 posix_compatible_flags_long() {
     let m = App::new("posix")