mirror of
https://github.com/openai/codex.git
synced 2026-10-22 21:40:34 +00:00
fix(tui): promote windows terminal diff ansi16 to truecolor
Windows Terminal supports 24-bit color but `supports-color` often reports only ANSI-16 because `COLORTERM` is not set. Introduce `diff_color_level_for_terminal`, a pure mapping from detected color level to intended diff palette, that promotes to truecolor when Windows Terminal is detected. Detection uses two independent signals — `TerminalName::WindowsTerminal` (from `terminal_info()`) and raw `WT_SESSION` presence — so the promotion fires even if one detection path changes. When `WT_SESSION` is present, promotion is unconditional (including `Unknown` level); without it, only ANSI-16 is promoted for named WindowsTerminal sessions. Setting `FORCE_COLOR` suppresses both paths to preserve explicit user intent. All env reads stay in the thin `diff_color_level()` wrapper; the core function is pure for testability. Six unit tests cover every branch.
This commit is contained in:
@@ -249,6 +249,7 @@ pub fn terminal_info() -> TerminalInfo {
|
||||
/// type is split on whitespace to extract a program name plus optional version (for example,
|
||||
/// `ghostty 1.2.3`), while the client term name becomes the `TERM` capability string.
|
||||
/// - Otherwise, `TERM_PROGRAM` (plus `TERM_PROGRAM_VERSION`) drives the detected terminal name.
|
||||
/// This means `TERM_PROGRAM` can mask later probes (for example `WT_SESSION`).
|
||||
/// - Next, terminal-specific variables (WEZTERM, iTerm2, Apple Terminal, kitty, etc.) are checked.
|
||||
/// - Finally, `TERM` is used as the capability fallback with `TerminalName::Unknown`.
|
||||
///
|
||||
|
||||
@@ -83,6 +83,8 @@ use crate::terminal_palette::indexed_color;
|
||||
use crate::terminal_palette::rgb_color;
|
||||
use crate::terminal_palette::stdout_color_level;
|
||||
use codex_core::git_info::get_git_repo_root;
|
||||
use codex_core::terminal::TerminalName;
|
||||
use codex_core::terminal::terminal_info;
|
||||
use codex_protocol::protocol::FileChange;
|
||||
|
||||
/// Classifies a diff line for gutter sign rendering and style selection.
|
||||
@@ -110,6 +112,14 @@ enum DiffTheme {
|
||||
Light,
|
||||
}
|
||||
|
||||
/// Palette depth the diff renderer will target.
|
||||
///
|
||||
/// This is the *renderer's own* notion of color depth, derived from — but not
|
||||
/// identical to — the raw [`StdoutColorLevel`] reported by `supports-color`.
|
||||
/// The indirection exists because some terminals (notably Windows Terminal)
|
||||
/// advertise only ANSI-16 support while actually rendering truecolor sequences
|
||||
/// correctly; [`diff_color_level_for_terminal`] promotes those cases so the
|
||||
/// diff output uses the richer palette.
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
enum DiffColorLevel {
|
||||
TrueColor,
|
||||
@@ -876,11 +886,75 @@ fn diff_theme() -> DiffTheme {
|
||||
diff_theme_for_bg(default_bg())
|
||||
}
|
||||
|
||||
/// Return the [`DiffColorLevel`] for the current terminal session.
|
||||
///
|
||||
/// This is the environment-reading adapter: it samples runtime signals
|
||||
/// (`supports-color` level, terminal name, `WT_SESSION`, and `FORCE_COLOR`)
|
||||
/// and forwards them to [`diff_color_level_for_terminal`].
|
||||
///
|
||||
/// Keeping env reads in this thin wrapper lets
|
||||
/// [`diff_color_level_for_terminal`] stay pure and easy to unit test.
|
||||
fn diff_color_level() -> DiffColorLevel {
|
||||
match stdout_color_level() {
|
||||
diff_color_level_for_terminal(
|
||||
stdout_color_level(),
|
||||
terminal_info().name,
|
||||
std::env::var_os("WT_SESSION").is_some(),
|
||||
has_force_color_override(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Returns whether `FORCE_COLOR` is explicitly set.
|
||||
fn has_force_color_override() -> bool {
|
||||
std::env::var_os("FORCE_COLOR").is_some()
|
||||
}
|
||||
|
||||
/// Map a raw [`StdoutColorLevel`] to a [`DiffColorLevel`], promoting
|
||||
/// ANSI-16 to truecolor when running inside Windows Terminal.
|
||||
///
|
||||
/// This helper is intentionally pure (no env access) so tests can validate
|
||||
/// the policy table by passing explicit inputs.
|
||||
///
|
||||
/// Windows Terminal fully supports 24-bit color but the `supports-color`
|
||||
/// crate often reports only ANSI-16 there because no `COLORTERM` variable
|
||||
/// is set. We detect Windows Terminal two ways — via `terminal_name`
|
||||
/// (parsed from `WT_SESSION` / `TERM_PROGRAM` by `terminal_info()`) and
|
||||
/// via the raw `has_wt_session` flag.
|
||||
///
|
||||
/// These signals are intentionally not equivalent: `terminal_name` is a
|
||||
/// derived classification with `TERM_PROGRAM` precedence, so `WT_SESSION`
|
||||
/// can be present while `terminal_name` is not `WindowsTerminal`.
|
||||
///
|
||||
/// When `WT_SESSION` is present, we promote to truecolor unconditionally
|
||||
/// unless `FORCE_COLOR` is set. This keeps Windows Terminal rendering rich
|
||||
/// by default while preserving explicit `FORCE_COLOR` user intent.
|
||||
///
|
||||
/// Outside `WT_SESSION`, only ANSI-16 is promoted for identified
|
||||
/// `WindowsTerminal` sessions; `Unknown` stays conservative.
|
||||
fn diff_color_level_for_terminal(
|
||||
stdout_level: StdoutColorLevel,
|
||||
terminal_name: TerminalName,
|
||||
has_wt_session: bool,
|
||||
has_force_color_override: bool,
|
||||
) -> DiffColorLevel {
|
||||
if has_wt_session && !has_force_color_override {
|
||||
return DiffColorLevel::TrueColor;
|
||||
}
|
||||
|
||||
let base = match stdout_level {
|
||||
StdoutColorLevel::TrueColor => DiffColorLevel::TrueColor,
|
||||
StdoutColorLevel::Ansi256 => DiffColorLevel::Ansi256,
|
||||
StdoutColorLevel::Ansi16 | StdoutColorLevel::Unknown => DiffColorLevel::Ansi16,
|
||||
};
|
||||
|
||||
// Outside `WT_SESSION`, keep the existing Windows Terminal promotion for
|
||||
// ANSI-16 sessions that likely support truecolor.
|
||||
if base == DiffColorLevel::Ansi16
|
||||
&& terminal_name == TerminalName::WindowsTerminal
|
||||
&& !has_force_color_override
|
||||
{
|
||||
DiffColorLevel::TrueColor
|
||||
} else {
|
||||
base
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1608,6 +1682,84 @@ mod tests {
|
||||
assert_eq!(lines[1].style.bg, Some(rgb_color(LIGHT_TC_ADD_LINE_BG_RGB)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn windows_terminal_promotes_ansi16_to_truecolor_for_diffs() {
|
||||
assert_eq!(
|
||||
diff_color_level_for_terminal(
|
||||
StdoutColorLevel::Ansi16,
|
||||
TerminalName::WindowsTerminal,
|
||||
false,
|
||||
false,
|
||||
),
|
||||
DiffColorLevel::TrueColor
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn wt_session_promotes_ansi16_to_truecolor_for_diffs() {
|
||||
assert_eq!(
|
||||
diff_color_level_for_terminal(
|
||||
StdoutColorLevel::Ansi16,
|
||||
TerminalName::Unknown,
|
||||
true,
|
||||
false,
|
||||
),
|
||||
DiffColorLevel::TrueColor
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_windows_terminal_keeps_ansi16_diff_palette() {
|
||||
assert_eq!(
|
||||
diff_color_level_for_terminal(
|
||||
StdoutColorLevel::Ansi16,
|
||||
TerminalName::WezTerm,
|
||||
false,
|
||||
false,
|
||||
),
|
||||
DiffColorLevel::Ansi16
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn wt_session_promotes_unknown_color_level_to_truecolor() {
|
||||
assert_eq!(
|
||||
diff_color_level_for_terminal(
|
||||
StdoutColorLevel::Unknown,
|
||||
TerminalName::WindowsTerminal,
|
||||
true,
|
||||
false,
|
||||
),
|
||||
DiffColorLevel::TrueColor
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_force_override_keeps_ansi16_on_windows_terminal() {
|
||||
assert_eq!(
|
||||
diff_color_level_for_terminal(
|
||||
StdoutColorLevel::Ansi16,
|
||||
TerminalName::WindowsTerminal,
|
||||
false,
|
||||
true,
|
||||
),
|
||||
DiffColorLevel::Ansi16
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_force_override_keeps_ansi256_on_windows_terminal() {
|
||||
assert_eq!(
|
||||
diff_color_level_for_terminal(
|
||||
StdoutColorLevel::Ansi256,
|
||||
TerminalName::WindowsTerminal,
|
||||
true,
|
||||
true,
|
||||
),
|
||||
DiffColorLevel::Ansi256
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_diff_uses_path_extension_for_highlighting() {
|
||||
let mut changes: HashMap<PathBuf, FileChange> = HashMap::new();
|
||||
|
||||
Reference in New Issue
Block a user