Skip to content

Commit

Permalink
Auto merge of #12751 - epage:color, r=arlosi
Browse files Browse the repository at this point in the history
refactor: Switch from termcolor to anstream

### What does this PR try to resolve?

`anstream` asks the question "what if you took `fwdansi` and removed `termcolor` underneath it.  It wraps output streams, adapting ANSI escape codes to what is needed
- Pass through if its supported
- Strip if its not
- Adapt to wincon API if needed

Benefits
- Lower boilerplate: we can use `write!` with styled text rather than the back-and-forth between colors and writing that termcolor needs
- Allows richer styling as `Shell` can accept styled messages and adapt them as needed

Side effects
- We'll now respect [NO_COLOR](https://no-color.org/), [CLICOLOR_FORCE](https://bixense.com/clicolors/), and [CLICOLOR](jhasse/clicolors@3a22aaa)

Fixes #12627

### How should we test and review this PR?

This is broken up by commits for easier browsing.

However, as there aren't really tests for colored output, this needs hand inspection to verify

### Additional information

This allowed us to remove the need for stripping ansi escape codes completely.  Even if it didn't, it exposes its internal stripping API for reuse, saving on a dependency and being significantly faster.
  • Loading branch information
bors committed Sep 29, 2023
2 parents 84445c6 + a770d36 commit 59596f0
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 294 deletions.
101 changes: 17 additions & 84 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 5 additions & 13 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ edition = "2021"
license = "MIT OR Apache-2.0"

[workspace.dependencies]
anstyle = "1.0.3"
anstyle-termcolor = "1.1.0"
anstream = "0.6.3"
anstyle = "1.0.4"
anyhow = "1.0.75"
base64 = "0.21.3"
bytesize = "1.3"
Expand All @@ -31,7 +31,7 @@ cargo-test-macro = { path = "crates/cargo-test-macro" }
cargo-test-support = { path = "crates/cargo-test-support" }
cargo-util = { version = "0.2.6", path = "crates/cargo-util" }
cargo_metadata = "0.17.0"
clap = "4.4.4"
clap = "4.4.6"
color-print = "0.3.4"
core-foundation = { version = "0.9.3", features = ["mac_os_10_7_support"] }
crates-io = { version = "0.39.0", path = "crates/crates-io" }
Expand All @@ -40,7 +40,6 @@ curl = "0.4.44"
curl-sys = "0.4.66"
filetime = "0.2.22"
flate2 = { version = "1.0.27", default-features = false, features = ["zlib"] }
fwdansi = "1.1.0"
git2 = "0.18.0"
git2-curl = "0.19.0"
gix = { version = "0.54.1", default-features = false, features = ["blocking-http-transport-curl", "progress-tree", "revision"] }
Expand Down Expand Up @@ -86,12 +85,10 @@ serde_json = "1.0.105"
sha1 = "0.10.5"
sha2 = "0.10.7"
shell-escape = "0.1.5"
snapbox = { version = "0.4.12", features = ["diff", "path"] }
strip-ansi-escapes = "0.1.1"
snapbox = { version = "0.4.13", features = ["diff", "path"] }
syn = { version = "2.0.29", features = ["extra-traits", "full"] }
tar = { version = "0.4.40", default-features = false }
tempfile = "3.8.0"
termcolor = "1.2.0"
thiserror = "1.0.47"
time = { version = "0.3", features = ["parsing", "formatting", "serde"] }
toml = "0.7.6"
Expand Down Expand Up @@ -123,8 +120,8 @@ name = "cargo"
path = "src/cargo/lib.rs"

[dependencies]
anstream.workspace = true
anstyle.workspace = true
anstyle-termcolor.workspace = true
anyhow.workspace = true
base64.workspace = true
bytesize.workspace = true
Expand Down Expand Up @@ -175,11 +172,9 @@ serde_ignored.workspace = true
serde_json = { workspace = true, features = ["raw_value"] }
sha1.workspace = true
shell-escape.workspace = true
strip-ansi-escapes.workspace = true
syn.workspace = true
tar.workspace = true
tempfile.workspace = true
termcolor.workspace = true
time.workspace = true
toml.workspace = true
toml_edit.workspace = true
Expand All @@ -194,9 +189,6 @@ walkdir.workspace = true
[target.'cfg(not(windows))'.dependencies]
openssl = { workspace = true, optional = true }

