From a5fd7fed1454a4a65223b4d2bf6aae2a27f23512 Mon Sep 17 00:00:00 2001 From: m Date: Thu, 28 Mar 2024 22:09:45 -0700 Subject: [PATCH 1/6] add --color setting --- src/config.rs | 6 ++++++ src/lib.rs | 23 +++++++++++++++++++++++ src/main.rs | 18 +++++++++++++++++- 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index adfdd285..6bacfddb 100644 --- a/src/config.rs +++ b/src/config.rs @@ -137,6 +137,12 @@ impl GlobalConfig { &mut self.stderr } + pub fn set_color_choice(mut self, choice: ColorChoice) -> Self { + self.stdout = StandardStream::stdout(choice); + self.stderr = StandardStream::stderr(choice); + self + } + /// Print a message with a colored title in the style of Cargo shell messages. pub fn shell_print( &mut self, diff --git a/src/lib.rs b/src/lib.rs index d52ee9f7..898e8eda 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,6 +35,7 @@ pub struct Check { current: Rustdoc, baseline: Rustdoc, log_level: Option, + color_choice: Option, release_type: Option, current_feature_config: rustdoc_gen::FeatureConfig, baseline_feature_config: rustdoc_gen::FeatureConfig, @@ -247,6 +248,7 @@ impl Check { current, baseline: Rustdoc::from_registry_latest_crate_version(), log_level: Default::default(), + color_choice: None, release_type: None, current_feature_config: rustdoc_gen::FeatureConfig::default_for_current(), baseline_feature_config: rustdoc_gen::FeatureConfig::default_for_baseline(), @@ -282,6 +284,21 @@ impl Check { pub fn log_level(&self) -> Option<&log::Level> { self.log_level.as_ref() } + + /// Set the termcolor [color choice](termcolor::ColorChoice) + /// If `None`, the use of colors will be determined automatically by + /// CARGO_TERM_COLOR env var and tty type of output` + pub fn with_color_choice(&mut self, choice: Option) -> &mut Self { + self.color_choice = choice; + self + } + + /// Get the current color choice. If `None`, the use of colors is determined + /// by the `CARGO_TERM_COLOR` env var and whether the output is a tty + #[inline] + pub fn color_choice(&self) -> Option<&termcolor::ColorChoice> { + self.color_choice.as_ref() + } pub fn with_release_type(&mut self, release_type: ReleaseType) -> &mut Self { self.release_type = Some(release_type); @@ -386,6 +403,12 @@ impl Check { pub fn check_release(&self) -> anyhow::Result { let mut config = GlobalConfig::new().set_level(self.log_level); + // we don't want to set auto if the choice is not set because it would overwrite + // the value if the CARGO_TERM_COLOR env var read in GlobalConfig::new() if it is set + if let Some(choice) = self.color_choice { + config = config.set_color_choice(choice); + } + let rustdoc_cmd = RustdocCommand::new().deps(false).silence(config.is_info()); // If both the current and baseline rustdoc are given explicitly as a file path, diff --git a/src/main.rs b/src/main.rs index 8624676f..81a7a4ae 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,6 +25,13 @@ fn main() { exit_on_error(true, || { let mut config = GlobalConfig::new().set_level(args.check_release.verbosity.log_level()); + + // we don't want to set auto if the choice is not set because it would overwrite + // the value if the CARGO_TERM_COLOR env var read in GlobalConfig::new() if it is set + if let Some(choice) = args.check_release.color_choice { + config = config.set_color_choice(choice); + } + let queries = SemverQuery::all_queries(); let mut rows = vec![["id", "type", "description"], ["==", "====", "==========="]]; for query in queries.values() { @@ -282,6 +289,14 @@ struct CheckRelease { #[command(flatten)] verbosity: clap_verbosity_flag::Verbosity, + + #[arg(value_enum, long = "color")] + /// Whether to print colors to the terminal: + /// always, alwaysansi (always use only ANSI color codes), + /// auto (based on whether output is a tty), and never + /// + /// Default is auto (use colors if output is a TTY, otherwise don't use colors) + color_choice: Option } impl From for cargo_semver_checks::Check { @@ -344,6 +359,7 @@ impl From for cargo_semver_checks::Check { } check.with_log_level(value.verbosity.log_level()); + check.with_color_choice(value.color_choice); if let Some(release_type) = value.release_type { check.with_release_type(release_type); @@ -365,7 +381,7 @@ impl From for cargo_semver_checks::Check { baseline_features.append(&mut mutual_features); // Treat --features="" as a no-op like cargo does - let trim_features = |features: &mut Vec| { + let trim_features= |features: &mut Vec| { features.retain(|feature| !(feature.is_empty() || feature == "\"\"")); }; trim_features(&mut current_features); From 156bda568a4f0213bbe8d9fbdf00e384d8d6d592 Mon Sep 17 00:00:00 2001 From: m Date: Thu, 28 Mar 2024 22:18:20 -0700 Subject: [PATCH 2/6] run fmt --- src/lib.rs | 4 ++-- src/main.rs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 898e8eda..8881ff68 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -284,7 +284,7 @@ impl Check { pub fn log_level(&self) -> Option<&log::Level> { self.log_level.as_ref() } - + /// Set the termcolor [color choice](termcolor::ColorChoice) /// If `None`, the use of colors will be determined automatically by /// CARGO_TERM_COLOR env var and tty type of output` @@ -292,7 +292,7 @@ impl Check { self.color_choice = choice; self } - + /// Get the current color choice. If `None`, the use of colors is determined /// by the `CARGO_TERM_COLOR` env var and whether the output is a tty #[inline] diff --git a/src/main.rs b/src/main.rs index 81a7a4ae..a0cc99f3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,7 +25,7 @@ fn main() { exit_on_error(true, || { let mut config = GlobalConfig::new().set_level(args.check_release.verbosity.log_level()); - + // we don't want to set auto if the choice is not set because it would overwrite // the value if the CARGO_TERM_COLOR env var read in GlobalConfig::new() if it is set if let Some(choice) = args.check_release.color_choice { @@ -289,14 +289,14 @@ struct CheckRelease { #[command(flatten)] verbosity: clap_verbosity_flag::Verbosity, - + #[arg(value_enum, long = "color")] /// Whether to print colors to the terminal: /// always, alwaysansi (always use only ANSI color codes), /// auto (based on whether output is a tty), and never - /// + /// /// Default is auto (use colors if output is a TTY, otherwise don't use colors) - color_choice: Option + color_choice: Option, } impl From for cargo_semver_checks::Check { @@ -381,7 +381,7 @@ impl From for cargo_semver_checks::Check { baseline_features.append(&mut mutual_features); // Treat --features="" as a no-op like cargo does - let trim_features= |features: &mut Vec| { + let trim_features = |features: &mut Vec| { features.retain(|feature| !(feature.is_empty() || feature == "\"\"")); }; trim_features(&mut current_features); From 195eacfab0cc94448fcbb6a981e88d9e7fcd43d6 Mon Sep 17 00:00:00 2001 From: m Date: Fri, 29 Mar 2024 12:27:09 -0700 Subject: [PATCH 3/6] add color flag and env var tests --- Cargo.lock | 19 +++++++++ Cargo.toml | 2 + tests/color_config.rs | 97 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 tests/color_config.rs diff --git a/Cargo.lock b/Cargo.lock index b250b68e..cde2d047 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -301,6 +301,7 @@ dependencies = [ "ignore", "itertools 0.12.1", "log", + "predicates", "rayon", "ron", "rustc_version", @@ -677,6 +678,15 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "float-cmp" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "98de4bbd547a563b716d8dfa9aad1cb19bfab00f4fa09a6a4ed21dbcf44ce9c4" +dependencies = [ + "num-traits", +] + [[package]] name = "fnv" version = "1.0.7" @@ -1902,6 +1912,12 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "normalize-line-endings" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" + [[package]] name = "num-conv" version = "0.1.0" @@ -2068,7 +2084,10 @@ checksum = "68b87bfd4605926cdfefc1c3b5f8fe560e3feca9d5552cf68c466d3d8236c7e8" dependencies = [ "anstyle", "difflib", + "float-cmp", + "normalize-line-endings", "predicates-core", + "regex", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 9de3763f..50873e26 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,9 @@ rayon = "1.8.0" [dev-dependencies] assert_cmd = "2.0" +lazy_static = "1.4.0" similar-asserts = { version = "1.5.0", features = ["serde"] } +predicates = "3.1.0" # In dev and test profiles, compile all dependencies with optimizations enabled, # but still checking debug assertions and overflows. diff --git a/tests/color_config.rs b/tests/color_config.rs new file mode 100644 index 00000000..77bb04c1 --- /dev/null +++ b/tests/color_config.rs @@ -0,0 +1,97 @@ +//! Tests the --color flag and CARGO_TERM_COLOR env var, and their interactions +//! +//! The --color flag should take precedence over the environment variable +use assert_cmd::Command; +use predicates::boolean::PredicateBooleanExt; + +/// helper function to run `check-release` on a given directory in `test_crates/` with the given color arg/env var +/// - `color_arg` is the argument to `--color=`, if set +/// - `color_env_var` sets the `CARGO_TERM_COLOR=choice`, if set +/// +/// Panics with a failed assertion if there are colors (i.e., ANSI color codes) in the output and `assert_colors` is false, or vice versa +fn run_on_crate_diff( + test_crate_name: &'static str, + color_arg: Option<&'static str>, + color_env_var: Option<&'static str>, + assert_colors: bool, +) { + let mut cmd = Command::cargo_bin("cargo-semver-checks").unwrap(); + + cmd.current_dir(format!("test_crates/{}", test_crate_name)) + .args([ + "semver-checks", + "check-release", + "--manifest-path=new/Cargo.toml", + "--baseline-root=old/", + ]); + + if let Some(color_arg) = color_arg { + cmd.arg("--color").arg(color_arg); + } + + if let Some(color_env) = color_env_var { + cmd.env("CARGO_TERM_COLOR", color_env); + } + + let pred = predicates::str::contains("\u{1b}["); + + let assert = cmd.assert(); + + if assert_colors { + assert.stderr(pred); + } else { + assert.stderr(pred.not()); + } +} + +/// test for colors when using the --color=always flag +#[test] +fn always_flag() { + // test on a crate diff returning no errors + run_on_crate_diff("template", Some("always"), None, true); + // test on a crate diff that will return errors + run_on_crate_diff("enum_missing", Some("always"), None, true); +} + +/// test for no colors when using the --color=never flag +#[test] +fn never_flag() { + // test on a crate diff returning no errors + run_on_crate_diff("template", Some("never"), None, false); + // test on a crate diff that will return errors + run_on_crate_diff("enum_missing", Some("never"), None, false); +} + +/// test for colors when using the CARGO_TERM_COLOR=always env var +#[test] +fn always_var() { + // test on a crate diff returning no errors + run_on_crate_diff("template", None, Some("always"), true); + // test on a crate diff that will return errors + run_on_crate_diff("enum_missing", None, Some("always"), true); +} + +/// test for colors when using the CARGO_TERM_COLOR=never env var +#[test] +fn never_var() { + // test on a crate diff returning no errors + run_on_crate_diff("template", None, Some("never"), false); + // test on a crate diff that will return errors + run_on_crate_diff("enum_missing", None, Some("never"), false); +} + +/// test for the behavior of the --color flag overriding the `CARGO_TERM_COLOR` env var when they conflict +#[test] +fn cli_flag_overrides_env_var() { + // test when --color=always and CARGO_TERM_COLOR=never + // test on a crate diff returning no errors + run_on_crate_diff("template", Some("always"), Some("never"), true); + // test on a crate diff that will return errors + run_on_crate_diff("enum_missing", Some("always"), Some("never"), true); + + // test when --color=never and CARGO_TERM_COLOR=always + // test on a crate diff returning no errors + run_on_crate_diff("template", Some("never"), Some("always"), false); + // test on a crate diff that will return errors + run_on_crate_diff("enum_missing", Some("never"), Some("always"), false); +} From d11594e9f424a6cfbf9c6d625a5b0ffed1234b12 Mon Sep 17 00:00:00 2001 From: Max Carr Date: Fri, 29 Mar 2024 12:31:49 -0700 Subject: [PATCH 4/6] Update Cargo.toml messed up my rebase and left in lazy_static --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 50873e26..975a7047 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,6 @@ rayon = "1.8.0" [dev-dependencies] assert_cmd = "2.0" -lazy_static = "1.4.0" similar-asserts = { version = "1.5.0", features = ["serde"] } predicates = "3.1.0" From 227b790252abb22c8374d21c2f3aef979fa7e31b Mon Sep 17 00:00:00 2001 From: Max Carr Date: Fri, 29 Mar 2024 12:32:14 -0700 Subject: [PATCH 5/6] Update src/lib.rs Co-authored-by: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 8881ff68..be4b141a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -287,7 +287,7 @@ impl Check { /// Set the termcolor [color choice](termcolor::ColorChoice) /// If `None`, the use of colors will be determined automatically by - /// CARGO_TERM_COLOR env var and tty type of output` + /// the `CARGO_TERM_COLOR` env var and tty type of output pub fn with_color_choice(&mut self, choice: Option) -> &mut Self { self.color_choice = choice; self From acf0e42cff3a863a98324bd1991d082f7d1b85c8 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> Date: Fri, 29 Mar 2024 16:49:24 -0400 Subject: [PATCH 6/6] Apply suggestions from code review --- src/lib.rs | 4 ++-- src/main.rs | 2 +- tests/color_config.rs | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index be4b141a..e83922e4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -285,9 +285,9 @@ impl Check { self.log_level.as_ref() } - /// Set the termcolor [color choice](termcolor::ColorChoice) + /// Set the termcolor [color choice](termcolor::ColorChoice). /// If `None`, the use of colors will be determined automatically by - /// the `CARGO_TERM_COLOR` env var and tty type of output + /// the `CARGO_TERM_COLOR` env var and tty type of output. pub fn with_color_choice(&mut self, choice: Option) -> &mut Self { self.color_choice = choice; self diff --git a/src/main.rs b/src/main.rs index a0cc99f3..21b0e695 100644 --- a/src/main.rs +++ b/src/main.rs @@ -290,12 +290,12 @@ struct CheckRelease { #[command(flatten)] verbosity: clap_verbosity_flag::Verbosity, - #[arg(value_enum, long = "color")] /// Whether to print colors to the terminal: /// always, alwaysansi (always use only ANSI color codes), /// auto (based on whether output is a tty), and never /// /// Default is auto (use colors if output is a TTY, otherwise don't use colors) + #[arg(value_enum, long = "color")] color_choice: Option, } diff --git a/tests/color_config.rs b/tests/color_config.rs index 77bb04c1..e90901c6 100644 --- a/tests/color_config.rs +++ b/tests/color_config.rs @@ -31,6 +31,8 @@ fn run_on_crate_diff( if let Some(color_env) = color_env_var { cmd.env("CARGO_TERM_COLOR", color_env); + } else { + cmd.env_remove("CARGO_TERM_COLOR"); } let pred = predicates::str::contains("\u{1b}[");