Skip to content

Replace term with termcolor #3351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 3, 2023
Merged

Replace term with termcolor #3351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f7579b7
replace term with termcolor
0xPoe May 8, 2023
f5e2a9e
Enable broken color in MSYS2 shells
0xPoe Jun 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions Cargo.lock
View file
Open in desktop

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

3 changes: 2 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ sharded-slab = "0.1.1"
strsim = "0.10"
tar = "0.4.26"
tempfile.workspace = true
term = "=0.5.1" # FIXME(issue #1818, #1826, and friends)
termcolor.workspace = true
thiserror.workspace = true
threadpool = "1"
tokio = { workspace = true, optional = true }
Expand Down Expand Up @@ -184,6 +184,7 @@ opentelemetry-otlp = { version = "0.11.0" }
proptest = "1.1.0"
rustup-macros = { path = "rustup-macros" }
tempfile = "3.5"
termcolor = "1.2"
thiserror = "1.0"
tokio = { version = "1.26.0", default-features = false, features = [
"rt-multi-thread",
Expand Down
1 change: 0 additions & 1 deletion src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ pub mod proxy_mode;
pub mod rustup_mode;
pub mod self_update;
pub mod setup_mode;
mod term2;
mod topical_doc;
119 changes: 65 additions & 54 deletions src/cli/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ use std::{cmp, env};
use anyhow::{anyhow, Context, Result};
use git_testament::{git_testament, render_testament};
use lazy_static::lazy_static;
use term2::Terminal;

use super::self_update;
use super::term2;
use crate::currentprocess::argsource::ArgSource;
use crate::currentprocess::{
argsource::ArgSource,
filesource::{StdinSource, StdoutSource},
terminalsource,
varsource::VarSource,
};
use crate::utils::notifications as util_notifications;
Expand All @@ -32,7 +31,7 @@ use crate::{Cfg, Notification};
pub(crate) const WARN_COMPLETE_PROFILE: &str = "downloading with complete profile isn't recommended unless you are a developer of the rust language";

pub(crate) fn confirm(question: &str, default: bool) -> Result<bool> {
write!(process().stdout(), "{question} ")?;
write!(process().stdout().lock(), "{question} ")?;
let _ = std::io::stdout().flush();
let input = read_line()?;

Expand All @@ -43,7 +42,7 @@ pub(crate) fn confirm(question: &str, default: bool) -> Result<bool> {
_ => false,
};

writeln!(process().stdout())?;
writeln!(process().stdout().lock())?;

Ok(r)
}
Expand All @@ -55,11 +54,14 @@ pub(crate) enum Confirm {
}

pub(crate) fn confirm_advanced() -> Result<Confirm> {
writeln!(process().stdout())?;
writeln!(process().stdout(), "1) Proceed with installation (default)")?;
writeln!(process().stdout(), "2) Customize installation")?;
writeln!(process().stdout(), "3) Cancel installation")?;
write!(process().stdout(), ">")?;
writeln!(process().stdout().lock())?;
writeln!(
process().stdout().lock(),
"1) Proceed with installation (default)"
)?;
writeln!(process().stdout().lock(), "2) Customize installation")?;
writeln!(process().stdout().lock(), "3) Cancel installation")?;
write!(process().stdout().lock(), ">")?;

let _ = std::io::stdout().flush();
let input = read_line()?;
Expand All @@ -70,17 +72,17 @@ pub(crate) fn confirm_advanced() -> Result<Confirm> {
_ => Confirm::No,
};

writeln!(process().stdout())?;
writeln!(process().stdout().lock())?;

Ok(r)
}

pub(crate) fn question_str(question: &str, default: &str) -> Result<String> {
writeln!(process().stdout(), "{question} [{default}]")?;
writeln!(process().stdout().lock(), "{question} [{default}]")?;
let _ = std::io::stdout().flush();
let input = read_line()?;

writeln!(process().stdout())?;
writeln!(process().stdout().lock())?;

if input.is_empty() {
Ok(default.to_string())
Expand All @@ -91,12 +93,12 @@ pub(crate) fn question_str(question: &str, default: &str) -> Result<String> {

pub(crate) fn question_bool(question: &str, default: bool) -> Result<bool> {
let default_text = if default { "(Y/n)" } else { "(y/N)" };
writeln!(process().stdout(), "{question} {default_text}")?;
writeln!(process().stdout().lock(), "{question} {default_text}")?;

let _ = std::io::stdout().flush();
let input = read_line()?;

writeln!(process().stdout())?;
writeln!(process().stdout().lock())?;

if input.is_empty() {
Ok(default)
Expand Down Expand Up @@ -212,10 +214,10 @@ fn show_channel_updates(
) -> Result<()> {
let data = updates.into_iter().map(|(pkg, result)| {
let (banner, color) = match &result {
Ok(UpdateStatus::Installed) => ("installed", Some(term2::color::GREEN)),
Ok(UpdateStatus::Updated(_)) => ("updated", Some(term2::color::GREEN)),
Ok(UpdateStatus::Installed) => ("installed", Some(terminalsource::Color::Green)),
Ok(UpdateStatus::Updated(_)) => ("updated", Some(terminalsource::Color::Green)),
Ok(UpdateStatus::Unchanged) => ("unchanged", None),
Err(_) => ("update failed", Some(term2::color::RED)),
Err(_) => ("update failed", Some(terminalsource::Color::Red)),
};

let (previous_version, version) = match &pkg {
Expand Down Expand Up @@ -253,7 +255,7 @@ fn show_channel_updates(
Ok((pkg, banner, width, color, version, previous_version))
});

let mut t = term2::stdout();
let mut t = process().stdout().terminal();

let data: Vec<_> = data.collect::<Result<_>>()?;
let max_width = data
Expand All @@ -263,20 +265,20 @@ fn show_channel_updates(
for (pkg, banner, width, color, version, previous_version) in data {
let padding = max_width - width;
let padding: String = " ".repeat(padding);
let _ = write!(t, " {padding}");
let _ = t.attr(term2::Attr::Bold);
let _ = write!(t.lock(), " {padding}");
let _ = t.attr(terminalsource::Attr::Bold);
if let Some(color) = color {
let _ = t.fg(color);
}
let _ = write!(t, "{pkg} {banner}");
let _ = write!(t.lock(), "{pkg} {banner}");
let _ = t.reset();
let _ = write!(t, " - {version}");
let _ = write!(t.lock(), " - {version}");
if let Some(previous_version) = previous_version {
let _ = write!(t, " (from {previous_version})");
let _ = write!(t.lock(), " (from {previous_version})");
}
let _ = writeln!(t);
let _ = writeln!(t.lock());
}
let _ = writeln!(t);
let _ = writeln!(t.lock());

Ok(())
}
Expand All @@ -294,7 +296,7 @@ pub(crate) fn update_all_channels(

let show_channel_updates = || {
if !toolchains.is_empty() {
writeln!(process().stdout())?;
writeln!(process().stdout().lock())?;

let t = toolchains
.into_iter()
Expand Down Expand Up @@ -374,7 +376,7 @@ where
}

pub(crate) fn list_targets(distributable: DistributableToolchain<'_>) -> Result<utils::ExitCode> {
let mut t = term2::stdout();
let mut t = process().stdout().terminal();
let manifestation = distributable.get_manifestation()?;
let config = manifestation.read_config()?.unwrap_or_default();
let manifest = distributable.get_manifest()?;
Expand All @@ -387,11 +389,11 @@ pub(crate) fn list_targets(distributable: DistributableToolchain<'_>) -> Result<
.as_ref()
.expect("rust-std should have a target");
if component.installed {
let _ = t.attr(term2::Attr::Bold);
let _ = writeln!(t, "{target} (installed)");
let _ = t.attr(terminalsource::Attr::Bold);
let _ = writeln!(t.lock(), "{target} (installed)");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note for future investigation .... is the let _ = needed still? I presume its discarding the fallible from the call, but we're in a function that returns Result, so ? should be usable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will take a look later.

let _ = t.reset();
} else if component.available {
let _ = writeln!(t, "{target}");
let _ = writeln!(t.lock(), "{target}");
}
}
}
Expand All @@ -402,7 +404,7 @@ pub(crate) fn list_targets(distributable: DistributableToolchain<'_>) -> Result<
pub(crate) fn list_installed_targets(
distributable: DistributableToolchain<'_>,
) -> Result<utils::ExitCode> {
let mut t = term2::stdout();
let t = process().stdout();
let manifestation = distributable.get_manifestation()?;
let config = manifestation.read_config()?.unwrap_or_default();
let manifest = distributable.get_manifest()?;
Expand All @@ -415,7 +417,7 @@ pub(crate) fn list_installed_targets(
.as_ref()
.expect("rust-std should have a target");
if component.installed {
writeln!(t, "{target}")?;
writeln!(t.lock(), "{target}")?;
}
}
}
Expand All @@ -425,35 +427,36 @@ pub(crate) fn list_installed_targets(
pub(crate) fn list_components(
distributable: DistributableToolchain<'_>,
) -> Result<utils::ExitCode> {
let mut t = term2::stdout();
let mut t = process().stdout().terminal();

let manifestation = distributable.get_manifestation()?;
let config = manifestation.read_config()?.unwrap_or_default();
let manifest = distributable.get_manifest()?;
let components = manifest.query_components(distributable.desc(), &config)?;
for component in components {
let name = component.name;
if component.installed {
t.attr(term2::Attr::Bold)?;
writeln!(t, "{name} (installed)")?;
t.attr(terminalsource::Attr::Bold)?;
writeln!(t.lock(), "{name} (installed)")?;
t.reset()?;
} else if component.available {
writeln!(t, "{name}")?;
writeln!(t.lock(), "{name}")?;
}
}

Ok(utils::ExitCode(0))
}

pub(crate) fn list_installed_components(distributable: DistributableToolchain<'_>) -> Result<()> {
let mut t = term2::stdout();
let t = process().stdout();
let manifestation = distributable.get_manifestation()?;
let config = manifestation.read_config()?.unwrap_or_default();
let manifest = distributable.get_manifest()?;
let components = manifest.query_components(distributable.desc(), &config)?;

for component in components {
if component.installed {
writeln!(t, "{}", component.name)?;
writeln!(t.lock(), "{}", component.name)?;
}
}
Ok(())
Expand All @@ -478,7 +481,7 @@ fn print_toolchain_path(
String::new()
};
writeln!(
process().stdout(),
process().stdout().lock(),
"{}{}{}{}",
&toolchain,
if_default,
Expand All @@ -496,7 +499,7 @@ pub(crate) fn list_toolchains(cfg: &Cfg, verbose: bool) -> Result<utils::ExitCod
.map(Into::into)
.collect::<Vec<_>>();
if toolchains.is_empty() {
writeln!(process().stdout(), "no installed toolchains")?;
writeln!(process().stdout().lock(), "no installed toolchains")?;
} else {
let def_toolchain_name = cfg.get_default()?.map(|t| (&t).into());
let cwd = utils::current_dir()?;
Expand Down Expand Up @@ -534,7 +537,7 @@ pub(crate) fn list_overrides(cfg: &Cfg) -> Result<utils::ExitCode> {
let overrides = cfg.settings_file.with(|s| Ok(s.overrides.clone()))?;

if overrides.is_empty() {
writeln!(process().stdout(), "no overrides")?;
writeln!(process().stdout().lock(), "no overrides")?;
} else {
let mut any_not_exist = false;
for (k, v) in overrides {
Expand All @@ -543,15 +546,15 @@ pub(crate) fn list_overrides(cfg: &Cfg) -> Result<utils::ExitCode> {
any_not_exist = true;
}
writeln!(
process().stdout(),
process().stdout().lock(),
"{:<40}\t{:<20}",
utils::format_path_for_display(&k)
+ if dir_exists { "" } else { " (not a directory)" },
v
)?
}
if any_not_exist {
writeln!(process().stdout())?;
writeln!(process().stdout().lock())?;
info!(
"you may remove overrides for non-existent directories with
`rustup override unset --nonexistent`"
Expand All @@ -576,43 +579,51 @@ pub(crate) fn version() -> &'static str {
pub(crate) fn dump_testament() -> Result<utils::ExitCode> {
use git_testament::GitModification::*;
writeln!(
process().stdout(),
process().stdout().lock(),
"Rustup version renders as: {}",
version()
)?;
writeln!(
process().stdout(),
process().stdout().lock(),
"Current crate version: {}",
env!("CARGO_PKG_VERSION")
)?;
if TESTAMENT.branch_name.is_some() {
writeln!(
process().stdout(),
process().stdout().lock(),
"Built from branch: {}",
TESTAMENT.branch_name.unwrap()
)?;
} else {
writeln!(process().stdout(), "Branch information missing")?;
writeln!(process().stdout().lock(), "Branch information missing")?;
}
writeln!(process().stdout(), "Commit info: {}", TESTAMENT.commit)?;
writeln!(
process().stdout().lock(),
"Commit info: {}",
TESTAMENT.commit
)?;
if TESTAMENT.modifications.is_empty() {
writeln!(process().stdout(), "Working tree is clean")?;
writeln!(process().stdout().lock(), "Working tree is clean")?;
} else {
for fmod in TESTAMENT.modifications {
match fmod {
Added(f) => writeln!(process().stdout(), "Added: {}", String::from_utf8_lossy(f))?,
Added(f) => writeln!(
process().stdout().lock(),
"Added: {}",
String::from_utf8_lossy(f)
)?,
Removed(f) => writeln!(
process().stdout(),
process().stdout().lock(),
"Removed: {}",
String::from_utf8_lossy(f)
)?,
Modified(f) => writeln!(
process().stdout(),
process().stdout().lock(),
"Modified: {}",
String::from_utf8_lossy(f)
)?,
Untracked(f) => writeln!(
process().stdout(),
process().stdout().lock(),
"Untracked: {}",
String::from_utf8_lossy(f)
)?,
Expand Down
Loading