From d95907cff6d011a901fe35fa00b0f4e18547a1fb Mon Sep 17 00:00:00 2001 From: Kevin K Date: Mon, 5 Feb 2018 19:46:00 -0500 Subject: [PATCH 1/4] fix(Overrides Self): fixes a bug where options with multiple values couldnt ever have multiple values Prior to this commit, an option with multiple values and that also overrode itself (either manually or via AppSettings::AllArgsOverrideSelf) could never have more than one value. This is fixed as options with multiple values now ignore the override of self. If one wants to have options with overrides of self, *and* multiple values you should set use_delimiter(false) and *not* set multiple(true). Then tell users to use a comma to seperate their values, i.e. `val1,val2`. --- src/args/arg.rs | 22 +++++++++++----------- src/args/arg_matcher.rs | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/args/arg.rs b/src/args/arg.rs index 6cbf3a76612..e412bd18141 100644 --- a/src/args/arg.rs +++ b/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,20 +1275,20 @@ 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::>(), &["other", "val"]); + /// assert_eq!(m.occurrences_of("opt"), 2); + /// assert_eq!(m.values_of("opt").unwrap().collect::>(), &["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); diff --git a/src/args/arg_matcher.rs b/src/args/arg_matcher.rs index 16ea79b1312..32b62829ece 100644 --- a/src/args/arg_matcher.rs +++ b/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 From d4401d8487ec20cefb6b4d350eea1d22abb5ca0c Mon Sep 17 00:00:00 2001 From: Kevin K Date: Mon, 5 Feb 2018 19:48:16 -0500 Subject: [PATCH 2/4] tests(Overrides Self): updates the tests to account for the bug fixes --- src/args/arg.rs | 2 +- tests/app_settings.rs | 23 ++++++++++++++++++----- tests/posix_compatible.rs | 21 +++++++++++++++++---- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/args/arg.rs b/src/args/arg.rs index e412bd18141..7875500e724 100644 --- a/src/args/arg.rs +++ b/src/args/arg.rs @@ -1292,7 +1292,7 @@ impl<'a, 'b> Arg<'a, 'b> { /// .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::>(), &["one", "two"]); + /// assert_eq!(m.values_of("opt").unwrap().collect::>(), &["one,two"]); /// ``` pub fn overrides_with(mut self, name: &'a str) -> Self { if let Some(ref mut vec) = self.b.overrides { diff --git a/tests/app_settings.rs b/tests/app_settings.rs index cbba0f060ba..2168c50093d 100644 --- a/tests/app_settings.rs +++ b/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::>(), &["one", "two"]); + assert_eq!(m.occurrences_of("opt"), 3); + assert_eq!(m.values_of("opt").unwrap().collect::>(), &["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::>(), &["some", "other", "val"]); + assert_eq!(m.occurrences_of("opt"), 2); + assert_eq!(m.values_of("opt").unwrap().collect::>(), &["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::>(), &["some", "other", "value"]); -} \ No newline at end of file +} + +#[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::>(), &["one,two"]); +} diff --git a/tests/posix_compatible.rs b/tests/posix_compatible.rs index 0a790c2cb87..26d9f71f39e 100644 --- a/tests/posix_compatible.rs +++ b/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::>(), &["one", "two"]); + assert_eq!(m.occurrences_of("opt"), 3); + assert_eq!(m.values_of("opt").unwrap().collect::>(), &["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::>(), &["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::>(), &["some", "other", "val"]); + assert_eq!(m.values_of("opt").unwrap().collect::>(), &["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")) From 12bb47309dc5e35f687a63a6064b938d68b88f74 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Mon, 5 Feb 2018 19:48:44 -0500 Subject: [PATCH 3/4] chore: adds more crates.io badges --- Cargo.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index f659c47121b..d318ddd1f54 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" From 790f1e3d50f2161d5a9bf1cf7ede1ffc25882561 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Mon, 5 Feb 2018 19:49:55 -0500 Subject: [PATCH 4/4] chore: increase version --- CHANGELOG.md | 10 ++++++++++ Cargo.toml | 2 +- README.md | 24 ++++-------------------- src/lib.rs | 2 +- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63a525f5c8f..a1fe2f5c191 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ + +### 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)) + + + ### v2.29.3 (2018-02-05) diff --git a/Cargo.toml b/Cargo.toml index d318ddd1f54..49270c87fca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "clap" -version = "2.29.3" +version = "2.29.4" authors = ["Kevin K. "] exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"] repository = "https://github.com/kbknapp/clap-rs" diff --git a/README.md b/README.md index de44e2bb766..91b6ac930bb 100644 --- a/README.md +++ b/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 diff --git a/src/lib.rs b/src/lib.rs index 3651344ec4e..b7aadc978cb 100644 --- a/src/lib.rs +++ b/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