Answer sixel handshake queries so apps emit and size sixel correctly#77
Merged
Conversation
A sixel-capable app (e.g. Terminal.Gui's UICatalog Mandelbrot scenario) only emits sixel after a terminal handshake. tuirec answered DA1 but with the form ESC [ ? 62 ; 4 c. Apps that detect support by splitting the response on ";" and looking for the token "4" (Terminal.Gui's SixelSupportDetector) saw "4c" and never enabled sixel. tuirec also did not answer the window/cell geometry queries (CSI 14/16/18 t), so the app could not determine the screen size (and drew nothing) or the cell size (sixel fell back to 1px per cell), and the rendered raster overflowed its on-screen cells. Changes: - DA1 response is now ESC [ ? 62 ; 4 ; 6 ; 22 c (sixel attribute mid-list, as real terminals report it). - The interceptor answers CSI 16 t (cell size), 18 t (text-area in chars), and 14 t (window in pixels). - The reported cell size is derived from the agg font-size/line-height (row = fontSize*lineHeight, column ~= 0.6*fontSize) so the app's sixel raster matches the cells agg renders. - Unit tests in pkg/record cover the DA1 token and the geometry replies. Docs: update agent guidance (RECORDING-AGENT.md, agent-guide.md) and README to describe sixel support, the Linux/macOS-only constraint (Windows ConPTY strips sixel DCS), the --trim=false requirement for alternate-screen apps, and how to record a Terminal.Gui sixel scenario. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80d9cb3388
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
record.Run accepts a zero-valued Config (pty.Start and gif.Render fill in defaults later), but the sixel interceptor was built from the raw config, so a zero Config produced cellW/cellH and rows/cols of 0. Apps that sent CSI 16/14/18 t then got reports like CSI 6;0;0t or CSI 8;0;0t, defeating the handshake (no layout, or a 0x0 raster) even though the PTY used default dimensions. Add sixelGeometry, which normalizes the terminal size and GIF font config with the same defaults pty.Start and gif.Render apply, and derive the reports from it. Exports pty.NormalizeSize and gif.NormalizeConfig so the geometry mirrors the size/metrics actually used. Covered by a unit test asserting a zero Config yields 120x30 cells and an 8x18 px cell. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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.
Summary
A sixel-capable TUI only emits sixel after a terminal handshake, and the rendered raster is sized from the cell metrics the terminal reports. tuirec answered DA1 but in a form many apps reject, and never answered the window/cell geometry queries ? so apps either never enabled sixel, drew nothing (no screen size), or emitted a raster that overflowed its on-screen cells.
Verified end-to-end: real Terminal.Gui UICatalog
Mandelbrotscenario ->tuirec(these fixes) -> sixel-awareagg-> the Mandelbrot renders in the GIF, fitting its view.What was wrong
ESC [ ? 62 ; 4 c. Apps that detect sixel by splitting the reply on;and checking for the token4(e.g. Terminal.Gui'sSixelSupportDetector:response.Split(";").Contains("4")) saw the token4cand never enabled sixel.CSI 14/16/18 t. Without18 t(text-area size) Terminal.Gui's ANSI driver could not determine the screen size and drew nothing; without16 t(cell size) sixel fell back to 1px/cell.Changes
ESC [ ? 62 ; 4 ; 6 ; 22 c(sixel attribute mid-list, as real terminals report it).CSI 16 t(cell size),CSI 18 t(text area in chars), andCSI 14 t(window in pixels).fontSize*lineHeight, column ~=0.6*fontSize) so the app's raster matches the cells agg renders.RECORDING-AGENT.md,agent-guide.md) andREADME.mdupdated: sixel support, the Linux/macOS-only constraint,--trim=falsefor alternate-screen apps, and how to record a Terminal.Gui sixel scenario.Test plan
go test ./pkg/record/? new unit tests cover the DA1 token (TestDA1ResponseAdvertisesSixelAsDelimitedToken) and the geometry replies (TestInterceptorAnswersCellSizeQuery,TestInterceptorAnswersTextAreaQuery); existing sixel pipeline tests still pass.Mandelbrot(--args Mandelbrot --trim=false, quit withEsc); cast holds 67+ sixel DCS payloads with Terminal.Gui's palette; the sixel-aware agg renders the Mandelbrot fitting its view.Known follow-ups (not in this PR)
--trimdiscards alternate-screen recordings. With trim enabled (default), a recording whose content is entirely inside the alternate screen is trimmed to nothing;--trim=falseis the current workaround. Worth a dedicated fix.agg(asciinema/agg sixel support).Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com