fix(renderer): column-align wide-glyph cells in libghostty-vt snapshots#118
Merged
Conversation
mapNativeCells packed one SnapshotCell per native cell record and dropped the native col/width, so a width-2 glyph (CJK/emoji) became a single entry with no spacer for its trailing column. Index-as-column consumers — the Session Dashboard projection (which pins libghostty-vt) and its cursor-cell highlight — then shifted every cell after the glyph one column left. Pack cells column-indexed instead: place each record at its true column and emit an empty spacer for a wide glyph's trailing column (and any gap), mirroring the ghostty-web backend, which already emits one cell per column. No protocol-schema change; visibleLines text was already correct. Adds regression tests for the producer (mocked records plus the real native engine, guarded by availability) and the dashboard projection. Closes #112 Change-Id: Ie1e757983e51ec60ff26ae423ad07a0865000828 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
Change-Id: Ib18c233c8a00d8b9ef3d46b48e39cc4de0b7abbf Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Kosiewski <tk@coder.com>
Member
Author
|
@codex review |
Member
Author
|
@claude review |
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.
Fixes #112.
Problem
Wide characters (CJK/emoji, native cell
width: 2) misaligned per-cell rendering: in a line containing a wide glyph, every cell after the glyph shifted one column left, and the dashboard cursor-cell highlight landed on the wrong cell.I verified the report against the source and reproduced it against the real native engine (in-process, no host needed):
The native engine emits one width-2 record and no record for the trailing column.
mapNativeCellspacked one array entry per record and dropped the nativecol/width, so the packed array was one entry short andcells[col]no longer matched terminal columncolpast the glyph.Root cause
src/renderer/libghosttyVt/backend.ts—mapNativeCellsdiscardedcol/width.src/dashboard/liveViewProjection.ts—SnapshotGrid.cellAtindexescells[col]as the terminal column, and the cursor flag comparessourceCol === cursorCol(a true column). The dashboard pinslibghostty-vt(PRD [codex] Harden darwin runtime checks #29/feat: replace ESLint and Prettier with Oxc tooling #71, ADR 0006), so it hit this by default for any screen containing wide glyphs.Fix (no protocol-schema change)
Pack cells column-indexed in
mapNativeCells: place each record at its truecoland emit an empty spacer for every trailing column a wide glyph covers (and defensively for any gap). The spacer carries the glyph's styling so the trailing half shades correctly. This reuses the exactcol/widththe native engine already provides (no text-width measurement needed) and matches theghostty-webbackend, which already emits one cell per column — so the two renderers are now consistent and the consumer's index-as-column assumption is correct.visibleLinestext was already right.This deliberately avoids option (a) from the issue (adding
col/widthto the publicSnapshotCellschema); that remains a possible future addition if a consumer ever needs explicit width.Verification
LibghosttyVtBackend.snapshot({includeCells:true})— the same pathagent-tty snapshot --include-cells --renderer libghostty-vtand the dashboard use — feeding a real🚀and asserting the glyph sits at its true column with an empty spacer after it.A漢字B→Bstays at true col 5, previously off-by-2).projectLiveViewtest asserts a wide-glyph row renders column-aligned and the cursor highlights its true column (the bug highlighted the next cell).npm run typecheck,npm run lint,npm run format:check, full unit suite (1212 passed), andnpm run buildall pass locally. The e2e/integration host-dependent suites run in CI.Existing cell tests used only ASCII/width-1 records (and the projection test helper built cells by code-point index), which is why this slipped through.
🤖 Generated with Claude Code