refactor(shell): Centralize Shell styling

This is a very rough pass at naming the styles with a focus on getting something in and keeping the colors the same.
epage · Sep 11, 2023 · da0d800 · da0d800
1 parent a41acc3
commit da0d800
Show file tree

Hide file tree

Showing 9 changed files with 64 additions and 58 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -17,6 +17,7 @@ license = "MIT OR Apache-2.0"
 
 [workspace.dependencies]
 anstyle = "1.0.3"
+anstyle-termcolor = "1.0.0"
 anyhow = "1.0.75"
 base64 = "0.21.3"
 bytesize = "1.3"
@@ -123,6 +124,7 @@ path = "src/cargo/lib.rs"
 
 [dependencies]
 anstyle.workspace = true
+anstyle-termcolor.workspace = true
 anyhow.workspace = true
 base64.workspace = true
 bytesize.workspace = true

diff --git a/src/cargo/core/compiler/timings.rs b/src/cargo/core/compiler/timings.rs
@@ -8,6 +8,7 @@ use crate::core::compiler::{BuildContext, Context, TimingOutput};
 use crate::core::PackageId;
 use crate::util::cpu::State;
 use crate::util::machine_message::{self, Message};
+use crate::util::style;
 use crate::util::{CargoResult, Config};
 use anyhow::Context as _;
 use cargo_util::paths;
@@ -352,7 +353,7 @@ impl<'cfg> Timings<'cfg> {
         paths::link_or_copy(&filename, &unstamped_filename)?;
         self.config
             .shell()
-            .status_with_color("Timing", msg, termcolor::Color::Cyan)?;
+            .status_with_color("Timing", msg, &style::NOTE)?;
         Ok(())
     }
 

diff --git a/src/cargo/core/shell.rs b/src/cargo/core/shell.rs
@@ -2,10 +2,12 @@ use std::fmt;
 use std::io::prelude::*;
 use std::io::IsTerminal;
 
-use termcolor::Color::{Cyan, Green, Red, Yellow};
-use termcolor::{self, Color, ColorSpec, StandardStream, WriteColor};
+use anstyle::Style;
+use anstyle_termcolor::to_termcolor_spec;
+use termcolor::{self, StandardStream, WriteColor};
 
 use crate::util::errors::CargoResult;
