a73x

1a8338f1

fix: a highlight edge inside a wide cell no longer shifts the row

a73x   2026-08-22 13:13

Commit message
fix: a highlight edge inside a wide cell no longer shifts the row

Dragging across 漢字 made the text move: every glyph right of the pointer
shifted by a column, and shifted back as the pointer moved on.

A wide cell is two columns, and a drag stops wherever the hand stopped, so
a span can cut one in half. ghostty resolves a partial wide cell by
emitting the WHOLE character — reaching back a column when a selection
starts on a spacer tail, and emitting both columns when one ends on a wide
cell. That is right for a selection dumped on its own. `dumpVtRowSpan`
cuts the row into three pieces and each resolves its own edge, so the
character straddling a cut was emitted TWICE and the row came out a column
wider than the grid:

    from=2 to=2:  ab \x1b[3G [7m漢[0m \x1b[4G 漢字cd
                              ^^^^         ^^^^

`snapWide` widens the span to whole characters before the row is cut, so
each character belongs to exactly one piece. Outwards, not inwards: half a
character under the pointer means the character is under the pointer, and
the copy already agreed — a drag from a spacer tail to a wide cell copies
`漢字テス`, whole glyphs at both ends.

The docstring claimed "nothing here walks a cell"; that assumption was the
bug, and one cell is now inspected at each edge.

Found by hand at the trial rig, which is what a hands-on gate is for. The
regression test walks every span of a row against the row itself — 400
boundary pairs over narrow, wide and spacer-head layouts — and catches a
neutered `snapWide` at 38 of them.

That regression test is one of two, because the bug's real lesson is
that no assertion this feature had could see it. Every one was a
substring search over the escape bytes the painter emitted, and a row
that emits the same glyph twice still contains every substring the
painter is supposed to emit. So the unit test replays what the client
WROTE through a fresh engine and reads the row back, turning an
assertion about bytes into one about the screen those bytes make; and
the e2e gains the first selection leg to use `assert_converged` at all —
the client's screen against the daemon's own, through a separate engine.
That leg clicks the highlight away before detaching, which buys the
byte-exact half of the check and with it the assertion that an overlay
leaves nothing behind when it goes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiryvVYESzBqqtf5cH1uoA
src/engine.zig
Old New
@@ -362,6 +362,42 @@ pub const Engine = struct {
362 std.debug.assert(y < self.term.rows); 362 std.debug.assert(y < self.term.rows);
363 return self.formatSelection(alloc, "\x1b[0m", self.viewportRows(y, y)); 363 return self.formatSelection(alloc, "\x1b[0m", self.viewportRows(y, y));
364 } 364 }
365 /// Widen a column span to whole characters.
366 ///
367 /// A wide cell is two columns — the character in the first, a spacer
368 /// tail in the second — and a drag stops wherever the hand stopped, so
369 /// a span may cut one in half. ghostty resolves a partial wide cell by
370 /// emitting the WHOLE character: ghostty's formatter reaches back a
371 /// column when a selection STARTS on a spacer tail, and a selection
372 /// ENDING on a wide cell still emits both of that cell's columns.
373 ///
374 /// Harmless for a selection dumped on its own, wrong here, because
375 /// `dumpVtRowSpan` cuts the row into three pieces and each resolves its
376 /// own edge independently — so a character straddling a cut is emitted
377 /// by BOTH pieces and the row lands one column wider than the grid.
378 /// Every glyph to the right of the pointer then shifts as the highlight
379 /// moves, which is how this was found: by dragging across 漢字.
380 ///
381 /// Snapping outwards rather than inwards because a highlight is a thing
382 /// a hand drew: half a character under the pointer means the character
383 /// is under the pointer.
384 fn snapWide(self: *Engine, y: u16, from: u16, to: u16) struct { from: u16, to: u16 } {
385 const screen = self.term.screens.active;
386 const last: u16 = @intCast(self.term.cols - 1);
387 var lo = from;
388 var hi = to;
389 if (lo > 0) {
390 if (screen.pages.pin(.{ .viewport = .{ .x = lo, .y = y } })) |p| {
391 if (p.rowAndCell().cell.wide == .spacer_tail) lo -= 1;
392 }
393 }
394 if (hi < last) {
395 if (screen.pages.pin(.{ .viewport = .{ .x = hi, .y = y } })) |p| {
396 if (p.rowAndCell().cell.wide == .wide) hi += 1;
397 }
398 }
399 return .{ .from = lo, .to = hi };
400 }
365 401
366 /// `dumpVtRow`, with grid columns [from, to] painted inverted. 402 /// `dumpVtRow`, with grid columns [from, to] painted inverted.
367 /// 403 ///
@@ -392,24 +428,27 @@ pub const Engine = struct {
392 std.debug.assert(from <= to); 428 std.debug.assert(from <= to);
393 std.debug.assert(to < self.term.cols); 429 std.debug.assert(to < self.term.cols);
394 const last: u16 = @intCast(self.term.cols - 1); 430 const last: u16 = @intCast(self.term.cols - 1);
431 const snapped = self.snapWide(y, from, to);
432 const lo = snapped.from;
433 const hi = snapped.to;
395 434
396 var aw: std.Io.Writer.Allocating = .init(alloc); 435 var aw: std.Io.Writer.Allocating = .init(alloc);
397 defer aw.deinit(); 436 defer aw.deinit();
398 try aw.writer.writeAll("\x1b[0m"); 437 try aw.writer.writeAll("\x1b[0m");
399 if (from > 0) try self.writeSelection(&aw.writer, .vt, self.viewportSpan(y, 0, from - 1)); 438 if (lo > 0) try self.writeSelection(&aw.writer, .vt, self.viewportSpan(y, 0, lo - 1));
400 try aw.writer.print("\x1b[{d}G\x1b[0m\x1b[7m", .{from + 1}); 439 try aw.writer.print("\x1b[{d}G\x1b[0m\x1b[7m", .{lo + 1});
401 // `trim = false`: a drag past the end of a short line selects the 440 // `trim = false`: a drag past the end of a short line selects the
402 // blanks after it, and a highlight that stopped at the last glyph 441 // blanks after it, and a highlight that stopped at the last glyph
403 // would be narrower than the one the hand made. 442 // would be narrower than the one the hand made.
404 try self.writeSelection( 443 try self.writeSelection(
405 &aw.writer, 444 &aw.writer,
406 .{ .emit = .plain, .trim = false }, 445 .{ .emit = .plain, .trim = false },
407 self.viewportSpan(y, from, to), 446 self.viewportSpan(y, lo, hi),
408 ); 447 );
409 try aw.writer.writeAll("\x1b[0m"); 448 try aw.writer.writeAll("\x1b[0m");
410 if (to < last) { 449 if (hi < last) {
411 try aw.writer.print("\x1b[{d}G", .{to + 2}); 450 try aw.writer.print("\x1b[{d}G", .{hi + 2});
412 try self.writeSelection(&aw.writer, .vt, self.viewportSpan(y, to + 1, last)); 451 try self.writeSelection(&aw.writer, .vt, self.viewportSpan(y, hi + 1, last));
413 } 452 }
414 return try aw.toOwnedSlice(); 453 return try aw.toOwnedSlice();
415 } 454 }
@@ -1570,3 +1609,62 @@ test "Engine: clearing side events frees their payloads" {
1570 test { 1609 test {
1571 std.testing.refAllDecls(@This()); 1610 std.testing.refAllDecls(@This());
1572 } 1611 }
1612
1613 test "engine: a span boundary inside a wide cell does not shift the row" {
1614 const alloc = std.testing.allocator;
1615 var a = try Engine.init(alloc, .{ .cols = 20, .rows = 2 });
1616 defer a.deinit();
1617 // Two wide cells with narrow text either side: every boundary in the
1618 // middle of the row lands either on a wide cell's first half or on its
1619 // spacer tail, which is the distinction under test.
1620 a.feed("ab\u{6f22}\u{5b57}cd");
1621
1622 try expectSpansAgree(alloc, a, 20);
1623
1624 // The third `Wide` variant: a wide character that does not fit before
1625 // the right edge leaves a SPACER HEAD in the last column and moves
1626 // itself to the next row. The formatter skips a row whose span starts
1627 // on one, so a boundary there is the case that blanks a row rather
1628 // than widening it.
1629 var c = try Engine.init(alloc, .{ .cols = 20, .rows = 3 });
1630 defer c.deinit();
1631 c.feed("aaaaaaaaaaaaaaaaaaa\u{6f22}");
1632 try expectSpansAgree(alloc, c, 20);
1633 }
1634
1635 /// Every span of row 0 must repaint that row exactly as it stands: a span
1636 /// changes which columns are INVERTED and nothing else, so the row's plain
1637 /// text through a fresh engine has to come back identical whatever the
1638 /// boundaries were. Row 0 only — a wide character that wrapped to row 1 is
1639 /// not what this paints.
1640 fn expectSpansAgree(alloc: std.mem.Allocator, eng: *Engine, cols: u16) !void {
1641 const full = try eng.dumpPlain(alloc);
1642 defer alloc.free(full);
1643 const want = firstLine(full);
1644
1645 var bad: usize = 0;
1646 var from: u16 = 0;
1647 while (from < cols) : (from += 1) {
1648 var to: u16 = from;
1649 while (to < cols) : (to += 1) {
1650 const span = try eng.dumpVtRowSpan(alloc, 0, from, to);
1651 defer alloc.free(span);
1652 var b = try Engine.init(alloc, .{ .cols = cols, .rows = @intCast(eng.term.rows) });
1653 defer b.deinit();
1654 b.feed("\x1b[1;1H\x1b[2K");
1655 b.feed(span);
1656 const got_full = try b.dumpPlain(alloc);
1657 defer alloc.free(got_full);
1658 const got = firstLine(got_full);
1659 if (!std.mem.eql(u8, want, got)) {
1660 bad += 1;
1661 std.debug.print("from={d} to={d}\n want |{s}|\n got |{s}|\n", .{ from, to, want, got });
1662 }
1663 }
1664 }
1665 try std.testing.expectEqual(@as(usize, 0), bad);
1666 }
1667
1668 fn firstLine(text: []const u8) []const u8 {
1669 return text[0 .. std.mem.indexOfScalar(u8, text, '\n') orelse text.len];
1670 }
src/interact.zig
Old New
@@ -3295,7 +3295,12 @@ fn dragFixture(alloc: std.mem.Allocator, out_fd: std.posix.fd_t) !Core {
3295 // exactly the promoted-tile case, and the reason an unzoomed tile 3295 // exactly the promoted-tile case, and the reason an unzoomed tile
3296 // never reaches any of this. 3296 // never reaches any of this.
3297 _ = core.claimTerminal(); 3297 _ = core.claimTerminal();
3298 core.rep.eng.feed("row-zero\r\nrow-one\r\nrow-two\r\nrow-three"); 3298 // Row 4 carries wide cells. Kept OFF the rows the column assertions
3299 // above use, rather than folded into them: a wide glyph shifts every
3300 // column to its right, so putting one in row 1 would have meant
3301 // re-deriving fifteen hand-checked column numbers — churn that hides
3302 // regressions instead of catching them.
3303 core.rep.eng.feed("row-zero\r\nrow-one\r\nrow-two\r\nrow-three\r\nw\u{6f22}\u{5b57}x");
3299 return core; 3304 return core;
3300 } 3305 }
3301 3306
@@ -3567,6 +3572,40 @@ test "interact: a drag repaints the rows it changed, not the screen" {
3567 try std.testing.expect(std.mem.indexOf(u8, painted, "\x1b[2;1H\x1b[2K") != null); 3572 try std.testing.expect(std.mem.indexOf(u8, painted, "\x1b[2;1H\x1b[2K") != null);
3568 } 3573 }
3569 3574
3575 test "interact: a drag edge inside a wide cell repaints the row unshifted" {
3576 const alloc = std.testing.allocator;
3577 const p = try std.posix.pipe2(.{ .NONBLOCK = true });
3578 defer std.posix.close(p[0]);
3579 defer std.posix.close(p[1]);
3580 var core = try dragFixture(alloc, p[1]);
3581 defer core.deinit();
3582 var tr: NullTransport = .{};
3583 var buf: [16384]u8 = undefined;
3584 _ = drainPipe(p[0], &buf); // the claim
3585
3586 // Row 4 is `w漢字x`: column 2 is the spacer tail of 漢 and column 3 the
3587 // first half of 字, so BOTH edges of this drag fall inside a wide cell.
3588 _ = try core.forward(&tr, "\x1b[<0;3;5M");
3589 _ = try core.forward(&tr, "\x1b[<32;4;5M");
3590 const painted = drainPipe(p[0], &buf);
3591
3592 // Snapped outwards to whole glyphs: the inversion opens at column 2
3593 // (0-based 1, where 漢 starts), not at the column the pointer stopped in.
3594 try std.testing.expect(std.mem.indexOf(u8, painted, "\x1b[2G\x1b[0m\x1b[7m") != null);
3595
3596 // The oracle, and the reason this test exists at all: replay what the
3597 // client actually WROTE through a fresh engine and read the row back.
3598 // Asserting escape bytes cannot see this class of bug — a row that
3599 // emits 漢 twice still contains every substring the painter is supposed
3600 // to emit, and lands a column wider than the grid.
3601 var screen = try Engine.init(alloc, .{ .cols = 20, .rows = 6 });
3602 defer screen.deinit();
3603 screen.feed(painted);
3604 const plain = try screen.dumpPlain(alloc);
3605 defer alloc.free(plain);
3606 try std.testing.expect(std.mem.indexOf(u8, plain, "w\u{6f22}\u{5b57}x") != null);
3607 }
3608
3570 test "interact: only the left button drags" { 3609 test "interact: only the left button drags" {
3571 const alloc = std.testing.allocator; 3610 const alloc = std.testing.allocator;
3572 const p = try std.posix.pipe2(.{ .NONBLOCK = true }); 3611 const p = try std.posix.pipe2(.{ .NONBLOCK = true });
test/e2e.sh
Old New
@@ -1549,6 +1549,7 @@ cleanup() {
1549 rm -f "$OUT.zsa" "$OUT.zsa.err" "$OUT.zsb" "$OUT.zsb.err" "$OUT.zscap" \ 1549 rm -f "$OUT.zsa" "$OUT.zsa.err" "$OUT.zsb" "$OUT.zsb.err" "$OUT.zscap" \
1550 "$OUT.zscap.err" "$OUT.zs.d" "$OUT.zsfa" "$OUT.zsfb" "$OUT.zspc" \ 1550 "$OUT.zscap.err" "$OUT.zs.d" "$OUT.zsfa" "$OUT.zsfb" "$OUT.zspc" \
1551 "$OUT.zssta" "$OUT.zsstb" "$OUT.zsstop" "$OUT.zswatch" 1551 "$OUT.zssta" "$OUT.zsstb" "$OUT.zsstop" "$OUT.zswatch"
1552 rm -f "$OUT.swcap" "$OUT.swcap.err" "$OUT.swpc"
1552 rm -f "$SOCK50" "$OUT.wmse.d" "$OUT.wmsa" "$OUT.wmsa.err" "$OUT.wmsb" \ 1553 rm -f "$SOCK50" "$OUT.wmse.d" "$OUT.wmsa" "$OUT.wmsa.err" "$OUT.wmsb" \
1553 "$OUT.wmsb.err" "$OUT.wmcap" "$OUT.wmcap.err" "$OUT.wmpc" \ 1554 "$OUT.wmsb.err" "$OUT.wmcap" "$OUT.wmcap.err" "$OUT.wmpc" \
1554 "$OUT.wmcap2" "$OUT.wmcap2.err" "$OUT.wmpc2" "$OUT.wmfa" "$OUT.wmfb" \ 1555 "$OUT.wmcap2" "$OUT.wmcap2.err" "$OUT.wmpc2" "$OUT.wmfa" "$OUT.wmfb" \
@@ -7763,6 +7764,62 @@ set -e
7763 SELCOPIES=$(grep -ao "$(printf '\033]52;')" "$OUT.selcap" | wc -l) 7764 SELCOPIES=$(grep -ao "$(printf '\033]52;')" "$OUT.selcap" | wc -l)
7764 [ "$SELCOPIES" = "1" ] || { 7765 [ "$SELCOPIES" = "1" ] || {
7765 echo "e2e FAIL: drag copy: $SELCOPIES OSC 52 writes on the client's tty, expected 1"; exit 1; } 7766 echo "e2e FAIL: drag copy: $SELCOPIES OSC 52 writes on the client's tty, expected 1"; exit 1; }
7767 # ---- a drag over WIDE cells leaves the client's screen converged -------
7768 #
7769 # The gate the wide-cell shift walked straight through. Every other
7770 # assertion this feature has is either on the daemon's state or on the
7771 # ESCAPE BYTES the painter emitted — and a row that emits the same wide
7772 # glyph TWICE still contains every substring the painter is supposed to
7773 # emit, while landing a column wider than the grid. Only a check that
7774 # judges the client's screen against the daemon's own, through a separate
7775 # engine, can see that: which is assert_converged, and which no selection
7776 # leg used until this one.
7777 #
7778 # Both edges of the drag land inside a wide cell on purpose. Row 3 is
7779 # `w漢字x-WIDEMARK`, so column 3 is the spacer tail of 漢 and column 4 the
7780 # first half of 字 — the two ways a span can cut a wide cell in half.
7781 #
7782 # The DEFAULT session, unlike the copy leg above: `converged_quiet` dumps
7783 # the daemon's default grid, so a leg that named a session would diff this
7784 # client's screen against a grid it never attached to and fail for a reason
7785 # that has nothing to do with wide cells.
7786 #
7787 # Asserted to fail, per the wan.sh rule: with `snapWide` stubbed out to
7788 # return its arguments, this leg renders `w 漢字x-WIDEMARK` against the
7789 # daemon's `w漢字x-WIDEMARK` — the extra column, which is the bug.
7790 #
7791 # The click after the drag is what makes the STYLED half of this check
7792 # mean something. A live highlight is reverse video the daemon has no
7793 # notion of, so a leg that detached still holding one would diverge on
7794 # style every time and could only ever assert the plain grid. Clearing it
7795 # first buys the byte-exact comparison, and with it the assertion that a
7796 # highlight leaves nothing behind when it goes — the overlay-never-becomes-
7797 # state rule, on the only overlay that paints through the replica.
7798 set +e
7799 timeout 60 "$PTYCLIENT" --cols 40 --rows 12 --out "$OUT.swcap" --err "$OUT.swcap.err" -- \
7800 "$MUX" --sock "$SOCK50" > "$OUT.swpc" 2>&1 <<'EOF'
7801 settle 900 20000
7802 send printf '\\033[2J\\033[3;1Hw\\346\\274\\242\\345\\255\\227x-WIDEMARK'\n
7803 expect WIDEMARK 15000
7804 settle 700 20000
7805 send \x1b[<0;3;3M\x1b[<32;4;3M\x1b[<0;4;3m
7806 settle 700 20000
7807 send \x1b[<0;1;3M\x1b[<0;1;3m
7808 settle 700 20000
7809 send \x1cd
7810 waitexit 10000
7811 EOF
7812 RC=$?
7813 set -e
7814 [ "$RC" -eq 0 ] || {
7815 echo "e2e FAIL: wide drag: ptyclient leg exited $RC:"
7816 # The client's own stderr as well as the fixture's: the fixture can only
7817 # report THAT the client closed the pty, and the reason is over here.
7818 cat "$OUT.swpc" "$OUT.swcap.err"; exit 1; }
7819 assert_converged "$OUT.swcap" "$SOCK50" "wide drag" 40 12
7820 ok "a drag across wide cells leaves the client's screen converged"
7821
7822
7766 assert_stopped "$SOCK50" "$D48PID" "wall mouse" "$OUT.wmstop" 7823 assert_stopped "$SOCK50" "$D48PID" "wall mouse" "$OUT.wmstop"
7767 D48PID="" 7824 D48PID=""
7768 ok "a drag copies on release, and a click copies nothing" 7825 ok "a drag copies on release, and a click copies nothing"
@@ -7783,7 +7840,9 @@ DPID=""
7783 7840
7784 # The pins. Literals, not variables set from counting something else — 7841 # The pins. Literals, not variables set from counting something else —
7785 # "assert the literal, never the constant the code under test reads" 7842 # "assert the literal, never the constant the code under test reads"
7786 # (decisions.md, M10). 43 scenario checkpoints; 35 convergence points. 7843 # (decisions.md, M10). 64 scenario checkpoints; 36 convergence points.
7844 # (The first of these two numbers had drifted to 43 while the pin below
7845 # said 63 — prose is not gated, which is why the pin is.)
7787 # Anyone adding a scenario updates these by hand, on purpose. 7846 # Anyone adding a scenario updates these by hand, on purpose.
7788 # 7847 #
7789 # M18 added three checkpoints and no convergence points: its wall block 7848 # M18 added three checkpoints and no convergence points: its wall block
@@ -7887,13 +7946,21 @@ DPID=""
7887 # are last in the file rather than beside the other wall legs on purpose: 7946 # are last in the file rather than beside the other wall legs on purpose:
7888 # the ordinals in this paragraph are positions, so a scenario inserted in 7947 # the ordinals in this paragraph are positions, so a scenario inserted in
7889 # the middle renumbers every sentence after it. 7948 # the middle renumbers every sentence after it.
7890 [ "$OK_COUNT" = "63" ] || { 7949 #
7891 echo "e2e FAIL: $OK_COUNT scenario checkpoints ran, the pin says 63 —" 7950 # The 64th is the drag across WIDE cells, and it IS a convergence point —
7951 # the first one this feature has. Everything else it asserts is the
7952 # daemon's state or the painter's own escape bytes, and neither can see a
7953 # row that emits the same wide glyph twice: the substrings a painter is
7954 # supposed to emit are all still present, and the row is a column too wide.
7955 # That shipped and was found by hand. This is the check that would have
7956 # caught it.
7957 [ "$OK_COUNT" = "64" ] || {
7958 echo "e2e FAIL: $OK_COUNT scenario checkpoints ran, the pin says 64 —"
7892 echo " a scenario was added (update the pin) or silently lost" 7959 echo " a scenario was added (update the pin) or silently lost"
7893 exit 1 7960 exit 1
7894 } 7961 }
7895 [ "$CONV_COUNT" = "35" ] || { 7962 [ "$CONV_COUNT" = "36" ] || {
7896 echo "e2e FAIL: $CONV_COUNT convergence points ran, the pin says 35" 7963 echo "e2e FAIL: $CONV_COUNT convergence points ran, the pin says 36"
7897 exit 1 7964 exit 1
7898 } 7965 }
7899 echo "e2e OK (63 scenarios, 35 convergence points)" 7966 echo "e2e OK (64 scenarios, 36 convergence points)"