Merge pull request #388 from kbknapp/issue-353

Issue 353

src/app/parser.rs

@@ -1058,24 +1058,34 @@ impl<'a, 'b> Parser<'a, 'b> where 'a: 'b {
         where A: AnyArg<'a, 'b> + Display {
         debugln!("fn=add_val_to_arg;");
         let mut ret = None;
-        for v in val.split(b',') {
-            debugln!("adding val: {:?}", v);
-            matcher.add_val_to(&*arg.name(), v);
-
-            // Increment or create the group "args"
-            if let Some(grps) = self.groups_for_arg(&*arg.name()) {
-                for grp in grps {
-                    matcher.add_val_to(&*grp, v);
-                }
+        if let Some(delim) = arg.val_delim() {
+            for v in val.split(delim as u32 as u8) {
+                ret = try!(self.add_single_val_to_arg(arg, v, matcher));
             }
-
-            // The validation must come AFTER inserting into 'matcher' or the usage string
-            // can't be built
-            ret = try!(self.validate_value(arg, v, matcher));
+        } else {
+            ret = try!(self.add_single_val_to_arg(arg, val, matcher));
         }
         Ok(ret)
     }
 
+    fn add_single_val_to_arg<A>(&self, arg: &A, v: &OsStr, matcher: &mut ArgMatcher<'a>) -> ClapResult<Option<&'a str>>
+        where A: AnyArg<'a, 'b>
+    {
+        debugln!("adding val: {:?}", v);
+        matcher.add_val_to(arg.name(), v);
+
+        // Increment or create the group "args"
+        if let Some(grps) = self.groups_for_arg(arg.name()) {
+            for grp in grps {
+                matcher.add_val_to(&*grp, v);
+            }
+        }
+
+        // The validation must come AFTER inserting into 'matcher' or the usage string
+        // can't be built
+        self.validate_value(arg, v, matcher)
+    }
+
     fn validate_value<A>(&self, arg: &A, val: &OsStr, matcher: &ArgMatcher<'a>) -> ClapResult<Option<&'a str>>
         where A: AnyArg<'a, 'b> {
         debugln!("fn=validate_value; val={:?}", val);

src/args/any_arg.rs

@@ -19,4 +19,5 @@ pub trait AnyArg<'n, 'e>: Display {
     fn validator(&self) -> Option<&Rc<Fn(String) -> Result<(), String>>>;
     fn short(&self) -> Option<char>;
     fn long(&self) -> Option<&'e str>;
+    fn val_delim(&self) -> Option<char>;
 }

src/args/arg.rs

@@ -90,6 +90,9 @@ pub struct Arg<'a, 'b> where 'a: 'b {
     pub overrides: Option<Vec<&'a str>>,
     #[doc(hidden)]
     pub settings: ArgFlags,
+    // Delimiting character for value separation
+    #[doc(hidden)]
+    pub val_delim: Option<char>,
 }
 
 impl<'a, 'b> Default for Arg<'a, 'b> {
@@ -111,6 +114,7 @@ impl<'a, 'b> Default for Arg<'a, 'b> {
             validator: None,
             overrides: None,
             settings: ArgFlags::new(),
+            val_delim: Some(','),
         }
     }
 }
@@ -175,6 +179,8 @@ impl<'a, 'b> Arg<'a, 'b> {
                 "max_values" => a.max_values(v.as_i64().unwrap() as u8),
                 "min_values" => a.min_values(v.as_i64().unwrap() as u8),
                 "value_name" => a.value_name(v.as_str().unwrap()),
+                "use_delimiter" => a.use_delimiter(v.as_bool().unwrap()),
+                "value_delimiter" => a.value_delimiter(v.as_str().unwrap()),
                 "value_names" => {
                     for ys in v.as_vec().unwrap() {
                         if let Some(s) = ys.as_str() {
@@ -199,10 +205,10 @@ impl<'a, 'b> Arg<'a, 'b> {
                     }
                     a
                 }
-                "mutually_overrides_with" => {
+                "overrides_with" => {
                     for ys in v.as_vec().unwrap() {
                         if let Some(s) = ys.as_str() {
-                            a = a.mutually_overrides_with(s);
+                            a = a.overrides_with(s);
                         }
                     }
                     a
@@ -430,9 +436,9 @@ impl<'a, 'b> Arg<'a, 'b> {
     /// ```no_run
     /// # use clap::{App, Arg};
     /// # let myprog = App::new("myprog").arg(Arg::with_name("config")
-    /// .mutually_overrides_with("debug")
+    /// .overrides_with("debug")
     /// # ).get_matches();
-    pub fn mutually_overrides_with(mut self, name: &'a str) -> Self {
+    pub fn overrides_with(mut self, name: &'a str) -> Self {
         if let Some(ref mut vec) = self.overrides {
             vec.push(name.as_ref());
         } else {
@@ -450,9 +456,9 @@ impl<'a, 'b> Arg<'a, 'b> {
     /// # use clap::{App, Arg};
     /// let config_overrides = ["debug", "input"];
     /// # let myprog = App::new("myprog").arg(Arg::with_name("config")
-    /// .mutually_overrides_with_all(&config_overrides)
+    /// .overrides_with_all(&config_overrides)
     /// # ).get_matches();
-    pub fn mutually_overrides_with_all(mut self, names: &[&'a str]) -> Self {
+    pub fn overrides_with_all(mut self, names: &[&'a str]) -> Self {
         if let Some(ref mut vec) = self.overrides {
             for s in names {
                 vec.push(s);
@@ -810,31 +816,115 @@ impl<'a, 'b> Arg<'a, 'b> {
         self
     }
 
+    /// Specifies whether or not an arugment should allow grouping of multiple values via a
+    /// delimter. I.e. shoulde `--option=val1,val2,val3` be parsed as three values (`val1`, `val2`,
+    /// 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 defalt is `true`. Setting the value to `true` will reset any previous use of
+    /// `Arg::value_delimiter` back to the default of `,` (comma).
+    ///
+    /// # Examples
+    ///
+    /// The following example shows the default behavior.
+    ///
+    /// ```no_run
+    /// # use clap::{App, Arg};
+    /// let delims = App::new("delims")
+    ///     .arg(Arg::with_name("option")
+    ///         .long("option")
+    ///         .takes_value(true))
+    ///     .get_matches_from(vec![
+    ///         "delims",
+    ///         "--option=val1,val2,val3",
+    ///     ]);
+    ///
+    /// assert!(delims.is_present("option"));
+    /// assert_eq!(delims.occurrences_of("option"), 1);
+    /// assert_eq!(delims.values_of("option").unwrap().collect::<Vec<_>>(), ["val1", "val2", "val3"]);
+    /// ```
+    /// The next example shows the difference when turning delimiters off.
+    ///
+    /// ```no_run
+    /// # use clap::{App, Arg};
+    /// let nodelims = App::new("nodelims")
+    ///     .arg(Arg::with_name("option")
+    ///         .long("option")
+    ///         .use_delimiter(false)
+    ///         .takes_value(true))
+    ///     .get_matches_from(vec![
+    ///         "nodelims",
+    ///         "--option=val1,val2,val3",
+    ///     ]);
+    ///
+    /// assert!(nodelims.is_present("option"));
+    /// assert_eq!(nodelims.occurrences_of("option"), 1);
+    /// assert_eq!(nodelims.value_of("option").unwrap(), "val1,val2,val3");
+    /// ```
+    pub fn use_delimiter(mut self, d: bool) -> Self {
+        if d {
+            self.val_delim = Some(',');
+            self.set(ArgSettings::UseValueDelimiter)
+        } else {
+            self.val_delim = None;
+            self.unset(ArgSettings::UseValueDelimiter)
+        }
+    }
+
+    /// Specifies the separator to use when values are clumped together, defaults to `,` (comma).
+    ///
+    /// **NOTE:** implicitly sets `Arg::use_delimiter(true)`
+    ///
+    /// **NOTE:** implicitly sets `Arg::takes_value(true)`
+    ///
+    /// # Examples
+    ///
+    /// ```no_run
+    /// # use clap::{App, Arg};
+    /// let app = App::new("fake")
+    ///     .arg(Arg::with_name("config")
+    ///         .short("c")
+    ///         .long("config")
+    ///         .value_delimiter(";"));
+    ///
+    /// let m = app.get_matches_from(vec![
+    ///     "fake", "--config=val1;val2;val3"
+    /// ]);
+    ///
+    /// assert_eq!(m.values_of("config").unwrap().collect::<Vec<_>>(), ["val1", "val2", "val3"])
+    /// ```
+    pub fn value_delimiter(mut self, d: &str) -> Self {
+        self = self.set(ArgSettings::TakesValue);
+        self = self.set(ArgSettings::UseValueDelimiter);
+        self.val_delim = Some(d.chars()
+                               .nth(0)
+                               .expect("Failed to get value_delimiter from arg"));
+        self
+    }
+
     /// Specifies names for values of option arguments. These names are cosmetic only, used for
     /// help and usage strings only. The names are **not** used to access arguments. The values of
     /// the arguments are accessed in numeric order (i.e. if you specify two names `one` and `two`
     /// `one` will be the first matched value, `two` will be the second).
     ///
-    /// **NOTE:** This implicitly sets `.number_of_values()` so there is no need to set that, but
-    /// be aware that the number of "names" you set for the values, will be the *exact* number of
-    /// values required to satisfy this argument
+    /// **NOTE:** This implicitly sets `.number_of_values()`, but be aware that the number of
+    /// "names" you set for the values, will be the *exact* number of values required to satisfy
+    /// this argument
     ///
-    /// **NOTE:** Does *not* require `.multiple(true)` to be set. Setting `.multiple(true)` would
-    /// allow `-f <file> <file> <file> -f <file> <file> <file>` where as *not* setting
-    /// `.multiple(true)` would only allow one occurrence of this argument.
+    /// **NOTE:** implicitly sets `Arg::takes_value(true)`
+    ///
+    /// **NOTE:** Does *not* require or imply `.multiple(true)`.
     ///
     /// # Examples
     ///
     /// ```no_run
     /// # use clap::{App, Arg};
-    /// let val_names = ["one", "two"];
-    /// # let matches = App::new("myprog")
-    /// #                 .arg(
-    /// # Arg::with_name("debug").index(1)
-    /// // ...
-    /// .value_names(&val_names)
-    /// # ).get_matches();
+    /// Arg::with_name("speed")
+    ///     .short("s")
+    ///     .value_names(&["fast", "slow"])
+    /// # ;
     pub fn value_names(mut self, names: &[&'b str]) -> Self {
+        self.setb(ArgSettings::TakesValue);
         if let Some(ref mut vals) = self.val_names {
             let mut l =  vals.len();
             for s in names {
@@ -878,20 +968,21 @@ impl<'a, 'b> Arg<'a, 'b> {
         self
     }
 
-    /// Specifies the name for value of option or positional arguments. This name is cosmetic only,
-    /// used for help and usage strings. The name is **not** used to access arguments.
+    /// Specifies the name for value of option or positional arguments inside of help documenation.
+    /// This name is cosmetic only, the name is **not** used to access arguments.
+    ///
+    /// **NOTE:** implicitly sets `Arg::takes_value(true)`
     ///
     /// # Examples
     ///
     /// ```no_run
     /// # use clap::{App, Arg};
-    /// # let matches = App::new("myprog")
-    /// #                 .arg(
-    /// Arg::with_name("debug")
+    /// Arg::with_name("input")
     ///     .index(1)
-    ///     .value_name("file")
-    /// # ).get_matches();
+    ///     .value_name("FILE")
+    /// # ;
     pub fn value_name(mut self, name: &'b str) -> Self {
+        self.setb(ArgSettings::TakesValue);
         if let Some(ref mut vals) = self.val_names {
             let l = vals.len();
             vals.insert(l, name);
@@ -924,6 +1015,7 @@ impl<'a, 'b, 'z> From<&'z Arg<'a, 'b>>
             validator: a.validator.clone(),
             overrides: a.overrides.clone(),
             settings: a.settings.clone(),
+            val_delim: a.val_delim,
         }
     }
 }

src/args/arg_builder/flag.rs

@@ -10,26 +10,14 @@ use args::AnyArg;
 use args::settings::{ArgFlags, ArgSettings};
 
 #[derive(Debug)]
+#[doc(hidden)]
 pub struct FlagBuilder<'n, 'e> {
     pub name: &'n str,
-    /// The long version of the flag (i.e. word)
-    /// without the preceding `--`
     pub long: Option<&'e str>,
-    /// The string of text that will displayed to
-    /// the user when the application's `help`
-    /// text is displayed
     pub help: Option<&'e str>,
-    /// A list of names for other arguments that
-    /// *may not* be used with this flag
     pub blacklist: Option<Vec<&'e str>>,
-    /// A list of names of other arguments that
-    /// are *required* to be used when this
-    /// flag is used
     pub requires: Option<Vec<&'e str>>,
-    /// The short version (i.e. single character)
-    /// of the argument, no preceding `-`
     pub short: Option<char>,
-    /// A list of names for other arguments that *mutually override* this flag
     pub overrides: Option<Vec<&'e str>>,
     pub settings: ArgFlags,
 }
@@ -192,6 +180,7 @@ impl<'n, 'e> AnyArg<'n, 'e> for FlagBuilder<'n, 'e> {
     fn min_vals(&self) -> Option<u8> { None }
     fn short(&self) -> Option<char> { self.short }
     fn long(&self) -> Option<&'e str> { self.long }
+    fn val_delim(&self) -> Option<char> { None }
 }
 
 #[cfg(test)]

src/args/arg_builder/option.rs

@@ -9,30 +9,23 @@ use args::{AnyArg, Arg};
 use args::settings::{ArgFlags, ArgSettings};
 
 #[allow(missing_debug_implementations)]
+#[doc(hidden)]
 pub struct OptBuilder<'n, 'e> {
     pub name: &'n str,
-    /// The short version (i.e. single character) of the argument, no preceding `-`
     pub short: Option<char>,
-    /// The long version of the flag (i.e. word) without the preceding `--`
     pub long: Option<&'e str>,
-    /// The string of text that will displayed to the user when the application's
-    /// `help` text is displayed
     pub help: Option<&'e str>,
-    /// A list of names for other arguments that *may not* be used with this flag
     pub blacklist: Option<Vec<&'e str>>,
-    /// A list of possible values for this argument
     pub possible_vals: Option<Vec<&'e str>>,
-    /// A list of names of other arguments that are *required* to be used when
-    /// this flag is used
     pub requires: Option<Vec<&'e str>>,
     pub num_vals: Option<u8>,
     pub min_vals: Option<u8>,
     pub max_vals: Option<u8>,
     pub val_names: Option<VecMap<&'e str>>,
     pub validator: Option<Rc<Fn(String) -> StdResult<(), String>>>,
-    /// A list of names for other arguments that *mutually override* this flag
     pub overrides: Option<Vec<&'e str>>,
     pub settings: ArgFlags,
+    pub val_delim: Option<char>,
 }
 
 impl<'n, 'e> Default for OptBuilder<'n, 'e> {
@@ -52,6 +45,7 @@ impl<'n, 'e> Default for OptBuilder<'n, 'e> {
             validator: None,
             overrides: None,
             settings: ArgFlags::new(),
+            val_delim: Some(','),
         }
     }
 }
@@ -81,6 +75,7 @@ impl<'n, 'e> OptBuilder<'n, 'e> {
             min_vals: a.min_vals,
             max_vals: a.max_vals,
             val_names: a.val_names.clone(),
+            val_delim: a.val_delim,
             ..Default::default()
         };
         if a.is_set(ArgSettings::Multiple) {
@@ -253,6 +248,7 @@ impl<'n, 'e> AnyArg<'n, 'e> for OptBuilder<'n, 'e> {
     fn min_vals(&self) -> Option<u8> { self.min_vals }
     fn short(&self) -> Option<char> { self.short }
     fn long(&self) -> Option<&'e str> { self.long }
+    fn val_delim(&self) -> Option<char> { self.val_delim }
 }
 
 #[cfg(test)]