diff --git a/src/build/arg/mod.rs b/src/build/arg/mod.rs index d458507aad8b..83c6dd7dae7a 100644 --- a/src/build/arg/mod.rs +++ b/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: diff --git a/src/parse/parser.rs b/src/parse/parser.rs index a01ab08c0adf..0836e3a81785 100644 --- a/src/parse/parser.rs +++ b/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 = 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); } } diff --git a/src/util/mod.rs b/src/util/mod.rs index 2e977b276a8e..eb7b8331e580 100644 --- a/src/util/mod.rs +++ b/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")] diff --git a/src/util/str_to_bool.rs b/src/util/str_to_bool.rs index 0b2fcb3b3147..58e2efa63d21 100644 --- a/src/util/str_to_bool.rs +++ b/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>(val: S) -> Result { +/// Any other value will be considered as `true`. +pub(crate) fn str_to_bool(val: impl AsRef) -> 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) } diff --git a/tests/env.rs b/tests/env.rs index d704272be019..2d4dc9123015 100644 --- a/tests/env.rs +++ b/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");