mirror of
https://github.com/openai/codex.git
synced 2026-05-26 05:55:36 +00:00
3b5996f988
## Summary - Promote ANSI-16 to truecolor for diff rendering when running inside Windows Terminal - Respect explicit `FORCE_COLOR` override, skipping promotion when set - Extract a pure `diff_color_level_for_terminal` function for testability - Strip background tints from ANSI-16 diff output, rendering add/delete lines with foreground color only - Introduce `RichDiffColorLevel` to type-safely restrict background fills to truecolor and ansi256 ## Problem Windows Terminal fully supports 24-bit (truecolor) rendering but often does not provide the usual TERM metadata (`TERM`, `TERM_PROGRAM`, `COLORTERM`) in `cmd.exe`/PowerShell sessions. In those environments, `supports-color` can report only ANSI-16 support. The diff renderer therefore falls back to a 16-color palette, producing washed-out, hard-to-read diffs. The screenshots below demonstrate that both PowerShell and cmd.exe don't set any `*TERM*` environment variables. | PowerShell | cmd.exe | |---|---| | <img width="2032" height="1162" alt="SCR-20260226-nfvy" src="https://github.com/user-attachments/assets/59e968cc-4add-4c7b-a415-07163297e86a" /> | <img width="2032" height="1162" alt="SCR-20260226-nfyc" src="https://github.com/user-attachments/assets/d06b3e39-bf91-4ce3-9705-82bf9563a01b" /> | ## Mental model `StdoutColorLevel` (from `supports-color`) is the _detected_ capability. `DiffColorLevel` is the _intended_ capability for diff rendering. A new intermediary — `diff_color_level_for_terminal` — maps one to the other and is the single place where terminal-specific overrides live. Windows Terminal is detected two independent ways: the `TerminalName` parsed by `terminal_info()` and the raw presence of `WT_SESSION`. When `WT_SESSION` is present and `FORCE_COLOR` is not set, we promote unconditionally to truecolor. When `WT_SESSION` is absent but `TerminalName::WindowsTerminal` is detected, we promote only the ANSI-16 level (not `Unknown`). A single override helper — `has_force_color_override()` — checks whether `FORCE_COLOR` is set. When it is, both the `WT_SESSION` fast-path and the `TerminalName`-based promotion are suppressed, preserving explicit user intent. | PowerShell | cmd.exe | WSL | Bash for Windows | |---|---|---|---| |  |  |  |  | ## Non-goals - This does not change color detection for anything outside the diff renderer (e.g. the chat widget, markdown rendering). - This does not add a user-facing config knob; `FORCE_COLOR` already serves that role. ## Tradeoffs - The `has_wt_session` signal is intentionally kept separate from `TerminalName::WindowsTerminal`. `terminal_info()` is derived with `TERM_PROGRAM` precedence, so it can differ from raw `WT_SESSION`. - Real-world validation in this issue: in both `cmd.exe` and PowerShell, `TERM`/`TERM_PROGRAM`/`COLORTERM` were absent, so TERM-based capability hints were unavailable in those sessions. - Checking `FORCE_COLOR` for presence rather than parsing its value is a simplification. In practice `supports-color` has already parsed it, so our check is a coarse "did the user set _anything_?" gate. The effective color level still comes from `supports-color`. - When `WT_SESSION` is present without `FORCE_COLOR`, we promote to truecolor regardless of `stdout_level` (including `Unknown`). This is aggressive but correct: `WT_SESSION` is a strong signal that we're in Windows Terminal. - ANSI-16 add/delete backgrounds (bright green/red) overpower syntax-highlighted token colors, making diffs harder to read. Foreground-only cues (colored text, gutter signs) preserve readability on low-color terminals. ## Architecture ``` stdout_color_level() ──┐ terminal_info().name ──┤ WT_SESSION presence ──┼──▶ diff_color_level_for_terminal() ──▶ DiffColorLevel FORCE_COLOR presence ──┘ │ ▼ RichDiffColorLevel::from_diff_color_level() │ ┌──────────┴──────────┐ │ Some(TrueColor|256) │ → bg tints │ None (Ansi16) │ → fg only └─────────────────────┘ ``` `diff_color_level()` is the environment-reading entry point; it gathers the four runtime signals and delegates to the pure, testable `diff_color_level_for_terminal()`. ## Observability No new logs or metrics. Incorrect color selection is immediately visible as broken diff rendering; the test suite covers the decision matrix exhaustively. ## Tests Six new unit tests exercise every branch of `diff_color_level_for_terminal`: | Test | Inputs | Expected | |------|--------|----------| | `windows_terminal_promotes_ansi16_to_truecolor_for_diffs` | Ansi16 + WindowsTerminal name | TrueColor | | `wt_session_promotes_ansi16_to_truecolor_for_diffs` | Ansi16 + WT_SESSION only | TrueColor | | `non_windows_terminal_keeps_ansi16_diff_palette` | Ansi16 + WezTerm | Ansi16 | | `wt_session_promotes_unknown_color_level_to_truecolor` | Unknown + WT_SESSION | TrueColor | | `explicit_force_override_keeps_ansi16_on_windows_terminal` | Ansi16 + WindowsTerminal + FORCE_COLOR | Ansi16 | | `explicit_force_override_keeps_ansi256_on_windows_terminal` | Ansi256 + WT_SESSION + FORCE_COLOR | Ansi256 | | `ansi16_add_style_uses_foreground_only` | Dark + Ansi16 | fg=Green, bg=None | | (and any other new snapshot/assertion tests from commitsd757feeandd7c78b3) | | | ## Test plan - [x] Verify all new unit tests pass (`cargo test -p codex-tui --lib`) - [x] On Windows Terminal: confirm diffs render with truecolor backgrounds - [x] On Windows Terminal with `FORCE_COLOR` set: confirm promotion is disabled and output follows the forced `supports-color` level - [x] On macOS/Linux terminals: confirm no behavior change Fixes https://github.com/openai/codex/issues/12904 Fixes https://github.com/openai/codex/issues/12890 Fixes https://github.com/openai/codex/issues/12912 Fixes https://github.com/openai/codex/issues/12840