From 90495b35d81c30db593f9345f124760b4d04335f Mon Sep 17 00:00:00 2001
From: Jiahao XU <Jiahao_XU@outlook.com>
Date: Sun, 13 Nov 2022 20:45:19 +1100
Subject: [PATCH] Enable coloring in console log (#528)

* Enable feat ansi of dep tracing-subscriber
* Rm use of `tracing_appender::non_blocking`
  since `cargo-binstall` is so simple that it doesn't need it.
* Use `tracing::{error, info}` in `MainExit::report`
* Use `tracing::{error, warn}` in `BinstallError::report`
* Add dep supports-color v1.3.1 to crates/bin
* Enable ansi of `tracing_subscriber::fmt` if stdout supports it

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
---
 Cargo.lock                    | 64 +++++++++++------------------------
 crates/bin/Cargo.toml         |  4 +--
 crates/bin/src/bin_util.rs    |  6 ++--
 crates/bin/src/logging.rs     | 45 ++++++++++++------------
 crates/bin/src/main.rs        |  2 +-
 crates/binstalk/src/errors.rs |  7 ++--
 6 files changed, 51 insertions(+), 77 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index ddeb5b20..eee57d7d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -283,10 +283,10 @@ dependencies = [
  "mimalloc",
  "once_cell",
  "semver",
+ "supports-color",
  "tempfile",
  "tokio",
  "tracing",
- "tracing-appender",
  "tracing-core",
  "tracing-log",
  "tracing-subscriber",
@@ -448,16 +448,6 @@ dependencies = [
  "cfg-if",
 ]
 
-[[package]]
-name = "crossbeam-channel"
-version = "0.5.6"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c2dd04ddaf88237dc3b8d8f9a3c1004b506b54b3313403944054d23c0870c521"
-dependencies = [
- "cfg-if",
- "crossbeam-utils",
-]
-
 [[package]]
 name = "crossbeam-utils"
 version = "0.8.11"
@@ -1249,6 +1239,16 @@ dependencies = [
 name = "normalize-path"
 version = "0.2.0"
 
+[[package]]
+name = "nu-ansi-term"
+version = "0.46.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "77a8165726e8236064dbb45459242600304b42a5ea24ee2948e18e023bf7ba84"
+dependencies = [
+ "overload",
+ "winapi",
+]
+
 [[package]]
 name = "num-integer"
 version = "0.1.45"
@@ -1278,15 +1278,6 @@ dependencies = [
  "libc",
 ]
 
-[[package]]
-name = "num_threads"
-version = "0.1.6"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2819ce041d2ee131036f4fc9d6ae7ae125a3a40e97ba64d04fe799ad9dabbb44"
-dependencies = [
- "libc",
-]
-
 [[package]]
 name = "object"
 version = "0.29.0"
@@ -1353,6 +1344,12 @@ version = "6.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9ff7415e9ae3fff1225851df9e0d9e4e5479f947619774677a63572e55e80eff"
 
+[[package]]
+name = "overload"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39"
+
 [[package]]
 name = "owo-colors"
 version = "3.5.0"
@@ -1898,9 +1895,9 @@ dependencies = [
 
 [[package]]
 name = "supports-color"
-version = "1.3.0"
+version = "1.3.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4872ced36b91d47bae8a214a683fe54e7078875b399dfa251df346c9b547d1f9"
+checksum = "8ba6faf2ca7ee42fdd458f4347ae0a9bd6bcc445ad7cb57ad82b383f18870d6f"
 dependencies = [
  "atty",
  "is_ci",
@@ -2008,17 +2005,6 @@ dependencies = [
  "once_cell",
 ]
 
-[[package]]
-name = "time"
-version = "0.3.14"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3c3f9a28b618c3a6b9251b6908e9c99e04b9e5c02e6581ccbb67d59c34ef7f9b"
-dependencies = [
- "itoa",
- "libc",
- "num_threads",
-]
-
 [[package]]
 name = "tinytemplate"
 version = "1.2.1"
@@ -2183,17 +2169,6 @@ dependencies = [
  "tracing-core",
 ]
 
-[[package]]
-name = "tracing-appender"
-version = "0.2.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "09d48f71a791638519505cefafe162606f706c25592e4bde4d97600c0195312e"
-dependencies = [
- "crossbeam-channel",
- "time",
- "tracing-subscriber",
-]
-
 [[package]]
 name = "tracing-attributes"
 version = "0.1.23"
@@ -2242,6 +2217,7 @@ version = "0.3.16"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a6176eae26dd70d0c919749377897b54a9276bd7061339665dd68777926b5a70"
 dependencies = [
+ "nu-ansi-term",
  "serde",
  "serde_json",
  "sharded-slab",
diff --git a/crates/bin/Cargo.toml b/crates/bin/Cargo.toml
index 4567dfc5..912ea0a1 100644
--- a/crates/bin/Cargo.toml
+++ b/crates/bin/Cargo.toml
@@ -31,13 +31,13 @@ miette = "5.4.1"
 mimalloc = { version = "0.1.31", default-features = false, optional = true }
 once_cell = "1.16.0"
 semver = "1.0.14"
+supports-color = "1.3.1"
 tempfile = "3.3.0"
 tokio = { version = "1.21.2", features = ["rt-multi-thread"], default-features = false }
 tracing-core = "0.1.30"
 tracing = { version = "0.1.37", default-features = false }
 tracing-log = { version = "0.1.3", default-features = false }
-tracing-appender = "0.2.2"
-tracing-subscriber = { version = "0.3.16", features = ["fmt", "json"], default-features = false }
+tracing-subscriber = { version = "0.3.16", features = ["fmt", "json", "ansi"], default-features = false }
 
 [build-dependencies]
 embed-resource = "1.7.4"
diff --git a/crates/bin/src/bin_util.rs b/crates/bin/src/bin_util.rs
index e1732dc9..d0d03532 100644
--- a/crates/bin/src/bin_util.rs
+++ b/crates/bin/src/bin_util.rs
@@ -1,6 +1,5 @@
 use std::{
     future::Future,
-    io::{self, Write},
     process::{ExitCode, Termination},
     time::Duration,
 };
@@ -9,6 +8,7 @@ use binstalk::errors::BinstallError;
 use binstalk::helpers::{signal::cancel_on_user_sig_term, tasks::AutoAbortJoinHandle};
 use miette::Result;
 use tokio::runtime::Runtime;
+use tracing::{error, info};
 
 pub enum MainExit {
     Success(Option<Duration>),
@@ -21,13 +21,13 @@ impl Termination for MainExit {
         match self {
             Self::Success(spent) => {
                 if let Some(spent) = spent {
-                    writeln!(io::stdout(), "Done in {spent:?}").ok();
+                    info!("Done in {spent:?}");
                 }
                 ExitCode::SUCCESS
             }
             Self::Error(err) => err.report(),
             Self::Report(err) => {
-                writeln!(io::stderr(), "Fatal error:\n{err:?}").ok();
+                error!("Fatal error:\n{err:?}");
                 ExitCode::from(16)
             }
         }
diff --git a/crates/bin/src/logging.rs b/crates/bin/src/logging.rs
index 6787d0d6..29869361 100644
--- a/crates/bin/src/logging.rs
+++ b/crates/bin/src/logging.rs
@@ -1,14 +1,14 @@
-use std::{cmp::min, io, iter::repeat};
+use std::{cmp::min, iter::repeat};
 
 use log::{LevelFilter, Log, STATIC_MAX_LEVEL};
 use once_cell::sync::Lazy;
+use supports_color::{on as supports_color_on_stream, Stream::Stdout};
 use tracing::{
     callsite::Callsite,
     dispatcher, field,
     subscriber::{self, set_global_default},
     Event, Level, Metadata,
 };
-use tracing_appender::non_blocking::{NonBlockingBuilder, WorkerGuard};
 use tracing_core::{identify_callsite, metadata::Kind, subscriber::Subscriber};
 use tracing_log::AsTrace;
 use tracing_subscriber::{filter::targets::Targets, fmt::fmt, layer::SubscriberExt};
@@ -131,7 +131,7 @@ impl Log for Logger {
     fn flush(&self) {}
 }
 
-pub fn logging(args: &Args) -> WorkerGuard {
+pub fn logging(args: &Args) {
     // Calculate log_level
     let log_level = min(args.log_level, STATIC_MAX_LEVEL);
 
@@ -141,31 +141,30 @@ pub fn logging(args: &Args) -> WorkerGuard {
     // Forward log to tracing
     Logger::init(log_level);
 
-    // Setup non-blocking stdout
-    let (non_blocking, guard) = NonBlockingBuilder::default()
-        .lossy(false)
-        .finish(io::stdout());
-
     // Build fmt subscriber
     let log_level = log_level.as_trace();
-    let subscriber_builder = fmt().with_writer(non_blocking).with_max_level(log_level);
+    let subscriber_builder = fmt().with_max_level(log_level);
 
     let subscriber: Box<dyn Subscriber + Send + Sync> = if args.json_output {
         Box::new(subscriber_builder.json().finish())
     } else {
-        Box::new(
-            subscriber_builder
-                .compact()
-                // Disable time, target, file, line_num, thread name/ids to make the
-                // output more readable
-                .without_time()
-                .with_target(false)
-                .with_file(false)
-                .with_line_number(false)
-                .with_thread_names(false)
-                .with_thread_ids(false)
-                .finish(),
-        )
+        // Disable time, target, file, line_num, thread name/ids to make the
+        // output more readable
+        let subscriber_builder = subscriber_builder
+            .without_time()
+            .with_target(false)
+            .with_file(false)
+            .with_line_number(false)
+            .with_thread_names(false)
+            .with_thread_ids(false);
+
+        // subscriber_builder defaults to write to io::stdout(),
+        // so tests whether it supports color.
+        let stdout_supports_color = supports_color_on_stream(Stdout)
+            .map(|color_level| color_level.has_basic)
+            .unwrap_or_default();
+
+        Box::new(subscriber_builder.with_ansi(stdout_supports_color).finish())
     };
 
     // Builder layer for filtering
@@ -178,6 +177,4 @@ pub fn logging(args: &Args) -> WorkerGuard {
 
     // Setup global subscriber
     set_global_default(subscriber).unwrap();
-
-    guard
 }
diff --git a/crates/bin/src/main.rs b/crates/bin/src/main.rs
index 9e57b764..b839f65a 100644
--- a/crates/bin/src/main.rs
+++ b/crates/bin/src/main.rs
@@ -27,7 +27,7 @@ fn main() -> MainExit {
         println!("{}", env!("CARGO_PKG_VERSION"));
         MainExit::Success(None)
     } else {
-        let _guard = logging(&args);
+        logging(&args);
 
         let start = Instant::now();
 
diff --git a/crates/binstalk/src/errors.rs b/crates/binstalk/src/errors.rs
index 5dae3396..266b6b43 100644
--- a/crates/binstalk/src/errors.rs
+++ b/crates/binstalk/src/errors.rs
@@ -1,5 +1,5 @@
 use std::{
-    io::{self, Write},
+    io,
     path::PathBuf,
     process::{ExitCode, ExitStatus, Termination},
 };
@@ -12,6 +12,7 @@ use compact_str::CompactString;
 use miette::{Diagnostic, Report};
 use thiserror::Error;
 use tokio::task;
+use tracing::{error, warn};
 
 /// Error kinds emitted by cargo-binstall.
 #[derive(Error, Diagnostic, Debug)]
@@ -399,9 +400,9 @@ impl Termination for BinstallError {
     fn report(self) -> ExitCode {
         let code = self.exit_code();
         if let BinstallError::UserAbort = self {
-            writeln!(io::stdout(), "Installation cancelled").ok();
+            warn!("Installation cancelled");
         } else {
-            writeln!(io::stderr(), "Fatal error:\n{:?}", Report::new(self)).ok();
+            error!("Fatal error:\n{:?}", Report::new(self));
         }
 
         code