openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-19 17:28:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Color paths and URLs in codex doctor by check status (#47358)

Browse Source 
## What changed

Highlight recognized paths and URLs in check descriptions and details in red for failed checks and cyan otherwise. Extend path recognition on Windows to drive-absolute, UNC, Win32, and NT DOS paths.

## Testing

Add Windows and non-Windows color snapshots covering successful, warning, and failed checks, punctuation, and backtick-quoted paths with spaces. Assert that descriptions and details remain unchanged when color is disabled.

GitOrigin-RevId: e8f9982505aa155fd312b12f91a71271effeb35e
This commit is contained in:
Dmitriy Kovalenko
2026-09-22 20:20:06 +00:00
committed by copyberry
parent 3ebfe7b300
commit 722dae7afd
3 changed files with 127 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/cli/src/doctor/output.rs
+107 -30
View File
@@ -146,7 +146,7 @@ fn write_check_row(out: &mut String, check: &DoctorCheck, options: HumanOutputOp
if options.show_details {
for detail in detail_lines(check, options) {
write_detail_line(out, detail, options);
write_detail_line(out, detail, status, options);
}
}
}
@@ -161,7 +161,12 @@ fn write_note_row(out: &mut String, note: &DoctorNote, options: HumanOutputOptio
);
}
fn write_detail_line(out: &mut String, detail: HumanDetail, options: HumanOutputOptions) {
fn write_detail_line(
out: &mut String,
detail: HumanDetail,
status: DisplayStatus,
options: HumanOutputOptions,
) {
match detail {
HumanDetail::Row {
label,
@@ -173,11 +178,11 @@ fn write_detail_line(out: &mut String, detail: HumanDetail, options: HumanOutput
let value = if let Some(expected) = expected {
format!(
"{} {}",
detail_value(&value, options),
detail_value(&value, status, options),
dim(&format!("(expected {expected})"), options)
)
} else {
detail_value(&value, options)
detail_value(&value, status, options)
};
let _ = writeln!(
out,
@@ -193,7 +198,7 @@ fn write_detail_line(out: &mut String, detail: HumanDetail, options: HumanOutput
out,
" {} {}",
detail_label(&spacer, options),
detail_value(&value, options)
detail_value(&value, status, options)
);
}
HumanDetail::Bullet(value) => {
@@ -201,7 +206,7 @@ fn write_detail_line(out: &mut String, detail: HumanDetail, options: HumanOutput
out,
" {} {}",
very_dim(if options.ascii { "-" } else { "·" }, options),
dim(&highlight_actions(&value, options), options)
dim(&highlight_actions(&value, status, options), options)
);
}
HumanDetail::Remedy(value) => {
@@ -210,7 +215,7 @@ fn write_detail_line(out: &mut String, detail: HumanDetail, options: HumanOutput
out,
" {} {}",
orange(marker, options),
highlight_actions(&value, options)
highlight_actions(&value, status, options)
);
}
}
@@ -321,7 +326,7 @@ fn style_description(
status: DisplayStatus,
options: HumanOutputOptions,
) -> String {
let highlighted = highlight_actions(description, options);
let highlighted = highlight_actions(description, status, options);
match status {
DisplayStatus::Ok | DisplayStatus::Idle => dim(&highlighted, options),
DisplayStatus::Update => amber(&highlighted, options),
@@ -805,7 +810,7 @@ fn separator(options: HumanOutputOptions) -> String {
}
}
fn highlight_actions(text: &str, options: HumanOutputOptions) -> String {
fn highlight_actions(text: &str, status: DisplayStatus, options: HumanOutputOptions) -> String {
if !options.color_enabled {
return text.to_string();
}
@@ -813,33 +818,40 @@ fn highlight_actions(text: &str, options: HumanOutputOptions) -> String {
let mut out = String::new();
let mut parts = text.split('`');
if let Some(first) = parts.next() {
out.push_str(&highlight_flags(first, options));
out.push_str(&highlight_plain_text(first, status, options));
}
let mut in_code = true;
for part in parts {
if in_code {
out.push_str(&cyan(part, options));
let highlighted = if status == DisplayStatus::Fail && looks_copyable(part) {
red(part, options)
} else {
cyan(part, options)
};
out.push_str(&highlighted);
} else {
out.push_str(&highlight_flags(part, options));
out.push_str(&highlight_plain_text(part, status, options));
}
in_code = !in_code;
}
out
}
fn highlight_flags(text: &str, options: HumanOutputOptions) -> String {
fn highlight_plain_text(text: &str, status: DisplayStatus, options: HumanOutputOptions) -> String {
text.split_inclusive(char::is_whitespace)
.map(|token| {
let trimmed = token.trim_end();
let suffix = &token[trimmed.len()..];
let bare = trimmed.trim_end_matches([',', '.', ':', ';', ')']);
let punctuation = &trimmed[bare.len()..];
if bare.starts_with("--") {
let highlighted = cyan(bare, options);
format!("{highlighted}{punctuation}{suffix}")
let highlighted = if status == DisplayStatus::Fail && looks_copyable(bare) {
red(bare, options)
} else if bare.starts_with("--") || looks_copyable(bare) {
cyan(bare, options)
} else {
token.to_string()
}
bare.to_string()
};
format!("{highlighted}{punctuation}{suffix}")
})
.collect()
}
@@ -1051,47 +1063,60 @@ fn detail_label(text: &str, options: HumanOutputOptions) -> String {
color256(text, /*code*/ 240, options)
}
fn detail_value(text: &str, options: HumanOutputOptions) -> String {
fn detail_value(text: &str, status: DisplayStatus, options: HumanOutputOptions) -> String {
if !options.color_enabled {
return text.to_string();
}
style_detail_text(text, options)
style_detail_text(text, status, options)
}
fn style_detail_text(text: &str, options: HumanOutputOptions) -> String {
fn style_detail_text(text: &str, status: DisplayStatus, options: HumanOutputOptions) -> String {
let mut out = String::new();
let mut parts = text.split('`');
if let Some(first) = parts.next() {
out.push_str(&style_detail_plain_text(first, options));
out.push_str(&style_detail_plain_text(first, status, options));
}
let mut in_code = true;
for part in parts {
if in_code {
out.push_str(&cyan(part, options));
let highlighted = if status == DisplayStatus::Fail && looks_copyable(part) {
red(part, options)
} else {
cyan(part, options)
};
out.push_str(&highlighted);
} else {
out.push_str(&style_detail_plain_text(part, options));
out.push_str(&style_detail_plain_text(part, status, options));
}
in_code = !in_code;
}
out
}
fn style_detail_plain_text(text: &str, options: HumanOutputOptions) -> String {
fn style_detail_plain_text(
text: &str,
status: DisplayStatus,
options: HumanOutputOptions,
) -> String {
text.split_inclusive(char::is_whitespace)
.map(|token| style_detail_token(token, options))
.map(|token| style_detail_token(token, status, options))
.collect()
}
fn style_detail_token(token: &str, options: HumanOutputOptions) -> String {
fn style_detail_token(token: &str, status: DisplayStatus, options: HumanOutputOptions) -> String {
let trimmed = token.trim_end();
let suffix = &token[trimmed.len()..];
let bare = trimmed.trim_end_matches([',', '.', ':', ';', ')']);
let punctuation = &trimmed[bare.len()..];
let styled = style_detail_bare_token(bare, options);
let styled = style_detail_bare_token(bare, status, options);
format!("{styled}{punctuation}{suffix}")
}
fn style_detail_bare_token(bare: &str, options: HumanOutputOptions) -> String {
fn style_detail_bare_token(
bare: &str,
status: DisplayStatus,
options: HumanOutputOptions,
) -> String {
if bare.is_empty() {
return String::new();
}
@@ -1109,6 +1134,9 @@ fn style_detail_bare_token(bare: &str, options: HumanOutputOptions) -> String {
if bare == "ok" {
return green(bare, options);
}
if status == DisplayStatus::Fail && looks_copyable(bare) {
return red(bare, options);
}
if bare.starts_with("--") || looks_copyable(bare) {
return cyan(bare, options);
}
@@ -1147,13 +1175,24 @@ fn color256(text: &str, code: u8, options: HumanOutputOptions) -> String {
}
fn looks_copyable(text: &str) -> bool {
text.starts_with("http://")
if text.starts_with("http://")
|| text.starts_with("https://")
|| text.starts_with("wss://")
|| text.starts_with("~/")
|| text.starts_with('/')
|| text.starts_with("./")
|| text.starts_with("../")
{
return true;
}
if cfg!(target_os = "windows") {
return text.starts_with(r"\\") // unc and win32 paths
|| text.starts_with(r"\??\") // NT DOS paths
|| matches!(text.as_bytes(), [drive, b':', b'/' | b'\\', ..] if drive.is_ascii_alphabetic()); // standard drive absolute C:/ and D:\ paths
}
false
}
#[cfg(test)]
@@ -1760,6 +1799,7 @@ Run codex doctor without --summary for detailed diagnostics.
fn detail_value_colors_inline_statuses_and_low_signal_values() {
let rendered = detail_value(
"npm: no · commit unknown · integrity ok · ~/code/codex/target/debug/codex · <redacted>",
DisplayStatus::Ok,
detailed_color_unicode_options(),
);
@@ -1924,4 +1964,41 @@ Run codex doctor without --summary for detailed diagnostics.
);
assert!(rendered.contains("\u{1b}["));
}
#[test]
fn copyable_items_use_check_status() {
let description =
"/tmp/logs_2.sqlite, ~/goals_1.sqlite integrity check; try --summary or `codex doctor`";
let details = r"see ./logs, ../goals. C:\logs, D:/goals. c:\logs, \\server\share\logs, \\?\C:\logs, \\?\UNC\server\share\logs, \\.\pipe\codex, \??\C:\logs, \DosDevices\C:\logs, \rooted. http://localhost:8080: https://example.com; wss://example.com) and `/tmp/my data/logs_2.sqlite`";
let mut rendered = String::new();
for status in [CheckStatus::Ok, CheckStatus::Warning, CheckStatus::Fail] {
let check = DoctorCheck::new("state.paths", "state", status, description)
.detail(format!("locations: {details}"));
write_check_row(&mut rendered, &check, detailed_color_unicode_options());
assert_eq!(
style_description(
description,
display_status(&check),
detailed_no_color_unicode_options()
),
description
);
assert_eq!(
detail_value(
details,
display_status(&check),
detailed_no_color_unicode_options()
),
details
);
}
let snapshot_name = if cfg!(windows) {
"copyable_items_color_windows"
} else {
"copyable_items_color"
};
insta::assert_snapshot!(snapshot_name, rendered.replace('\u{1b}', "<ESC>"));
}
}
codex-rs/cli/src/doctor/snapshots/codex__doctor__output__tests__copyable_items_color.snap
+10
View File
@@ -0,0 +1,10 @@
---
source: cli/src/doctor/output.rs
expression: "rendered.replace('\\u{1b}', \"<ESC>\")"
---
<ESC>[38;5;10m✓<ESC>[39m state <ESC>[2m<ESC>[38;5;117m/tmp/logs_2.sqlite<ESC>[39m, <ESC>[38;5;117m~/goals_1.sqlite<ESC>[39m integrity check; try <ESC>[38;5;117m--summary<ESC>[39m or <ESC>[38;5;117mcodex doctor<ESC>[39m<ESC>[0m
<ESC>[38;5;240mlocations <ESC>[39m see <ESC>[38;5;117m./logs<ESC>[39m, <ESC>[38;5;117m../goals<ESC>[39m. C:\logs, D:/goals. c:\logs, \\server\share\logs, \\?\C:\logs, \\?\UNC\server\share\logs, \\.\pipe\codex, \??\C:\logs, \DosDevices\C:\logs, \rooted. <ESC>[38;5;117mhttp://localhost:8080<ESC>[39m: <ESC>[38;5;117mhttps://example.com<ESC>[39m; <ESC>[38;5;117mwss://example.com<ESC>[39m) and <ESC>[38;5;117m/tmp/my data/logs_2.sqlite<ESC>[39m
<ESC>[38;5;214m⚠<ESC>[39m state <ESC>[38;5;117m/tmp/logs_2.sqlite<ESC>[39m, <ESC>[38;5;117m~/goals_1.sqlite<ESC>[39m integrity check; try <ESC>[38;5;117m--summary<ESC>[39m or <ESC>[38;5;117mcodex doctor<ESC>[39m
<ESC>[38;5;240mlocations <ESC>[39m see <ESC>[38;5;117m./logs<ESC>[39m, <ESC>[38;5;117m../goals<ESC>[39m. C:\logs, D:/goals. c:\logs, \\server\share\logs, \\?\C:\logs, \\?\UNC\server\share\logs, \\.\pipe\codex, \??\C:\logs, \DosDevices\C:\logs, \rooted. <ESC>[38;5;117mhttp://localhost:8080<ESC>[39m: <ESC>[38;5;117mhttps://example.com<ESC>[39m; <ESC>[38;5;117mwss://example.com<ESC>[39m) and <ESC>[38;5;117m/tmp/my data/logs_2.sqlite<ESC>[39m
<ESC>[38;5;196m✗<ESC>[39m state <ESC>[38;5;196m/tmp/logs_2.sqlite<ESC>[39m, <ESC>[38;5;196m~/goals_1.sqlite<ESC>[39m integrity check; try <ESC>[38;5;117m--summary<ESC>[39m or <ESC>[38;5;117mcodex doctor<ESC>[39m
<ESC>[38;5;240mlocations <ESC>[39m see <ESC>[38;5;196m./logs<ESC>[39m, <ESC>[38;5;196m../goals<ESC>[39m. C:\logs, D:/goals. c:\logs, \\server\share\logs, \\?\C:\logs, \\?\UNC\server\share\logs, \\.\pipe\codex, \??\C:\logs, \DosDevices\C:\logs, \rooted. <ESC>[38;5;196mhttp://localhost:8080<ESC>[39m: <ESC>[38;5;196mhttps://example.com<ESC>[39m; <ESC>[38;5;196mwss://example.com<ESC>[39m) and <ESC>[38;5;196m/tmp/my data/logs_2.sqlite<ESC>[39m
codex-rs/cli/src/doctor/snapshots/codex__doctor__output__tests__copyable_items_color_windows.snap
+10
View File
@@ -0,0 +1,10 @@
---
source: cli/src/doctor/output.rs
expression: "rendered.replace('\\u{1b}', \"<ESC>\")"
---
<ESC>[38;5;10m✓<ESC>[39m state <ESC>[2m<ESC>[38;5;117m/tmp/logs_2.sqlite<ESC>[39m, <ESC>[38;5;117m~/goals_1.sqlite<ESC>[39m integrity check; try <ESC>[38;5;117m--summary<ESC>[39m or <ESC>[38;5;117mcodex doctor<ESC>[39m<ESC>[0m
<ESC>[38;5;240mlocations <ESC>[39m see <ESC>[38;5;117m./logs<ESC>[39m, <ESC>[38;5;117m../goals<ESC>[39m. <ESC>[38;5;117mC:\logs<ESC>[39m, <ESC>[38;5;117mD:/goals<ESC>[39m. <ESC>[38;5;117mc:\logs<ESC>[39m, <ESC>[38;5;117m\\server\share\logs<ESC>[39m, <ESC>[38;5;117m\\?\C:\logs<ESC>[39m, <ESC>[38;5;117m\\?\UNC\server\share\logs<ESC>[39m, <ESC>[38;5;117m\\.\pipe\codex<ESC>[39m, <ESC>[38;5;117m\??\C:\logs<ESC>[39m, \DosDevices\C:\logs, \rooted. <ESC>[38;5;117mhttp://localhost:8080<ESC>[39m: <ESC>[38;5;117mhttps://example.com<ESC>[39m; <ESC>[38;5;117mwss://example.com<ESC>[39m) and <ESC>[38;5;117m/tmp/my data/logs_2.sqlite<ESC>[39m
<ESC>[38;5;214m⚠<ESC>[39m state <ESC>[38;5;117m/tmp/logs_2.sqlite<ESC>[39m, <ESC>[38;5;117m~/goals_1.sqlite<ESC>[39m integrity check; try <ESC>[38;5;117m--summary<ESC>[39m or <ESC>[38;5;117mcodex doctor<ESC>[39m
<ESC>[38;5;240mlocations <ESC>[39m see <ESC>[38;5;117m./logs<ESC>[39m, <ESC>[38;5;117m../goals<ESC>[39m. <ESC>[38;5;117mC:\logs<ESC>[39m, <ESC>[38;5;117mD:/goals<ESC>[39m. <ESC>[38;5;117mc:\logs<ESC>[39m, <ESC>[38;5;117m\\server\share\logs<ESC>[39m, <ESC>[38;5;117m\\?\C:\logs<ESC>[39m, <ESC>[38;5;117m\\?\UNC\server\share\logs<ESC>[39m, <ESC>[38;5;117m\\.\pipe\codex<ESC>[39m, <ESC>[38;5;117m\??\C:\logs<ESC>[39m, \DosDevices\C:\logs, \rooted. <ESC>[38;5;117mhttp://localhost:8080<ESC>[39m: <ESC>[38;5;117mhttps://example.com<ESC>[39m; <ESC>[38;5;117mwss://example.com<ESC>[39m) and <ESC>[38;5;117m/tmp/my data/logs_2.sqlite<ESC>[39m
<ESC>[38;5;196m✗<ESC>[39m state <ESC>[38;5;196m/tmp/logs_2.sqlite<ESC>[39m, <ESC>[38;5;196m~/goals_1.sqlite<ESC>[39m integrity check; try <ESC>[38;5;117m--summary<ESC>[39m or <ESC>[38;5;117mcodex doctor<ESC>[39m
<ESC>[38;5;240mlocations <ESC>[39m see <ESC>[38;5;196m./logs<ESC>[39m, <ESC>[38;5;196m../goals<ESC>[39m. <ESC>[38;5;196mC:\logs<ESC>[39m, <ESC>[38;5;196mD:/goals<ESC>[39m. <ESC>[38;5;196mc:\logs<ESC>[39m, <ESC>[38;5;196m\\server\share\logs<ESC>[39m, <ESC>[38;5;196m\\?\C:\logs<ESC>[39m, <ESC>[38;5;196m\\?\UNC\server\share\logs<ESC>[39m, <ESC>[38;5;196m\\.\pipe\codex<ESC>[39m, <ESC>[38;5;196m\??\C:\logs<ESC>[39m, \DosDevices\C:\logs, \rooted. <ESC>[38;5;196mhttp://localhost:8080<ESC>[39m: <ESC>[38;5;196mhttps://example.com<ESC>[39m; <ESC>[38;5;196mwss://example.com<ESC>[39m) and <ESC>[38;5;196m/tmp/my data/logs_2.sqlite<ESC>[39m