fix: render OSC 4 palette overrides instead of forwarding the index - #2162
fix: render OSC 4 palette overrides instead of forwarding the index#2162hamidi-dev wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe change adds Ghostty default palette retrieval and applies only OSC 4-modified palette entries during full-frame and dirty-patch rendering. Changed indexed colors become RGB colors. Unchanged indexed and direct RGB colors retain their existing representation. ChangesPalette-aware terminal rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Renderer
participant Terminal
participant Ghostty
participant PaletteOverrides
Renderer->>Terminal: request default_palette()
Terminal->>Ghostty: query default 256-entry palette
Ghostty-->>Terminal: return palette or error
Renderer->>PaletteOverrides: compare active and default palettes
PaletteOverrides-->>Renderer: return changed palette entries
Renderer->>Renderer: convert changed indexed colors to RGB
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR renders palette indices changed through OSC 4 as explicit RGB values while continuing to forward untouched indices to the host terminal.
Confidence Score: 5/5The PR appears safe to merge, with palette overrides correctly resolved and palette-only changes forcing a redraw. The new FFI getter matches libghostty's documented array contract, active/default comparison isolates runtime overrides, and all relevant cell-style and redraw paths apply the new resolution.
|
| Filename | Overview |
|---|---|
| src/ghostty/mod.rs | Adds an ABI-compatible getter for the terminal's default 256-entry RGB palette. |
| src/pane/terminal.rs | Detects runtime palette overrides, resolves affected indexed cell colors to RGB across full and dirty rendering, and adds focused unit tests. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Cell palette index] --> B{Active entry differs<br/>from default palette?}
B -- Yes: OSC 4 override --> C[Emit explicit RGB]
B -- No --> D[Emit indexed color]
D --> E[Host terminal resolves theme color]
Reviews (1): Last reviewed commit: "fix: render OSC 4 palette overrides inst..." | Re-trigger Greptile
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pane/terminal.rs (1)
3088-3135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd integration coverage for both rendering paths.
The tests validate
PaletteOverridesandghostty_cell_colorin isolation. They do not exerciseTerminal::default_palette(),render(), orcollect_dirty_patch(). Add one full-frame and one dirty-patch test for an OSC 4 override. Assert that the changed index renders asColor::Rgband an unchanged index remainsColor::Indexed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f5002ed-7923-4e50-b266-f18df9c006a3
📒 Files selected for processing (2)
src/ghostty/mod.rssrc/pane/terminal.rs
What's broken
A cell carrying a palette index is emitted to the host terminal as that index, so the host resolves it against its own palette. When a program in a pane redefines an entry with OSC 4, libghostty parses and stores it —
palette_color_query_responseeven answers OSC 4 queries with the new value — but the redefinition never reaches the screen.Repro
In a herdr pane this renders in the host's colour for index 18; outside herdr it's lavender.
Found via hamidi-dev/opentab#12, reported by SimonEisenhauer — a curses TUI that themes itself with
init_color. Every theme rendered identically inside herdr while other TUIs looked fine, because those emit truecolor SGR and take theCellColor::Rgbpath. I've shipped a workaround on my side, but since this is the root cause I thought I'd submit a PR to fix it here too.Fix
Compare the render state's active palette against
COLOR_PALETTE_DEFAULTto find exactly the entries a program overrode, and emit those as RGB. Every other index is still forwarded, so panes that never use OSC 4 render as before and keep following the host terminal's theme.Resolving every index would be simpler, but it would replace the host's palette for users whose terminal doesn't answer herdr's colour queries — their theme would silently become libghostty's default. Only resolving real overrides can't regress anyone.