Skip to content

Commit

Permalink
Auto merge of #12655 - epage:style, r=weihanglo
Browse files Browse the repository at this point in the history
refactor: Consolidate clap/shell styles

### What does this PR try to resolve?

This is a follow up to #12578 to reduce duplication of terminal styling.

This still leaves styling in `color_print::cstr!`.

This is also prep for copy/pasting into `clap-cargo` for others to use.  Another step might be to extract `cargo::util::style` into its own crate.

### How should we test and review this PR?

We have no styling tests so this can only be verified by inspection or running the commands

### Additional information

I chose `anstyle` for expressing these as its an API designed specifically for stable style definitions to put in public APIs.
  • Loading branch information
bors committed Sep 13, 2023
2 parents 7541cc7 + 51e1058 commit 00bf8b0
Show file tree
Hide file tree
Showing 11 changed files with 88 additions and 69 deletions.
16 changes: 14 additions & 2 deletions Cargo.lock

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

4 changes: 4 additions & 0 deletions Cargo.toml
Expand Up @@ -16,6 +16,8 @@ edition = "2021"
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"
Expand Down Expand Up @@ -121,6 +123,8 @@ name = "cargo"
path = "src/cargo/lib.rs"

[dependencies]
anstyle.workspace = true
anstyle-termcolor.workspace = true
anyhow.workspace = true
base64.workspace = true
bytesize.workspace = true
Expand Down
18 changes: 9 additions & 9 deletions src/bin/cargo/cli.rs
Expand Up @@ -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()?;
Expand Down Expand Up @@ -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")
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/core/compiler/timings.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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(())
}

Expand Down
46 changes: 20 additions & 26 deletions src/cargo/core/shell.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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()?;
Expand All @@ -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()?;
}
Expand All @@ -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()?;
}
Expand Down
21 changes: 8 additions & 13 deletions src/cargo/ops/cargo_add/mod.rs
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)?;
}
}

Expand Down
13 changes: 7 additions & 6 deletions src/cargo/ops/cargo_generate_lockfile.rs
Expand Up @@ -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> {
Expand Down Expand Up @@ -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) {
Expand All @@ -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)?;
}
}
}
Expand Down
16 changes: 6 additions & 10 deletions src/cargo/ops/registry/search.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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() {
Expand All @@ -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,
);
}

Expand Down

0 comments on commit 00bf8b0

Please sign in to comment.