feat(render): draw the cursor in screenshots - #86
Open
aymanbagabas wants to merge 9 commits into
Open
Conversation
A screenshot and a color assertion disagreed about what a cell was painted. `render/svg.rs` carried a private sixteen-color table and `assert/color.rs` carried a different one, so `expect --fg "#800000"` passed on a cell the screenshot drew `#e88388`. Both tables are deleted here and both callers resolve through one profile, which is what makes them agree by construction rather than by coincidence. The palette had to become configurable to fix it anyway: the two tables could only be collapsed by choosing which one was right, and that choice belongs to the user rather than to whichever module was read first. The shipped default is the VGA/xterm palette that `TERM=xterm-256color` already promises, which is what the assertion side used. A profile is read from `shell-use.toml` and sets scrollback and colors. Only the sixteen ANSI slots and the three defaults are configurable; indices 16-255 are the xterm color cube and gray ramp, which are fixed by the spec, so a config that could move them would let two sessions disagree about what `--fg 196` means. Profiles are named, and `--profile` selects one. The file is looked up nearest first, project before user, so a repository can pin the terminal its tests expect. Resolution happens in the CLI rather than the daemon: the daemon is long-lived and shared, so it has no single working directory to resolve a project-local config against, and a resolved profile travels on `Request::Open` the same way timeouts already do. Absent settings take the default, and the field is `#[serde(default)]`, so a client that predates this keeps the behavior it had. Scrollback moves from a hardcoded 5,000 to a configurable 10,000, matching alacritty's own default. Two things are deliberately errors rather than silent fallbacks: an unknown profile name, which reports the ones that exist, and a config file that does not parse, which would otherwise run the session with settings nobody asked for. A *missing* file stays fine, since running without one is normal. Screenshots will look different: the default background is now black rather than the previous dark blue-gray, and the palette is saturated rather than muted. Both are recoverable in a profile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
Programs ask the terminal what color it is before deciding whether to draw for a light or a dark background. Nothing answered, so every one of them blocked until it timed out and guessed. The emulator answers now, through `take_pending_writes`, which already exists for exactly this: the replies a terminal owes to device queries. alacritty parses the sequence, tracks what a program set, and hands back a formatter with the query's own prefix and terminator already captured, so the only missing piece was the color itself. That comes from the session profile, which the emulator is now constructed with. The alternative was parsing the sequences off the PTY stream, the way shell integration is tracked. That would have meant reimplementing color parsing, the runtime table, reply formatting, and terminator tracking, all of which the emulator already does — and getting the terminator wrong, since it is only visible to whoever parsed the sequence. Reading what the emulator already knows is both less code and more faithful. Colors resolve in three layers: what a program set, else the session profile, else the table the specification defines. A reset clears only the first, so the profile is unreachable from the byte stream and there is always something to restore. That is what the specification asks for, describing a reset as restoring "the color specified by the corresponding X resource". `Emulator` gains `color(slot)`, which every backend answers from its own state, plus a `palette()` snapshot of all 259 slots. The screenshot renderer and `expect --fg/--bg` take that snapshot rather than the emulator, so neither holds the session lock while it renders, and neither knows which backend produced the colors. Five conformance cases cover queries, terminator echo, set-then-reset, unconfigured indices, and that a cell follows whatever its slot now holds. They run against every backend, so a future one cannot answer differently. An end-to-end test drives a real program through the whole path: it reads the configured background, sets its own, resets, and gets the configured one back. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
Follow-up to the previous commit, which routed color resolution through a `Palette` snapshot the session passed around. That put the fallback in the wrong place: the emulator reported only what a program had set, and every consumer had to know how to fill in the rest. The emulator now takes the session profile at construction and answers `color(slot)` for any slot, mapping its own table onto the profile when nothing has overridden it. Consumers ask the emulator and get a color, with no second layer to consult. `Palette` is gone. The 256-color table above the sixteen configurable slots is a static built at compile time rather than arithmetic run per lookup. It is the same in every terminal, so computing it repeatedly only invited the two implementations of it to drift. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
The profile addressed colors by index, using 256, 257, 258 for the foreground, background, and cursor. That is alacritty's layout — it stores the dynamic colors after the palette — and it had spread into the profile, the emulator trait, and the conformance suite, none of which have any reason to know it. A backend that numbered its own table differently would have had to pretend otherwise. Slots are a `ColorSlot` enum now: `Indexed(u8)`, `Foreground`, `Background`, `Cursor`. The alacritty backend translates that to its own indices inside the one match that reads its table, and resolves an unset slot to the profile, falling back to the xterm table for an index the profile does not name. The tests for that resolution moved to the backend that performs it, where they exercise the real path rather than a helper that mirrored it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
A snapshot stores the palette slot a cell chose rather than the color that slot resolves to, which is what lets a saved baseline outlive a profile change: recoloring a terminal would otherwise invalidate every snapshot in a suite at once. That was already true and nothing checked it, so a change to how colors are serialized could have quietly made snapshots profile-dependent. The companion case pins the exception: a true-color cell names its own color, so that one is recorded literally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
…fter The color tests leaned almost entirely on `OSC 11`. That is the sequence programs actually reach for, but it meant the foreground and cursor were never set, never queried, and never reset, so wiring any of them to the wrong slot would have gone unnoticed. Three conformance cases now set all three to distinct colors and check that each reset frees only its own, that each answers its own query, and that a bare `OSC 104` resets the palette without touching them. Nothing covered the path from an escape sequence to a rendered pixel either. Both halves are pinned now: a screenshot paints the background a program set and recolors a cell whose slot it moved, and an assertion matches that same color while still comparing the index unchanged. Both return to the profile after a reset. They read the same state, so this is the earlier "a screenshot and an assertion agree" guarantee held at every point in a session rather than only at the start. The end-to-end test drives all three dynamic colors over a real PTY. It needs a wide terminal: its report is one line, and `text` returns the grid, so a narrower one wrapped the reply out of the assertion's reach. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
A screenshot showed the grid but never where the terminal was about to write, so a reader could not tell an editor's caret position, whether a program had hidden the cursor, or which mode it was in. The emulator now reports visibility (`DECTCEM`) and shape (`DECSCUSR`), and the renderer draws the cursor after the text pass: a block fills the cell, an underline sits on its bottom edge, and a bar on its left. A block redraws the character beneath it in the cell's background color, so it stays readable rather than being swallowed. The redraw goes through the same path as the text pass, which keeps two cases right that a naive redraw gets wrong: a double-width character is covered across both of its cells instead of being clipped and squashed into one, and a nerd font glyph comes back as a vector glyph rather than as a character the text font has no glyph for. Blink is deliberately not represented; a screenshot is a single moment, and a blinking cursor is drawn in the half of the cycle where it shows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
This was referenced Aug 4, 2026
aymanbagabas
marked this pull request as ready for review
August 4, 2026 21:07
Member
Author
The probe puts its own terminal in raw mode so it can read a reply that arrives without a newline and must not be echoed. That needs `termios`, which Windows CPython does not ship, so the test could only ever fail there, and a fail-fast matrix let it cancel the other two platforms. Nothing about the reply is platform specific. Its format is covered by conformance cases that run against every backend, and the write that carries it to the child is the same `pty.write` that every `type` and `submit` already exercises on Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
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.
Third of a three PR stack: #84 → #85 → #86. Only the last commit is new here; a cross-repo PR cannot be based on a fork branch, so GitHub shows the whole stack. Review
feat(render): draw the cursor in screenshots.A screenshot showed the grid but never showed where the terminal was about to write. You could not tell an editor's caret position, whether a program had hidden the cursor, or which mode it was in.
Before / after
The same session, screenshotted by the same command. The cursor is parked mid-word on
The:The emulator now reports visibility (
DECTCEM,CSI ?25 handl) and shape (DECSCUSR,CSI Ps SP q). The renderer draws the cursor after the text pass: a block fills the cell, an underline sits on its bottom edge, a bar on its left.Visibility matters as much as position. Full-screen programs hide the cursor while repainting, so a screenshot that ignored
DECTCEMwould show one parked wherever the last write happened to land.The cursor never hides content
A block redraws the character beneath it in the cell background, the way a terminal does. The redraw goes through the same path as the text pass, which keeps right two cases a naive redraw gets wrong:
日)The cursor follows a color a program sets
OSC 12sets the cursor color andOSC 112resets it, both from #85. The cursor had nothing to paint until this PR, so this is where they become visible. One session, screenshotted after each step, cursor parked on theq:OSC 12sets#ff8700OSC 112resetsThe default cursor color follows the profile foreground, so the block is the same grey as the text. The
qstays legible in all three.Notes for review
Emulator::cursoris relative to the visible screen, soscreenshot --fulloffsets it past the scrollback above.HollowBlock(what it draws for an unfocused window) maps toBlock, since a headless terminal has no notion of focus. ItsHiddenshape means the same as the mode being off.