From 9881a4a23ac3234e11f764e8e8c452e142705997 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 10 Sep 2016 18:18:43 -0400 Subject: [PATCH 1/3] tests(Value Delimiters): updates tests to new value delimiter rules --- tests/app_settings.rs | 27 ++++++++++++++++++++++++++- tests/multiple_occurrences.rs | 8 ++++---- tests/multiple_values.rs | 1 + 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/tests/app_settings.rs b/tests/app_settings.rs index 74b1ae687e3..ccb9461cc1a 100644 --- a/tests/app_settings.rs +++ b/tests/app_settings.rs @@ -223,7 +223,7 @@ fn delim_values_only_pos_follows() { let m = r.unwrap(); assert!(m.is_present("arg")); assert!(!m.is_present("f")); - assert_eq!(m.values_of("arg").unwrap().collect::>(), &["-f", "-g", "x"]); + assert_eq!(m.values_of("arg").unwrap().collect::>(), &["-f", "-g,x"]); } #[test] @@ -235,6 +235,31 @@ fn delim_values_trailingvararg() { ) .get_matches_from(vec!["", "test", "--foo", "-Wl,-bar"]); assert!(m.is_present("opt")); + assert_eq!(m.values_of("opt").unwrap().collect::>(), &["test", "--foo", "-Wl,-bar"]); +} + +#[test] +fn delim_values_only_pos_follows_with_delim() { + let r = App::new("onlypos") + .args(&[Arg::from_usage("-f [flag] 'some opt'"), + Arg::from_usage("[arg]... 'some arg'").use_delimiter(true)]) + .get_matches_from_safe(vec!["", "--", "-f", "-g,x"]); + assert!(r.is_ok()); + let m = r.unwrap(); + assert!(m.is_present("arg")); + assert!(!m.is_present("f")); + assert_eq!(m.values_of("arg").unwrap().collect::>(), &["-f", "-g", "x"]); +} + +#[test] +fn delim_values_trailingvararg_with_delim() { + let m = App::new("positional") + .setting(AppSettings::TrailingVarArg) + .arg( + Arg::from_usage("[opt]... 'some pos'").use_delimiter(true), + ) + .get_matches_from(vec!["", "test", "--foo", "-Wl,-bar"]); + assert!(m.is_present("opt")); assert_eq!(m.values_of("opt").unwrap().collect::>(), &["test", "--foo", "-Wl", "-bar"]); } diff --git a/tests/multiple_occurrences.rs b/tests/multiple_occurrences.rs index 6b31b7322fd..9da337cfc81 100644 --- a/tests/multiple_occurrences.rs +++ b/tests/multiple_occurrences.rs @@ -5,7 +5,7 @@ use clap::{App, Arg}; #[test] fn multiple_occurrences_of_flags_long() { - let m = App::new("multiple_occurrences") + let m = App::new("mo_flags_long") .arg(Arg::from_usage("--multflag 'allowed multiple flag'") .multiple(true)) .arg(Arg::from_usage("--flag 'disallowed multiple flag'")) @@ -23,7 +23,7 @@ fn multiple_occurrences_of_flags_long() { #[test] fn multiple_occurrences_of_flags_short() { - let m = App::new("multiple_occurrences") + let m = App::new("mo_flags_short") .arg(Arg::from_usage("-m --multflag 'allowed multiple flag'") .multiple(true)) .arg(Arg::from_usage("-f --flag 'disallowed multiple flag'")) @@ -41,7 +41,7 @@ fn multiple_occurrences_of_flags_short() { #[test] fn multiple_occurrences_of_flags_mixed() { - let m = App::new("multiple_occurrences") + let m = App::new("mo_flags_mixed") .arg(Arg::from_usage("-m, --multflag1 'allowed multiple flag'") .multiple(true)) .arg(Arg::from_usage("-n, --multflag2 'another allowed multiple flag'") @@ -67,7 +67,7 @@ fn multiple_occurrences_of_flags_mixed() { #[test] fn multiple_occurrences_of_flags_large_quantity() { let args : Vec<&str> = vec![""].into_iter().chain(vec!["-m"; 1024].into_iter()).collect(); - let m = App::new("multiple_occurrences") + let m = App::new("mo_flags_larg_qty") .arg(Arg::from_usage("-m --multflag 'allowed multiple flag'") .multiple(true)) .get_matches_from(args); diff --git a/tests/multiple_values.rs b/tests/multiple_values.rs index 3941bf0a35f..194b8303134 100644 --- a/tests/multiple_values.rs +++ b/tests/multiple_values.rs @@ -630,6 +630,7 @@ fn multiple_values_sep_positional() { let m = App::new("multiple_values") .arg(Arg::with_name("option") .help("multiple options") + .use_delimiter(true) .multiple(true)) .get_matches_from_safe(vec![ "", From f9e692548e8c94de15f909432de301407d6bb834 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 10 Sep 2016 18:19:33 -0400 Subject: [PATCH 2/3] imp(Value Delimiters): changes the default value delimiter rules Prior to this change, values were always delimited by default. This was causing issues with code where the arg had a single value, and contained valid commas and shouldn't be delimited. This commit changes the rules slightly so that values are not delimited by default, *unless* one of the methods which implies multiple values was used (max_values, value_names, etc.). This means single value args should *not* be delimited by default. If one wishes to use the old way, they can add `Arg::use_delimiter(true)` to such code. Closes #655 --- src/args/arg.rs | 43 ++++++++++++++++++++++++++++++++++++------- src/args/settings.rs | 29 +++++++++++++++++------------ src/usage_parser.rs | 7 +++++++ 3 files changed, 60 insertions(+), 19 deletions(-) diff --git a/src/args/arg.rs b/src/args/arg.rs index 6e63bf6cf4c..be382a0454e 100644 --- a/src/args/arg.rs +++ b/src/args/arg.rs @@ -96,7 +96,7 @@ impl<'a, 'b> Default for Arg<'a, 'b> { validator: None, overrides: None, settings: ArgFlags::new(), - val_delim: Some(','), + val_delim: None, default_val: None, disp_ord: 999, r_unless: None, @@ -1277,8 +1277,12 @@ impl<'a, 'b> Arg<'a, 'b> { /// [option]: ./struct.Arg.html#method.takes_value /// [`Arg::number_of_values(1)`]: ./struct.Arg.html#method.number_of_values /// [`Arg::multiple(true)`]: ./struct.Arg.html#method.multiple - pub fn multiple(self, multi: bool) -> Self { + pub fn multiple(mut self, multi: bool) -> Self { if multi { + if self.settings.is_set(ArgSettings::ValueDelimiterNotSet) && + self.settings.is_set(ArgSettings::TakesValue) { + self = self.use_delimiter(true); + } self.set(ArgSettings::Multiple) } else { self.unset(ArgSettings::Multiple) @@ -1649,6 +1653,13 @@ impl<'a, 'b> Arg<'a, 'b> { /// ``` /// [`Arg::multiple(true)`]: ./struct.Arg.html#method.multiple pub fn number_of_values(mut self, qty: u64) -> Self { + if qty > 1 && self.settings.is_set(ArgSettings::ValueDelimiterNotSet) { + self.unsetb(ArgSettings::ValueDelimiterNotSet); + self.setb(ArgSettings::UseValueDelimiter); + } else { + self = self.use_delimiter(false); + } + self.setb(ArgSettings::TakesValue); self.num_vals = Some(qty); self } @@ -1747,6 +1758,13 @@ impl<'a, 'b> Arg<'a, 'b> { /// ``` /// [`Arg::multiple(true)`]: ./struct.Arg.html#method.multiple pub fn max_values(mut self, qty: u64) -> Self { + if qty > 1 && self.settings.is_set(ArgSettings::ValueDelimiterNotSet) { + self.unsetb(ArgSettings::ValueDelimiterNotSet); + self.setb(ArgSettings::UseValueDelimiter); + } else { + self = self.use_delimiter(false); + } + self.setb(ArgSettings::TakesValue); self.max_vals = Some(qty); self } @@ -1857,11 +1875,16 @@ impl<'a, 'b> Arg<'a, 'b> { /// [`Arg::value_delimiter`]: ./struct.Arg.html#method.value_delimiter pub fn use_delimiter(mut self, d: bool) -> Self { if d { - self.val_delim = Some(','); - self.set(ArgSettings::UseValueDelimiter) + if self.val_delim.is_none() { + self.val_delim = Some(','); + } + self.setb(ArgSettings::TakesValue); + self.setb(ArgSettings::UseValueDelimiter); + self.unset(ArgSettings::ValueDelimiterNotSet) } else { self.val_delim = None; - self.unset(ArgSettings::UseValueDelimiter) + self.unsetb(ArgSettings::UseValueDelimiter); + self.unset(ArgSettings::ValueDelimiterNotSet) } } @@ -1939,6 +1962,7 @@ impl<'a, 'b> Arg<'a, 'b> { /// ``` pub fn require_delimiter(mut self, d: bool) -> Self { if d { + self.unsetb(ArgSettings::ValueDelimiterNotSet); self.setb(ArgSettings::UseValueDelimiter); self.set(ArgSettings::RequireDelimiter) } else { @@ -1972,8 +1996,9 @@ impl<'a, 'b> Arg<'a, 'b> { /// [`Arg::use_delimiter(true)`]: ./struct.Arg.html#method.use_delimiter /// [`Arg::takes_value(true)`]: ./struct.Arg.html#method.takes_value pub fn value_delimiter(mut self, d: &str) -> Self { - self = self.set(ArgSettings::TakesValue); - self = self.set(ArgSettings::UseValueDelimiter); + self.unsetb(ArgSettings::ValueDelimiterNotSet); + self.setb(ArgSettings::TakesValue); + self.setb(ArgSettings::UseValueDelimiter); self.val_delim = Some(d.chars() .nth(0) .expect("Failed to get value_delimiter from arg")); @@ -2041,6 +2066,10 @@ impl<'a, 'b> Arg<'a, 'b> { /// [`Arg::multiple(true)`]: ./struct.Arg.html#method.multiple pub fn value_names(mut self, names: &[&'b str]) -> Self { self.setb(ArgSettings::TakesValue); + if self.settings.is_set(ArgSettings::ValueDelimiterNotSet) { + self.unsetb(ArgSettings::ValueDelimiterNotSet); + self.setb(ArgSettings::UseValueDelimiter); + } if let Some(ref mut vals) = self.val_names { let mut l = vals.len(); for s in names { diff --git a/src/args/settings.rs b/src/args/settings.rs index 4b67274fe07..dd48e4cbec5 100644 --- a/src/args/settings.rs +++ b/src/args/settings.rs @@ -4,16 +4,17 @@ use std::str::FromStr; bitflags! { flags Flags: u16 { - const REQUIRED = 0b0000000001, - const MULTIPLE = 0b0000000010, - const EMPTY_VALS = 0b0000000100, - const GLOBAL = 0b0000001000, - const HIDDEN = 0b0000010000, - const TAKES_VAL = 0b0000100000, - const USE_DELIM = 0b0001000000, - const NEXT_LINE_HELP = 0b0010000000, - const R_UNLESS_ALL = 0b0100000000, - const REQ_DELIM = 0b1000000000, + const REQUIRED = 0b00000000001, + const MULTIPLE = 0b00000000010, + const EMPTY_VALS = 0b00000000100, + const GLOBAL = 0b00000001000, + const HIDDEN = 0b00000010000, + const TAKES_VAL = 0b00000100000, + const USE_DELIM = 0b00001000000, + const NEXT_LINE_HELP = 0b00010000000, + const R_UNLESS_ALL = 0b00100000000, + const REQ_DELIM = 0b01000000000, + const DELIM_NOT_SET = 0b10000000000, } } @@ -36,13 +37,14 @@ impl ArgFlags { UseValueDelimiter => USE_DELIM, NextLineHelp => NEXT_LINE_HELP, RequiredUnlessAll => R_UNLESS_ALL, - RequireDelimiter => REQ_DELIM + RequireDelimiter => REQ_DELIM, + ValueDelimiterNotSet => DELIM_NOT_SET } } impl Default for ArgFlags { fn default() -> Self { - ArgFlags(EMPTY_VALS | USE_DELIM) + ArgFlags(EMPTY_VALS | DELIM_NOT_SET) } } @@ -74,6 +76,8 @@ pub enum ArgSettings { RequireDelimiter, #[doc(hidden)] RequiredUnlessAll, + #[doc(hidden)] + ValueDelimiterNotSet, } impl FromStr for ArgSettings { @@ -90,6 +94,7 @@ impl FromStr for ArgSettings { "nextlinehelp" => Ok(ArgSettings::NextLineHelp), "requiredunlessall" => Ok(ArgSettings::RequiredUnlessAll), "requiredelimiter" => Ok(ArgSettings::RequireDelimiter), + "valuedelimiternotset" => Ok(ArgSettings::ValueDelimiterNotSet), _ => Err("unknown ArgSetting, cannot convert from str".to_owned()), } } diff --git a/src/usage_parser.rs b/src/usage_parser.rs index 47dfb8d112c..d2d17a7e00f 100644 --- a/src/usage_parser.rs +++ b/src/usage_parser.rs @@ -174,6 +174,13 @@ impl<'a> UsageParser<'a> { if dot_counter == 3 { debugln!("setting multiple"); arg.setb(ArgSettings::Multiple); + if arg.settings.is_set(ArgSettings::TakesValue) { + arg.setb(ArgSettings::UseValueDelimiter); + arg.unsetb(ArgSettings::ValueDelimiterNotSet); + if arg.val_delim.is_none() { + arg.val_delim = Some(','); + } + } self.prev = UsageToken::Multiple; self.pos += 1; break; From f9d17a060aa53f10d0a6e1a7eed5d989d1a59533 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 10 Sep 2016 18:23:31 -0400 Subject: [PATCH 3/3] docs(Value Delimiters): updates the docs for the Arg::multiple method WRT value delimiters and default settings --- src/args/arg.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/args/arg.rs b/src/args/arg.rs index be382a0454e..a3ee1245a38 100644 --- a/src/args/arg.rs +++ b/src/args/arg.rs @@ -1711,7 +1711,7 @@ impl<'a, 'b> Arg<'a, 'b> { /// /// **NOTE:** This does *not* implicitly set [`Arg::multiple(true)`]. This is because /// `-o val -o val` is multiple occurrences but a single value and `-o val1 val2` is a single - /// occurence with multple values. For positional arguments this **does** set + /// occurence with multiple values. For positional arguments this **does** set /// [`Arg::multiple(true)`] because there is no way to determine the difference between multiple /// occurences and multiple values. /// @@ -1832,8 +1832,12 @@ impl<'a, 'b> Arg<'a, 'b> { /// and `val3`) or as a single value (`val1,val2,val3`). Defaults to using `,` (comma) as the /// value delimiter for all arguments that accept values (options and positional arguments) /// - /// **NOTE:** The default is `true`. Setting the value to `true` will reset any previous use of - /// [`Arg::value_delimiter`] back to the default of `,` (comma). + /// **NOTE:** The default is `false`. When set to `true` the default [`Arg::value_delimiter`] + /// is the comma `,`. + /// + /// **NOTE:** When using methods like [`Arg::multiple`] or [`Arg::max_values`] (i.e. methods + /// that imply multiple values, this `use_delimiter` setting will automatically be set to + /// `true` and use the comma (`,`) by default. /// /// # Examples /// @@ -1844,6 +1848,7 @@ impl<'a, 'b> Arg<'a, 'b> { /// let delims = App::new("delims") /// .arg(Arg::with_name("option") /// .long("option") + /// .use_delimiter(true) /// .takes_value(true)) /// .get_matches_from(vec![ /// "delims", @@ -1854,7 +1859,8 @@ impl<'a, 'b> Arg<'a, 'b> { /// assert_eq!(delims.occurrences_of("option"), 1); /// assert_eq!(delims.values_of("option").unwrap().collect::>(), ["val1", "val2", "val3"]); /// ``` - /// The next example shows the difference when turning delimiters off. + /// The next example shows the difference when turning delimiters off. This is the default + /// behavior, unless one of the methods/settings which implies multiple values is set. /// /// ```rust /// # use clap::{App, Arg};