Skip to content

Commit

Permalink
switch from termcolor to anstream and anstyle (#737)
Browse files Browse the repository at this point in the history 
* ready for some initial feedback

* soliciting feedback here

* switch to `colorchoice-clap` crate for color handling

it still breaks tests, i think it's an issue with how we handle the
`check-release` subcommand

* store stdout and stderr in config

* add some tests

Squashed commit of the following:

commit 59a3a7f
Author: m <[email protected]>
Date:   Fri Apr 12 15:15:42 2024 -0700

    add some simple tests

commit 03b2494
Author: m <[email protected]>
Date:   Fri Apr 12 14:16:55 2024 -0700

    current work

* fix clippy

* (re-)add ColorChoice::global & CARGO_TERM_COLOR

- document that `ColorChoice::write_global` must be called before
  `GlobalConfig::new` or `set_std{err,out}`
- add tests for `GlobalConfig`
- add parsing for `CARGO_TERM_COLOR` env var in binary part of crate

* Apply suggestions from code review

Co-authored-by: Predrag Gruevski <[email protected]>

* make `configure_color` more clear

* clarify rustdoc child process color setting

* handle env vars with flags

we had to make our own colorchoice clap handler
because the mixin doesn't work with Option<_>

* make `Check::check_release` take &mut GlobalConfig

* respect our color preference in `rustdoc`

* minor: save config.stderr() when we use it a lot

this also cleans up lines that get unnecessarily changed in the diff

---------

Co-authored-by: Predrag Gruevski <[email protected]>
  • Loading branch information
@suaviloquence @obi1kenobi
suaviloquence and obi1kenobi authored Apr 18, 2024
1 parent 0bfda4f commit aa44576
Show file tree
Hide file tree
Showing 8 changed files with 451 additions and 272 deletions.
29 changes: 2 additions & 27 deletions Cargo.lock
View file

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

4 changes: 2 additions & 2 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ serde = { version = "1.0.185", features = ["derive"] }
semver = "1.0.12"
handlebars = "5.1.0"
atty = "0.2.14"
termcolor = "1.1.3"
termcolor_output = "1.0.1"
cargo_metadata = "0.18.1"
clap-cargo = { version = "0.14.0", features = ["cargo_metadata"] }
ignore = "0.4.18"
Expand All @@ -45,6 +43,8 @@ directories = "5.0.1"
sha2 = "0.10.6"
rustc_version = "0.4.0"
rayon = "1.8.0"
anstyle = "1.0.6"
anstream = "0.6.13"

[dev-dependencies]
assert_cmd = "2.0"
Expand Down
173 changes: 79 additions & 94 deletions src/check_release.rs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::io::Write as _;
use std::{collections::BTreeMap, sync::Arc, time::Instant};

use anstyle::{AnsiColor, Color, Reset, Style};

use anyhow::Context;
use clap::crate_version;
use itertools::Itertools;
use rayon::prelude::*;
use termcolor::Color;
use termcolor_output::{colored, colored_ln};
use trustfall::TransparentValue;
use trustfall_rustdoc::{VersionedCrate, VersionedIndexedCrate, VersionedRustdocAdapter};

Expand Down Expand Up @@ -163,33 +164,31 @@ pub(super) fn run_check_release(
RequiredSemverUpdate::Minor => "minor",
};
if results.is_empty() {
colored_ln(config.stderr(), |w| {
colored!(
w,
"{}{}{:>12}{} [{:>8.3}s] {:^18} {}",
fg!(Some(Color::Green)),
bold!(true),
"PASS",
reset!(),
time_to_decide.as_secs_f32(),
category,
semver_query.id,
)
})?;
writeln!(
config.stderr(),
"{}{:>12}{} [{:8.3}s] {:^18} {}",
Style::new()
.fg_color(Some(Color::Ansi(AnsiColor::Green)))
.bold(),
"PASS",
Reset,
time_to_decide.as_secs_f32(),
category,
semver_query.id
)?;
} else {
colored_ln(config.stderr(), |w| {
colored!(
w,
"{}{}{:>12}{} [{:>8.3}s] {:^18} {}",
fg!(Some(Color::Red)),
bold!(true),
"FAIL",
reset!(),
time_to_decide.as_secs_f32(),
category,
semver_query.id,
)
})?;
writeln!(
config.stderr(),
"{}{:>12}{} [{:>8.3}s] {:^18} {}",
Style::new()
.fg_color(Some(Color::Ansi(AnsiColor::Red)))
.bold(),
"FAIL",
Reset,
time_to_decide.as_secs_f32(),
category,
semver_query.id
)?;
}
Ok(())
})
Expand All @@ -211,7 +210,7 @@ pub(super) fn run_check_release(
results_with_errors.len(),
skipped_queries,
),
Color::Red,
Color::Ansi(AnsiColor::Red),
true,
)
.expect("print failed");
Expand All @@ -222,67 +221,55 @@ pub(super) fn run_check_release(
required_versions.push(semver_query.required_update);
config
.log_info(|config| {
colored_ln(config.stdout(), |w| {
colored!(
w,
"\n--- failure {}: {} ---\n",
&semver_query.id,
&semver_query.human_readable_name,
)
})?;
writeln!(
config.stdout(),
"\n--- failure {}: {} ---\n",
&semver_query.id,
&semver_query.human_readable_name
)?;
Ok(())
})
.expect("print failed");

if let Some(ref_link) = semver_query.reference_link.as_deref() {
config.log_info(|config| {
colored_ln(config.stdout(), |w| {
colored!(
w,
"{}Description:{}\n{}\n{:>12} {}\n{:>12} {}\n",
bold!(true),
reset!(),
&semver_query.error_message,
"ref:",
ref_link,
"impl:",
format!(
"https://github.com/obi1kenobi/cargo-semver-checks/tree/v{}/src/lints/{}.ron",
crate_version!(),
semver_query.id,
)
)
})?;
writeln!(config.stdout(), "{}Description:{}\n{}\n{:>12} {}\n{:>12} https://github.com/obi1kenobi/cargo-semver-checks/tree/v{}/src/lints/{}.ron\n",
Style::new().bold(), Reset,
&semver_query.error_message,
"ref:",
ref_link,
"impl:",
crate_version!(),
semver_query.id,
)?;
Ok(())
})
.expect("print failed");
} else {
config.log_info(|config| {
colored_ln(config.stdout(), |w| {
colored!(
w,
"{}Description:{}\n{}\n{:>12} {}\n",
bold!(true),
reset!(),
&semver_query.error_message,
"impl:",
format!(
"https://github.com/obi1kenobi/cargo-semver-checks/tree/v{}/src/lints/{}.ron",
crate_version!(),
semver_query.id,
)
)
})?;
writeln!(
config.stdout(),
"{}Description:{}\n{}\n{:>12} https://github.com/obi1kenobi/cargo-semver-checks/tree/v{}/src/lints/{}.ron",
Style::new().bold(),
Reset,
&semver_query.error_message,
"impl:",
crate_version!(),
semver_query.id,
)?;
Ok(())
})
.expect("print failed");
}

config
.log_info(|config| {
colored_ln(config.stdout(), |w| {
colored!(w, "{}Failed in:{}", bold!(true), reset!())
})?;
writeln!(
config.stdout(),
"{}Failed in:{}",
Style::new().bold(),
Reset
)?;
Ok(())
})
.expect("print failed");
Expand All @@ -301,37 +288,35 @@ pub(super) fn run_check_release(
.expect("could not materialize template");
config
.log_info(|config| {
colored_ln(config.stdout(), |w| colored!(w, " {}", message,))?;
writeln!(config.stdout(), " {}", message)?;
Ok(())
})
.expect("print failed");

config
.log_extra_verbose(|config| {
colored_ln(config.stdout(), |w| {
let serde_pretty = serde_json::to_string_pretty(&pretty_result)
.expect("serde failed");
let indented_serde = serde_pretty
.split('\n')
.map(|line| format!(" {line}"))
.join("\n");
colored!(w, " lint rule output values:\n{}", indented_serde)
})
.map_err(|e| e.into())
let serde_pretty =
serde_json::to_string_pretty(&pretty_result).expect("serde failed");
let indented_serde = serde_pretty
.split('\n')
.map(|line| format!(" {line}"))
.join("\n");
writeln!(
config.stdout(),
"\tlint rule output values:\n{}",
indented_serde
)?;
Ok(())
})
.expect("print failed");
} else {
config
.log_info(|config| {
colored_ln(config.stdout(), |w| {
colored!(
w,
"{}\n",
serde_json::to_string_pretty(&pretty_result)
.expect("serde failed"),
)
})
.expect("print failed");
writeln!(
config.stdout(),
"{}\n",
serde_json::to_string_pretty(&pretty_result)?
)?;
Ok(())
})
.expect("print failed");
Expand Down Expand Up @@ -362,7 +347,7 @@ pub(super) fn run_check_release(
.filter(|x| *x == &RequiredSemverUpdate::Minor)
.count(),
),
Color::Red,
Color::Ansi(AnsiColor::Red),
true,
)
.expect("print failed");
Expand All @@ -382,7 +367,7 @@ pub(super) fn run_check_release(
queries_to_run.len(),
skipped_queries,
),
Color::Green,
Color::Ansi(AnsiColor::Green),
true,
)
.expect("print failed");
Expand Down
Loading

0 comments on commit aa44576

Please sign in to comment.