Skip to content

Commit

Permalink
Auto merge of #13463 - epage:term_color, r=weihanglo
Browse files Browse the repository at this point in the history
fix(cli): Control clap colors through config

### What does this PR try to resolve?

Part of #9012

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

To accomplish this, I pivoted in how we handle `-C`.  In #11029, I made the config lazily loaded.  Instead, we are now reloading the config for `-C` like we do for "cargo script" and `cargo install`.  If there is any regression, it will be felt by those commands as well and we can fix all together.  As this is unstable, if there is a regression, the impact is less.  This allowed us access to the full config for getting the color setting, rather than taking more limited approaches like checking only `CARGO_TERM_CONFIG`.

### Additional information
  • Loading branch information
bors committed Feb 21, 2024
2 parents 3e69220 + 6747425 commit d325f9b
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 144 deletions.
85 changes: 27 additions & 58 deletions src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use anyhow::{anyhow, Context as _};
use cargo::core::shell::Shell;
use cargo::core::{features, CliUnstable};
use cargo::util::config::TermConfig;
use cargo::{drop_print, drop_println, CargoResult};
use clap::builder::UnknownArgumentValueParser;
use itertools::Itertools;
Expand All @@ -13,14 +13,17 @@ use super::commands;
use super::list_commands;
use crate::command_prelude::*;
use crate::util::is_rustup;
use cargo::core::shell::ColorChoice;
use cargo::util::style;

