fix(ui): theme rendering bugs surfaced by colorblind-light - #160
Merged
Merged
Conversation
The highlighted line styles set only background and rely on chroma to paint per-token fg inline. The +/-/~ prefix is prepended outside the chroma path, so the prefix bytes have no fg ANSI applied and fall through to the terminal default fg. On light themes (e.g. colorblind-light) in kitty sessions with light terminal default fg, this makes the markers near-invisible against the light add/remove backgrounds. Add Resolver.LineFg helper (mirrors LineBg) plus ColorKeyModifyLineFg for the collapsed-mode ~ prefix. Introduce Model.wrapPrefixForHighlight that injects raw truecolor ANSI fg + ResetFg only when chroma highlighting is active. Apply at all three highlighted-line render sites: styleDiffContent in diffview.go and both collapsed-mode paths in collapsed.go.
revdiff's raw-ANSI helpers in style.ansiColor always emit truecolor escape sequences, but lipgloss respects the termenv-detected color profile, which can downgrade to ANSI256/ANSI/Ascii based on TERM and COLORTERM. Inside tmux or terminals where detection regresses, lipgloss renders pane borders and other styled elements in degraded colors while raw-ANSI paths (line prefix wrap, overlay title injection) keep truecolor. The mismatch shows up as washed-out pane borders that don't reflect the theme's accent and border colors and indistinct active vs inactive panes on light themes. Force lipgloss to truecolor at startup when colors are enabled, so both paths emit identical ANSI and rendered output matches the theme.
formatEntry returns " " + swatch + " " + name for non-selected rows. The swatch wraps its glyph in fg + reset, so after the swatch the trailing name has no fg ANSI applied and falls through to the terminal default fg. On light themes rendered in a session whose terminal default fg is light (e.g. kitty default light scheme), this makes non-selected theme names near-invisible against the popup's light background. Wrap name in raw normal-fg ANSI + ResetFg so it renders in the theme's configured normal foreground, matching the contrast the selected row gets from StyleKeyFileSelected.
color-border was #595959 (luminance ~0.10), close to color-accent #0031a9 (~0.07). At thin border widths active vs inactive panes appeared nearly identical on light bg. Bump border to #b0b0b0 so inactive panes recede visually and the deep-navy accent stands out, matching the contrast level of catppuccin-latte. The colorblind-friendly aspect (no red/green pair) is preserved.
There was a problem hiding this comment.
Pull request overview
Fixes multiple theme rendering issues revealed by the new colorblind-light theme, primarily around foreground coloring on syntax-highlighted lines and ensuring consistent truecolor rendering across lipgloss and raw-ANSI code paths.
Changes:
- Add
Resolver.LineFg+ColorKeyModifyLineFg, and use them to explicitly color diff prefixes (+/-/~) when chroma highlighting is active. - Force lipgloss to render using a truecolor profile when colors are enabled to avoid profile-detection downgrades (e.g., in tmux).
- Fix theme selector rendering where non-selected theme names could inherit terminal-default foreground and become invisible on light themes; adjust
colorblind-lightborder color for better contrast.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| themes/gallery/colorblind-light | Raises border luminance to improve inactive pane contrast on light backgrounds. |
| app/ui/style/resolver_test.go | Adds unit coverage for the new Resolver.LineFg behavior. |
| app/ui/style/resolver.go | Introduces LineFg and adds ColorKeyModifyLineFg mapping. |
| app/ui/style/enums.go | Extends the color key generator input with colorKeyModifyLineFg. |
| app/ui/style/color_key_enum.go | Updates generated enum mappings to include ModifyLineFg. |
| app/ui/overlay/themeselect.go | Ensures non-selected theme names render in the theme’s normal fg rather than terminal default. |
| app/ui/model.go | Extends styleResolver interface to include LineFg. |
| app/ui/mocks/style_resolver.go | Updates the generated mock to implement/capture LineFg calls. |
| app/ui/diffview.go | Adds wrapPrefixForHighlight and applies it to highlighted diff rendering. |
| app/ui/collapsed.go | Applies prefix foreground wrapping to collapsed-mode add/modify rendering (incl. wrapped lines). |
| app/main.go | Forces lipgloss color profile to termenv.TrueColor when colors are enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if modified { | ||
| bgColor = m.resolver.Color(style.ColorKeyModifyLineBg) | ||
| prefixFg = m.resolver.Color(style.ColorKeyModifyLineFg) | ||
| } |
There was a problem hiding this comment.
In collapsed mode, when a line is a search match you switch to StyleKeySearchMatch and unset the foreground for the highlighted path, but the prefix fg is still taken from add/modify (LineFg(ChangeAdd) or ColorKeyModifyLineFg). This makes the gutter prefix color differ between highlighted vs non-highlighted search matches, and it also contradicts the intent that search matches render with search styling instead of add/modify styling. Consider deriving the prefix fg from the search-match style (e.g., keep the search fg for the prefix even when unsetting foreground for chroma content, or introduce a raw ColorKeySearchFg similar to ColorKeyNormalFg and use it here when isSearchMatch).
Suggested change
| } | |
| } | |
| if isSearchMatch { | |
| prefixFg = lineStyle.GetForeground() | |
| } |
Addresses Copilot's review on PR #160. When a line in collapsed mode is a search match and chroma highlighting is on, the previous fix wrapped the +/~ prefix in the add-fg or modify-fg color, mismatching the search-match background. The non-highlighted path uses the SearchMatch lipgloss style (fg + bg), so the prefix renders in search-fg there. With highlighting on, the lipgloss style drops the foreground for chroma to own content fg, so the prefix wrap must inject search-fg explicitly to match. Add ColorKeySearchFg (mirrors the existing ColorKeyAddLineFg / ModifyLineFg pattern) and use it in renderCollapsedAddLine when isSearchMatch is true.
Deploying revdiff with
|
| Latest commit: |
69439ce
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://99471d4a.revdiff.pages.dev |
| Branch Preview URL: | https://fix-prefix-fg-on-highlighted.revdiff.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four related fixes for theme-rendering bugs surfaced while reviewing the new
colorblind-lighttheme in #157.1.
+/-/~prefix invisible on syntax-highlighted lines (fc3e092)The highlighted line styles set only background and rely on chroma to paint per-token foreground inline. The
+/-/~prefix is prepended outside the chroma path, so the prefix bytes have no foreground ANSI applied and fall through to the terminal default fg. On light themes rendered in a kitty session with light terminal default fg, the markers were near-invisible against the light add/remove backgrounds.Adds
Resolver.LineFg(mirrors existingLineBg) plusColorKeyModifyLineFgfor the collapsed-mode~prefix. NewModel.wrapPrefixForHighlightinjects raw truecolor ANSI fg + ResetFg only when chroma highlighting is active. Applied at all three render sites:styleDiffContentinapp/ui/diffview.go, plus both collapsed-mode paths inapp/ui/collapsed.go.2. Force lipgloss to truecolor profile (
052ff80)revdiff's raw-ANSI helpers in
style.ansiColoralways emit truecolor. Lipgloss respects the termenv-detected profile, which can downgrade to ANSI256/ANSI/Ascii based onTERM/COLORTERM. Inside tmux or terminals where detection regresses, lipgloss-rendered pane borders looked washed out while raw-ANSI paths kept truecolor, leaving active vs inactive panes hard to distinguish on light themes.lipgloss.SetColorProfile(termenv.TrueColor)at startup (gated on!opts.NoColors) unifies both paths.3. Wrap theme list non-selected names in normal fg ANSI (
f3b5cfd)formatEntryinapp/ui/overlay/themeselect.goreturned" " + swatch + " " + namefor non-selected rows. The swatch wraps its glyph in fg + reset, so the trailingnamehad no fg ANSI and inherited terminal default fg, which is invisible on light kitty sessions.Wrap
namein rawColorKeyNormalFg+ResetFgso it renders in the theme's configured normal fg.4. Bump
colorblind-lightborder luminance (1213940)Theme had
color-border = #595959(luminance ~0.10), perceptually close tocolor-accent = #0031a9(~0.07). At thin border widths the eye reads brightness more than hue, so active vs inactive panes still appeared similar after fix #2. Bump border to#b0b0b0so inactive panes recede visually. The colorblind-friendly aspect (no red/green pair) is preserved.Tests, race, and lint all green. Verified visually in kitty against
colorblind-light,colorblind-dark,catppuccin-latte, anddracula.