Skip to content

Commit 9ffbb0f

Browse files
authored
fix(cli): handle terminals with limited color support (#4458)
* fix(cli): panic on limited color support ### Motivation - `term` crate panicked on terminals lacking color support. ### Solution - Swap `term` crate for raw ANSI escape codes. - Emit color only if TTY and `NO_COLOR` is unset. - Delete obsolete `stderr_wrapper` module. * Updated CHANGELOG.md
1 parent 650c497 commit 9ffbb0f

File tree

6 files changed

+34
-112
lines changed

6 files changed

+34
-112
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ and its status can be checked out using `dfx canister migration-status`.
1212
The optimization functionality provided by `ic_wasm::optimize" cannot handle Wasm modules that contains 64-bit table.
1313
Instead of blocking the build, such optimization failure will issue a warning.
1414

15+
### fix: prevent panic on terminals with limited color support
16+
17+
Fixed a panic that could occur when running `dfx` on terminals lacking color support. The `term` crate has been replaced with raw ANSI escape codes, and colors are now only emitted when stderr is a TTY and the `NO_COLOR` environment variable is not set.
18+
1519
## Dependencies
1620

1721
### Motoko

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/dfx/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ supports-color = "2.1.0"
107107
sysinfo = "0.28.4"
108108
tar.workspace = true
109109
tempfile.workspace = true
110-
term = "0.7.0"
111110
thiserror.workspace = true
112111
time = { workspace = true, features = ["macros", "serde", "serde-human-readable"] }
113112
tokio = { workspace = true, features = ["full"] }

src/dfx/src/main.rs

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use dfx_core::extension::manager::ExtensionManager;
1515
use indicatif::MultiProgress;
1616
use std::collections::HashMap;
1717
use std::ffi::OsString;
18+
use std::io::{IsTerminal, Write, stderr};
1819
use std::path::PathBuf;
1920
use std::time::Instant;
2021
use util::default_allowlisted_canisters;
@@ -72,8 +73,19 @@ fn setup_logging(opts: &CliOpts) -> (i64, slog::Logger, MultiProgress) {
7273
(verbose_level, logger, spinners)
7374
}
7475

76+
/// Returns true if colored output should be used on stderr.
77+
/// Respects the NO_COLOR environment variable (https://no-color.org/).
78+
fn use_color() -> bool {
79+
std::env::var_os("NO_COLOR").is_none() && stderr().is_terminal()
80+
}
81+
7582
fn print_error_and_diagnosis(log_level: Option<i64>, err: Error, error_diagnosis: DiagnosedError) {
76-
let mut stderr = util::stderr_wrapper::stderr_wrapper();
83+
let mut stderr = stderr().lock();
84+
let use_color = use_color();
85+
86+
const RED: &str = "\x1b[31m";
87+
const YELLOW: &str = "\x1b[33m";
88+
const RESET: &str = "\x1b[0m";
7789

7890
// print error chain stack
7991
if log_level.unwrap_or_default() > 0 // DEBUG or more verbose
@@ -85,43 +97,35 @@ fn print_error_and_diagnosis(log_level: Option<i64>, err: Error, error_diagnosis
8597
}
8698

8799
let (color, prefix) = if level == 0 {
88-
(term::color::RED, "Error")
100+
(RED, "Error")
89101
} else {
90-
(term::color::YELLOW, "Caused by")
102+
(YELLOW, "Caused by")
91103
};
92-
stderr
93-
.fg(color)
94-
.expect("Failed to set stderr output color.");
95-
write!(stderr, "{prefix}: ").expect("Failed to write to stderr.");
96-
stderr
97-
.reset()
98-
.expect("Failed to reset stderr output color.");
104+
if use_color {
105+
write!(stderr, "{color}{prefix}: {RESET}").expect("Failed to write to stderr.");
106+
} else {
107+
write!(stderr, "{prefix}: ").expect("Failed to write to stderr.");
108+
}
99109

100110
writeln!(stderr, "{cause}").expect("Failed to write to stderr.");
101111
}
102112
}
103113

104114
// print diagnosis
105115
if let Some(explanation) = error_diagnosis.explanation {
106-
stderr
107-
.fg(term::color::RED)
108-
.expect("Failed to set stderr output color.");
109-
write!(stderr, "Error: ").expect("Failed to write to stderr.");
110-
stderr
111-
.reset()
112-
.expect("Failed to reset stderr output color.");
113-
116+
if use_color {
117+
write!(stderr, "{RED}Error: {RESET}").expect("Failed to write to stderr.");
118+
} else {
119+
write!(stderr, "Error: ").expect("Failed to write to stderr.");
120+
}
114121
writeln!(stderr, "{explanation}").expect("Failed to write to stderr.");
115122
}
116123
if let Some(action_suggestion) = error_diagnosis.action_suggestion {
117-
stderr
118-
.fg(term::color::YELLOW)
119-
.expect("Failed to set stderr output color.");
120-
write!(stderr, "To fix: ").expect("Failed to write to stderr.");
121-
stderr
122-
.reset()
123-
.expect("Failed to reset stderr output color.");
124-
124+
if use_color {
125+
write!(stderr, "{YELLOW}To fix: {RESET}").expect("Failed to write to stderr.");
126+
} else {
127+
write!(stderr, "To fix: ").expect("Failed to write to stderr.");
128+
}
125129
writeln!(stderr, "{action_suggestion}").expect("Failed to write to stderr.");
126130
}
127131
}

src/dfx/src/util/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ pub mod assets;
3131
pub mod clap;
3232
pub mod command;
3333
pub mod currency_conversion;
34-
pub mod stderr_wrapper;
3534
pub mod url;
3635

3736
const DECIMAL_POINT: char = '.';

src/dfx/src/util/stderr_wrapper.rs

Lines changed: 0 additions & 83 deletions
This file was deleted.

0 commit comments

Comments
 (0)