Allow empty Value by default, introduce NoEquals Error

clap_derive/tests/non_literal_attributes.rs

@@ -12,7 +12,7 @@
 // commit#ea76fa1b1b273e65e3b0b1046643715b49bec51f which is licensed under the
 // MIT/Apache 2.0 license.
 
-use clap::{AppSettings, Clap};
+use clap::{AppSettings, Clap, ErrorKind};
 use std::num::ParseIntError;
 
 pub const DISPLAY_ORDER: usize = 2;
@@ -121,6 +121,7 @@ fn test_bool() {
     );
     let result = Opt::try_parse_from(&["test", "-l", "1", "--x", "1"]);
     assert!(result.is_err());
+    assert_eq!(result.unwrap_err().kind, ErrorKind::NoEquals);
 }
 
 fn parse_hex(input: &str) -> Result<u64, ParseIntError> {

src/build/app/settings.rs

@@ -1041,6 +1041,7 @@ pub enum AppSettings {
     NoAutoVersion,
 
     #[doc(hidden)]
+    /// If the user already passed '--'. Meaning only positional args follow.
     TrailingValues,
 
     #[doc(hidden)]

src/build/arg/mod.rs

@@ -3245,8 +3245,8 @@ impl<'help> Arg<'help> {
     /// assert!(res.is_ok());
     /// ```
     ///
-    /// Setting [`RequireEquals`] and *not* supplying the equals will cause an error
-    /// unless [`ArgSettings::EmptyValues`] is set.
+    /// Setting [`RequireEquals`] and *not* supplying the equals, the arg will emits
+    /// an error.
     ///
     /// ```rust
     /// # use clap::{App, Arg, ErrorKind, ArgSettings};
@@ -3259,16 +3259,13 @@ impl<'help> Arg<'help> {
     ///     ]);
     ///
     /// assert!(res.is_err());
-    /// assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue);
+    /// assert_eq!(res.unwrap_err().kind, ErrorKind::NoEquals);
     /// ```
     /// [`RequireEquals`]: ./enum.ArgSettings.html#variant.RequireEquals
-    /// [`ArgSettings::EmptyValues`]: ./enum.ArgSettings.html#variant.EmptyValues
-    /// [`ArgSettings::EmptyValues`]: ./enum.ArgSettings.html#variant.TakesValue
     #[inline]
     pub fn require_equals(self, r: bool) -> Self {
         if r {
-            self.unset_setting(ArgSettings::AllowEmptyValues)
-                .setting(ArgSettings::RequireEquals)
+            self.setting(ArgSettings::RequireEquals)
         } else {
             self.unset_setting(ArgSettings::RequireEquals)
         }
@@ -3946,55 +3943,7 @@ impl<'help> Arg<'help> {
         self.multiple_occurrences(multi).multiple_values(multi)
     }
 
-    /// Allows an argument to accept explicitly empty values. An empty value must be specified at
-    /// the command line with an explicit `""`, `''`, or `--option=`
-    ///
-    /// **NOTE:** By default empty values are *not* allowed
-    ///
-    /// **NOTE:** Implicitly sets [`ArgSettings::TakesValue`]
-    ///
-    /// # Examples
-    ///
-    /// ```rust
-    /// # use clap::{App, Arg, ArgSettings};
-    /// Arg::new("file")
-    ///     .long("file")
-    ///     .setting(ArgSettings::AllowEmptyValues)
-    /// # ;
-    /// ```
-    /// The default is to *not* allow empty values.
-    ///
-    /// ```rust
-    /// # use clap::{App, Arg, ErrorKind, ArgSettings};
-    /// let res = App::new("prog")
-    ///     .arg(Arg::new("cfg")
-    ///         .long("config")
-    ///         .short('v')
-    ///         .setting(ArgSettings::TakesValue))
-    ///     .try_get_matches_from(vec![
-    ///         "prog", "--config="
-    ///     ]);
-    ///
-    /// assert!(res.is_err());
-    /// assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue);
-    /// ```
-    /// By adding this setting, we can allow empty values
-    ///
-    /// ```rust
-    /// # use clap::{App, Arg, ArgSettings};
-    /// let res = App::new("prog")
-    ///     .arg(Arg::new("cfg")
-    ///         .long("config")
-    ///         .short('v')
-    ///         .setting(ArgSettings::AllowEmptyValues)) // implies TakesValue
-    ///     .try_get_matches_from(vec![
-    ///         "prog", "--config="
-    ///     ]);
-    ///
-    /// assert!(res.is_ok());
-    /// assert_eq!(res.unwrap().value_of("config"), None);
-    /// ```
-    /// [`ArgSettings::TakesValue`]: ./enum.ArgSettings.html#variant.TakesValue
+    /// Placeholder documentation
     #[inline]
     pub fn multiple_values(self, multi: bool) -> Self {
         if multi {

src/build/arg/settings.rs

@@ -8,7 +8,7 @@ bitflags! {
     struct Flags: u32 {
         const REQUIRED         = 1;
         const MULTIPLE_OCC     = 1 << 1;
-        const EMPTY_VALS       = 1 << 2 | Self::TAKES_VAL.bits;
+        const NO_EMPTY_VALS    = 1 << 2 | Self::TAKES_VAL.bits;
         const GLOBAL           = 1 << 3;
         const HIDDEN           = 1 << 4;
         const TAKES_VAL        = 1 << 5;
@@ -39,7 +39,7 @@ impl_settings! { ArgSettings, ArgFlags,
     Required("required") => Flags::REQUIRED,
     MultipleOccurrences("multipleoccurrences") => Flags::MULTIPLE_OCC,
     MultipleValues("multiplevalues") => Flags::MULTIPLE_VALS,
-    AllowEmptyValues("allowemptyvalues") => Flags::EMPTY_VALS,
+    NoEmptyValues("noemptyvalues") => Flags::NO_EMPTY_VALS,
     Hidden("hidden") => Flags::HIDDEN,
     TakesValue("takesvalue") => Flags::TAKES_VAL,
     UseValueDelimiter("usevaluedelimiter") => Flags::USE_DELIM,
@@ -80,8 +80,57 @@ pub enum ArgSettings {
     MultipleValues,
     /// Allows an arg to appear multiple times
     MultipleOccurrences,
-    /// Allows an arg accept empty values such as `""`
-    AllowEmptyValues,
+    /// Don't allow an argument to accept explicitly empty values. An empty value
+    /// must be specified at the command line with an explicit `""`, `''`, or
+    /// `--option=`
+    ///
+    /// **NOTE:** By default empty values are allowed.
+    ///
+    /// **NOTE:** Implicitly sets [`ArgSettings::TakesValue`]
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # use clap::{App, Arg, ArgSettings};
+    /// Arg::new("file")
+    ///     .long("file")
+    ///     .setting(ArgSettings::NoEmptyValues)
+    /// # ;
+    /// ```
+    /// The default is allowing empty values.
+    ///
+    /// ```rust
+    /// # use clap::{App, Arg, ErrorKind, ArgSettings};
+    /// let res = App::new("prog")
+    ///     .arg(Arg::new("cfg")
+    ///         .long("config")
+    ///         .short('v')
+    ///         .setting(ArgSettings::TakesValue))
+    ///     .try_get_matches_from(vec![
+    ///         "prog", "--config="
+    ///     ]);
+    ///
+    /// assert!(res.is_ok());
+    /// assert_eq!(res.unwrap().value_of("config"), None);
+    /// ```
+    /// By adding this setting, we can allow empty values
+    ///
+    /// ```rust
+    /// # use clap::{App, Arg, ErrorKind, ArgSettings};
+    /// let res = App::new("prog")
+    ///     .arg(Arg::new("cfg")
+    ///         .long("config")
+    ///         .short('v')
+    ///         .setting(ArgSettings::NoEmptyValues)) // implies TakesValue
+    ///     .try_get_matches_from(vec![
+    ///         "prog", "--config="
+    ///     ]);
+    ///
+    /// assert!(res.is_err());
+    /// assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue);
+    /// ```
+    /// [`ArgSettings::TakesValue`]: ./enum.ArgSettings.html#variant.TakesValue
+    NoEmptyValues,
     /// Hides an arg from the help message
     Hidden,
     /// Allows an argument to take a value (such as `--option value`)
@@ -130,8 +179,8 @@ mod test {
             ArgSettings::AllowHyphenValues
         );
         assert_eq!(
-            "allowemptyvalues".parse::<ArgSettings>().unwrap(),
-            ArgSettings::AllowEmptyValues
+            "noemptyvalues".parse::<ArgSettings>().unwrap(),
+            ArgSettings::NoEmptyValues
         );
         assert_eq!(
             "hidepossiblevalues".parse::<ArgSettings>().unwrap(),

src/parse/errors.rs

@@ -116,13 +116,30 @@ pub enum ErrorKind {
     /// let res = App::new("prog")
     ///     .arg(Arg::new("color")
     ///          .setting(ArgSettings::TakesValue)
+    ///          .setting(ArgSettings::NoEmptyValues)
     ///          .long("color"))
     ///     .try_get_matches_from(vec!["prog", "--color="]);
     /// assert!(res.is_err());
     /// assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue);
     /// ```
     EmptyValue,
 
+    /// Occurs when the user doesn't use equals for an option that requres equal
+    /// sign to provide values.
+    ///
+    /// ```rust
+    /// # use clap::{App, Arg, ErrorKind, ArgSettings};
+    /// let res = App::new("prog")
+    ///     .arg(Arg::new("color")
+    ///          .setting(ArgSettings::TakesValue)
+    ///          .setting(ArgSettings::RequireEquals)
+    ///          .long("color"))
+    ///     .try_get_matches_from(vec!["prog", "--color", "red"]);
+    /// assert!(res.is_err());
+    /// assert_eq!(res.unwrap_err().kind, ErrorKind::NoEquals);
+    /// ```
+    NoEquals,
+
     /// Occurs when the user provides a value for an argument with a custom validation and the
     /// value fails that validation.
     ///
@@ -504,6 +521,24 @@ impl Error {
         }
     }
 
+    pub(crate) fn no_equals(arg: &Arg, usage: String, color: ColorChoice) -> Self {
+        let mut c = Colorizer::new(true, color);
+        let arg = arg.to_string();
+
+        start_error(&mut c, "The argument '");
+        c.warning(arg.clone());
+        c.none("' requires equals on value providing.");
+        put_usage(&mut c, usage);
+        try_help(&mut c);
+
+        Error {
+            message: c,
+            kind: ErrorKind::NoEquals,
+            info: vec![arg],
+            source: None,
+        }
+    }
+
     pub(crate) fn invalid_value<G>(
         bad_val: String,
         good_vals: &[G],

src/parse/parser.rs

@@ -1152,15 +1152,15 @@ impl<'help, 'app> Parser<'help, 'app> {
         // has_eq: --flag=value
         let mut has_eq = false;
         let no_val = val.is_none();
-        let empty_vals = opt.is_set(ArgSettings::AllowEmptyValues);
+        let no_empty_vals = opt.is_set(ArgSettings::NoEmptyValues);
         let min_vals_zero = opt.min_vals.unwrap_or(1) == 0;
         let require_equals = opt.is_set(ArgSettings::RequireEquals);
 
         debug!("Parser::parse_opt; Checking for val...");
         if let Some(fv) = val {
             has_eq = fv.starts_with("=");
             let v = fv.trim_start_n_matches(1, b'=');
-            if !empty_vals && (v.is_empty() || (require_equals && !has_eq)) {
+            if no_empty_vals && (v.is_empty() || (require_equals && !has_eq)) {
                 debug!("Found Empty - Error");
                 return Err(ClapError::empty_value(
                     opt,
@@ -1175,24 +1175,32 @@ impl<'help, 'app> Parser<'help, 'app> {
                 fv.starts_with("=")
             );
             self.add_val_to_arg(opt, &v, matcher, ValueType::CommandLine, false);
-        } else if require_equals && !empty_vals && !min_vals_zero {
-            debug!("None, but requires equals...Error");
-            return Err(ClapError::empty_value(
-                opt,
-                Usage::new(self).create_usage_with_title(&[]),
-                self.app.color(),
-            ));
-        } else if require_equals && min_vals_zero {
-            debug!("None and requires equals, but min_vals == 0");
-            if !opt.default_missing_vals.is_empty() {
-                debug!("Parser::parse_opt: has default_missing_vals");
-                let vals = opt
-                    .default_missing_vals
-                    .iter()
-                    .map(OsString::from)
-                    .collect();
-                self.add_multiple_vals_to_arg(opt, vals, matcher, ValueType::CommandLine, false);
-            };
+        } else if require_equals {
+            if min_vals_zero {
+                debug!("Requires equals, but min_vals == 0");
+                if !opt.default_missing_vals.is_empty() {
+                    debug!("Parser::parse_opt: has default_missing_vals");
+                    let vals = opt
+                        .default_missing_vals
+                        .iter()
+                        .map(OsString::from)
+                        .collect();
+                    self.add_multiple_vals_to_arg(
+                        opt,
+                        vals,
+                        matcher,
+                        ValueType::CommandLine,
+                        false,
+                    );
+                };
+            } else {
+                debug!("Requires equals but not provided. Error.");
+                return Err(ClapError::no_equals(
+                    opt,
+                    Usage::new(self).create_usage_with_title(&[]),
+                    self.app.color(),
+                ));
+            }
         } else {
             debug!("None");
         }

src/parse/validator.rs

@@ -125,9 +125,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
                     ));
                 }
             }
-            if !arg.is_set(ArgSettings::AllowEmptyValues)
-                && val.is_empty()
-                && matcher.contains(&arg.id)
+            if arg.is_set(ArgSettings::NoEmptyValues) && val.is_empty() && matcher.contains(&arg.id)
             {
                 debug!("Validator::validate_arg_values: illegal empty val found");
                 return Err(Error::empty_value(

tests/default_vals.rs

@@ -1,4 +1,4 @@
-use clap::{App, Arg, ErrorKind};
+use clap::{App, Arg, ArgSettings, ErrorKind};
 
 #[test]
 fn opts() {
@@ -14,7 +14,11 @@ fn opts() {
 #[test]
 fn opt_without_value_fail() {
     let r = App::new("df")
-        .arg(Arg::from("-o [opt] 'some opt'").default_value("default"))
+        .arg(
+            Arg::from("-o [opt] 'some opt'")
+                .default_value("default")
+                .setting(ArgSettings::NoEmptyValues),
+        )
         .try_get_matches_from(vec!["", "-o"]);
     assert!(r.is_err());
     let err = r.unwrap_err();