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

Author: kbknapp

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 {

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()),
         }
     }

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;