Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DRAFT: refactor: COLOR_MODE to OnceCell #365

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

DRAFT: refactor: COLOR_MODE to OnceCell #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e6f0127
refactor: COLOR_MODE to OnceCell
bconn98 Apr 2, 2024
cfe8960
refactor: set_color_mode => get_color_mode
bconn98 Jul 10, 2024
4a4e7cf
refactor: change args of get_color_mode to be Option wrapped to suppo…
bconn98 Jul 12, 2024
0b7cf7c
chore: format
bconn98 Jul 12, 2024
863c336
chore: clippy
bconn98 Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 161 additions & 27 deletions src/encode/writer/console.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,57 @@
//!
//! Requires the `console_writer` feature.

use std::{fmt, io};
use std::{env, fmt, io};

use crate::encode::{self, Style};
use once_cell::sync::Lazy;
use once_cell::sync::OnceCell;

static COLOR_MODE: OnceCell<ColorMode> = OnceCell::new();

/// Determine if coloration of logs should be enabled or disabled at runtime.
/// Arguments are passed as Option<Strings> to support dependency injection in
/// unit testing.
fn get_color_mode(
no_color: Option<Result<String, env::VarError>>,
clicolor_force: Option<Result<String, env::VarError>>,
clicolor: Option<Result<String, env::VarError>>,
) -> ColorMode {
let no_color = match no_color {
Some(result) => result,
None => env::var("NO_COLOR"),
}
.map(|var| var != "0")
.unwrap_or(false);

let clicolor_force = match clicolor_force {
Some(result) => result,
None => env::var("CLICOLOR_FORCE"),
}
.map(|var| var != "0")
.unwrap_or(false);

static COLOR_MODE: Lazy<ColorMode> = Lazy::new(|| {
let no_color = std::env::var("NO_COLOR")
.map(|var| var != "0")
.unwrap_or(false);
let clicolor_force = std::env::var("CLICOLOR_FORCE")
.map(|var| var != "0")
.unwrap_or(false);
if no_color {
ColorMode::Never
} else if clicolor_force {
ColorMode::Always
} else {
let clicolor = std::env::var("CLICOLOR")
.map(|var| var != "0")
.unwrap_or(true);
let clicolor = match clicolor {
Some(result) => result,
None => env::var("CLICOLOR"),
}
.map(|var| var != "0")
.unwrap_or(true);

if clicolor {
ColorMode::Auto
} else {
ColorMode::Never
}
}
});
}

