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.

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

feat: add optional colored log output #52

Merged
merged 3 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ license agreements:
Dual-licensed under [Apache 2.0][Apache2] or [MIT].
- [chrono](https://crates.io/crates/chrono):
Dual-licensed under [Apache 2.0][Apache2] or [MIT].
- [colored](https://crates.io/crates/colored): Licensed under [MPL-2.0]

The python binding uses

Expand All @@ -223,3 +224,4 @@ The node binding uses

[MIT]: https://choosealicense.com/licenses/mit
[Apache2]: https://choosealicense.com/licenses/apache-2.0/
[MPL-2.0]: https://choosealicense.com/licenses/mpl-2.0
3 changes: 2 additions & 1 deletion cpp-linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ license.workspace = true
anyhow = "1.0.89"
chrono = "0.4.38"
clap = "4.5.17"
colored = "2.1.0"
fast-glob = "0.4.0"
futures = "0.3.30"
git2 = "0.19.0"
lenient_semver = "0.4.2"
log = "0.4.22"
log = { version = "0.4.22", features = ["std"] }
openssl = { version = "0.10", features = ["vendored"], optional = true }
openssl-probe = { version = "0.1", optional = true }
regex = "1.10.6"
Expand Down
8 changes: 4 additions & 4 deletions cpp-linter/src/clang_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ use which::{which, which_in};
use super::common_fs::FileObj;
use crate::{
cli::ClangParams,
logger::{end_log_group, start_log_group},
rest_api::{COMMENT_MARKER, USER_OUTREACH},
rest_api::{RestApiClient, COMMENT_MARKER, USER_OUTREACH},
};
pub mod clang_format;
use clang_format::run_clang_format;
Expand Down Expand Up @@ -144,6 +143,7 @@ pub async fn capture_clang_tools_output(
files: &mut Vec<Arc<Mutex<FileObj>>>,
version: &str,
clang_params: &mut ClangParams,
rest_api_client: &impl RestApiClient,
) -> Result<()> {
// find the executable paths for clang-tidy and/or clang-format and show version
// info as debugging output.
Expand Down Expand Up @@ -192,11 +192,11 @@ pub async fn capture_clang_tools_output(
while let Some(output) = executors.join_next().await {
if let Ok(out) = output? {
let (file_name, logs) = out;
start_log_group(format!("Analyzing {}", file_name.to_string_lossy()));
rest_api_client.start_log_group(format!("Analyzing {}", file_name.to_string_lossy()));
for (level, msg) in logs {
log::log!(level, "{}", msg);
}
end_log_group();
rest_api_client.end_log_group();
}
}
Ok(())
Expand Down
81 changes: 50 additions & 31 deletions cpp-linter/src/logger.rs
Original file line number Diff line number Diff line change
@@ -1,58 +1,77 @@
//! A module to initialize and customize the logger object used in (most) stdout.

// non-std crates
use log::{Level, LevelFilter, Metadata, Record, SetLoggerError};
use std::env;

use anyhow::{Error, Result};
use colored::{control::set_override, Colorize};
use log::{Level, LevelFilter, Metadata, Record};

#[derive(Default)]
struct SimpleLogger;

impl SimpleLogger {
fn level_color(level: &Level) -> String {
let name = format!("{:>5}", level.as_str().to_uppercase());
match level {
Level::Error => name.red().bold().to_string(),
Level::Warn => name.yellow().bold().to_string(),
Level::Info => name.green().bold().to_string(),
Level::Debug => name.blue().bold().to_string(),
Level::Trace => name.magenta().bold().to_string(),
}
}
}

impl log::Log for SimpleLogger {
fn enabled(&self, metadata: &Metadata) -> bool {
metadata.level() <= Level::Debug
metadata.level() <= log::max_level()
}

fn log(&self, record: &Record) {
if self.enabled(record.metadata()) {
println!("{}: {}", record.level(), record.args());
if record.target() == "CI_LOG_GROUPING" {
// this log is meant to manipulate a CI workflow's log grouping
println!("{}", record.args());
} else if self.enabled(record.metadata()) {
println!(
"[{}]: {}",
Self::level_color(&record.level()),
record.args()
);
}
}

fn flush(&self) {}
}

/// A private constant to manage the application's logger object.
static LOGGER: SimpleLogger = SimpleLogger;

/// A function to initialize the private `LOGGER`.
///
/// The logging level defaults to [`LevelFilter::Info`].
/// Returns a [`SetLoggerError`] if the `LOGGER` is already initialized.
pub fn init() -> Result<(), SetLoggerError> {
log::set_logger(&LOGGER).map(|()| log::set_max_level(LevelFilter::Info))
}

/// This prints a line to indicate the beginning of a related group of log statements.
///
/// This function may or may not get moved to [crate::rest_api::RestApiClient] trait
/// if/when platforms other than GitHub are supported.
pub fn start_log_group(name: String) {
println!("::group::{}", name);
}

/// This prints a line to indicate the ending of a related group of log statements.
///
/// This function may or may not get moved to [crate::rest_api::RestApiClient] trait
/// if/when platforms other than GitHub are supported.
pub fn end_log_group() {
println!("::endgroup::");
pub fn init() -> Result<()> {
let logger: SimpleLogger = SimpleLogger;
if matches!(
env::var("CPP_LINTER_COLOR").as_deref(),
Ok("on" | "1" | "true")
) {
set_override(true);
}
log::set_boxed_logger(Box::new(logger))
.map(|()| log::set_max_level(LevelFilter::Info))
.map_err(Error::from)
}

#[cfg(test)]
mod tests {
use super::{end_log_group, start_log_group};
mod test {
use std::env;

use super::{init, SimpleLogger};

#[test]
fn issue_log_grouping_stdout() {
start_log_group(String::from("a dumb test"));
end_log_group();
fn trace_log() {
env::set_var("CPP_LINTER_COLOR", "true");
init().unwrap_or(());
assert!(SimpleLogger::level_color(&log::Level::Trace).contains("TRACE"));
log::set_max_level(log::LevelFilter::Trace);
log::trace!("A dummy log statement for code coverage");
}
}
10 changes: 10 additions & 0 deletions cpp-linter/src/rest_api/github/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ impl RestApiClient for GithubApiClient {
checks_failed
}

/// This prints a line to indicate the beginning of a related group of log statements.
fn start_log_group(&self, name: String) {
log::info!(target: "CI_LOG_GROUPING", "::group::{}", name);
}

/// This prints a line to indicate the ending of a related group of log statements.
fn end_log_group(&self) {
log::info!(target: "CI_LOG_GROUPING", "::endgroup::");
}

fn make_headers() -> Result<HeaderMap<HeaderValue>> {
let mut headers = HeaderMap::new();
headers.insert(
Expand Down
18 changes: 17 additions & 1 deletion cpp-linter/src/rest_api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ pub trait RestApiClient {
tidy_checks_failed: Option<u64>,
) -> u64;

/// This prints a line to indicate the beginning of a related group of log statements.
fn start_log_group(&self, name: String);

/// This prints a line to indicate the ending of a related group of log statements.
fn end_log_group(&self);

/// A convenience method to create the headers attached to all REST API calls.
///
/// If an authentication token is provided (via environment variable),
Expand Down Expand Up @@ -453,12 +459,21 @@ mod test {
) -> Result<u64> {
Err(anyhow!("Not implemented"))
}

fn start_log_group(&self, name: String) {
log::info!(target: "CI_LOG_GROUPING", "start_log_group: {name}");
}

fn end_log_group(&self) {
log::info!(target: "CI_LOG_GROUPING", "end_log_group");
}
}

#[tokio::test]
async fn dummy_coverage() {
assert!(TestClient::make_headers().is_err());
let dummy = TestClient::default();
dummy.start_log_group("Dummy test".to_string());
assert_eq!(dummy.set_exit_code(1, None, None), 0);
assert!(dummy
.get_list_of_changed_files(&FileFilter::new(&[], vec![]))
Expand All @@ -474,7 +489,8 @@ mod test {
assert!(dummy
.post_feedback(&[], FeedbackInput::default())
.await
.is_err())
.is_err());
dummy.end_log_group();
}

// ************************************************* try_next_page() tests
Expand Down
18 changes: 12 additions & 6 deletions cpp-linter/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use openssl_probe;
use crate::clang_tools::capture_clang_tools_output;
use crate::cli::{get_arg_parser, ClangParams, Cli, FeedbackInput, LinesChangedOnly};
use crate::common_fs::FileFilter;
use crate::logger::{self, end_log_group, start_log_group};
use crate::logger;
use crate::rest_api::{github::GithubApiClient, RestApiClient};

const VERSION: &str = env!("CARGO_PKG_VERSION");
Expand Down Expand Up @@ -93,7 +93,7 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
}
}

start_log_group(String::from("Get list of specified source files"));
rest_api_client.start_log_group(String::from("Get list of specified source files"));
let files = if cli.lines_changed_only != LinesChangedOnly::Off || cli.files_changed_only {
// parse_diff(github_rest_api_payload)
rest_api_client
Expand Down Expand Up @@ -124,16 +124,22 @@ pub async fn run_main(args: Vec<String>) -> Result<()> {
log::info!(" ./{}", file.name.to_string_lossy().replace('\\', "/"));
arc_files.push(Arc::new(Mutex::new(file)));
}
end_log_group();
rest_api_client.end_log_group();

let mut clang_params = ClangParams::from(&cli);
let user_inputs = FeedbackInput::from(&cli);
capture_clang_tools_output(&mut arc_files, cli.version.as_str(), &mut clang_params).await?;
start_log_group(String::from("Posting feedback"));
capture_clang_tools_output(
&mut arc_files,
cli.version.as_str(),
&mut clang_params,
&rest_api_client,
)
.await?;
rest_api_client.start_log_group(String::from("Posting feedback"));
let checks_failed = rest_api_client
.post_feedback(&arc_files, user_inputs)
.await?;
end_log_group();
rest_api_client.end_log_group();
if env::var("PRE_COMMIT").is_ok_and(|v| v == "1") {
if checks_failed > 1 {
return Err(anyhow!("Some checks did not pass"));
Expand Down
Loading