From 2d5354adc9f4a948e7c41daf788d966714204a1d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 11 Sep 2023 09:32:13 -0500 Subject: [PATCH 1/2] refactor(cli): Extract CLI style definitions --- Cargo.lock | 5 +++-- Cargo.toml | 2 ++ src/bin/cargo/cli.rs | 18 +++++++++--------- src/cargo/util/mod.rs | 1 + src/cargo/util/style.rs | 9 +++++++++ 5 files changed, 24 insertions(+), 11 deletions(-) create mode 100644 src/cargo/util/style.rs diff --git a/Cargo.lock b/Cargo.lock index 4f0757617a8..b7f3cc8f600 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -60,9 +60,9 @@ dependencies = [ [[package]] name = "anstyle" -version = "1.0.0" +version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41ed9a86bf92ae6580e0a31281f65a1b1d867c0cc68d5346e2ae128dddfa6a7d" +checksum = "b84bf0a05bbb2a83e5eb6fa36bb6e87baa08193c35ff52bbf6b38d8af2890e46" [[package]] name = "anstyle-parse" @@ -258,6 +258,7 @@ dependencies = [ name = "cargo" version = "0.75.0" dependencies = [ + "anstyle", "anyhow", "base64", "bytesize", diff --git a/Cargo.toml b/Cargo.toml index d80e755dd4b..8acba4c7d6f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ edition = "2021" license = "MIT OR Apache-2.0" [workspace.dependencies] +anstyle = "1.0.3" anyhow = "1.0.75" base64 = "0.21.3" bytesize = "1.3" @@ -121,6 +122,7 @@ name = "cargo" path = "src/cargo/lib.rs" [dependencies] +anstyle.workspace = true anyhow.workspace = true base64.workspace = true bytesize.workspace = true diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs index dc0a34bfa50..ade9602c56e 100644 --- a/src/bin/cargo/cli.rs +++ b/src/bin/cargo/cli.rs @@ -14,6 +14,7 @@ use super::list_commands; use crate::command_prelude::*; use crate::util::is_rustup; use cargo::core::features::HIDDEN; +use cargo::util::style; pub fn main(config: &mut LazyConfig) -> CliResult { let args = cli().try_get_matches()?; @@ -519,15 +520,14 @@ pub fn cli() -> Command { }; let styles = { - use clap::builder::styling::*; - Styles::styled() - .header(AnsiColor::Green.on_default() | Effects::BOLD) - .usage(AnsiColor::Green.on_default() | Effects::BOLD) - .literal(AnsiColor::Cyan.on_default() | Effects::BOLD) - .placeholder(AnsiColor::Cyan.on_default()) - .error(AnsiColor::Red.on_default() | Effects::BOLD) - .valid(AnsiColor::Cyan.on_default() | Effects::BOLD) - .invalid(AnsiColor::Yellow.on_default() | Effects::BOLD) + clap::builder::styling::Styles::styled() + .header(style::HEADER) + .usage(style::USAGE) + .literal(style::LITERAL) + .placeholder(style::PLACEHOLDER) + .error(style::ERROR) + .valid(style::VALID) + .invalid(style::INVALID) }; Command::new("cargo") diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 2c3e5a80274..d74351cd016 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -60,6 +60,7 @@ mod queue; pub mod restricted_names; pub mod rustc; mod semver_ext; +pub mod style; pub mod to_semver; pub mod toml; pub mod toml_mut; diff --git a/src/cargo/util/style.rs b/src/cargo/util/style.rs new file mode 100644 index 00000000000..15ce7b3257c --- /dev/null +++ b/src/cargo/util/style.rs @@ -0,0 +1,9 @@ +use anstyle::*; + +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 VALID: Style = AnsiColor::Cyan.on_default().effects(Effects::BOLD); +pub const INVALID: Style = AnsiColor::Yellow.on_default().effects(Effects::BOLD); From 51e10583793034e84be99f0558e38f3f942fc3c3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 11 Sep 2023 09:51:09 -0500 Subject: [PATCH 2/2] 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. --- Cargo.lock | 11 ++++++ Cargo.toml | 2 ++ src/cargo/core/compiler/timings.rs | 3 +- src/cargo/core/shell.rs | 46 +++++++++++------------- src/cargo/ops/cargo_add/mod.rs | 21 +++++------ src/cargo/ops/cargo_generate_lockfile.rs | 13 +++---- src/cargo/ops/registry/search.rs | 16 ++++----- src/cargo/util/flock.rs | 6 ++-- src/cargo/util/style.rs | 4 +++ 9 files changed, 64 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b7f3cc8f600..f6cab086800 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,6 +82,16 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "anstyle-termcolor" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11c3d1411f1f4c8a7b177caec3c71b51290f9e8ad9f99124fd3fe9aa96e56834" +dependencies = [ + "anstyle", + "termcolor", +] + [[package]] name = "anstyle-wincon" version = "2.1.0" @@ -259,6 +269,7 @@ name = "cargo" version = "0.75.0" dependencies = [ "anstyle", + "anstyle-termcolor", "anyhow", "base64", "bytesize", diff --git a/Cargo.toml b/Cargo.toml index 8acba4c7d6f..d2e825155da 100644 --- 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.1.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 index 57ded9bf84b..300719438ea 100644 --- 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 index 4d45e609889..e029c37c7d5 100644 --- 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,14 +205,14 @@ 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(&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. @@ -218,7 +220,7 @@ impl Shell { &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(&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(&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 index ccd249d8c13..2fbc3233864 100644 --- 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 index 5e6fda7551d..5b5f177b187 100644 --- 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 index 5e01ea98fef..6c4a10324de 100644 --- 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 index 295eb1e140b..32ad2c90b34 100644 --- 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 index 15ce7b3257c..618a10876bf 100644 --- 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);