feat(parser): accept non-false literals with env vars as true

src/build/arg/mod.rs

@@ -3051,28 +3051,36 @@ impl<'help> Arg<'help> {
     /// ```
     ///
     /// In this example, because [`Arg::takes_value(false)`] (by default),
-    /// `prog` is a flag that only accepts optional, case-insensitive boolean literals.
-    /// A `true` literal is `y`, `yes`, `t`, `true`, `on` or `1`;
-    /// a `false` literal is `n`, `no`, `f`, `false`, `off` or `0`.
-    /// An absent environment variable will be considered as `false`.
-    /// Anything else will cause a parsing error.
+    /// `prog` is a flag that accepts an optional, case-insensitive boolean literal.
+    /// A `false` literal is `n`, `no`, `f`, `false`, `off` or `0`.
+    /// An absent environment variable will also be considered as `false`.
+    /// Anything else will considered as `true`.
     ///
     /// ```rust
     /// # use std::env;
     /// # use clap::{App, Arg};
     ///
-    /// env::set_var("MY_FLAG", "true");
+    /// env::set_var("TRUE_FLAG", "true");
+    /// env::set_var("FALSE_FLAG", "OFF");
     ///
     /// let m = App::new("prog")
-    ///     .arg(Arg::new("flag")
-    ///         .long("flag")
-    ///         .env("MY_FLAG"))
+    ///     .arg(Arg::new("true_flag")
+    ///         .long("true_flag")
+    ///         .env("TRUE_FLAG"))
+    ///     .arg(Arg::new("false_flag")
+    ///         .long("false_flag")
+    ///         .env("FALSE_FLAG"))
+    ///     .arg(Arg::new("absent_flag")
+    ///         .long("absent_flag")
+    ///         .env("ABSENT_FLAG"))
     ///     .get_matches_from(vec![
     ///         "prog"
     ///     ]);
     ///
-    /// assert!(m.is_present("flag"));
-    /// assert_eq!(m.value_of("flag"), None);
+    /// assert!(m.is_present("true_flag"));
+    /// assert_eq!(m.value_of("true_flag"), None);
+    /// assert!(!m.is_present("false_flag"));
+    /// assert!(!m.is_present("absent_flag"));
     /// ```
     ///
     /// In this example, we show the variable coming from an option on the CLI:

src/parse/parser.rs

@@ -1772,7 +1772,7 @@ impl<'help, 'app> Parser<'help, 'app> {
         matcher: &mut ArgMatcher,
         trailing_values: bool,
     ) -> ClapResult<()> {
-        use crate::util::{str_to_bool, FALSE_LITERALS, TRUE_LITERALS};
+        use crate::util::str_to_bool;
 
         if self.app.is_set(AS::DisableEnv) {
             debug!("Parser::add_env: Env vars disabled, quitting");
@@ -1820,38 +1820,10 @@ impl<'help, 'app> Parser<'help, 'app> {
                 }
 
                 debug!("Parser::add_env: Found a flag with value `{:?}`", val);
-                match str_to_bool(val.to_string_lossy()) {
-                    Ok(predicate) => {
-                        debug!("Parser::add_env: Found boolean literal `{}`", predicate);
-                        if predicate {
-                            matcher.add_index_to(&a.id, self.cur_idx.get(), ValueType::EnvVariable);
-                        }
-                    }
-                    Err(rest) => {
-                        debug!("Parser::parse_long_arg: Got invalid literal `{}`", rest);
-                        let used: Vec<Id> = matcher
-                            .arg_names()
-                            .filter(|&n| {
-                                self.app.find(n).map_or(true, |a| {
-                                    !(a.is_set(ArgSettings::Hidden)
-                                        || self.required.contains(&a.id))
-                                })
-                            })
-                            .cloned()
-                            .collect();
-                        let good_vals: Vec<&str> = TRUE_LITERALS
-                            .iter()
-                            .chain(FALSE_LITERALS.iter())
-                            .copied()
-                            .collect();
-                        return Err(ClapError::invalid_value(
-                            rest.into(),
-                            &good_vals,
-                            a,
-                            Usage::new(self).create_usage_no_title(&used),
-                            self.app.color(),
-                        ));
-                    }
+                let predicate = str_to_bool(val.to_string_lossy());
+                debug!("Parser::add_env: Found boolean literal `{}`", predicate);
+                if predicate {
+                    matcher.add_index_to(&a.id, self.cur_idx.get(), ValueType::EnvVariable);
                 }
             }
 

src/util/mod.rs

@@ -8,12 +8,7 @@ mod str_to_bool;
 
 pub use self::fnv::Key;
 
-pub(crate) use self::{
-    argstr::ArgStr,
-    graph::ChildGraph,
-    id::Id,
-    str_to_bool::{str_to_bool, FALSE_LITERALS, TRUE_LITERALS},
-};
+pub(crate) use self::{argstr::ArgStr, graph::ChildGraph, id::Id, str_to_bool::str_to_bool};
 pub(crate) use vec_map::VecMap;
 
 #[cfg(feature = "color")]

src/util/str_to_bool.rs

@@ -1,24 +1,15 @@
 /// True values are `y`, `yes`, `t`, `true`, `on`, and `1`.
-pub(crate) const TRUE_LITERALS: [&str; 6] = ["y", "yes", "t", "true", "on", "1"];
+// pub(crate) const TRUE_LITERALS: [&str; 6] = ["y", "yes", "t", "true", "on", "1"];
 
 /// False values are `n`, `no`, `f`, `false`, `off`, and `0`.
-pub(crate) const FALSE_LITERALS: [&str; 6] = ["n", "no", "f", "false", "off", "0"];
+const FALSE_LITERALS: [&str; 6] = ["n", "no", "f", "false", "off", "0"];
 
 /// Converts a string literal representation of truth to true or false.
 ///
-/// Translated from the Python function [`strtobool`].
+/// `false` values are `n`, `no`, `f`, `false`, `off`, and `0` (case insensitive).
 ///
-/// [`strtobool`]: https://docs.python.org/3/distutils/apiref.html#distutils.util.strtobool
-///
-/// # Errors
-/// Returns Err(val) if `val` is anything else.
-pub(crate) fn str_to_bool<S: AsRef<str>>(val: S) -> Result<bool, S> {
+/// Any other value will be considered as `true`.
+pub(crate) fn str_to_bool(val: impl AsRef<str>) -> bool {
     let pat: &str = &val.as_ref().to_lowercase();
-    if TRUE_LITERALS.contains(&pat) {
-        Ok(true)
-    } else if FALSE_LITERALS.contains(&pat) {
-        Ok(false)
-    } else {
-        Err(val)
-    }
+    !FALSE_LITERALS.contains(&pat)
 }

tests/env.rs

@@ -1,7 +1,7 @@
 use std::env;
 use std::ffi::OsStr;
 
-use clap::{App, AppSettings, Arg, Error as ClapError};
+use clap::{App, AppSettings, Arg};
 
 #[test]
 fn env() {
@@ -22,23 +22,6 @@ fn env() {
     assert_eq!(m.value_of("arg").unwrap(), "env");
 }
 
-#[test]
-fn env_no_takes_value() {
-    env::set_var("CLP_TEST_ENV", "env");
-
-    let r = App::new("df")
-        .arg(Arg::from("[arg] 'some opt'").env("CLP_TEST_ENV"))
-        .try_get_matches_from(vec![""]);
-
-    assert!(matches!(
-        dbg!(r),
-        Err(ClapError {
-            kind: clap::ErrorKind::InvalidValue,
-            ..
-        })
-    ));
-}
-
 #[test]
 fn env_bool_literal() {
     env::set_var("CLP_TEST_FLAG_TRUE", "On");