/// The color output mode for a `ConsoleAppender`
#[derive(Clone, Copy, Default)]
#[derive(Clone, Copy, Default, Debug, PartialEq)]
pub enum ColorMode {
/// Print color only if the output is recognized as a console
#[default]
Expand Down Expand Up @@ -128,7 +149,7 @@ mod imp {
self,
writer::{
ansi::AnsiWriter,
console::{ColorMode, COLOR_MODE},
console::{get_color_mode, ColorMode, COLOR_MODE},
},
Style,
},
Expand All @@ -140,7 +161,7 @@ mod imp {
impl Writer {
pub fn stdout() -> Option<Writer> {
let writer = || Writer(AnsiWriter(StdWriter::stdout()));
match *COLOR_MODE {
match COLOR_MODE.get_or_init(|| get_color_mode(None, None, None)) {
ColorMode::Auto => {
if unsafe { libc::isatty(libc::STDOUT_FILENO) } != 1 {
None
Expand All @@ -155,7 +176,7 @@ mod imp {

pub fn stderr() -> Option<Writer> {
let writer = || Writer(AnsiWriter(StdWriter::stderr()));
match *COLOR_MODE {
match COLOR_MODE.get_or_init(|| get_color_mode(None, None, None)) {
ColorMode::Auto => {
if unsafe { libc::isatty(libc::STDERR_FILENO) } != 1 {
None
Expand Down Expand Up @@ -227,7 +248,7 @@ mod imp {
#[cfg(windows)]
mod imp {
use std::{
fmt,
env, fmt,
io::{self, Write},
mem,
};
Expand All @@ -239,7 +260,7 @@ mod imp {
use crate::{
encode::{
self,
writer::console::{ColorMode, COLOR_MODE},
writer::console::{get_color_mode, ColorMode, COLOR_MODE},
Color, Style,
},
priv_io::{StdWriter, StdWriterLock},
Expand Down Expand Up @@ -335,7 +356,7 @@ mod imp {
inner: StdWriter::stdout(),
};

match *COLOR_MODE {
match COLOR_MODE.get_or_init(|| get_color_mode(None, None, None)) {
ColorMode::Auto | ColorMode::Always => Some(writer),
ColorMode::Never => None,
}
Expand All @@ -362,7 +383,7 @@ mod imp {
inner: StdWriter::stdout(),
};

match *COLOR_MODE {
match COLOR_MODE.get_or_init(|| get_color_mode(None, None, None)) {
ColorMode::Auto | ColorMode::Always => Some(writer),
ColorMode::Never => None,
}
Expand Down Expand Up @@ -435,20 +456,23 @@ mod imp {

#[cfg(test)]
mod test {
use std::io::Write;

use super::*;
use crate::encode::{Color, Style, Write as EncodeWrite};
use std::{env::VarError, io::Write};

// Unable to test the non locked Console as by definition, the unlocked
// console results in race conditions. Codecov tooling does not seem to
// see this test as coverage of the ConsoleWritterLock or WriterLock
// class, however, it should completely cover either.
#[test]
fn basic() {
fn test_stdout_console_writer_lock() {
let w = match ConsoleWriter::stdout() {
Some(w) => w,
None => return,
};
let mut w = w.lock();

w.write_all(b"normal ").unwrap();
w.write(b"normal ").unwrap();
w.set_style(
Style::new()
.text(Color::Red)
Expand All @@ -457,10 +481,120 @@ mod test {
)
.unwrap();
w.write_all(b"styled").unwrap();
w.set_style(Style::new().text(Color::Green)).unwrap();
w.set_style(&Style::new().text(Color::Green).intense(false))
.unwrap();
w.write_all(b" styled2").unwrap();
w.set_style(&Style::new()).unwrap();
w.write_all(b" normal\n").unwrap();
w.write_fmt(format_args!(" {} \n", "normal")).unwrap();
w.flush().unwrap();
}

#[test]
fn test_color_mode_default() {
let no_color = Some(Err(VarError::NotPresent));
let clicolor_force = Some(Err(VarError::NotPresent));
let clicolor = Some(Err(VarError::NotPresent));

let color_mode: OnceCell<ColorMode> = OnceCell::new();
assert_eq!(
color_mode.get_or_init(|| get_color_mode(no_color, clicolor_force, clicolor)),
&ColorMode::Auto
);
}

// Note that NO_COLOR has priority over all other fields
#[test]
fn test_no_color() {
let no_color = Some(Ok("1".to_owned()));
let clicolor_force = Some(Err(VarError::NotPresent));
let clicolor = Some(Err(VarError::NotPresent));

let mut color_mode: OnceCell<ColorMode> = OnceCell::new();
assert_eq!(
color_mode.get_or_init(|| get_color_mode(no_color, clicolor_force, clicolor)),
&ColorMode::Never
);

let no_color = Some(Ok("1".to_owned()));
let clicolor_force = Some(Ok("1".to_owned()));
let clicolor = Some(Ok("1".to_owned()));

let _ = color_mode.take(); // Clear the owned value
assert_eq!(
color_mode.get_or_init(|| get_color_mode(no_color, clicolor_force, clicolor)),
&ColorMode::Never
);
}

#[test]
fn test_cli_force() {
// CLICOLOR_FORCE is the only set field
let no_color = Some(Err(VarError::NotPresent));
let clicolor_force = Some(Ok("1".to_owned()));
let clicolor = Some(Err(VarError::NotPresent));

let mut color_mode: OnceCell<ColorMode> = OnceCell::new();
assert_eq!(
color_mode.get_or_init(|| get_color_mode(no_color, clicolor_force, clicolor)),
&ColorMode::Always
);

// Although NO_COLOR has priority, when set to 0 next in line
// is CLICOLOR_FORCE which maintains precedence over clicolor
// regardless of how it's set. Attempt both settings below
let no_color = Some(Ok("0".to_owned()));
let clicolor_force = Some(Ok("1".to_owned()));
let clicolor = Some(Ok("1".to_owned()));

let _ = color_mode.take(); // Clear the owned value
assert_eq!(
color_mode.get_or_init(|| get_color_mode(no_color, clicolor_force, clicolor)),
&ColorMode::Always
);

let no_color = Some(Ok("0".to_owned()));
let clicolor_force = Some(Ok("1".to_owned()));
let clicolor = Some(Ok("0".to_owned()));

let _ = color_mode.take(); // Clear the owned value
assert_eq!(
color_mode.get_or_init(|| get_color_mode(no_color, clicolor_force, clicolor)),
&ColorMode::Always
);
}

#[test]
fn test_cli_on() {
// CLICOLOR is the only set field
let no_color = Some(Err(VarError::NotPresent));
let clicolor_force = Some(Err(VarError::NotPresent));
let clicolor = Some(Ok("1".to_owned()));

let mut color_mode: OnceCell<ColorMode> = OnceCell::new();
assert_eq!(
color_mode.get_or_init(|| get_color_mode(no_color, clicolor_force, clicolor)),
&ColorMode::Auto
);

let no_color = Some(Err(VarError::NotPresent));
let clicolor_force = Some(Err(VarError::NotPresent));
let clicolor = Some(Ok("0".to_owned()));

let _ = color_mode.take(); // Clear the owned value
assert_eq!(
color_mode.get_or_init(|| get_color_mode(no_color, clicolor_force, clicolor)),
&ColorMode::Never
);

// CLICOLOR_FORCE is disabled
let no_color = Some(Err(VarError::NotPresent));
let clicolor_force = Some(Ok("0".to_owned()));
let clicolor = Some(Ok("1".to_owned()));

let _ = color_mode.take(); // Clear the owned value
assert_eq!(
color_mode.get_or_init(|| get_color_mode(no_color, clicolor_force, clicolor)),
&ColorMode::Auto
);
}
}
2 changes: 1 addition & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@
//! For more examples see the [examples](https://github.com/estk/log4rs/tree/main/examples).
//!

#![allow(where_clauses_object_safety, clippy::manual_non_exhaustive)]
#![allow(clippy::manual_non_exhaustive)]
#![warn(missing_docs)]

use std::{
Expand Down
Loading