Merge pull request clap-rs#1166 from kbknapp/v2.29.4

V2.29.4

CHANGELOG.md

@@ -1,3 +1,13 @@
+<a name="v2.29.4"></a>
+### v2.29.4 (2018-02-06)
+
+
+#### Bug Fixes
+
+* **Overrides Self:**  fixes a bug where options with multiple values couldnt ever have multiple values ([d95907cf](https://github.com/kbknapp/clap-rs/commit/d95907cff6d011a901fe35fa00b0f4e18547a1fb))
+
+
+
 <a name="v2.29.3"></a>
 ### v2.29.3 (2018-02-05)
 

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name = "clap"
-version = "2.29.3"
+version = "2.29.4"
 authors = ["Kevin K. <kbknapp@gmail.com>"]
 exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"]
 repository = "https://github.com/kbknapp/clap-rs"
@@ -18,6 +18,10 @@ 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"

README.md

@@ -42,6 +42,10 @@ Created by [gh-md-toc](https://github.com/ekalinin/github-markdown-toc)
 
 ## What's New
 
+Here's whats new in 2.29.4:
+
+* **Overrides Self:**  fixes a bug where options with multiple values couldnt ever have multiple values ([d95907cf](https://github.com/kbknapp/clap-rs/commit/d95907cff6d011a901fe35fa00b0f4e18547a1fb))
+
 Here's whats new in 2.29.3:
 
 * **Self Overrides:** now supports arguments which override themselves (Allows true shell aliases, config files, etc!)
@@ -69,26 +73,6 @@ Here's whats new in 2.29.1:
 * Updates contributors list
 * Fixes the ripgrep benchmark by adding a value to a flag that expects it
 
-Here's whats new in 2.29.0:
-
-* **Arg:**  adds Arg::hide_env_values(bool) which allows one to hide any current env values and display only the key in help messages
-
-Here's whats new in 2.28.0:
-
-The minimum required Rust is now 1.20. This was done to start using bitflags 1.0 and having >1.0 deps is a *very good* thing!
-
-* Updates `bitflags` to 1.0
-* Adds the traits to be used with the `clap-derive` crate to be able to use Custom Derive (for now must be accessed with `unstable` feature flag)
-* Adds Arg::case_insensitive(bool) which allows matching Arg::possible_values without worrying about ASCII case
-* Fixes a regression where --help couldn't be overridden
-* adds '[SUBCOMMAND]' to usage strings with only AppSettings::AllowExternalSubcommands is used with no other subcommands
-* uses `.bash` for Bash completion scripts now instead of `.bash-completion` due to convention and `.bash-completion` not being supported by completion projects
-* Fix URL path to github hosted files
-* fix typos in docs
-* **README.md:**  updates the readme and pulls out some redundant sections
-* fixes a bug that allowed options to pass parsing when no value was provided
-* ignore PropagateGlobalValuesDown deprecation warning
-
 For full details, see [CHANGELOG.md](https://github.com/kbknapp/clap-rs/blob/master/CHANGELOG.md)
 
 ## About

src/args/arg.rs

@@ -1242,7 +1242,7 @@ impl<'a, 'b> Arg<'a, 'b> {
     /// assert!(m.is_present("flag"));
     /// assert_eq!(m.occurrences_of("flag"), 1);
     /// ```
-    /// Making a flag `multiple(true)` and override itself is essentially meaningless. Therefore
+    /// Making a arg `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.
     ///
     /// ```
@@ -1253,7 +1253,8 @@ impl<'a, 'b> Arg<'a, 'b> {
     /// 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
+    /// Now notice with options (which *do not* set `multiple(true)`), it's as if only the last
+    /// occurrence happened.
     ///
     /// ```
     /// # use clap::{App, Arg};
@@ -1265,8 +1266,7 @@ impl<'a, 'b> Arg<'a, 'b> {
     /// 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.
+    /// Just like flags, options with `multiple(true)` set, will ignore the "override self" setting.
     ///
     /// ```
     /// # use clap::{App, Arg};
@@ -1275,24 +1275,24 @@ impl<'a, 'b> Arg<'a, 'b> {
     ///                 .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"]);
+    /// 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, to ensure there is no confusion is to require an argument delimiter and
-    /// and only one "value set" per instance of the option.
+    /// 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 use `use_delimiter(false)` and *not* use 
+    /// `multiple(true)` while telling users to seperate 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'")
+    ///             .arg(Arg::from_usage("--opt [val] 'some option'")
     ///                 .overrides_with("opt")
-    ///                 .number_of_values(1)
-    ///                 .require_delimiter(true))
+    ///                 .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"]);
+    /// 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 {

src/args/arg_matcher.rs

@@ -56,7 +56,7 @@ impl<'a> ArgMatcher<'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)) {
+            if !aa.has_switch() || 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 

src/lib.rs

@@ -513,7 +513,7 @@
 //! this repository for more information.
 
 #![crate_type = "lib"]
-#![doc(html_root_url = "https://docs.rs/clap/2.29.3")]
+#![doc(html_root_url = "https://docs.rs/clap/2.29.4")]
 #![deny(missing_docs, missing_debug_implementations, missing_copy_implementations, trivial_casts,
         unused_import_braces, unused_allocation)]
 // Lints we'd like to deny but are currently failing for upstream crates

tests/app_settings.rs

@@ -784,8 +784,8 @@ fn aaos_opts_mult() {
     assert!(res.is_ok());
     let m = res.unwrap();
     assert!(m.is_present("opt"));
-    assert_eq!(m.occurrences_of("opt"), 1);
-    assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["one", "two"]);
+    assert_eq!(m.occurrences_of("opt"), 3);
+    assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["some", "other", "one", "two"]);
 }
 
 #[test]
@@ -798,8 +798,8 @@ fn aaos_opts_mult_req_delims() {
     assert!(res.is_ok());
     let m = res.unwrap();
     assert!(m.is_present("opt"));
-    assert_eq!(m.occurrences_of("opt"), 1);
-    assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["some", "other", "val"]);
+    assert_eq!(m.occurrences_of("opt"), 2);
+    assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["first", "overides", "some", "other", "val"]);
 }
 
 #[test]
@@ -814,4 +814,17 @@ fn aaos_pos_mult() {
     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"]);
+}

tests/posix_compatible.rs

@@ -47,8 +47,8 @@ fn mult_option_require_delim_overrides_itself() {
     assert!(res.is_ok());
     let m = res.unwrap();
     assert!(m.is_present("opt"));
-    assert_eq!(m.occurrences_of("opt"), 1);
-    assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["one", "two"]);
+    assert_eq!(m.occurrences_of("opt"), 3);
+    assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["some", "other", "one", "two"]);
 }
 
 #[test]
@@ -60,12 +60,25 @@ fn mult_option_overrides_itself() {
     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")
+                    .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<_>>(), &["some", "other", "val"]);
+    assert_eq!(m.values_of("opt").unwrap().collect::<Vec<_>>(), &["one,two"]);
 }
 
 #[test]
-fn aaos_pos_mult() {
+fn pos_mult_overrides_itself() {
     // opts with multiple
     let res = App::new("posix")
                 .arg(Arg::from_usage("[val]... 'some pos'").overrides_with("val"))