a73x

5db58b82

fix: CellRowWriter owns each cell's text until the run closes

a73x   2026-09-04 16:59

Commit message
fix: CellRowWriter owns each cell's text until the run closes

The writer holds a run's cells back until the style changes, so the ascii
decision is made over the whole run. It held the caller's text SLICE in
that list, which is only correct while the caller keeps every cell's bytes
alive — and the daemon encoder renders each cell into one reused stack
buffer, so every cell in a run would have emitted the last cell's glyph.

The run list now stores an offset into a writer-owned text buffer, and
`deinit` frees both so an encoder can errdefer it mid-row. `cell` also
asserts the style does not carry bit 15, which is the run's ascii marker
on the wire and never a ghostty style flag.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CsWfuJFQbTfGtKZLS5qw4q
src/engine/protocol.zig
Old New
@@ -1312,13 +1312,20 @@ fn asciiCell(wide: Wide, text: []const u8) bool {
1312 } 1312 }
1313 1313
1314 pub const CellRowWriter = struct { 1314 pub const CellRowWriter = struct {
1315 /// One held-back cell. The text is an offset into the writer's own `text`
1316 /// buffer, never the caller's slice: an encoder renders every cell into
1317 /// one reused buffer, and a run outlives the call that added its cells.
1318 const RunCell = struct { wide: Wide, text_off: u32, text_len: u8 };
1319
1315 list: *std.ArrayList(u8), 1320 list: *std.ArrayList(u8),
1316 alloc: std.mem.Allocator, 1321 alloc: std.mem.Allocator,
1317 prefix_at: usize, 1322 prefix_at: usize,
1318 ncells: u16 = 0, 1323 ncells: u16 = 0,
1319 /// The open run's cells, held back until the run closes so the ascii 1324 /// The open run's cells, held back until the run closes so the ascii
1320 /// decision is made over the whole run. 1325 /// decision is made over the whole run.
1321 run: std.ArrayListUnmanaged(DecodedCell) = .empty, 1326 run: std.ArrayListUnmanaged(RunCell) = .empty,
1327 /// The open run's cell texts, back to back.
1328 text: std.ArrayListUnmanaged(u8) = .empty,
1322 run_style: CellStyle = .{}, 1329 run_style: CellStyle = .{},
1323 1330
1324 pub fn begin(list: *std.ArrayList(u8), alloc: std.mem.Allocator) !CellRowWriter { 1331 pub fn begin(list: *std.ArrayList(u8), alloc: std.mem.Allocator) !CellRowWriter {
@@ -1329,18 +1336,27 @@ pub const CellRowWriter = struct {
1329 1336
1330 pub fn cell(self: *CellRowWriter, style: CellStyle, wide: Wide, text: []const u8) !void { 1337 pub fn cell(self: *CellRowWriter, style: CellStyle, wide: Wide, text: []const u8) !void {
1331 std.debug.assert(text.len <= cell_text_max); 1338 std.debug.assert(text.len <= cell_text_max);
1339 // Bit 15 is the run's own ascii marker on the wire, so a style may not
1340 // carry it: ghostty's Style.Flags only reaches bit 10.
1341 std.debug.assert(style.flags & run_ascii == 0);
1332 if (self.run.items.len > 0 and !style.eql(self.run_style)) try self.flush(); 1342 if (self.run.items.len > 0 and !style.eql(self.run_style)) try self.flush();
1333 if (self.run.items.len == 0) self.run_style = style; 1343 if (self.run.items.len == 0) self.run_style = style;
1334 try self.run.append(self.alloc, .{ .style = style, .wide = wide, .text = text }); 1344 const off: u32 = @intCast(self.text.items.len);
1345 try self.text.appendSlice(self.alloc, text);
1346 try self.run.append(self.alloc, .{ .wide = wide, .text_off = off, .text_len = @intCast(text.len) });
1335 self.ncells += 1; 1347 self.ncells += 1;
1336 } 1348 }
1337 1349
1350 fn cellText(self: *const CellRowWriter, c: RunCell) []const u8 {
1351 return self.text.items[c.text_off..][0..c.text_len];
1352 }
1353
1338 fn flush(self: *CellRowWriter) !void { 1354 fn flush(self: *CellRowWriter) !void {
1339 const cells = self.run.items; 1355 const cells = self.run.items;
1340 if (cells.len == 0) return; 1356 if (cells.len == 0) return;
1341 var ascii = true; 1357 var ascii = true;
1342 for (cells) |c| { 1358 for (cells) |c| {
1343 if (!asciiCell(c.wide, c.text)) { 1359 if (!asciiCell(c.wide, self.cellText(c))) {
1344 ascii = false; 1360 ascii = false;
1345 break; 1361 break;
1346 } 1362 }
@@ -1353,15 +1369,26 @@ pub const CellRowWriter = struct {
1353 std.mem.writeInt(u32, hdr[12..16], self.run_style.ul, .little); 1369 std.mem.writeInt(u32, hdr[12..16], self.run_style.ul, .little);
1354 try self.list.appendSlice(self.alloc, &hdr); 1370 try self.list.appendSlice(self.alloc, &hdr);
1355 for (cells) |c| { 1371 for (cells) |c| {
1372 const text = self.cellText(c);
1356 if (ascii) { 1373 if (ascii) {
1357 try self.list.append(self.alloc, c.text[0]); 1374 try self.list.append(self.alloc, text[0]);
1358 } else { 1375 } else {
1359 const head: u8 = (@as(u8, @intFromEnum(c.wide)) << 6) | @as(u8, @intCast(c.text.len)); 1376 const head: u8 = (@as(u8, @intFromEnum(c.wide)) << 6) | c.text_len;
1360 try self.list.append(self.alloc, head); 1377 try self.list.append(self.alloc, head);
1361 try self.list.appendSlice(self.alloc, c.text); 1378 try self.list.appendSlice(self.alloc, text);
1362 } 1379 }
1363 } 1380 }
1364 self.run.clearRetainingCapacity(); 1381 self.run.clearRetainingCapacity();
1382 self.text.clearRetainingCapacity();
1383 }
1384
1385 /// Frees the writer's held-back run. Safe after `finish`, and safe twice,
1386 /// so a caller that may fail mid-row can `errdefer w.deinit()`.
1387 pub fn deinit(self: *CellRowWriter) void {
1388 self.run.deinit(self.alloc);
1389 self.run = .empty;
1390 self.text.deinit(self.alloc);
1391 self.text = .empty;
1365 } 1392 }
1366 1393
1367 /// Closes the open run and stamps ncells. The writer is spent after this. 1394 /// Closes the open run and stamps ncells. The writer is spent after this.
@@ -1370,7 +1397,7 @@ pub const CellRowWriter = struct {
1370 error.OutOfMemory => @panic("CellRowWriter.finish: out of memory"), 1397 error.OutOfMemory => @panic("CellRowWriter.finish: out of memory"),
1371 }; 1398 };
1372 std.mem.writeInt(u16, self.list.items[self.prefix_at..][0..2], self.ncells, .little); 1399 std.mem.writeInt(u16, self.list.items[self.prefix_at..][0..2], self.ncells, .little);
1373 self.run.deinit(self.alloc); 1400 self.deinit();
1374 } 1401 }
1375 }; 1402 };
1376 1403
@@ -2794,6 +2821,26 @@ test "cellrow: two default ascii cells are one ascii run" {
2794 }, list.items); 2821 }, list.items);
2795 } 2822 }
2796 2823
2824 test "cellrow: the writer owns each cell's text, so a reused caller buffer is safe" {
2825 const alloc = std.testing.allocator;
2826 var list: std.ArrayList(u8) = .empty;
2827 defer list.deinit(alloc);
2828 // An encoder renders each cell into one reused stack buffer; the run holds
2829 // its cells until the style changes, so the writer must copy the bytes
2830 // rather than remember the caller's slice — a borrowed slice would emit
2831 // the last cell's glyph for every cell of the run.
2832 var buf: [4]u8 = undefined;
2833 var w = try CellRowWriter.begin(&list, alloc);
2834 buf[0] = 'a';
2835 try w.cell(.{}, .narrow, buf[0..1]);
2836 buf[0] = 'b';
2837 try w.cell(.{}, .narrow, buf[0..1]);
2838 w.finish();
2839 var r = try CellRowReader.init(list.items);
2840 try std.testing.expectEqualStrings("a", (try r.next()).?.text);
2841 try std.testing.expectEqualStrings("b", (try r.next()).?.text);
2842 }
2843
2797 test "cellrow: a wide glyph and its spacer, a grapheme, and a styled run round-trip" { 2844 test "cellrow: a wide glyph and its spacer, a grapheme, and a styled run round-trip" {
2798 const alloc = std.testing.allocator; 2845 const alloc = std.testing.allocator;
2799 var list: std.ArrayList(u8) = .empty; 2846 var list: std.ArrayList(u8) = .empty;