Merge pull request #2664 from rami3l/env-literal

feat(parser): accept boolean literal with env vars

src/build/arg/mod.rs

@@ -3050,25 +3050,37 @@ impl<'help> Arg<'help> {
     /// assert_eq!(m.value_of("flag"), Some("env"));
     /// ```
     ///
-    /// In this example, we show the flag being raised but with no value because
-    /// of not setting [`Arg::takes_value(true)`]:
+    /// In this example, because [`Arg::takes_value(false)`] (by default),
+    /// `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", "env");
+    /// env::set_var("TRUE_FLAG", "true");
+    /// env::set_var("FALSE_FLAG", "0");
     ///
     /// 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

@@ -16,7 +16,7 @@ use crate::{
     parse::features::suggestions,
     parse::{ArgMatcher, SubCommand},
     parse::{Validator, ValueType},
-    util::{termcolor::ColorChoice, ArgStr, ChildGraph, Id},
+    util::{str_to_bool, termcolor::ColorChoice, ArgStr, ChildGraph, Id},
     INTERNAL_ERROR_MSG, INVALID_UTF8,
 };
 
@@ -1773,39 +1773,60 @@ impl<'help, 'app> Parser<'help, 'app> {
         trailing_values: bool,
     ) -> ClapResult<()> {
         if self.app.is_set(AS::DisableEnv) {
+            debug!("Parser::add_env: Env vars disabled, quitting");
             return Ok(());
         }
 
-        for a in self.app.args.args() {
-            // Use env only if the arg was not present among command line args
-            if matcher.get(&a.id).map_or(true, |a| a.occurs == 0) {
-                if let Some((_, Some(ref val))) = a.env {
-                    let val = ArgStr::new(val);
-                    if a.is_set(ArgSettings::TakesValue) {
-                        self.add_val_to_arg(
-                            a,
-                            val,
-                            matcher,
-                            ValueType::EnvVariable,
-                            false,
-                            trailing_values,
-                        );
-                    } else {
-                        match self.check_for_help_and_version_str(&val) {
-                            Some(ParseResult::HelpFlag) => {
-                                return Err(self.help_err(true));
-                            }
-                            Some(ParseResult::VersionFlag) => {
-                                return Err(self.version_err(true));
-                            }
-                            _ => (),
-                        }
-                        matcher.add_index_to(&a.id, self.cur_idx.get(), ValueType::EnvVariable);
+        self.app.args.args().try_for_each(|a| {
+            // Use env only if the arg was absent among command line args,
+            // early return if this is not the case.
+            if matcher.get(&a.id).map_or(false, |a| a.occurs != 0) {
+                debug!("Parser::add_env: Skipping existing arg `{}`", a);
+                return Ok(());
+            }
+
+            debug!("Parser::add_env: Checking arg `{}`", a);
+            if let Some((_, Some(ref val))) = a.env {
+                let val = ArgStr::new(val);
+
+                if a.is_set(ArgSettings::TakesValue) {
+                    debug!(
+                        "Parser::add_env: Found an opt with value={:?}, trailing={:?}",
+                        val, trailing_values
+                    );
+                    self.add_val_to_arg(
+                        a,
+                        val,
+                        matcher,
+                        ValueType::EnvVariable,
+                        false,
+                        trailing_values,
+                    );
+                    return Ok(());
+                }
+
+                debug!("Parser::add_env: Checking for help and version");
+                // Early return on `HelpFlag` or `VersionFlag`.
+                match self.check_for_help_and_version_str(&val) {
+                    Some(ParseResult::HelpFlag) => {
+                        return Err(self.help_err(true));
+                    }
+                    Some(ParseResult::VersionFlag) => {
+                        return Err(self.version_err(true));
                     }
+                    _ => (),
+                }
+
+                debug!("Parser::add_env: Found a flag with value `{:?}`", val);
+                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);
                 }
             }
-        }
-        Ok(())
+
+            Ok(())
+        })
     }
 
     /// Increase occurrence of specific argument and the grouped arg it's in.

src/util/mod.rs

@@ -4,10 +4,11 @@ mod argstr;
 mod fnv;
 mod graph;
 mod id;
+mod str_to_bool;
 
 pub use self::fnv::Key;
 
-pub(crate) use self::{argstr::ArgStr, graph::ChildGraph, id::Id};
+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

@@ -0,0 +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"];
+
+/// False values are `n`, `no`, `f`, `false`, `off`, and `0`.
+const FALSE_LITERALS: [&str; 6] = ["n", "no", "f", "false", "off", "0"];
+
+/// Converts a string literal representation of truth to true or false.
+///
+/// `false` values are `n`, `no`, `f`, `false`, `off`, and `0` (case insensitive).
+///
+/// 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();
+    !FALSE_LITERALS.contains(&pat)
+}

tests/env.rs

@@ -23,18 +23,23 @@ fn env() {
 }
 
 #[test]
-fn env_no_takes_value() {
-    env::set_var("CLP_TEST_ENV", "env");
+fn env_bool_literal() {
+    env::set_var("CLP_TEST_FLAG_TRUE", "On");
+    env::set_var("CLP_TEST_FLAG_FALSE", "nO");
 
     let r = App::new("df")
-        .arg(Arg::from("[arg] 'some opt'").env("CLP_TEST_ENV"))
+        .arg(Arg::from("[present] 'some opt'").env("CLP_TEST_FLAG_TRUE"))
+        .arg(Arg::from("[negated] 'some another opt'").env("CLP_TEST_FLAG_FALSE"))
+        .arg(Arg::from("[absent] 'some third opt'").env("CLP_TEST_FLAG_ABSENT"))
         .try_get_matches_from(vec![""]);
 
     assert!(r.is_ok());
     let m = r.unwrap();
-    assert!(m.is_present("arg"));
-    assert_eq!(m.occurrences_of("arg"), 0);
-    assert_eq!(m.value_of("arg"), None);
+    assert!(m.is_present("present"));
+    assert_eq!(m.occurrences_of("present"), 0);
+    assert_eq!(m.value_of("present"), None);
+    assert!(!m.is_present("negated"));
+    assert!(!m.is_present("absent"));
 }
 
 #[test]