Skip to content

Commit f352be2

Browse files
authored
refactor: add colors to --help output and uniform logging (#374)
Following DRY principal, I unified all logging implementation used in the workspace to clang-tools-manager crate. I also took the time to add colors to all `--help` output. These colors still adhere to `FORCE`, `CLICOLOR`, `CLICOLOR_FORCE`, and `NO_COLOR` env vars. The color theme resides in the same logger module used by each binary in the workspace. Lastly, I made clang-tools CLI's `--tool` argument into a repeatable positional argument (with the same default value). PS - I adjusted cargo-nextest config to auto-cancel on tests that get caught in an infinite loop/hang. This happened recently in CI, and had it to be cancel manually.
1 parent c149eea commit f352be2

18 files changed

Lines changed: 99 additions & 243 deletions

File tree

.config/nextest.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ default-filter = "(kind(test) + test(use_extra_args)) - package(clang-tools-mana
3232
# show log output from each failing test
3333
failure-output = "final"
3434

35+
# set slow-timeout to 30 seconds and
36+
# terminate tests after hitting the period 8 times (30s x 8 = 4 minutes).
37+
slow-timeout = { period = "30s", terminate-after = 8 }
38+
3539
[profile.all]
3640
# A profile to run all tests (including tests that run longer than 10 seconds)
3741
default-filter = "all() - test(#*eval_version)"
38-
39-
# Revert slow-timeout to nextest default value.
40-
# Otherwise, default profile value (10s) is inherited.
41-
slow-timeout = "60s"

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ strip = "symbols"
2222
[workspace.dependencies]
2323
anyhow = "1.0.102"
2424
clap = { version = "4.6.1", features = ["derive"] }
25-
colored = "3.1.1"
2625
log = { version = "0.4.31", features = ["std"] }
2726
mockito = "1.7.2"
2827
pyo3 = {version = "0.29.0", features = ["extension-module"] }

clang-tools-manager/Cargo.toml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ required-features = ["bin"]
2424
[dependencies]
2525
anyhow = { workspace = true, optional = true }
2626
blake2 = "0.10.6"
27-
clap = { workspace = true, features = ["derive"], optional = true }
28-
colored = { workspace = true, optional = true }
27+
clap = { workspace = true, optional = true }
28+
colored = { version = "3.1.1", optional = true }
2929
directories = "6.0.0"
3030
log = { workspace = true }
3131
regex = { workspace = true }
@@ -41,7 +41,6 @@ which = "8.0.2"
4141
zip = { version = "8.6.0", default-features = false, features = ["deflate"] }
4242

4343
[dev-dependencies]
44-
colored = { workspace = true }
4544
mockito = { workspace = true }
4645
reqwest = { workspace = true, features = ["default-tls"] }
4746
tempfile = { workspace = true }

clang-tools-manager/README.md

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,15 @@ The `clang-tools` binary's Command Line Interface (CLI) is rather simple.
4141
<details><summary><code>clang-tools --help</code></summary>
4242
<p>
4343

44-
```sh
45-
Usage: clang-tools [OPTIONS]
44+
```txt
45+
Usage: clang-tools.exe [OPTIONS] [TOOL]...
46+
47+
Arguments:
48+
[TOOL]...
49+
The clang tool to install
50+
51+
[default: clang-format clang-tidy]
52+
[possible values: clang-tidy, clang-format]
4653
4754
Options:
4855
-v, --version [<VERSION>]
@@ -56,12 +63,6 @@ Options:
5663
This will include more DEBUG level log messages.
5764
Without it, log level is set to INFO by default.
5865
59-
-t, --tool <TOOL>
60-
The clang tool to install
61-
62-
[default: "clang-format clang-tidy"]
63-
[possible values: clang-tidy, clang-format]
64-
6566
-d, --directory <DIRECTORY>
6667
The directory where the clang tools should be installed
6768
@@ -70,6 +71,24 @@ Options:
7071
7172
This will only overwrite an existing symlink.
7273
74+
--mod-sys
75+
Whether to use the system's available package managers.
76+
77+
By default, this matches the value of a CI environment variable.
78+
For non-CI contexts, this allows users to opt-in to using
79+
system package managers as a fallback in case PyPI offerings are
80+
unsatisfactory.
81+
82+
If system package managers are not allowed or fail, then
83+
static binaries built by cpp-linter are sought
84+
(for compatible platforms).
85+
86+
--no-mod-sys
87+
Strictly disallow using system package managers.
88+
89+
This can be used to override the default behavior of `--mod-sys`,
90+
useful in sensitive CI environments, like self-hosted runners.
91+
7392
-h, --help
7493
Print help (see a summary with '-h')
7594
```

clang-tools-manager/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,5 @@ pub use version::{ClangVersion, GetToolError, RequestedVersion, RequestedVersion
2323

2424
mod progress_bar;
2525
pub use progress_bar::ProgressBar;
26+
27+
pub mod logger;
Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,26 @@ use std::{
66
io::{Write, stdout},
77
};
88

9+
use clap::builder::{
10+
Styles,
11+
styling::{AnsiColor, Color, Style},
12+
};
913
use colored::{Colorize, control::set_override};
1014
use log::{Level, LevelFilter, Log, Metadata, Record};
1115

16+
/// A color scheme to use for CLI `--help` output.
17+
pub const CLI_HELP_STYLE: Styles = Styles::styled()
18+
.usage(Style::new().fg_color(Some(Color::Ansi(AnsiColor::BrightBlue))))
19+
.header(
20+
Style::new()
21+
.fg_color(Some(Color::Ansi(AnsiColor::BrightBlue)))
22+
.underline(),
23+
)
24+
.placeholder(Style::new().fg_color(Some(Color::Ansi(AnsiColor::Yellow))))
25+
.literal(Style::new().fg_color(Some(Color::Ansi(AnsiColor::BrightGreen))))
26+
.context_value(Style::new().fg_color(Some(Color::Ansi(AnsiColor::Cyan))))
27+
.invalid(Style::new().fg_color(Some(Color::Ansi(AnsiColor::BrightRed))));
28+
1229
#[derive(Default)]
1330
struct SimpleLogger;
1431

@@ -41,7 +58,7 @@ impl Log for SimpleLogger {
4158
.expect("Failed to flush log command in stdout");
4259
} else if self.enabled(record.metadata()) {
4360
let module = record.module_path();
44-
if module.is_none_or(|v| v.starts_with("cpp_linter")) {
61+
if module.is_none_or(|v| v.starts_with("cpp_linter") || v.starts_with("clang_tools")) {
4562
writeln!(
4663
stdout,
4764
"[{}]: {}",
@@ -74,7 +91,7 @@ impl Log for SimpleLogger {
7491
/// The logging level defaults to [`LevelFilter::Info`].
7592
/// This logs a debug message about [`SetLoggerError`](struct@log::SetLoggerError)
7693
/// if the `LOGGER` is already initialized.
77-
pub fn try_init() {
94+
pub fn try_init_logger() {
7895
let logger: SimpleLogger = SimpleLogger;
7996
if matches!(
8097
env::var("CPP_LINTER_COLOR").as_deref(),
@@ -95,15 +112,14 @@ pub fn try_init() {
95112
mod test {
96113
use std::env;
97114

98-
use super::{SimpleLogger, try_init};
115+
use super::try_init_logger;
99116

100117
#[test]
101118
fn trace_log() {
102119
unsafe {
103120
env::set_var("CPP_LINTER_COLOR", "true");
104121
}
105-
try_init();
106-
assert!(SimpleLogger::level_color(&log::Level::Trace).contains("TRACE"));
122+
try_init_logger();
107123
log::set_max_level(log::LevelFilter::Trace);
108124
log::trace!("A dummy log statement for code coverage");
109125
}

clang-tools-manager/src/main.rs

Lines changed: 10 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1,101 +1,14 @@
11
use anyhow::Result;
2-
use clang_tools_manager::{ClangTool, RequestedVersion};
2+
use clang_tools_manager::{
3+
ClangTool, RequestedVersion,
4+
logger::{CLI_HELP_STYLE, try_init_logger},
5+
};
36
use clap::Parser;
47

58
use std::{collections::HashMap, path::PathBuf, str::FromStr};
6-
mod logging {
7-
use colored::{Colorize, control::set_override};
8-
use log::{Level, LevelFilter, Log, Metadata, Record};
9-
use std::{
10-
env,
11-
io::{Write, stdout},
12-
};
13-
14-
struct SimpleLogger;
15-
16-
impl SimpleLogger {
17-
fn level_color(level: &Level) -> String {
18-
let name = format!("{:>5}", level.as_str().to_uppercase());
19-
match level {
20-
Level::Error => name.red().bold().to_string(),
21-
Level::Warn => name.yellow().bold().to_string(),
22-
Level::Info => name.green().bold().to_string(),
23-
Level::Debug => name.blue().bold().to_string(),
24-
Level::Trace => name.magenta().bold().to_string(),
25-
}
26-
}
27-
}
28-
29-
impl Log for SimpleLogger {
30-
fn enabled(&self, metadata: &Metadata) -> bool {
31-
metadata.level() <= log::max_level()
32-
}
33-
34-
fn log(&self, record: &Record) {
35-
let mut stdout = stdout().lock();
36-
if record.target() == "CI_LOG_GROUPING" {
37-
// this log is meant to manipulate a CI workflow's log grouping
38-
writeln!(stdout, "{}", record.args())
39-
.expect("Failed to write log command to stdout");
40-
stdout
41-
.flush()
42-
.expect("Failed to flush log command in stdout");
43-
} else if self.enabled(record.metadata()) {
44-
let module = record.module_path();
45-
if module.is_none_or(|v| {
46-
v.starts_with("clang_tools_manager") || v.starts_with("clang_tools")
47-
}) {
48-
writeln!(
49-
stdout,
50-
"[{}]: {}",
51-
Self::level_color(&record.level()),
52-
record.args()
53-
)
54-
.expect("Failed to write log message to stdout");
55-
} else if let Some(module) = module {
56-
writeln!(
57-
stdout,
58-
"[{}]{{{}:{}}}: {}",
59-
Self::level_color(&record.level()),
60-
module,
61-
record.line().unwrap_or_default(),
62-
record.args()
63-
)
64-
.expect("Failed to write detailed log message to stdout");
65-
}
66-
stdout
67-
.flush()
68-
.expect("Failed to flush log message in stdout");
69-
}
70-
}
71-
72-
fn flush(&self) {}
73-
}
74-
75-
/// A function to initialize the private `LOGGER`.
76-
///
77-
/// The logging level defaults to [`LevelFilter::Info`].
78-
/// This logs a debug message about [`SetLoggerError`](struct@log::SetLoggerError)
79-
/// if the `LOGGER` is already initialized.
80-
pub fn initialize_logger() {
81-
let logger: SimpleLogger = SimpleLogger;
82-
if env::var("CPP_LINTER_COLOR")
83-
.as_deref()
84-
.is_ok_and(|v| matches!(v, "on" | "1" | "true"))
85-
{
86-
set_override(true);
87-
}
88-
if let Err(e) =
89-
log::set_boxed_logger(Box::new(logger)).map(|()| log::set_max_level(LevelFilter::Info))
90-
{
91-
// logger singleton already instantiated.
92-
// we'll just use whatever the current config is.
93-
log::debug!("{e:?}");
94-
}
95-
}
96-
}
979

9810
#[derive(clap::Parser, Debug)]
11+
#[command(styles(CLI_HELP_STYLE))]
9912
pub struct CliOptions {
10013
/// The desired version of clang to install.
10114
#[arg(
@@ -122,12 +35,10 @@ pub struct CliOptions {
12235

12336
/// The clang tool to install.
12437
#[arg(
125-
short,
126-
long,
127-
value_delimiter = ' ',
128-
default_value = "clang-format clang-tidy"
38+
num_args = 0..,
39+
default_values_t = vec![ClangTool::ClangFormat, ClangTool::ClangTidy],
12940
)]
130-
pub tool: Option<Vec<ClangTool>>,
41+
pub tool: Vec<ClangTool>,
13142

13243
/// The directory where the clang tools should be installed.
13344
#[arg(short, long)]
@@ -162,14 +73,11 @@ pub struct CliOptions {
16273

16374
#[tokio::main]
16475
async fn main() -> Result<()> {
165-
logging::initialize_logger();
76+
try_init_logger();
16677
let options = CliOptions::parse();
16778
if options.verbose {
16879
log::set_max_level(log::LevelFilter::Debug);
16980
}
170-
let tool = options
171-
.tool
172-
.expect("--tool should have a default value: [clang-format, clang-tidy]");
17381
match options.version.unwrap_or(RequestedVersion::default()) {
17482
RequestedVersion::NoValue => {
17583
log::info!(
@@ -179,7 +87,7 @@ async fn main() -> Result<()> {
17987
}
18088
req_ver => {
18189
let mut map_tools = HashMap::new();
182-
for t in tool {
90+
for t in options.tool {
18391
if let Some(version) = req_ver
18492
.eval_tool(
18593
&t,

0 commit comments

Comments
 (0)