Skip to content

Commit

Permalink
Merge pull request #55 from vn971/master
Browse files Browse the repository at this point in the history 
respect tty discovery for CLICOLOR, fix handling of CLICOLOR_FORCE value of 0, fix CLICOLOR_FORCE and NO_COLOR priorities
  • Loading branch information
@kurtlawrence
kurtlawrence authored Nov 11, 2019
2 parents 98667ba + 4ab870f commit 060d3a1
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 46 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
83 changes: 38 additions & 45 deletions src/control.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub fn set_virtual_terminal(use_virtual: bool) -> Result<(), ()> {
}

pub struct ShouldColorize {
clicolor: Option<bool>,
clicolor: bool,
clicolor_force: Option<bool>,
// XXX we can't use Option<Atomic> because we can't use &mut references to ShouldColorize
has_manual_override: AtomicBool,
Expand All @@ -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),
Expand All @@ -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"),
Expand All @@ -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) {
Expand All @@ -157,13 +144,12 @@ impl ShouldColorize {
no_color: Result<String, env::VarError>,
clicolor_force: Result<String, env::VarError>,
) -> Option<bool> {
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
}
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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(
Expand All @@ -285,15 +278,15 @@ 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()
});

ctx.it("clicolor == true means colors !", || {
let colorize_control = ShouldColorize {
clicolor: Some(true),
clicolor: true,
..ShouldColorize::default()
};
true == colorize_control.should_colorize()
Expand All @@ -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()
};
Expand All @@ -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()
};
Expand All @@ -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),
Expand All @@ -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),
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down

0 comments on commit 060d3a1

Please sign in to comment.