diff --git a/Cargo.toml b/Cargo.toml index c470bf5..a88433a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,9 +14,9 @@ keywords = ["color", "string", "term", "ansi_term", "term-painter"] no-color = [] [dependencies] +atty = "0.2.11" lazy_static = "1.4.0" - [target.'cfg(windows)'.dependencies.winapi] version = "0.3" default-features = false diff --git a/src/control.rs b/src/control.rs index f620745..ab6289a 100644 --- a/src/control.rs +++ b/src/control.rs @@ -56,7 +56,7 @@ pub fn set_virtual_terminal(use_virtual: bool) -> Result<(), ()> { } pub struct ShouldColorize { - clicolor: Option, + clicolor: bool, clicolor_force: Option, // XXX we can't use Option because we can't use &mut references to ShouldColorize has_manual_override: AtomicBool, @@ -82,7 +82,7 @@ lazy_static! { impl Default for ShouldColorize { fn default() -> ShouldColorize { ShouldColorize { - clicolor: None, + clicolor: true, clicolor_force: None, has_manual_override: AtomicBool::new(false), manual_override: AtomicBool::new(false), @@ -91,25 +91,16 @@ impl Default for ShouldColorize { } impl ShouldColorize { - /// Reads environment variables to determine whether colorization should - /// be used or not. `CLICOLOR_FORCE` takes highest priority, followed by - /// `NO_COLOR`, followed by `CLICOLOR`. In the absence of manual overrides, - /// which take precedence over all environment variables, the priority - /// of these variables can be expressed as follows. - /// - /// `NO_COLOR` | `CLICOLOR` | `CLICOLOR_FORCE` | colorize? - /// :--------- | :--------- | :--------------- | :-------- - /// unset | unset | unset | true (default) - /// unset | `!= 0` | unset | true - /// unset | `== 0` | unset | false - /// set | unset/`== 0`/`!= 0` | unset | false - /// set/unset | unset/`== 0`/`!= 0` | `== 0` | false - /// set/unset | unset/`== 0`/`!= 0` | `!= 0` | true + /// Reads environment variables and checks if output is a tty to determine + /// whether colorization should be used or not. + /// `CLICOLOR_FORCE` takes highest priority, followed by `NO_COLOR`, + /// followed by `CLICOLOR` combined with tty check. pub fn from_env() -> Self { use std::io; ShouldColorize { - clicolor: ShouldColorize::normalize_env(env::var("CLICOLOR")), + clicolor: ShouldColorize::normalize_env(env::var("CLICOLOR")).unwrap_or_else(|| true) + && atty::is(atty::Stream::Stdout), clicolor_force: ShouldColorize::resolve_clicolor_force( env::var("NO_COLOR"), env::var("CLICOLOR_FORCE"), @@ -127,11 +118,7 @@ impl ShouldColorize { return forced_value; } - if let Some(value) = self.clicolor { - return value; - } - - true + self.clicolor } pub fn set_override(&self, override_colorize: bool) { @@ -157,13 +144,12 @@ impl ShouldColorize { no_color: Result, clicolor_force: Result, ) -> Option { - match ( - ShouldColorize::normalize_env(no_color), - ShouldColorize::normalize_env(clicolor_force), - ) { - (_, Some(b)) => Some(b), - (Some(_), None) => Some(false), - (None, None) => None, + if ShouldColorize::normalize_env(clicolor_force) == Some(true) { + Some(true) + } else if ShouldColorize::normalize_env(no_color).is_some() { + Some(false) + } else { + None } } } @@ -205,7 +191,7 @@ mod specs { ctx.describe("::resolve_clicolor_force", |ctx| { ctx.it( - "should return None if neither NO_COLOR nor CLICOLOR_FORCE are set", + "should return None if NO_COLOR is not set and CLICOLOR_FORCE is not set or set to 0", || { assert_eq!( None, @@ -214,11 +200,18 @@ mod specs { Err(env::VarError::NotPresent) ) ); + assert_eq!( + None, + ShouldColorize::resolve_clicolor_force( + Err(env::VarError::NotPresent), + Ok(String::from("0")), + ) + ); }, ); ctx.it( - "should return Some(false) if NO_COLOR is set and CLICOLOR_FORCE is unset", + "should return Some(false) if NO_COLOR is set and CLICOLOR_FORCE is not enabled", || { assert_eq!( Some(false), @@ -234,11 +227,18 @@ mod specs { Err(env::VarError::NotPresent) ) ); + assert_eq!( + Some(false), + ShouldColorize::resolve_clicolor_force( + Ok(String::from("1")), + Ok(String::from("0")), + ) + ); }, ); ctx.it( - "should ignore NO_COLOR and return Some(forced_value) if CLICOLOR_FORCE is set to forced_value", + "should prioritize CLICOLOR_FORCE over NO_COLOR if CLICOLOR_FORCE is set to non-zero value", || { assert_eq!( Some(true), @@ -254,13 +254,6 @@ mod specs { Ok(String::from("0")), ) ); - assert_eq!( - Some(false), - ShouldColorize::resolve_clicolor_force( - Err(env::VarError::NotPresent), - Ok(String::from("0")), - ) - ); assert_eq!( Some(true), ShouldColorize::resolve_clicolor_force( @@ -285,7 +278,7 @@ mod specs { ctx.describe("when only changing clicolors", |ctx| { ctx.it("clicolor == false means no colors", || { let colorize_control = ShouldColorize { - clicolor: Some(false), + clicolor: false, ..ShouldColorize::default() }; false == colorize_control.should_colorize() @@ -293,7 +286,7 @@ mod specs { ctx.it("clicolor == true means colors !", || { let colorize_control = ShouldColorize { - clicolor: Some(true), + clicolor: true, ..ShouldColorize::default() }; true == colorize_control.should_colorize() @@ -309,7 +302,7 @@ mod specs { "clicolor_force should force to true no matter clicolor", || { let colorize_control = ShouldColorize { - clicolor: Some(false), + clicolor: false, clicolor_force: Some(true), ..ShouldColorize::default() }; @@ -322,7 +315,7 @@ mod specs { "clicolor_force should force to false no matter clicolor", || { let colorize_control = ShouldColorize { - clicolor: Some(true), + clicolor: true, clicolor_force: Some(false), ..ShouldColorize::default() }; @@ -335,7 +328,7 @@ mod specs { ctx.describe("using a manual override", |ctx| { ctx.it("shoud colorize if manual_override is true, but clicolor is false and clicolor_force also false", || { let colorize_control = ShouldColorize { - clicolor: Some(false), + clicolor: false, clicolor_force: None, has_manual_override: AtomicBool::new(true), manual_override: AtomicBool::new(true), @@ -347,7 +340,7 @@ mod specs { ctx.it("should not colorize if manual_override is false, but clicolor is true or clicolor_force is true", || { let colorize_control = ShouldColorize { - clicolor: Some(true), + clicolor: true, clicolor_force: Some(true), has_manual_override: AtomicBool::new(true), manual_override: AtomicBool::new(false), diff --git a/src/lib.rs b/src/lib.rs index 069df26..d6b591d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,7 @@ //! See [the `Colorize` trait](./trait.Colorize.html) for all the methods. //! +extern crate atty; #[macro_use] extern crate lazy_static; #[cfg(windows)]