+use crate::util::style::*;
 
 pub enum TtyWidth {
     NoTty,
@@ -130,7 +132,7 @@ impl Shell {
         &mut self,
         status: &dyn fmt::Display,
         message: Option<&dyn fmt::Display>,
-        color: Color,
+        color: &Style,
         justified: bool,
     ) -> CargoResult<()> {
         match self.verbosity {
@@ -203,22 +205,22 @@ impl Shell {
         T: fmt::Display,
         U: fmt::Display,
     {
-        self.print(&status, Some(&message), Green, true)
+        self.print(&status, Some(&message), &HEADER, true)
     }
 
     pub fn status_header<T>(&mut self, status: T) -> CargoResult<()>
     where
         T: fmt::Display,
     {
-        self.print(&status, None, Cyan, true)
+        self.print(&status, None, &NOTE, true)
     }
 
     /// Shortcut to right-align a status message.
     pub fn status_with_color<T, U>(
         &mut self,
         status: T,
         message: U,
-        color: Color,
+        color: &Style,
     ) -> CargoResult<()>
     where
         T: fmt::Display,
@@ -255,20 +257,20 @@ impl Shell {
             self.err_erase_line();
         }
         self.output
-            .message_stderr(&"error", Some(&message), Red, false)
+            .message_stderr(&"error", Some(&message), &ERROR, false)
     }
 
     /// Prints an amber 'warning' message.
     pub fn warn<T: fmt::Display>(&mut self, message: T) -> CargoResult<()> {
         match self.verbosity {
             Verbosity::Quiet => Ok(()),
-            _ => self.print(&"warning", Some(&message), Yellow, false),
+            _ => self.print(&"warning", Some(&message), &WARN, false),
         }
     }
 
     /// Prints a cyan 'note' message.
     pub fn note<T: fmt::Display>(&mut self, message: T) -> CargoResult<()> {
-        self.print(&"note", Some(&message), Cyan, false)
+        self.print(&"note", Some(&message), &NOTE, false)
     }
 
     /// Updates the verbosity of the shell.
@@ -338,22 +340,14 @@ impl Shell {
     /// Write a styled fragment
     ///
     /// Caller is responsible for deciding whether [`Shell::verbosity`] is affects output.
-    pub fn write_stdout(
-        &mut self,
-        fragment: impl fmt::Display,
-        color: &ColorSpec,
-    ) -> CargoResult<()> {
+    pub fn write_stdout(&mut self, fragment: impl fmt::Display, color: &Style) -> CargoResult<()> {
         self.output.write_stdout(fragment, color)
     }
 
     /// Write a styled fragment
     ///
     /// Caller is responsible for deciding whether [`Shell::verbosity`] is affects output.
-    pub fn write_stderr(
-        &mut self,
-        fragment: impl fmt::Display,
-        color: &ColorSpec,
-    ) -> CargoResult<()> {
+    pub fn write_stderr(&mut self, fragment: impl fmt::Display, color: &Style) -> CargoResult<()> {
         self.output.write_stderr(fragment, color)
     }
 
@@ -412,18 +406,18 @@ impl ShellOut {
         &mut self,
         status: &dyn fmt::Display,
         message: Option<&dyn fmt::Display>,
-        color: Color,
+        style: &Style,
         justified: bool,
     ) -> CargoResult<()> {
         match *self {
             ShellOut::Stream { ref mut stderr, .. } => {
                 stderr.reset()?;
-                stderr.set_color(ColorSpec::new().set_bold(true).set_fg(Some(color)))?;
+                stderr.set_color(&to_termcolor_spec(*style))?;
                 if justified {
                     write!(stderr, "{:>12}", status)?;
                 } else {
                     write!(stderr, "{}", status)?;
-                    stderr.set_color(ColorSpec::new().set_bold(true))?;
+                    stderr.set_color(termcolor::ColorSpec::new().set_bold(true))?;
                     write!(stderr, ":")?;
                 }
                 stderr.reset()?;
@@ -448,11 +442,11 @@ impl ShellOut {
     }
 
     /// Write a styled fragment
-    fn write_stdout(&mut self, fragment: impl fmt::Display, color: &ColorSpec) -> CargoResult<()> {
+    fn write_stdout(&mut self, fragment: impl fmt::Display, color: &Style) -> CargoResult<()> {
         match *self {
             ShellOut::Stream { ref mut stdout, .. } => {
                 stdout.reset()?;
-                stdout.set_color(&color)?;
+                stdout.set_color(&to_termcolor_spec(*color))?;
                 write!(stdout, "{}", fragment)?;
                 stdout.reset()?;
             }
@@ -464,11 +458,11 @@ impl ShellOut {
     }
 
     /// Write a styled fragment
-    fn write_stderr(&mut self, fragment: impl fmt::Display, color: &ColorSpec) -> CargoResult<()> {
+    fn write_stderr(&mut self, fragment: impl fmt::Display, color: &Style) -> CargoResult<()> {
         match *self {
             ShellOut::Stream { ref mut stderr, .. } => {
                 stderr.reset()?;
-                stderr.set_color(&color)?;
+                stderr.set_color(&to_termcolor_spec(*color))?;
                 write!(stderr, "{}", fragment)?;
                 stderr.reset()?;
             }

diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs
@@ -12,9 +12,6 @@ use anyhow::Context as _;
 use cargo_util::paths;
 use indexmap::IndexSet;
 use itertools::Itertools;
-use termcolor::Color::Green;
-use termcolor::Color::Red;
-use termcolor::ColorSpec;
 use toml_edit::Item as TomlItem;
 
 use crate::core::dependency::DepKind;
@@ -26,6 +23,7 @@ use crate::core::Shell;
 use crate::core::Summary;
 use crate::core::Workspace;
 use crate::sources::source::QueryKind;
+use crate::util::style;
 use crate::util::toml_mut::dependency::Dependency;
 use crate::util::toml_mut::dependency::GitSource;
 use crate::util::toml_mut::dependency::MaybeWorkspace;
@@ -968,19 +966,16 @@ fn print_dep_table_msg(shell: &mut Shell, dep: &DependencyUI) -> CargoResult<()>
         } else {
             "".to_owned()
         };
-        shell.write_stderr(
-            format_args!("{}Features{}:\n", prefix, suffix),
-            &ColorSpec::new(),
-        )?;
+        shell.write_stderr(format_args!("{}Features{}:\n", prefix, suffix), &style::NOP)?;
         for feat in activated {
-            shell.write_stderr(&prefix, &ColorSpec::new())?;
-            shell.write_stderr('+', &ColorSpec::new().set_bold(true).set_fg(Some(Green)))?;
-            shell.write_stderr(format_args!(" {}\n", feat), &ColorSpec::new())?;
+            shell.write_stderr(&prefix, &style::NOP)?;
+            shell.write_stderr('+', &style::GOOD)?;
+            shell.write_stderr(format_args!(" {}\n", feat), &style::NOP)?;
         }
         for feat in deactivated {
-            shell.write_stderr(&prefix, &ColorSpec::new())?;
-            shell.write_stderr('-', &ColorSpec::new().set_bold(true).set_fg(Some(Red)))?;
-            shell.write_stderr(format_args!(" {}\n", feat), &ColorSpec::new())?;
+            shell.write_stderr(&prefix, &style::NOP)?;
+            shell.write_stderr('-', &style::ERROR)?;
+            shell.write_stderr(format_args!(" {}\n", feat), &style::NOP)?;
         }
     }
 

diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs
@@ -4,9 +4,10 @@ use crate::core::{PackageId, PackageIdSpec};
 use crate::core::{Resolve, SourceId, Workspace};
 use crate::ops;
 use crate::util::config::Config;
+use crate::util::style;
 use crate::util::CargoResult;
+use anstyle::Style;
 use std::collections::{BTreeMap, HashSet};
-use termcolor::Color::{self, Cyan, Green, Red, Yellow};
 use tracing::debug;
 
 pub struct UpdateOptions<'a> {
@@ -133,7 +134,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
     )?;
 
     // Summarize what is changing for the user.
-    let print_change = |status: &str, msg: String, color: Color| {
+    let print_change = |status: &str, msg: String, color: &Style| {
         opts.config.shell().status_with_color(status, msg, color)
     };
     for (removed, added) in compare_dependency_graphs(&previous_resolve, &resolve) {
@@ -149,16 +150,16 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
             };
 
             if removed[0].version() > added[0].version() {
-                print_change("Downgrading", msg, Yellow)?;
+                print_change("Downgrading", msg, &style::WARN)?;
             } else {
-                print_change("Updating", msg, Green)?;
+                print_change("Updating", msg, &style::GOOD)?;
             }
         } else {
             for package in removed.iter() {
-                print_change("Removing", format!("{}", package), Red)?;
+                print_change("Removing", format!("{}", package), &style::ERROR)?;
             }
             for package in added.iter() {
-                print_change("Adding", format!("{}", package), Cyan)?;
+                print_change("Adding", format!("{}", package), &style::NOTE)?;
             }
         }
     }

diff --git a/src/cargo/ops/registry/search.rs b/src/cargo/ops/registry/search.rs
@@ -6,10 +6,9 @@ use std::cmp;
 use std::iter::repeat;
 
 use anyhow::Context as _;
-use termcolor::Color;
-use termcolor::ColorSpec;
 use url::Url;
 
+use crate::util::style;
 use crate::util::truncate_with_ellipsis;
 use crate::CargoResult;
 use crate::Config;
@@ -58,15 +57,12 @@ pub fn search(
         };
         let mut fragments = line.split(query).peekable();
         while let Some(fragment) = fragments.next() {
-            let _ = config.shell().write_stdout(fragment, &ColorSpec::new());
+            let _ = config.shell().write_stdout(fragment, &style::NOP);
             if fragments.peek().is_some() {
-                let _ = config.shell().write_stdout(
-                    query,
-                    &ColorSpec::new().set_bold(true).set_fg(Some(Color::Green)),
-                );
+                let _ = config.shell().write_stdout(query, &style::GOOD);
             }
         }
-        let _ = config.shell().write_stdout("\n", &ColorSpec::new());
+        let _ = config.shell().write_stdout("\n", &style::NOP);
     }
 
     let search_max_limit = 100;
@@ -76,7 +72,7 @@ pub fn search(
                 "... and {} crates more (use --limit N to see more)\n",
                 total_crates - limit
             ),
-            &ColorSpec::new(),
+            &style::NOP,
         );
     } else if total_crates > limit && limit >= search_max_limit {
         let extra = if source_ids.original.is_crates_io() {
@@ -87,7 +83,7 @@ pub fn search(
         };
         let _ = config.shell().write_stdout(
             format_args!("... and {} crates more{}\n", total_crates - limit, extra),
-            &ColorSpec::new(),
+            &style::NOP,
         );
     }
 

diff --git a/src/cargo/util/flock.rs b/src/cargo/util/flock.rs
@@ -4,11 +4,11 @@ use std::io::{Read, Seek, SeekFrom, Write};
 use std::path::{Display, Path, PathBuf};
 
 use crate::util::errors::CargoResult;
+use crate::util::style;
 use crate::util::Config;
 use anyhow::Context as _;
 use cargo_util::paths;
 use sys::*;
-use termcolor::Color::Cyan;
 
 #[derive(Debug)]
 pub struct FileLock {
@@ -312,7 +312,9 @@ fn acquire(
         }
     }
     let msg = format!("waiting for file lock on {}", msg);
-    config.shell().status_with_color("Blocking", &msg, Cyan)?;
+    config
+        .shell()
+        .status_with_color("Blocking", &msg, &style::NOTE)?;
 
     lock_block().with_context(|| format!("failed to lock file: {}", path.display()))?;
     return Ok(());

diff --git a/src/cargo/util/style.rs b/src/cargo/util/style.rs
@@ -1,9 +1,13 @@
 use anstyle::*;
 
+pub const NOP: Style = Style::new();
 pub const HEADER: Style = AnsiColor::Green.on_default().effects(Effects::BOLD);
 pub const USAGE: Style = AnsiColor::Green.on_default().effects(Effects::BOLD);
 pub const LITERAL: Style = AnsiColor::Cyan.on_default().effects(Effects::BOLD);
 pub const PLACEHOLDER: Style = AnsiColor::Cyan.on_default();
 pub const ERROR: Style = AnsiColor::Red.on_default().effects(Effects::BOLD);
+pub const WARN: Style = AnsiColor::Yellow.on_default().effects(Effects::BOLD);
+pub const NOTE: Style = AnsiColor::Cyan.on_default().effects(Effects::BOLD);
+pub const GOOD: Style = AnsiColor::Green.on_default().effects(Effects::BOLD);
 pub const VALID: Style = AnsiColor::Cyan.on_default().effects(Effects::BOLD);
 pub const INVALID: Style = AnsiColor::Yellow.on_default().effects(Effects::BOLD);