feat: add optional colored log output (#52)

Author: 2bndy5

Cargo.lock

@@ -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
 
@@ -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

README.md

@@ -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"

cpp-linter/Cargo.toml

@@ -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;
@@ -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.
@@ -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(())

cpp-linter/src/clang_tools/mod.rs

@@ -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");
     }
 }

cpp-linter/src/logger.rs

@@ -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(

cpp-linter/src/rest_api/github/mod.rs

@@ -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),
@@ -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![]))
@@ -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

cpp-linter/src/rest_api/mod.rs

@@ -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");
@@ -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
@@ -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"));