From 553ffa0ad43797868dc2e3884e9179922b58d50b Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Thu, 26 Feb 2026 12:50:54 -0300 Subject: [PATCH] fix(tui): promote windows terminal diff ansi16 to truecolor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- codex-rs/core/src/terminal.rs | 1 + codex-rs/tui/src/diff_render.rs | 154 +++++++++++++++++++++++++++++++- 2 files changed, 154 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/terminal.rs b/codex-rs/core/src/terminal.rs index 8437962f6c..b91aef106b 100644 --- a/codex-rs/core/src/terminal.rs +++ b/codex-rs/core/src/terminal.rs @@ -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`. /// diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs index 32591945fa..4dab25333a 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -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 = HashMap::new();