mirror of
https://github.com/openai/codex.git
synced 2026-05-04 11:26:33 +00:00
98923e53cc
## Problem The `ansi`, `base16`, and `base16-256` syntax themes are designed to emit ANSI palette colors so that highlighted code respects the user's terminal color scheme. Syntect encodes this intent in the alpha channel of its `Color` struct — a convention shared with `bat` — but `convert_style` was ignoring it entirely, treating every foreground color as raw RGB. This caused ANSI-family themes to produce hard-coded RGB values (e.g. `Rgb(0x02, 0, 0)` instead of `Green`), defeating their purpose and rendering them as near-invisible dark colors on most terminals. Reported in #12890. ## Mental model Syntect themes use a compact encoding in their `Color` struct: | `alpha` | Meaning of `r` | Mapped to | |---------|----------------|-----------| | `0x00` | ANSI palette index (0–255) | `RtColor::Black`…`Gray` for 0–7, `Indexed(n)` for 8–255 | | `0x01` | Unused (sentinel) | `None` — inherit terminal default fg/bg | | `0xFF` | True RGB red channel | `RtColor::Rgb(r, g, b)` | | other | Unexpected | `RtColor::Rgb(r, g, b)` (silent fallback) | This encoding is a bat convention that three bundled themes rely on. The new `convert_syntect_color` function decodes it; `ansi_palette_color` maps indices 0–7 to ratatui's named ANSI variants. | macOS - Dark | macOS - Light | Windows - ansi | Windows - base16 | |---|---|---|---| | <img width="1064" height="1205" alt="macos-dark" src="https://github.com/user-attachments/assets/f03d92fb-b44b-4939-b2b9-503fde133811" /> | <img width="1073" height="1227" alt="macos-light" src="https://github.com/user-attachments/assets/2ecb2089-73b5-4676-bed8-e4e6794250b4" /> |  |  | ## Non-goals - Background color decoding — we intentionally skip backgrounds to preserve the terminal's own background. The decoder supports it, but `convert_style` does not apply it. - Italic/underline changes — those remain suppressed as before. - Custom `.tmTheme` support for ANSI encoding — only the bundled themes use this convention. ## Tradeoffs - The alpha-channel encoding is an undocumented bat/syntect convention, not a formal spec. We match bat's behavior exactly, trading formality for ecosystem compatibility. - Indices 0–7 are mapped to ratatui's named variants (`Black`, `Red`, …, `Gray`) rather than `Indexed(0)`…`Indexed(7)`. This lets terminals apply bold/bright semantics to named colors, which is the expected behavior for ANSI themes, but means the two representations are not perfectly round-trippable. ## Architecture All changes are in `codex-rs/tui/src/render/highlight.rs`, within the style-conversion layer between syntect and ratatui: ``` syntect::highlighting::Color └─ convert_syntect_color(color) [NEW — alpha-dispatch] ├─ a=0x00 → ansi_palette_color() [NEW — index→named/indexed] ├─ a=0x01 → None (terminal default) ├─ a=0xFF → Rgb(r,g,b) (standard opaque path) └─ other → Rgb(r,g,b) (silent fallback) ``` `convert_style` delegates foreground mapping to `convert_syntect_color` instead of inlining the `Rgb(r,g,b)` conversion. The core highlighter is refactored into `highlight_to_line_spans_with_theme` (accepts an explicit theme reference) so tests can highlight against specific themes without mutating process-global state. ### ANSI-family theme contract The ANSI-family themes (`ansi`, `base16`, `base16-256`) rely on upstream alpha-channel encoding from two_face/syntect. We intentionally do **not** validate this contract at runtime — if the upstream format changes, the `ansi_themes_use_only_ansi_palette_colors` test catches it at build time, long before it reaches users. A runtime warning would be unactionable noise. ### Warning copy cleanup User-facing warning messages were rewritten for clarity: - Removed internal jargon ("alpha-encoded ANSI color markers", "RGB fallback semantics", "persisted override config") - Dropped "syntax" prefix from "syntax theme" — users just think "theme" - Downgraded developer-only diagnostics (duplicate override, resolve fallback) from `warn` to `debug` ## Observability - The `ansi_themes_use_only_ansi_palette_colors` test enforces the ANSI-family contract at build time. - The snapshot test provides a regression tripwire for palette color output. - User-facing warnings are limited to actionable issues: unknown theme names and invalid custom `.tmTheme` files. ## Tests - **Unit tests for each alpha branch:** `alpha=0x00` with low index (named color), `alpha=0x00` with high index (`Indexed`), `alpha=0x01` (terminal default), unexpected alpha (falls back to RGB), ANSI white → Gray mapping. - **Integration test:** `ansi_family_themes_use_terminal_palette_colors_not_rgb` — highlights a Rust snippet with each ANSI-family theme and asserts zero `Rgb` foreground colors appear. - **Snapshot test:** `ansi_family_foreground_palette_snapshot` — records the exact set of unique foreground colors each ANSI-family theme produces, guarding against regressions. - **Warning validation tests:** verify user-facing warnings for missing custom themes, invalid `.tmTheme` files, and bundled theme resolution. ## Test plan - [ ] `cargo test -p codex-tui` passes all new and existing tests - [ ] Select `ansi`, `base16`, or `base16-256` theme and verify code blocks render with terminal palette colors (not near-black RGB) - [ ] Select a standard RGB theme (e.g. `dracula`) and verify no regression in color output