fix(tui): theme-aware diff backgrounds with fallback behavior#13037
fix(tui): theme-aware diff backgrounds with fallback behavior#13037etraut-openai merged 5 commits intoopenai:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the TUI diff renderer to respect syntax theme-provided insert/delete background scopes (with sensible fallbacks), and to avoid unreadable background blocks on ANSI-16 terminals by switching to foreground-only styling.
Changes:
- Extract diff/markup scope background RGBs from the active syntect theme (
markup.*preferred,diff.*fallback). - Resolve diff line backgrounds once per render using theme scopes when available; quantize RGBs for ANSI-256 and disable backgrounds entirely for ANSI-16 via
RichDiffColorLevel. - Improve terminal color-level detection with Windows Terminal promotion rules and add unit + snapshot coverage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
codex-rs/tui/src/render/highlight.rs |
Adds theme-scope background extraction helpers and tests for bundled/custom theme behavior. |
codex-rs/tui/src/diff_render.rs |
Threads resolved backgrounds through style helpers, adds ANSI-256 quantization, ANSI-16 foreground-only behavior, and Windows Terminal promotion logic with tests/snapshots. |
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__theme_scope_background_resolution.snap |
New snapshot covering theme-scope override resolution output. |
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__ansi16_insert_delete_no_background.snap |
New snapshot verifying ANSI-16 renders without insert/delete backgrounds. |
codex-rs/core/src/terminal.rs |
Documentation note clarifying that TERM_PROGRAM can mask later probes like WT_SESSION. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use syntax theme scope backgrounds for diff insert/delete rows when available in rich color modes, preferring `markup.*` and then `diff.*` scopes. Keep ANSI16 behavior foreground-only and fall back to the existing palette when no matching scope background exists. Add focused coverage for scope resolution, ansi256 quantization, and snapshot output, plus built-in and custom `.tmTheme` background resolution paths.
Document how diff backgrounds are resolved from syntax theme scopes and how fallback colors are applied per color level. Clarify helper invariants around rich color modes, ANSI-256 quantization, and ANSI-16 foreground-only behavior.
33a8cda to
a6813f7
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Precompute `DiffRenderStyleContext` once per render pass and thread it through diff line render helpers. This avoids repeated theme and diff-background resolution for each rendered preview line. Update `diff_render` tests to call the context-aware helpers directly and remove legacy wrappers, so tests exercise the same rendering path used in production.
Problem
The TUI diff renderer uses hardcoded background palettes for insert/delete lines that don't respect the user's chosen syntax theme. When a theme defines
markup.inserted/markup.deletedscope backgrounds (the convention used by GitHub, Solarized, Monokai, and most VS Code themes), those colors are ignored — the diff always renders with the same green/red tints regardless of theme selection.Separately, ANSI-16 terminals (and Windows Terminal sessions misreported as ANSI-16) rendered diff backgrounds as full-saturation blocks that obliterated syntax token colors, making highlighted diffs unreadable.
Mental model
Diff backgrounds are resolved in three layers:
Color level detection —
diff_color_level_for_terminal()maps the rawsupports-colorprobe + Windows Terminal heuristics to aDiffColorLevel(TrueColor / Ansi256 / Ansi16). Windows Terminal gets promoted from Ansi16 to TrueColor whenWT_SESSIONis present.Background resolution —
resolve_diff_backgrounds()queries the active syntax theme formarkup.inserted/markup.deleted(falling back todiff.inserted/diff.deleted), then overlays those on top of the hardcoded palette. For ANSI-256, theme RGB values are quantized to the nearest xterm-256 index. For ANSI-16, backgrounds areNone(foreground-only).Style composition — The resolved
ResolvedDiffBackgroundsis threaded through every call tostyle_add,style_del,style_sign_*, andstyle_line_bg_for, which decide how to compose foreground+background for each line kind and theme variant.A new
RichDiffColorLeveltype (a subset ofDiffColorLevelwithout Ansi16) encodes the invariant "we have enough depth for tinted backgrounds" at the type level, so background-producing functions have exhaustive matches without unreachable arms.Non-goals
highlight_code_to_styled_spanspath.DiffThemeis still determined by querying the terminal's background color.Tradeoffs
Nonebackgrounds (foreground-only) is intentionally conservative.resolve_diff_backgroundsis called once perrender_changeinvocation (i.e., once per file in a diff). If the theme changes mid-render (live preview), the next file picks up the new backgrounds.Architecture
Files changed:
tui/src/render/highlight.rsDiffScopeBackgroundRgbs,diff_scope_background_rgbs(), scope extraction helperstui/src/diff_render.rsRichDiffColorLevel,ResolvedDiffBackgrounds,resolve_diff_backgrounds*,quantize_rgb_to_ansi256, Windows Terminal promotion; modified: all style helpers to accept/threadResolvedDiffBackgroundsThe scope-extraction code lives in
highlight.rsbecause it usessyntect::highlighting::Highlighterand the theme singleton. The resolution and quantization logic lives indiff_render.rsbecause it depends on diff-specific types (DiffTheme,DiffColorLevel, ratatuiColor).Observability
No runtime logging was added. The most useful debugging aid is the
diff_color_level_for_terminalfunction, which is pure and fully unit-tested — to diagnose a color-depth mismatch, log its four inputs (StdoutColorLevel,TerminalName,WT_SESSIONpresence,FORCE_COLORpresence).Scope resolution can be tested by loading a custom
.tmThemewith knownmarkup.inserted/markup.deletedbackgrounds and checking the diff output in a truecolor terminal.Tests
diff_color_level_for_terminal(ANSI-16 promotion,WT_SESSIONunconditional promotion,FORCE_COLORsuppression, conservativeUnknownlevel).style_add,style_del,style_sign_*,style_line_bg_for, andstyle_gutter_forall returnNonebackgrounds on ANSI-16.markup.*preference overdiff.*,Nonewhen no scope matches, bundled theme resolution, and custom.tmThemeround-trip.ansi16_insert_delete_no_background,theme_scope_background_resolution) lock visual output.