diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 79aa96f18..a7c02bda7 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -3636,6 +3636,36 @@ pub const IncreaseCapacity = enum { grapheme_bytes, hyperlink_bytes, string_bytes, + + /// Returns the capacity dimension that must be increased for the + /// given row clone error to succeed on retry, or null if the page + /// only needs to be rehashed at its current capacity. + pub fn forCloneError(err: Page.CloneFromError) ?IncreaseCapacity { + return switch (err) { + // Rehash the sets + error.StyleSetNeedsRehash, + error.HyperlinkSetNeedsRehash, + => null, + + // Increase style memory + error.StyleSetOutOfMemory, + => .styles, + + // Increase string memory + error.StringAllocOutOfMemory, + => .string_bytes, + + // Increase hyperlink memory + error.HyperlinkSetOutOfMemory, + error.HyperlinkMapOutOfMemory, + => .hyperlink_bytes, + + // Increase grapheme memory + error.GraphemeMapOutOfMemory, + error.GraphemeAllocOutOfMemory, + => .grapheme_bytes, + }; + } }; pub const IncreaseCapacityError = error{ @@ -4315,6 +4345,57 @@ fn destroyNodeExt( pool.nodes.destroy(node); } +/// Clone the given source row into the row at `dst_y` of the given +/// node's page, increasing the node's capacity as necessary to fit the +/// source row's managed memory (styles, hyperlinks, etc.). +/// +/// Since increasing capacity replaces the node in the page list, the +/// (possibly replaced) node is returned and the caller must use it in +/// place of the old node. The source must NOT be on the given node +/// since the node's page memory may be freed on capacity increase. +fn cloneRowGrowCapacity( + self: *PageList, + node: *List.Node, + dst_y: usize, + src_page: *Page, + src_row: *const Row, +) *List.Node { + assert(src_page != node.page()); + + var current = node; + while (true) { + const cur_page = current.page(); + const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); + cur_page.cloneRowFrom( + src_page, + &cur_rows[dst_y], + src_row, + ) catch |err| { + // Adjust our page capacity to make room for what we + // didn't have space for. + current = self.increaseCapacity( + current, + IncreaseCapacity.forCloneError(err), + ) catch |e| switch (e) { + // We can't gracefully recover from either of these + // here: our callers have already rotated rows, so + // returning an error would leave the page list + // half-mutated (and corrupt), so a crash is better. + error.OutOfMemory, + => @panic("increaseCapacity system allocator OOM"), + + error.OutOfSpace, + => @panic("increaseCapacity OutOfSpace"), + }; + + // Retry the row copy with the increased capacity. + continue; + }; + + return current; + } +} + /// Fast-path function to erase exactly 1 row. Erasing means that the row /// is completely REMOVED, not just cleared. All rows following the removed /// row will be shifted up by 1 to fill the empty space. @@ -4384,9 +4465,16 @@ pub fn eraseRow( // 5 5 5 | 6 // 6 6 6 | 7 // 7 7 7 <' 4 - try page.cloneRowFrom( + // + // The copy may replace the destination node in order to + // increase its capacity. We can discard the replacement + // because we advance to the next node below, and the + // replacement is already linked in its place (so e.g. the + // `node.prev` access in the pin fixups below is correct). + _ = self.cloneRowGrowCapacity( + node, + node.rows() - 1, next_page, - &rows[node.rows() - 1], &next_rows[0], ); @@ -4542,9 +4630,15 @@ pub fn eraseRowBounded( const next_page = next.page(); const next_rows = next_page.rows.ptr(next_page.memory.ptr); - try page.cloneRowFrom( + // The copy may replace the destination node in order to + // increase its capacity. We can discard the replacement + // because we advance to the next node below, and the + // replacement is already linked in its place (so e.g. the + // `node.prev` access in the pin fixups below is correct). + _ = self.cloneRowGrowCapacity( + node, + node.rows() - 1, next_page, - &rows[node.rows() - 1], &next_rows[0], ); @@ -12359,6 +12453,227 @@ test "PageList eraseRowBounded full rows two pages" { try testing.expectEqual(@as(usize, 0), p_out.x); } +test "PageList eraseRow hyperlink-dense row crosses page boundary" { + // Regression test: when eraseRow shifts rows up across a page + // boundary, the top row of the next page is cloned into the last + // row of the previous page. If the previous page doesn't have + // enough capacity for the managed memory of that row (hyperlinks, + // styles, etc.) the error propagated out AFTER the previous page + // had already been rotated and its tracked pins moved, leaving + // the page list half-mutated. + // + // eraseRow must instead increase the destination page's capacity + // and retry, the same way insertLines/deleteLines and + // cursorScrollAbove handle their cross-page copies. + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 10, null); + defer s.deinit(); + + // Grow to two pages so our active area straddles them: the first + // page is exactly full and the second page holds the last 5 rows + // of the active area. + { + const page = s.pages.last.?.page(); + page.pauseIntegrityChecks(true); + for (0..page.capacity.rows - page.size.rows) |_| _ = try s.grow(); + page.pauseIntegrityChecks(false); + try s.growRows(5); + try testing.expectEqual(@as(usize, 2), s.totalPages()); + try testing.expectEqual(@as(usize, 5), s.pages.last.?.rows()); + } + + // Mark each active row with a codepoint so we can verify the + // shift afterwards. Row y gets codepoint '0' + y at x = 0. + for (0..10) |y| { + const row_pin = s.pin(.{ .active = .{ .y = @intCast(y) } }).?; + row_pin.rowAndCell().cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast('0' + y) }, + }; + } + + // Fill the top row of the second page (active y=5) with more + // unique hyperlinks ('A' through 'J') than the first page's + // default hyperlink capacity can hold. We must increase the + // second page's capacity to even create such a row; the first + // page keeps its default capacity. + const link_count: usize = 10; + while (s.pages.last.?.page().hyperlink_set.layout.cap <= link_count) { + _ = try s.increaseCapacity(s.pages.last.?, .hyperlink_bytes); + } + try testing.expect(s.pages.first.?.page().hyperlink_set.layout.cap < link_count); + { + const page = s.pages.last.?.page(); + for (0..link_count) |x| { + var buf: [64]u8 = undefined; + const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{x}); + const id = try page.insertHyperlink(.{ + .id = .{ .implicit = @intCast(x) }, + .uri = uri, + }); + const rac = page.getRowAndCell(x, 0); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast('A' + x) }, + }; + try page.setHyperlink(rac.row, rac.cell, id); + page.hyperlink_set.use(page.memory, id); + } + } + + // Track a pin in the shifted region of the first page to verify + // it survives the capacity change of its node. + const p = try s.trackPin(s.pin(.{ .active = .{ .x = 3, .y = 1 } }).?); + defer s.untrackPin(p); + + // Erase the first active row. The dense hyperlink row must cross + // the page boundary into the first page, which requires growing + // the first page's hyperlink capacity. + try s.eraseRow(.{ .active = .{ .y = 0 } }); + + // Every remaining row shifted up by one: the '0' marker row was + // erased, the dense row moved up across the page boundary to + // row 4, and the last row was cleared. + const expected = [10]u21{ '1', '2', '3', '4', 'A', '6', '7', '8', '9', 0 }; + for (expected, 0..) |cp, y| { + const list_cell = s.getCell(.{ .active = .{ .y = @intCast(y) } }).?; + try testing.expectEqual(cp, list_cell.cell.content.codepoint); + } + + // Every cell of the dense row must still resolve to a real + // hyperlink entry with the correct URI. A half-applied erase + // leaves cells whose hyperlink flag is set but that have no map + // entry, which aborts in clearCells later. + for (0..link_count) |x| { + const list_cell = s.getCell(.{ .active = .{ + .x = @intCast(x), + .y = 4, + } }).?; + try testing.expect(list_cell.cell.hyperlink); + const page: *Page = list_cell.node.page(); + const id = page.lookupHyperlink(list_cell.cell).?; + const link = page.hyperlink_set.get(page.memory, id); + var buf: [64]u8 = undefined; + const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{x}); + try testing.expectEqualStrings(uri, link.uri.slice(page.memory)); + } + + // All pages must pass integrity checks. + var node_: ?*List.Node = s.pages.first; + while (node_) |node| : (node_ = node.next) node.page().assertIntegrity(); + + // Our tracked pin shifted up by one row and still points into + // the (possibly replaced) first page. + try testing.expectEqual(s.pages.first.?, p.node); + const p_pt = s.pointFromPin(.active, p.*).?.active; + try testing.expectEqual(@as(u32, 3), p_pt.x); + try testing.expectEqual(@as(u32, 0), p_pt.y); +} + +test "PageList eraseRowBounded hyperlink-dense row crosses page boundary" { + // Same as the eraseRow variant above but for eraseRowBounded, + // which has the same rotate-then-clone structure and had the + // same bug: a cross-page row clone failure propagated out after + // the first page had already been rotated. + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 10, null); + defer s.deinit(); + + // Grow to two pages so our active area straddles them: the first + // page is exactly full and the second page holds the last 5 rows + // of the active area. + { + const page = s.pages.last.?.page(); + page.pauseIntegrityChecks(true); + for (0..page.capacity.rows - page.size.rows) |_| _ = try s.grow(); + page.pauseIntegrityChecks(false); + try s.growRows(5); + try testing.expectEqual(@as(usize, 2), s.totalPages()); + try testing.expectEqual(@as(usize, 5), s.pages.last.?.rows()); + } + + // Mark each active row with a codepoint so we can verify the + // shift afterwards. Row y gets codepoint '0' + y at x = 0. + for (0..10) |y| { + const row_pin = s.pin(.{ .active = .{ .y = @intCast(y) } }).?; + row_pin.rowAndCell().cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast('0' + y) }, + }; + } + + // Fill the top row of the second page (active y=5) with more + // unique hyperlinks ('A' through 'J') than the first page's + // default hyperlink capacity can hold. We must increase the + // second page's capacity to even create such a row; the first + // page keeps its default capacity. + const link_count: usize = 10; + while (s.pages.last.?.page().hyperlink_set.layout.cap <= link_count) { + _ = try s.increaseCapacity(s.pages.last.?, .hyperlink_bytes); + } + try testing.expect(s.pages.first.?.page().hyperlink_set.layout.cap < link_count); + { + const page = s.pages.last.?.page(); + for (0..link_count) |x| { + var buf: [64]u8 = undefined; + const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{x}); + const id = try page.insertHyperlink(.{ + .id = .{ .implicit = @intCast(x) }, + .uri = uri, + }); + const rac = page.getRowAndCell(x, 0); + rac.cell.* = .{ + .content_tag = .codepoint, + .content = .{ .codepoint = @intCast('A' + x) }, + }; + try page.setHyperlink(rac.row, rac.cell, id); + page.hyperlink_set.use(page.memory, id); + } + } + + // Erase the first active row with a limit that extends into the + // second page (5 rows remain in the first page, so a limit of 6 + // forces the cross-page path). The dense hyperlink row must cross + // the page boundary into the first page. + try s.eraseRowBounded(.{ .active = .{ .y = 0 } }, 6); + + // Rows within the limit shifted up by one: the '0' marker row was + // erased, the dense row moved up across the page boundary to + // row 4, row 6 is the new blank row, and rows past the limit are + // unchanged. + const expected = [10]u21{ '1', '2', '3', '4', 'A', '6', 0, '7', '8', '9' }; + for (expected, 0..) |cp, y| { + const list_cell = s.getCell(.{ .active = .{ .y = @intCast(y) } }).?; + try testing.expectEqual(cp, list_cell.cell.content.codepoint); + } + + // Every cell of the dense row must still resolve to a real + // hyperlink entry with the correct URI. A half-applied erase + // leaves cells whose hyperlink flag is set but that have no map + // entry, which aborts in clearCells later. + for (0..link_count) |x| { + const list_cell = s.getCell(.{ .active = .{ + .x = @intCast(x), + .y = 4, + } }).?; + try testing.expect(list_cell.cell.hyperlink); + const page: *Page = list_cell.node.page(); + const id = page.lookupHyperlink(list_cell.cell).?; + const link = page.hyperlink_set.get(page.memory, id); + var buf: [64]u8 = undefined; + const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{x}); + try testing.expectEqualStrings(uri, link.uri.slice(page.memory)); + } + + // All pages must pass integrity checks. + var node_: ?*List.Node = s.pages.first; + while (node_) |node| : (node_ = node.next) node.page().assertIntegrity(); +} + test "PageList clone" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 8a0da9001..85c33100f 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -575,6 +575,7 @@ pub fn clone( result.assertIntegrity(); return result; } + pub fn increaseCapacity( self: *Screen, node: *PageList.List.Node, @@ -645,6 +646,74 @@ pub fn increaseCapacity( return new_node; } +/// Clone the cells in columns [x_start, x_end) of a source row into +/// the row at `dst_y` of the given node's page, increasing the node's +/// capacity as necessary to fit the managed memory (styles, +/// hyperlinks, etc.) of the copied cells. +/// +/// This is the Screen-level analog of PageList.cloneRowGrowCapacity: +/// capacity increases are routed through Screen.increaseCapacity so +/// that the cursor's style/hyperlink references are migrated when the +/// destination node is the cursor's page. +/// +/// Since increasing capacity replaces the node in the page list, the +/// (possibly replaced) node is returned and the caller must use it in +/// place of the old node. Tracked pins are updated automatically. The +/// source must NOT be on the given node since the node's page memory +/// may be freed on capacity increase. +/// +/// Callers use this mid-mutation (after rows have been rotated or +/// shifted), so a failure can't be propagated without leaving the +/// page list half-mutated (and corrupt). If the capacity can't be +/// increased (system OOM or the page is already at max capacity), +/// this panics: a crash is better than corruption. +pub fn clonePartialRowGrowCapacity( + self: *Screen, + node: *PageList.List.Node, + dst_y: usize, + src_page: *Page, + src_row: *const Row, + x_start: usize, + x_end: usize, +) *PageList.List.Node { + assert(src_page != node.page()); + + var current = node; + while (true) { + const cur_page: *Page = current.page(); + const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); + cur_page.clonePartialRowFrom( + src_page, + &cur_rows[dst_y], + src_row, + x_start, + x_end, + ) catch |err| { + // Adjust our page capacity to make room for what we + // didn't have space for and retry the copy. + current = self.increaseCapacity( + current, + PageList.IncreaseCapacity.forCloneError(err), + ) catch |e| switch (e) { + // We can't gracefully recover from either of these + // here: our callers have already rotated or shifted + // rows, so returning an error would leave the page + // list half-mutated (and corrupt), so a crash is + // better. + error.OutOfMemory, + => @panic("increaseCapacity system allocator OOM"), + + error.OutOfSpace, + => @panic("increaseCapacity OutOfSpace"), + }; + + continue; + }; + + return current; + } +} + pub inline fn cursorCellRight(self: *Screen, n: size.CellCountInt) *pagepkg.Cell { assert(self.cursor.x + n < self.pages.cols); const cell: [*]pagepkg.Cell = @ptrCast(self.cursor.page_cell); @@ -1068,70 +1137,21 @@ fn cursorScrollAboveRotate( // Copy the last row of the previous page to the top of current. // If the current page doesn't have enough capacity for the // managed memory of the copied row (styles, hyperlinks, etc.) - // then we increase its capacity and retry, the same way that - // insertLines/deleteLines handle their cross-page copies. - while (true) { + // then its capacity is increased and the copy retried, which + // may replace `current` in the page list. Our loop condition + // guarantees `current` is never the cursor page, and `prev` is + // unaffected by the replacement. + { const prev_page = prev.page(); - const cur_page = current.page(); const prev_rows = prev_page.rows.ptr(prev_page.memory.ptr); - const cur_rows = cur_page.rows.ptr(cur_page.memory.ptr); - cur_page.cloneRowFrom( + current = self.clonePartialRowGrowCapacity( + current, + 0, prev_page, - &cur_rows[0], &prev_rows[prev_page.size.rows - 1], - ) catch |err| { - // Adjust our page capacity to make - // room for what we didn't have space for - const new_node = self.increaseCapacity( - current, - switch (err) { - // Rehash the sets - error.StyleSetNeedsRehash, - error.HyperlinkSetNeedsRehash, - => null, - - // Increase style memory - error.StyleSetOutOfMemory, - => .styles, - - // Increase string memory - error.StringAllocOutOfMemory, - => .string_bytes, - - // Increase hyperlink memory - error.HyperlinkSetOutOfMemory, - error.HyperlinkMapOutOfMemory, - => .hyperlink_bytes, - - // Increase grapheme memory - error.GraphemeMapOutOfMemory, - error.GraphemeAllocOutOfMemory, - => .grapheme_bytes, - }, - ) catch |e| switch (e) { - // See insertLines which takes the same approach for - // the same errors. We can't gracefully recover from - // either of these here: the rows have already been - // rotated, so returning an error would leave the - // page list half-mutated (and corrupt), so a crash - // is better. - error.OutOfMemory, - => @panic("increaseCapacity system allocator OOM"), - - error.OutOfSpace, - => @panic("increaseCapacity OutOfSpace"), - }; - - // increaseCapacity replaces the node in the page list - // so our iteration node must be updated to the - // replacement. - current = new_node; - - // Retry the row copy with the increased capacity. - continue; - }; - - break; + 0, + self.pages.cols, + ); } // Mark dirty on the page, since we are dirtying all rows with this. diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 014d2c8b0..bc5c9834a 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -2642,60 +2642,19 @@ pub fn insertLines(self: *Terminal, count: usize) void { // If our page doesn't match, then we need to do a copy from // one page to another. This is the slow path. if (src_p.node != dst_p.node) { - dst_p.node.page().clonePartialRowFrom( + // The copy may replace the destination node in order + // to increase its capacity. Our pins are tracked so + // they update automatically; we can discard the + // replacement because the remainder of this iteration + // only accesses rows through the pins. + _ = self.screens.active.clonePartialRowGrowCapacity( + dst_p.node, + dst_p.y, src_p.node.page(), - dst_row, src_row, self.scrolling_region.left, self.scrolling_region.right + 1, - ) catch |err| { - // Adjust our page capacity to make - // room for we didn't have space for - _ = self.screens.active.increaseCapacity( - dst_p.node, - switch (err) { - // Rehash the sets - error.StyleSetNeedsRehash, - error.HyperlinkSetNeedsRehash, - => null, - - // Increase style memory - error.StyleSetOutOfMemory, - => .styles, - - // Increase string memory - error.StringAllocOutOfMemory, - => .string_bytes, - - // Increase hyperlink memory - error.HyperlinkSetOutOfMemory, - error.HyperlinkMapOutOfMemory, - => .hyperlink_bytes, - - // Increase grapheme memory - error.GraphemeMapOutOfMemory, - error.GraphemeAllocOutOfMemory, - => .grapheme_bytes, - }, - ) catch |e| switch (e) { - // System OOM. We have no way to recover from this - // currently. We should probably change insertLines - // to raise an error here. - error.OutOfMemory, - => @panic("increaseCapacity system allocator OOM"), - - // The page can't accommodate the managed memory required - // for this operation. We previously just corrupted - // memory here so a crash is better. The right long - // term solution is to allocate a new page here - // move this row to the new page, and start over. - error.OutOfSpace, - => @panic("increaseCapacity OutOfSpace"), - }; - - // Continue the loop to try handling this row again. - continue; - }; + ); } else { if (!left_right) { // Swap the src/dst cells. This ensures that our dst gets the @@ -2843,53 +2802,19 @@ pub fn deleteLines(self: *Terminal, count: usize) void { // If our page doesn't match, then we need to do a copy from // one page to another. This is the slow path. if (src_p.node != dst_p.node) { - dst_p.node.page().clonePartialRowFrom( + // The copy may replace the destination node in order + // to increase its capacity. Our pins are tracked so + // they update automatically; we can discard the + // replacement because the remainder of this iteration + // only accesses rows through the pins. + _ = self.screens.active.clonePartialRowGrowCapacity( + dst_p.node, + dst_p.y, src_p.node.page(), - dst_row, src_row, self.scrolling_region.left, self.scrolling_region.right + 1, - ) catch |err| { - // Adjust our page capacity to make - // room for we didn't have space for - _ = self.screens.active.increaseCapacity( - dst_p.node, - switch (err) { - // Rehash the sets - error.StyleSetNeedsRehash, - error.HyperlinkSetNeedsRehash, - => null, - - // Increase style memory - error.StyleSetOutOfMemory, - => .styles, - - // Increase string memory - error.StringAllocOutOfMemory, - => .string_bytes, - - // Increase hyperlink memory - error.HyperlinkSetOutOfMemory, - error.HyperlinkMapOutOfMemory, - => .hyperlink_bytes, - - // Increase grapheme memory - error.GraphemeMapOutOfMemory, - error.GraphemeAllocOutOfMemory, - => .grapheme_bytes, - }, - ) catch |e| switch (e) { - // See insertLines - error.OutOfMemory, - => @panic("increaseCapacity system allocator OOM"), - - error.OutOfSpace, - => @panic("increaseCapacity OutOfSpace"), - }; - - // Continue the loop to try handling this row again. - continue; - }; + ); } else { if (!left_right) { // Swap the src/dst cells. This ensures that our dst gets the @@ -7606,6 +7531,91 @@ test "Terminal: insertLines across page boundary marks all shifted rows dirty" { } } +test "Terminal: insertLines hyperlink-dense row crosses page boundary" { + // Regression test for the cross-page copy of insertLines: when the + // shifted row carries more unique hyperlinks than the destination + // page's hyperlink capacity, the copy must increase the destination + // page's capacity and retry rather than corrupting the page list. + const alloc = testing.allocator; + var t = try init(alloc, .{ .rows = 5, .cols = 10, .max_scrollback = 1024 }); + defer t.deinit(alloc); + + const pages = &t.screens.active.pages; + + // Fill the first page so it is exactly full, then two more rows so + // the second page holds the last two active rows (y=3 and y=4). + const first_page_rows = pages.pages.first.?.capacity().rows; + for (0..first_page_rows + 1) |_| try t.linefeed(); + try testing.expect(pages.pages.first != pages.pages.last); + try testing.expectEqual(@as(usize, 2), pages.pages.last.?.rows()); + + // Marker rows so we can verify the shift afterwards. + t.setCursorPos(1, 1); + try t.printString("0"); + t.setCursorPos(2, 1); + try t.printString("1"); + t.setCursorPos(4, 1); + try t.printString("3"); + t.setCursorPos(5, 1); + try t.printString("4"); + + // Fill the last row of the first page (active y=2) with unique + // hyperlinks: more than the second page can hold with its default + // hyperlink capacity. Writing them grows the first page's capacity + // as needed; the second page keeps its default capacity. + t.setCursorPos(3, 1); + for (0..10) |i| { + var buf: [64]u8 = undefined; + const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{i}); + try t.screens.active.startHyperlink(uri, null); + try t.print(@intCast('A' + i)); + t.screens.active.endHyperlink(); + } + { + const pin = pages.pin(.{ .active = .{ .y = 2 } }).?; + try testing.expectEqual(pages.pages.first.?, pin.node); + try testing.expectEqual(pin.node.rows() - 1, @as(usize, pin.y)); + } + try testing.expect(pages.pages.last.?.page().hyperlink_set.layout.cap < 10); + + // Insert a line at the top: every row shifts down by one and the + // dense row crosses the page boundary into the second page. + t.setCursorPos(1, 1); + t.insertLines(1); + + { + const str = try t.plainString(testing.allocator); + defer testing.allocator.free(str); + try testing.expectEqualStrings("\n0\n1\nABCDEFGHIJ\n3", str); + } + + // The second page's hyperlink capacity had to grow to receive the + // row, proving the capacity-retry path ran. + try testing.expect(pages.pages.last.?.page().hyperlink_set.layout.cap >= 10); + + // Every cell of the dense row must still resolve to a real + // hyperlink entry with the correct URI. A half-applied shift + // leaves cells whose hyperlink flag is set but that have no map + // entry, which aborts in clearCells later. + for (0..10) |x| { + const list_cell = pages.getCell(.{ .active = .{ + .x = @intCast(x), + .y = 3, + } }).?; + try testing.expect(list_cell.cell.hyperlink); + const page: *Page = list_cell.node.page(); + const id = page.lookupHyperlink(list_cell.cell).?; + const link = page.hyperlink_set.get(page.memory, id); + var buf: [64]u8 = undefined; + const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{x}); + try testing.expectEqualStrings(uri, link.uri.slice(page.memory)); + } + + // All pages must pass integrity checks. + var node_: ?*PageList.List.Node = pages.pages.first; + while (node_) |node| : (node_ = node.next) node.page().assertIntegrity(); +} + test "Terminal: insertLines (legacy test)" { const alloc = testing.allocator; var t = try init(alloc, .{ .cols = 2, .rows = 5 }); @@ -10404,6 +10414,96 @@ test "Terminal: deleteLines across page boundary marks all shifted rows dirty" { } } +test "Terminal: deleteLines hyperlink-dense row crosses page boundary" { + // Regression test for the cross-page copy of deleteLines: when the + // shifted row carries more unique hyperlinks than the destination + // page's hyperlink capacity, the copy must increase the destination + // page's capacity and retry rather than corrupting the page list. + // + // This is the mirror of the insertLines variant: the dense row + // starts as the first row of the second page and is pulled up into + // the first page, which also happens to be the cursor's page so + // this exercises the cursor accounting of the capacity increase. + const alloc = testing.allocator; + var t = try init(alloc, .{ .rows = 5, .cols = 10, .max_scrollback = 1024 }); + defer t.deinit(alloc); + + const pages = &t.screens.active.pages; + + // Fill the first page so it is exactly full, then two more rows so + // the second page holds the last two active rows (y=3 and y=4). + const first_page_rows = pages.pages.first.?.capacity().rows; + for (0..first_page_rows + 1) |_| try t.linefeed(); + try testing.expect(pages.pages.first != pages.pages.last); + try testing.expectEqual(@as(usize, 2), pages.pages.last.?.rows()); + + // Marker rows so we can verify the shift afterwards. + t.setCursorPos(1, 1); + try t.printString("0"); + t.setCursorPos(2, 1); + try t.printString("1"); + t.setCursorPos(3, 1); + try t.printString("2"); + t.setCursorPos(5, 1); + try t.printString("4"); + + // Fill the first row of the second page (active y=3) with unique + // hyperlinks: more than the first page can hold with its default + // hyperlink capacity. Writing them grows the second page's + // capacity as needed; the first page keeps its default capacity. + t.setCursorPos(4, 1); + for (0..10) |i| { + var buf: [64]u8 = undefined; + const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{i}); + try t.screens.active.startHyperlink(uri, null); + try t.print(@intCast('A' + i)); + t.screens.active.endHyperlink(); + } + { + const pin = pages.pin(.{ .active = .{ .y = 3 } }).?; + try testing.expectEqual(pages.pages.last.?, pin.node); + try testing.expectEqual(@as(usize, 0), @as(usize, pin.y)); + } + try testing.expect(pages.pages.first.?.page().hyperlink_set.layout.cap < 10); + + // Delete the top line: every row shifts up by one and the dense + // row crosses the page boundary into the first page. + t.setCursorPos(1, 1); + t.deleteLines(1); + + { + const str = try t.plainString(testing.allocator); + defer testing.allocator.free(str); + try testing.expectEqualStrings("1\n2\nABCDEFGHIJ\n4", str); + } + + // The first page's hyperlink capacity had to grow to receive the + // row, proving the capacity-retry path ran. + try testing.expect(pages.pages.first.?.page().hyperlink_set.layout.cap >= 10); + + // Every cell of the dense row must still resolve to a real + // hyperlink entry with the correct URI. A half-applied shift + // leaves cells whose hyperlink flag is set but that have no map + // entry, which aborts in clearCells later. + for (0..10) |x| { + const list_cell = pages.getCell(.{ .active = .{ + .x = @intCast(x), + .y = 2, + } }).?; + try testing.expect(list_cell.cell.hyperlink); + const page: *Page = list_cell.node.page(); + const id = page.lookupHyperlink(list_cell.cell).?; + const link = page.hyperlink_set.get(page.memory, id); + var buf: [64]u8 = undefined; + const uri = try std.fmt.bufPrint(&buf, "http://example.com/{d}", .{x}); + try testing.expectEqualStrings(uri, link.uri.slice(page.memory)); + } + + // All pages must pass integrity checks. + var node_: ?*PageList.List.Node = pages.pages.first; + while (node_) |node| : (node_ = node.next) node.page().assertIntegrity(); +} + test "Terminal: deleteLines (legacy)" { const alloc = testing.allocator; var t = try init(alloc, .{ .cols = 80, .rows = 80 });