pub fn main(lazy_gctx: &mut LazyContext) -> CliResult {
let args = cli().try_get_matches()?;
pub fn main(gctx: &mut GlobalContext) -> CliResult {
// CAUTION: Be careful with using `config` until it is configured below.
// In general, try to avoid loading config values unless necessary (like
// the [alias] table).

let args = cli(gctx).try_get_matches()?;

// Update the process-level notion of cwd
// This must be completed before config is initialized
assert_eq!(lazy_gctx.is_init(), false);
if let Some(new_cwd) = args.get_one::<std::path::PathBuf>("directory") {
// This is a temporary hack. This cannot access `Config`, so this is a bit messy.
// This does not properly parse `-Z` flags that appear after the subcommand.
Expand All @@ -40,13 +43,9 @@ pub fn main(lazy_gctx: &mut LazyContext) -> CliResult {
.into());
}
std::env::set_current_dir(&new_cwd).context("could not change to requested directory")?;
gctx.reload_cwd()?;
}

// CAUTION: Be careful with using `config` until it is configured below.
// In general, try to avoid loading config values unless necessary (like
// the [alias] table).
let gctx = lazy_gctx.get_mut();

let (expanded_args, global_args) = expand_aliases(gctx, args, vec![])?;

if expanded_args
Expand Down Expand Up @@ -175,7 +174,7 @@ Run with `{literal}cargo -Z{literal:#} {placeholder}[FLAG] [COMMAND]{placeholder
Some((cmd, args)) => (cmd, args),
_ => {
// No subcommand provided.
cli().print_help()?;
cli(gctx).print_help()?;
return Ok(());
}
};
Expand Down Expand Up @@ -338,7 +337,7 @@ For more information, see issue #12207 <https://github.com/rust-lang/cargo/issue
// Note that an alias to an external command will not receive
// these arguments. That may be confusing, but such is life.
let global_args = GlobalArgs::new(sub_args);
let new_args = cli().no_binary_name(true).try_get_matches_from(alias)?;
let new_args = cli(gctx).no_binary_name(true).try_get_matches_from(alias)?;

let new_cmd = new_args.subcommand_name().expect("subcommand is required");
already_expanded.push(cmd.to_string());
Expand Down Expand Up @@ -514,7 +513,19 @@ impl GlobalArgs {
}
}

pub fn cli() -> Command {
pub fn cli(gctx: &GlobalContext) -> Command {
// Don't let config errors get in the way of parsing arguments
let term = gctx.get::<TermConfig>("term").unwrap_or_default();
let color = term
.color
.and_then(|c| c.parse().ok())
.unwrap_or(ColorChoice::CargoAuto);
let color = match color {
ColorChoice::Always => clap::ColorChoice::Always,
ColorChoice::Never => clap::ColorChoice::Never,
ColorChoice::CargoAuto => clap::ColorChoice::Auto,
};

let usage = if is_rustup() {
color_print::cstr!("<cyan,bold>cargo</> <cyan>[+toolchain] [OPTIONS] [COMMAND]</>\n <cyan,bold>cargo</> <cyan>[+toolchain] [OPTIONS]</> <cyan,bold>-Zscript</> <cyan><<MANIFEST_RS>> [ARGS]...</>")
} else {
Expand All @@ -539,6 +550,7 @@ pub fn cli() -> Command {
// We also want these to come before auto-generated `--help`
.next_display_order(800)
.allow_external_subcommands(true)
.color(color)
.styles(styles)
// Provide a custom help subcommand for calling into man pages
.disable_help_subcommand(true)
Expand Down Expand Up @@ -646,53 +658,10 @@ See '<cyan,bold>cargo help</> <cyan><<command>></>' for more information on a sp
.subcommands(commands::builtin())
}

/// Delay loading [`GlobalContext`] until access.
///
/// In the common path, the [`GlobalContext`] is dependent on CLI parsing and shouldn't be loaded until
/// after that is done but some other paths (like fix or earlier errors) might need access to it,
/// so this provides a way to share the instance and the implementation across these different
/// accesses.
pub struct LazyContext {
gctx: Option<GlobalContext>,
}

impl LazyContext {
pub fn new() -> Self {
Self { gctx: None }
}

/// Check whether the config is loaded
///
/// This is useful for asserts in case the environment needs to be setup before loading
pub fn is_init(&self) -> bool {
self.gctx.is_some()
}

/// Get the config, loading it if needed
///
/// On error, the process is terminated
pub fn get(&mut self) -> &GlobalContext {
self.get_mut()
}

/// Get the config, loading it if needed
///
/// On error, the process is terminated
pub fn get_mut(&mut self) -> &mut GlobalContext {
self.gctx
.get_or_insert_with(|| match GlobalContext::default() {
Ok(cfg) => cfg,
Err(e) => {
let mut shell = Shell::new();
cargo::exit_with_error(e.into(), &mut shell)
}
})
}
}

#[test]
fn verify_cli() {
cli().debug_assert();
let gctx = GlobalContext::default().unwrap();
cli(&gctx).debug_assert();
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
}
}
} else {
let mut cmd = crate::cli::cli();
let mut cmd = crate::cli::cli(gctx);
let _ = cmd.print_help();
}
Ok(())
Expand Down
15 changes: 11 additions & 4 deletions src/bin/cargo/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![allow(clippy::self_named_module_files)] // false positive in `commands/build.rs`

use cargo::core::shell::Shell;
use cargo::util::network::http::http_handle;
use cargo::util::network::http::needs_custom_http_transport;
use cargo::util::{self, closest_msg, command_prelude, CargoResult};
Expand All @@ -19,17 +20,23 @@ use crate::command_prelude::*;
fn main() {
setup_logger();

let mut lazy_gctx = cli::LazyContext::new();
let mut gctx = match GlobalContext::default() {
Ok(gctx) => gctx,
Err(e) => {
let mut shell = Shell::new();
cargo::exit_with_error(e.into(), &mut shell)
}
};

let result = if let Some(lock_addr) = cargo::ops::fix_get_proxy_lock_addr() {
cargo::ops::fix_exec_rustc(lazy_gctx.get(), &lock_addr).map_err(|e| CliError::from(e))
cargo::ops::fix_exec_rustc(&gctx, &lock_addr).map_err(|e| CliError::from(e))
} else {
let _token = cargo::util::job::setup();
cli::main(&mut lazy_gctx)
cli::main(&mut gctx)
};

match result {
Err(e) => cargo::exit_with_error(e, &mut lazy_gctx.get_mut().shell()),
Err(e) => cargo::exit_with_error(e, &mut *gctx.shell()),
Ok(()) => {}
}
}
Expand Down
161 changes: 86 additions & 75 deletions src/cargo/core/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,44 +9,6 @@ use crate::util::errors::CargoResult;
use crate::util::hostname;
use crate::util::style::*;

pub enum TtyWidth {
NoTty,
Known(usize),
Guess(usize),
}

impl TtyWidth {
/// Returns the width of the terminal to use for diagnostics (which is
/// relayed to rustc via `--diagnostic-width`).
pub fn diagnostic_terminal_width(&self) -> Option<usize> {
// ALLOWED: For testing cargo itself only.
#[allow(clippy::disallowed_methods)]
if let Ok(width) = std::env::var("__CARGO_TEST_TTY_WIDTH_DO_NOT_USE_THIS") {
return Some(width.parse().unwrap());
}
match *self {
TtyWidth::NoTty | TtyWidth::Guess(_) => None,
TtyWidth::Known(width) => Some(width),
}
}

/// Returns the width used by progress bars for the tty.
pub fn progress_max_width(&self) -> Option<usize> {
match *self {
TtyWidth::NoTty => None,
TtyWidth::Known(width) | TtyWidth::Guess(width) => Some(width),
}
}
}

/// The requested verbosity of output.
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum Verbosity {
Verbose,
Normal,
Quiet,
}

/// An abstraction around console output that remembers preferences for output
/// verbosity and color.
pub struct Shell {
Expand Down Expand Up @@ -77,31 +39,6 @@ impl fmt::Debug for Shell {
}
}

/// A `Write`able object, either with or without color support
enum ShellOut {
/// A plain write object without color support
Write(AutoStream<Box<dyn Write>>),
/// Color-enabled stdio, with information on whether color should be used
Stream {
stdout: AutoStream<std::io::Stdout>,
stderr: AutoStream<std::io::Stderr>,
stderr_tty: bool,
color_choice: ColorChoice,
hyperlinks: bool,
},
}

/// Whether messages should use color output
#[derive(Debug, PartialEq, Clone, Copy)]
pub enum ColorChoice {
/// Force color output
Always,
/// Force disable color output
Never,
/// Intelligently guess whether to use color output
CargoAuto,
}

impl Shell {
/// Creates a new shell (color choice and verbosity), defaulting to 'auto' color and verbose
/// output.
Expand Down Expand Up @@ -299,18 +236,10 @@ impl Shell {
..
} = self.output
{
let cfg = match color {
Some("always") => ColorChoice::Always,
Some("never") => ColorChoice::Never,

Some("auto") | None => ColorChoice::CargoAuto,

Some(arg) => anyhow::bail!(
"argument for --color must be auto, always, or \
never, but found `{}`",
arg
),
};
let cfg = color
.map(|c| c.parse())
.transpose()?
.unwrap_or(ColorChoice::CargoAuto);
*color_choice = cfg;
let stdout_choice = cfg.to_anstream_color_choice();
let stderr_choice = cfg.to_anstream_color_choice();
Expand Down Expand Up @@ -444,6 +373,20 @@ impl Default for Shell {
}
}

/// A `Write`able object, either with or without color support
enum ShellOut {
/// A plain write object without color support
Write(AutoStream<Box<dyn Write>>),
/// Color-enabled stdio, with information on whether color should be used
Stream {
stdout: AutoStream<std::io::Stdout>,
stderr: AutoStream<std::io::Stderr>,
stderr_tty: bool,
color_choice: ColorChoice,
hyperlinks: bool,
},
}

impl ShellOut {
/// Prints out a message with a status. The status comes first, and is bold plus the given
/// color. The status can be justified, in which case the max width that will right align is
Expand Down Expand Up @@ -488,6 +431,55 @@ impl ShellOut {
}
}

pub enum TtyWidth {
NoTty,
Known(usize),
Guess(usize),
}

impl TtyWidth {
/// Returns the width of the terminal to use for diagnostics (which is
/// relayed to rustc via `--diagnostic-width`).
pub fn diagnostic_terminal_width(&self) -> Option<usize> {
// ALLOWED: For testing cargo itself only.
#[allow(clippy::disallowed_methods)]
if let Ok(width) = std::env::var("__CARGO_TEST_TTY_WIDTH_DO_NOT_USE_THIS") {
return Some(width.parse().unwrap());
}
match *self {
TtyWidth::NoTty | TtyWidth::Guess(_) => None,
TtyWidth::Known(width) => Some(width),
}
}

/// Returns the width used by progress bars for the tty.
pub fn progress_max_width(&self) -> Option<usize> {
match *self {
TtyWidth::NoTty => None,
TtyWidth::Known(width) | TtyWidth::Guess(width) => Some(width),
}
}
}

/// The requested verbosity of output.
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum Verbosity {
Verbose,
Normal,
Quiet,
}

/// Whether messages should use color output
#[derive(Debug, PartialEq, Clone, Copy)]
pub enum ColorChoice {
/// Force color output
Always,
/// Force disable color output
Never,
/// Intelligently guess whether to use color output
CargoAuto,
}

impl ColorChoice {
/// Converts our color choice to anstream's version.
fn to_anstream_color_choice(self) -> anstream::ColorChoice {
Expand All @@ -499,6 +491,25 @@ impl ColorChoice {
}
}

impl std::str::FromStr for ColorChoice {
type Err = anyhow::Error;
fn from_str(color: &str) -> Result<Self, Self::Err> {
let cfg = match color {
"always" => ColorChoice::Always,
"never" => ColorChoice::Never,

"auto" => ColorChoice::CargoAuto,

arg => anyhow::bail!(
"argument for --color must be auto, always, or \
never, but found `{}`",
arg
),
};
Ok(cfg)
}
}

fn supports_color(choice: anstream::ColorChoice) -> bool {
match choice {
anstream::ColorChoice::Always
Expand Down
Loading

0 comments on commit d325f9b

Please sign in to comment.