[target.'cfg(windows)'.dependencies]
fwdansi.workspace = true

[target.'cfg(windows)'.dependencies.windows-sys]
workspace = true
features = [
Expand Down
3 changes: 2 additions & 1 deletion crates/cargo-test-support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ publish = false
doctest = false

[dependencies]
anstream.workspace = true
anstyle.workspace = true
anyhow.workspace = true
cargo-test-macro.workspace = true
cargo-util.workspace = true
Expand All @@ -24,7 +26,6 @@ serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
snapbox.workspace = true
tar.workspace = true
termcolor.workspace = true
time.workspace = true
toml.workspace = true
url.workspace = true
Expand Down
48 changes: 20 additions & 28 deletions crates/cargo-test-support/src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

use std::fmt;
use std::io::Write;
use termcolor::{Ansi, Color, ColorSpec, NoColor, WriteColor};

/// A single line change to be applied to the original.
#[derive(Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -111,42 +110,35 @@ where
}

pub fn render_colored_changes<T: fmt::Display>(changes: &[Change<T>]) -> String {
// termcolor is not very ergonomic, but I don't want to bring in another dependency.
let mut red = ColorSpec::new();
red.set_fg(Some(Color::Red));
let mut green = ColorSpec::new();
green.set_fg(Some(Color::Green));
let mut dim = ColorSpec::new();
dim.set_dimmed(true);
let mut v = Vec::new();
let mut result: Box<dyn WriteColor> = if crate::is_ci() {
// anstyle is not very ergonomic, but I don't want to bring in another dependency.
let red = anstyle::AnsiColor::Red.on_default().render();
let green = anstyle::AnsiColor::Green.on_default().render();
let dim = (anstyle::Style::new() | anstyle::Effects::DIMMED).render();
let bold = (anstyle::Style::new() | anstyle::Effects::BOLD).render();
let reset = anstyle::Reset.render();

let choice = if crate::is_ci() {
// Don't use color on CI. Even though GitHub can display colors, it
// makes reading the raw logs more difficult.
Box::new(NoColor::new(&mut v))
anstream::ColorChoice::Never
} else {
Box::new(Ansi::new(&mut v))
anstream::AutoStream::choice(&std::io::stdout())
};
let mut buffer = anstream::AutoStream::new(Vec::new(), choice);

for change in changes {
let (nums, sign, color, text) = match change {
Change::Add(i, s) => (format!(" {:<4} ", i), '+', &green, s),
Change::Remove(i, s) => (format!("{:<4} ", i), '-', &red, s),
Change::Keep(x, y, s) => (format!("{:<4}{:<4} ", x, y), ' ', &dim, s),
Change::Add(i, s) => (format!(" {:<4} ", i), '+', green, s),
Change::Remove(i, s) => (format!("{:<4} ", i), '-', red, s),
Change::Keep(x, y, s) => (format!("{:<4}{:<4} ", x, y), ' ', dim, s),
};
result.set_color(&dim).unwrap();
write!(result, "{}", nums).unwrap();
let mut bold = color.clone();
bold.set_bold(true);
result.set_color(&bold).unwrap();
write!(result, "{}", sign).unwrap();
result.reset().unwrap();
result.set_color(&color).unwrap();
write!(result, "{}", text).unwrap();
result.reset().unwrap();
writeln!(result).unwrap();
write!(
buffer,
"{dim}{nums}{reset}{bold}{sign}{reset}{color}{text}{reset}"
)
.unwrap();
}
drop(result);
String::from_utf8(v).unwrap()
String::from_utf8(buffer.into_inner()).unwrap()
}

#[cfg(test)]
Expand Down
Loading

0 comments on commit 59596f0

Please sign in to comment.