diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 04b58b99b..1ba939f8f 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2907,6 +2907,8 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { }, .delta_prompt => |n| self.scrollPrompt(n), .delta_row => |n| delta_row: { + const amount: usize = @abs(n); + switch (self.viewport) { // If we're at the top and we're scrolling backwards, // we don't have to do anything, because there's nowhere to go. @@ -2923,11 +2925,11 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { .pin => switch (std.math.order(n, 0)) { .eq => break :delta_row, - .lt => switch (self.viewport_pin.upOverflow(@intCast(-n))) { + .lt => switch (self.viewport_pin.upOverflow(amount)) { .offset => |new_pin| { self.viewport_pin.* = new_pin; if (self.viewport_pin_row_offset) |*v| { - v.* -= @as(usize, @intCast(-n)); + v.* -= amount; } break :delta_row; }, @@ -2939,7 +2941,7 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { }, }, - .gt => switch (self.viewport_pin.downOverflow(@intCast(n))) { + .gt => switch (self.viewport_pin.downOverflow(amount)) { // If we offset its a valid pin but we still have to // check if we're in the active area. .offset => |new_pin| { @@ -2948,7 +2950,7 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { } else { self.viewport_pin.* = new_pin; if (self.viewport_pin_row_offset) |*v| { - v.* += @intCast(n); + v.* += amount; } } break :delta_row; @@ -2966,10 +2968,10 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { // Slow path: we have to calculate the new pin by moving // from our viewport. const top = self.getTopLeft(.viewport); - const p: Pin = if (n < 0) switch (top.upOverflow(@intCast(-n))) { + const p: Pin = if (n < 0) switch (top.upOverflow(amount)) { .offset => |v| v, .overflow => |v| v.end, - } else switch (top.downOverflow(@intCast(n))) { + } else switch (top.downOverflow(amount)) { .offset => |v| v, .overflow => |v| v.end, }; @@ -3007,7 +3009,7 @@ pub fn scroll(self: *PageList, behavior: Scroll) void { fn scrollPrompt(self: *PageList, delta: isize) void { // If we aren't jumping any prompts then we don't need to do anything. if (delta == 0) return; - const delta_start: usize = @intCast(if (delta > 0) delta else -delta); + const delta_start: usize = @abs(delta); var delta_rem: usize = delta_start; // We start at the row before or after our viewport depending on the @@ -4737,6 +4739,9 @@ pub fn pin(self: *const PageList, pt: point.Point) ?Pin { // Grab the top left and move to the point. var p = self.getTopLeft(pt).down(pt.coord().y) orelse return null; + // Incomplete reflow can leave a page narrower than the desired width. + // Never manufacture an out-of-bounds pin for that page. + if (x >= p.node.cols()) return null; p.x = x; return p; } @@ -4796,6 +4801,24 @@ pub fn pinIsValid(self: *const PageList, p: Pin) bool { return false; } +/// Returns whether a node pointer and serial still identify the same page in +/// this list. The node pointer is only compared and is safe even if the node +/// has been destroyed or reused since the serial was captured. +pub fn nodeIsValid( + self: *const PageList, + target: *List.Node, + serial: u64, +) bool { + if (serial < self.page_serial_min) return false; + + var it = self.pages.first; + while (it) |node| : (it = node.next) { + if (node == target) return node.serial == serial; + } + + return false; +} + /// Returns the viewport for the given pin, preferring to pin to /// "active" if the pin is within the active area. fn pinIsActive(self: *const PageList, p: Pin) bool { @@ -4844,15 +4867,23 @@ pub fn pointFromPin(self: *const PageList, tag: point.Tag, p: Pin) ?point.Point if (tl.y > p.y) return null; coord.y = p.y - tl.y; } else { - coord.y += tl.node.rows() - tl.y; + coord.y = std.math.add( + u32, + coord.y, + tl.node.rows() - tl.y, + ) catch return null; var node_ = tl.node.next; while (node_) |node| : (node_ = node.next) { if (node == p.node) { - coord.y += p.y; + coord.y = std.math.add(u32, coord.y, p.y) catch return null; break; } - coord.y += node.rows(); + coord.y = std.math.add( + u32, + coord.y, + node.rows(), + ) catch return null; } else { // We never saw our node, meaning we're outside the range. return null; @@ -5577,18 +5608,15 @@ pub const PageIterator = struct { .count => |*limit| count: { assert(limit.* > 0); // should be handled already - const len = @min(row.node.rows() - row.y, limit.*); - if (len > limit.*) { - self.row = row.down(len); - limit.* -= len; - } else { - self.row = null; - } + const available: usize = row.node.rows() - row.y; + const len = @min(available, limit.*); + limit.* -= len; + self.row = if (limit.* > 0) row.down(len) else null; break :count .{ .node = row.node, .start = row.y, - .end = row.y + len, + .end = @intCast(@as(usize, row.y) + len), }; }, @@ -5646,18 +5674,15 @@ pub const PageIterator = struct { .count => |*limit| count: { assert(limit.* > 0); // should be handled already - const len = @min(row.y, limit.*); - if (len > limit.*) { - self.row = row.up(len); - limit.* -= len; - } else { - self.row = null; - } + const available: usize = @as(usize, row.y) + 1; + const len = @min(available, limit.*); + limit.* -= len; + self.row = if (limit.* > 0) row.up(len) else null; break :count .{ .node = row.node, - .start = row.y - len, - .end = row.y - 1, + .start = @intCast(available - len), + .end = @intCast(available), }; }, @@ -6243,26 +6268,18 @@ pub const Pin = struct { /// /// TODO: Unit tests. pub fn leftWrap(self: Pin, n: usize) ?Pin { - // NOTE: This assumes that all pages have the same width, which may - // be violated under certain circumstances by incomplete reflow. - const cols = self.node.cols(); - const remaining_in_row = self.x; - - if (n <= remaining_in_row) return self.left(n); - - const extra_after_remaining = n - remaining_in_row; - const rows_off = 1 + (extra_after_remaining - 1) / cols; - - switch (self.upOverflow(rows_off)) { - .offset => |v| { - var result = v; - result.x = @intCast( - cols - 1 - (extra_after_remaining - 1) % cols, - ); - return result; - }, - .overflow => return null, + var result = self; + var remaining = n; + while (remaining > result.x) { + remaining -= @as(usize, result.x) + 1; + result = result.up(1) orelse return null; + // Crossing a row boundary lands on that destination row's final + // cell, whose width may differ from ours during reflow. + result.x = result.node.cols() - 1; } + + result.x -= @intCast(remaining); + return result; } /// Move the pin right n cells, wrapping to the next row as needed. @@ -6271,23 +6288,18 @@ pub const Pin = struct { /// /// TODO: Unit tests. pub fn rightWrap(self: Pin, n: usize) ?Pin { - // NOTE: This assumes that all pages have the same width, which may - // be violated under certain circumstances by incomplete reflow. - const cols = self.node.cols(); - const remaining_in_row = cols - self.x - 1; - - if (n <= remaining_in_row) return self.right(n); - - const extra_after_remaining = n - remaining_in_row; - const rows_off = 1 + (extra_after_remaining - 1) / cols; - - switch (self.downOverflow(rows_off)) { - .offset => |v| { - var result = v; - result.x = @intCast((extra_after_remaining - 1) % cols); + var result = self; + var remaining = n; + while (true) { + const row_remaining = result.node.cols() - result.x - 1; + if (remaining <= row_remaining) { + result.x += @intCast(remaining); return result; - }, - .overflow => return null, + } + + remaining -= @as(usize, row_remaining) + 1; + result = result.down(1) orelse return null; + result.x = 0; } } @@ -6335,7 +6347,7 @@ pub const Pin = struct { .end = .{ .node = node, .y = node.rows() - 1, - .x = self.x, + .x = @min(self.x, node.cols() - 1), }, .remaining = n_left, } }; @@ -6343,7 +6355,7 @@ pub const Pin = struct { .node = node, .y = std.math.cast(size.CellCountInt, n_left - 1) orelse std.math.maxInt(size.CellCountInt), - .x = self.x, + .x = @min(self.x, node.cols() - 1), } }; n_left -= node.rows(); } @@ -6371,20 +6383,111 @@ pub const Pin = struct { var n_left: usize = n - self.y; while (true) { node = node.prev orelse return .{ .overflow = .{ - .end = .{ .node = node, .y = 0, .x = self.x }, + .end = .{ + .node = node, + .y = 0, + .x = @min(self.x, node.cols() - 1), + }, .remaining = n_left, } }; if (n_left <= node.rows()) return .{ .offset = .{ .node = node, .y = std.math.cast(size.CellCountInt, node.rows() - n_left) orelse std.math.maxInt(size.CellCountInt), - .x = self.x, + .x = @min(self.x, node.cols() - 1), } }; n_left -= node.rows(); } } }; +fn mixedWidthPinListForTest(alloc: Allocator) !PageList { + var result = try init(alloc, 2, 1, null); + errdefer result.deinit(); + + // This deliberately constructs a layout that normal PageList operations + // do not expose yet. Keep integrity checks paused through deinit so the + // fixture can exercise mixed-width traversal in isolation. + result.pauseIntegrityChecks(true); + + inline for (.{ 4, 3 }) |cols| { + const node = try result.createPage(.{ .cap = .{ + .cols = cols, + .rows = 1, + } }); + node.page().size.rows = 1; + result.pages.append(node); + result.total_rows += 1; + } + + // Desired geometry is wider than the first and last stored pages. + result.cols = 4; + return result; +} + +test "PageList Pin row movement clamps across mixed-width pages" { + const testing = std.testing; + + var s = try mixedWidthPinListForTest(testing.allocator); + defer s.deinit(); + + const first = s.pages.first.?; + const second = first.next.?; + const third = second.next.?; + + try testing.expect((Pin{ .node = third, .x = 2 }).eql( + (Pin{ .node = second, .x = 3 }).down(1).?, + )); + try testing.expect((Pin{ .node = first, .x = 1 }).eql( + (Pin{ .node = second, .x = 3 }).up(1).?, + )); + + switch ((Pin{ .node = second, .x = 3 }).downOverflow(10)) { + .offset => try testing.expect(false), + .overflow => |overflow| try testing.expect( + (Pin{ .node = third, .x = 2 }).eql(overflow.end), + ), + } + switch ((Pin{ .node = second, .x = 3 }).upOverflow(10)) { + .offset => try testing.expect(false), + .overflow => |overflow| try testing.expect( + (Pin{ .node = first, .x = 1 }).eql(overflow.end), + ), + } +} + +test "PageList Pin wrapping crosses mixed-width pages" { + const testing = std.testing; + + var s = try mixedWidthPinListForTest(testing.allocator); + defer s.deinit(); + + const first = s.pages.first.?; + const second = first.next.?; + const third = second.next.?; + + try testing.expect((Pin{ .node = third, .x = 2 }).eql( + (Pin{ .node = first, .x = 1 }).rightWrap(7).?, + )); + try testing.expect((Pin{ .node = second, .x = 0 }).eql( + (Pin{ .node = third, .x = 2 }).leftWrap(6).?, + )); + try testing.expect((Pin{ .node = first }).leftWrap(1) == null); + try testing.expect((Pin{ .node = third, .x = 2 }).rightWrap(1) == null); +} + +test "PageList Pin rejects columns beyond mixed-width page bounds" { + const testing = std.testing; + + var s = try mixedWidthPinListForTest(testing.allocator); + defer s.deinit(); + + try testing.expect(s.pin(.{ .screen = .{ .x = 1, .y = 0 } }) != null); + try testing.expect(s.pin(.{ .screen = .{ .x = 2, .y = 0 } }) == null); + try testing.expect(s.pin(.{ .screen = .{ .x = 3, .y = 1 } }) != null); + try testing.expect(s.pin(.{ .screen = .{ .x = 3, .y = 2 } }) == null); +} + pub const Cell = struct { node: *List.Node, row: *pagepkg.Row, @@ -6419,7 +6522,7 @@ pub const Cell = struct { /// this file then consider a different approach and ask yourself very /// carefully if you really need this. pub fn screenPoint(self: Cell) point.Point { - var y: size.CellCountInt = self.row_idx; + var y: u32 = self.row_idx; var node_ = self.node; while (node_.prev) |node| { y += node.rows(); @@ -7802,6 +7905,44 @@ test "PageList pointFromPin traverse pages" { }) == null); } } + +test "PageList pointFromPin rejects overflowing screen coordinate" { + const testing = std.testing; + + // Use maximum-height metadata-only pages to model a valid scrollback just + // beyond the u32 coordinate range without allocating their backing cells. + const page_count = 65_539; + const rows_per_page = std.math.maxInt(size.CellCountInt); + const nodes = try testing.allocator.alloc(Node, page_count); + defer testing.allocator.free(nodes); + + for (nodes, 0..) |*node, i| { + node.* = .{ + .prev = if (i > 0) &nodes[i - 1] else null, + .next = if (i + 1 < nodes.len) &nodes[i + 1] else null, + .data = .{ .resident = undefined }, + .serial = @intCast(i), + .owned = .heap, + }; + node.data.resident.size = .{ + .cols = 1, + .rows = rows_per_page, + }; + } + + var s: PageList = undefined; + s.pages = .{ + .first = &nodes[0], + .last = &nodes[nodes.len - 1], + }; + + try testing.expect(s.pointFromPin(.screen, .{ + .node = &nodes[nodes.len - 1], + .y = 0, + .x = 0, + }) == null); +} + test "PageList active after grow" { const testing = std.testing; const alloc = testing.allocator; @@ -8132,6 +8273,20 @@ test "PageList scroll delta row back overflow" { }, s.scrollbar()); } +test "PageList scroll minimum row delta" { + const testing = std.testing; + + var s = try init(testing.allocator, 10, 3, null); + defer s.deinit(); + + // Create one row of history so scrolling all the way back has an + // observable result. + try s.growRows(1); + s.scroll(.{ .delta_row = std.math.minInt(isize) }); + + try testing.expectEqual(Viewport.top, s.viewport); +} + test "PageList scroll delta row forward" { const testing = std.testing; const alloc = testing.allocator; @@ -8817,6 +8972,16 @@ test "PageList: jump zero prompts" { }, s.scrollbar()); } +test "PageList: jump minimum prompt delta" { + const testing = std.testing; + + var s = try init(testing.allocator, 10, 3, null); + defer s.deinit(); + + s.scroll(.{ .delta_prompt = std.math.minInt(isize) }); + try testing.expectEqual(Viewport.active, s.viewport); +} + test "Screen: jump back one prompt" { const testing = std.testing; const alloc = testing.allocator; @@ -8985,6 +9150,47 @@ test "PageList grow allocate" { } } +test "PageList Cell screenPoint supports long scrollback" { + const testing = std.testing; + + // A modest number of full-size page nodes is enough to exceed the u16 + // row range without allocating any page backing memory. screenPoint only + // reads the linked metadata while calculating the absolute coordinate. + const page_count = 307; + const rows_per_page = std_capacity.rows; + const nodes = try testing.allocator.alloc(Node, page_count); + defer testing.allocator.free(nodes); + + for (nodes, 0..) |*node, i| { + node.* = .{ + .prev = if (i > 0) &nodes[i - 1] else null, + .next = if (i + 1 < nodes.len) &nodes[i + 1] else null, + .data = .{ .resident = undefined }, + .serial = @intCast(i), + .owned = .heap, + }; + node.data.resident.size = .{ + .cols = 1, + .rows = rows_per_page, + }; + } + + const expected_y: u32 = (page_count - 1) * @as(u32, rows_per_page); + try testing.expect(expected_y > std.math.maxInt(size.CellCountInt)); + + const cell: Cell = .{ + .node = &nodes[nodes.len - 1], + .row = undefined, + .cell = undefined, + .row_idx = 0, + .col_idx = 0, + }; + try testing.expectEqual(point.Point{ .screen = .{ + .x = 0, + .y = expected_y, + } }, cell.screenPoint()); +} + test "PageList grow prune scrollback" { const testing = std.testing; const alloc = testing.allocator; @@ -9870,6 +10076,76 @@ test "PageList pageIterator reverse history two pages" { try testing.expect(it.next() == null); } +test "PageList PageIterator reverse count includes row zero" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 2, 2, null); + defer s.deinit(); + + var it: PageIterator = .{ + .row = s.getTopLeft(.screen), + .limit = .{ .count = 1 }, + .direction = .left_up, + }; + const chunk = it.next().?; + try testing.expectEqual(@as(size.CellCountInt, 0), chunk.start); + try testing.expectEqual(@as(size.CellCountInt, 1), chunk.end); + try testing.expect(it.next() == null); +} + +test "PageList PageIterator count crosses page boundaries" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 80, 24, null); + defer s.deinit(); + + const first = s.pages.first.?; + first.page().pauseIntegrityChecks(true); + while (first.rows() < first.capacity().rows) _ = try s.grow(); + first.page().pauseIntegrityChecks(false); + const second = (try s.grow()).?; + + var down: PageIterator = .{ + .row = .{ .node = first, .y = first.rows() - 1 }, + .limit = .{ .count = 2 }, + .direction = .right_down, + }; + { + const chunk = down.next().?; + try testing.expectEqual(first, chunk.node); + try testing.expectEqual(first.rows() - 1, chunk.start); + try testing.expectEqual(first.rows(), chunk.end); + } + { + const chunk = down.next().?; + try testing.expectEqual(second, chunk.node); + try testing.expectEqual(@as(size.CellCountInt, 0), chunk.start); + try testing.expectEqual(@as(size.CellCountInt, 1), chunk.end); + } + try testing.expect(down.next() == null); + + var up: PageIterator = .{ + .row = .{ .node = second }, + .limit = .{ .count = 2 }, + .direction = .left_up, + }; + { + const chunk = up.next().?; + try testing.expectEqual(second, chunk.node); + try testing.expectEqual(@as(size.CellCountInt, 0), chunk.start); + try testing.expectEqual(@as(size.CellCountInt, 1), chunk.end); + } + { + const chunk = up.next().?; + try testing.expectEqual(first, chunk.node); + try testing.expectEqual(first.rows() - 1, chunk.start); + try testing.expectEqual(first.rows(), chunk.end); + } + try testing.expect(up.next() == null); +} + test "PageList cellIterator" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 95cba1bf9..8798411b0 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -530,20 +530,30 @@ pub fn clone( } // We move the top pin back in bounds to the top row. + const node = pages.pages.first.?; break :start try pages.trackPin(.{ - .node = pages.pages.first.?, - .x = if (sel.rectangle) ordered.tl.x else 0, + .node = node, + .x = if (sel.rectangle) + @min(ordered.tl.x, node.cols() - 1) + else + 0, }); }; // If we got to this point it means that the selection is not // fully out of bounds, so we move the bottom right pin back // in bounds if it isn't already. - const end_pin = pin_remap.get(ordered.br) orelse try pages.trackPin(.{ - .node = pages.pages.last.?, - .x = if (sel.rectangle) ordered.br.x else pages.cols - 1, - .y = pages.pages.last.?.rows() - 1, - }); + const end_pin = pin_remap.get(ordered.br) orelse end: { + const node = pages.pages.last.?; + break :end try pages.trackPin(.{ + .node = node, + .x = if (sel.rectangle) + @min(ordered.br.x, node.cols() - 1) + else + node.cols() - 1, + .y = node.rows() - 1, + }); + }; break :sel .{ .bounds = .{ .tracked = .{ @@ -651,7 +661,7 @@ pub fn cursorCellEndOfPrev(self: *Screen) *pagepkg.Cell { assert(self.cursor.y > 0); var page_pin = self.cursor.page_pin.up(1).?; - page_pin.x = self.pages.cols - 1; + page_pin.x = page_pin.node.cols() - 1; const page_rac = page_pin.rowAndCell(); return page_rac.cell; } @@ -1516,19 +1526,19 @@ pub fn clearRows( var it = self.pages.pageIterator(.right_down, tl, bl); while (it.next()) |chunk| { + const page = chunk.node.page(); for (chunk.rows()) |*row| { const cells_offset = row.cells; - const cells_multi: [*]Cell = row.cells.ptr(chunk.node.page().memory); - const cells = cells_multi[0..self.pages.cols]; + const cells = page.getCells(row); // Clear all cells if (protected) { - self.clearUnprotectedCells(chunk.node.page(), row, cells); + self.clearUnprotectedCells(page, row, cells); // We need to preserve other row attributes since we only // cleared unprotected cells. row.cells = cells_offset; } else { - self.clearCells(chunk.node.page(), row, cells); + self.clearCells(page, row, cells); row.* = .{ .cells = cells_offset }; } @@ -1546,6 +1556,8 @@ pub fn clearCells( row: *Row, cells: []Cell, ) void { + if (cells.len == 0) return; + // This whole operation does unsafe things, so we just want to assert // the end state. page.pauseIntegrityChecks(true); @@ -1577,7 +1589,7 @@ pub fn clearCells( // If we have no left/right scroll region we can be sure // that we've cleared all the graphemes, so we clear the // flag, otherwise we ask the page to update the flag. - if (cells.len == self.pages.cols) { + if (cells.len == page.size.cols) { row.grapheme = false; } else { page.updateRowGraphemeFlag(row); @@ -1593,7 +1605,7 @@ pub fn clearCells( // If we have no left/right scroll region we can be sure // that we've cleared all the hyperlinks, so we clear the // flag, otherwise we ask the page to update the flag. - if (cells.len == self.pages.cols) { + if (cells.len == page.size.cols) { row.hyperlink = false; } else { page.updateRowHyperlinkFlag(row); @@ -1621,7 +1633,7 @@ pub fn clearCells( // If we have no left/right scroll region we can be sure // that we've cleared all the styles, so we clear the // flag, otherwise we ask the page to update the flag. - if (cells.len == self.pages.cols) { + if (cells.len == page.size.cols) { row.styled = false; } else { page.updateRowStyledFlag(row); @@ -1630,7 +1642,7 @@ pub fn clearCells( if (comptime build_options.kitty_graphics) { if (row.kitty_virtual_placeholder and - cells.len == self.pages.cols) + cells.len == page.size.cols) { for (cells) |c| { if (c.codepoint() == kitty.graphics.unicode.placeholder) { @@ -2401,13 +2413,13 @@ pub fn startHyperlink( .id = if (id_) |id| .{ .explicit = id, } else implicit: { - defer self.cursor.hyperlink_implicit_id += 1; + defer self.cursor.hyperlink_implicit_id +%= 1; break :implicit .{ .implicit = self.cursor.hyperlink_implicit_id }; }, }; errdefer switch (link.id) { .explicit => {}, - .implicit => self.cursor.hyperlink_implicit_id -= 1, + .implicit => self.cursor.hyperlink_implicit_id -%= 1, }; // Loop until we have enough page memory to add the hyperlink @@ -2448,9 +2460,6 @@ fn startHyperlinkOnce( self: *Screen, source: hyperlink.Hyperlink, ) (Allocator.Error || Page.InsertHyperlinkError)!void { - // End any prior hyperlink - self.endHyperlink(); - // Allocate our new Hyperlink entry in non-page memory. This // lets us quickly get access to URI, ID. const link = try self.alloc.create(hyperlink.Hyperlink); @@ -2458,6 +2467,10 @@ fn startHyperlinkOnce( link.* = try source.dupe(self.alloc); errdefer link.deinit(self.alloc); + // End any prior hyperlink only after duplicating the new value. The + // source slices are allowed to reference our current hyperlink. + self.endHyperlink(); + // Insert the hyperlink into page memory var page = self.cursor.page_pin.node.page(); const id: hyperlink.Id = try page.insertHyperlink(link.*); @@ -2612,8 +2625,28 @@ pub fn select(self: *Screen, sel_: ?Selection) Allocator.Error!void { const tracked_sel = if (sel.tracked()) sel else try sel.track(self); errdefer if (!sel.tracked()) tracked_sel.deinit(self); - // Untrack prior selection - if (self.selection) |*old| old.deinit(self); + // Untrack prior selection pins that aren't also owned by the replacement. + // A caller may pass our current tracked selection back to us by value, so + // releasing both old pins unconditionally would leave the replacement + // pointing at freed pool entries. + if (self.selection) |old| { + const new_bounds = tracked_sel.bounds.tracked; + switch (old.bounds) { + .untracked => old.deinit(self), + .tracked => |old_bounds| { + if (old_bounds.start != new_bounds.start and + old_bounds.start != new_bounds.end) + { + self.pages.untrackPin(old_bounds.start); + } + if (old_bounds.end != new_bounds.start and + old_bounds.end != new_bounds.end) + { + self.pages.untrackPin(old_bounds.end); + } + }, + } + } self.selection = tracked_sel; self.dirty.selection = true; } @@ -2807,7 +2840,7 @@ pub fn selectLine(self: *const Screen, opts: SelectLine) ?Selection { // so we scan forward to find where our content ends. if (start_offset == 0 and cells[0].semantic_content != v) { var prev = p.up(1).?; - prev.x = p.node.cols() - 1; + prev.x = prev.node.cols() - 1; break :end_pin prev; } @@ -4084,6 +4117,38 @@ test "Screen clearRows active styled line" { try testing.expectEqualStrings("", str); } +test "Screen clearCells empty range" { + const testing = std.testing; + + var s = try Screen.init(testing.allocator, .default); + defer s.deinit(); + + const page = s.cursor.page_pin.node.page(); + const row = s.cursor.page_row; + const cells = page.getCells(row); + s.clearCells(page, row, cells[0..0]); +} + +test "Screen clearRows uses stored page width" { + const testing = std.testing; + + var s = try Screen.init(testing.allocator, .{ + .cols = 2, + .rows = 1, + .max_scrollback = 0, + }); + defer s.deinit(); + + try s.testWriteString("AB"); + const node = s.pages.pages.first.?; + s.pages.cols = 4; + + s.clearRows(.{ .screen = .{} }, null, false); + const cells = node.page().getCells(&node.page().rows.ptr(node.page().memory)[0]); + try testing.expectEqual(@as(usize, 2), cells.len); + for (cells) |cell| try testing.expect(cell.isEmpty()); +} + test "Screen clearRows protected" { const testing = std.testing; const alloc = testing.allocator; @@ -4209,6 +4274,27 @@ test "Screen eraseRows active partial" { } } +test "Screen: cursorCellEndOfPrev across mixed-width pages" { + const testing = std.testing; + var s = try init(testing.allocator, .{ + .cols = 4, + .rows = 2, + .max_scrollback = 0, + }); + defer s.deinit(); + + try s.testWriteString("ABCDE"); + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 1 }); + s.cursorReload(); + const second = first.next.?; + first.page().size.cols = 2; + + try testing.expectEqual(second, s.cursor.page_pin.node); + const expected = (Pin{ .node = first, .x = 1 }).rowAndCell().cell; + try testing.expectEqual(expected, s.cursorCellEndOfPrev()); +} + test "Screen: cursorDown across pages preserves style" { const testing = std.testing; const alloc = testing.allocator; @@ -5934,6 +6020,51 @@ test "Screen: clone contains subset of selection" { } } +test "Screen: clone clamps clipped selections to mixed-width pages" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 4, .rows = 3, .max_scrollback = 0 }); + defer s.deinit(); + + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 2 }); + try s.pages.split(.{ .node = first, .y = 1 }); + const middle = first.next.?; + const last = middle.next.?; + middle.page().size.cols = 2; + + try s.select(Selection.init( + .{ .node = first }, + .{ .node = last, .x = 3 }, + false, + )); + var linear = try s.clone( + alloc, + .{ .screen = .{} }, + .{ .screen = .{ .y = 1 } }, + ); + defer linear.deinit(); + const linear_end = linear.selection.?.end(); + _ = linear_end.rowAndCell(); + try testing.expectEqual(@as(size.CellCountInt, 1), linear_end.x); + + try s.select(Selection.init( + .{ .node = first, .x = 3 }, + .{ .node = last, .x = 3 }, + true, + )); + var rectangle = try s.clone( + alloc, + .{ .screen = .{ .y = 1 } }, + null, + ); + defer rectangle.deinit(); + const rectangle_start = rectangle.selection.?.start(); + _ = rectangle_start.rowAndCell(); + try testing.expectEqual(@as(size.CellCountInt, 1), rectangle_start.x); +} + test "Screen: clone contains subset of rectangle selection" { const testing = std.testing; const alloc = testing.allocator; @@ -8114,6 +8245,23 @@ test "Screen: select replaces existing pins" { try testing.expectEqual(tracked + 2, s.pages.countTrackedPins()); } +test "Screen: reselecting tracked selection preserves its pins" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 10, .rows = 2, .max_scrollback = 0 }); + defer s.deinit(); + + try s.select(Selection.init( + s.pages.pin(.{ .active = .{ .x = 1, .y = 0 } }).?, + s.pages.pin(.{ .active = .{ .x = 3, .y = 0 } }).?, + false, + )); + + try s.select(s.selection.?); + try testing.expectEqual(Selection.Order.forward, s.selection.?.order(&s)); +} + test "Screen: selectAll" { const testing = std.testing; const alloc = testing.allocator; @@ -8804,6 +8952,37 @@ test "Screen: selectLine semantic boundary first cell of row" { } } +test "Screen: selectLine semantic boundary across mixed-width pages" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 4, .rows = 2, .max_scrollback = 0 }); + defer s.deinit(); + + s.cursorSetSemanticContent(.{ .input = .clear_explicit }); + try s.testWriteString("ABCD"); + s.cursorSetSemanticContent(.output); + try s.testWriteString("E"); + + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 1 }); + const second = first.next.?; + first.page().size.cols = 2; + s.pages.pauseIntegrityChecks(true); + defer s.pages.pauseIntegrityChecks(false); + + try testing.expectEqual(@as(size.CellCountInt, 2), first.cols()); + try testing.expectEqual(@as(size.CellCountInt, 4), second.cols()); + + var sel = s.selectLine(.{ .pin = .{ + .node = first, + .x = 1, + } }).?; + defer sel.deinit(&s); + try testing.expect((Pin{ .node = first, .x = 0 }).eql(sel.start())); + try testing.expect((Pin{ .node = first, .x = 1 }).eql(sel.end())); +} + test "Screen: selectLine semantic all same content" { const testing = std.testing; const alloc = testing.allocator; @@ -9855,6 +10034,58 @@ test "Screen: hyperlink start/end" { } } +test "Screen: hyperlink accepts its current values" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 5, .rows = 5, .max_scrollback = 0 }); + defer s.deinit(); + + try s.startHyperlink("http://example.com", "current"); + const current = s.cursor.hyperlink.?; + try s.startHyperlink(current.uri, current.id.explicit); + + try testing.expectEqualStrings("http://example.com", s.cursor.hyperlink.?.uri); + try testing.expectEqualStrings("current", s.cursor.hyperlink.?.id.explicit); +} + +test "Screen: implicit hyperlink ID wraps" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, .{ .cols = 5, .rows = 5, .max_scrollback = 0 }); + defer s.deinit(); + + s.cursor.hyperlink_implicit_id = std.math.maxInt(size.OffsetInt); + try s.startHyperlink("http://example.com", null); + + try testing.expectEqual(@as(size.OffsetInt, 0), s.cursor.hyperlink_implicit_id); + try testing.expectEqual( + std.math.maxInt(size.OffsetInt), + s.cursor.hyperlink.?.id.implicit, + ); + + // A failed allocation must roll the wrapped counter back to its + // original value as well. + s.endHyperlink(); + s.cursor.hyperlink_implicit_id = std.math.maxInt(size.OffsetInt); + var failing = testing.FailingAllocator.init(alloc, .{}); + failing.fail_index = failing.alloc_index; + { + const original_alloc = s.alloc; + defer s.alloc = original_alloc; + s.alloc = failing.allocator(); + try testing.expectError( + error.OutOfMemory, + s.startHyperlink("http://example.com", null), + ); + } + try testing.expectEqual( + std.math.maxInt(size.OffsetInt), + s.cursor.hyperlink_implicit_id, + ); +} + test "Screen: hyperlink reuse" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/Selection.zig b/src/terminal/Selection.zig index 8aacc9ba9..be6854e5f 100644 --- a/src/terminal/Selection.zig +++ b/src/terminal/Selection.zig @@ -155,12 +155,12 @@ pub fn topLeft(self: Selection, s: *const Screen) Pin { .reverse => self.end(), .mirrored_forward => pin: { var p = self.start(); - p.x = self.end().x; + p.x = @min(self.end().x, p.node.cols() - 1); break :pin p; }, .mirrored_reverse => pin: { var p = self.end(); - p.x = self.start().x; + p.x = @min(self.start().x, p.node.cols() - 1); break :pin p; }, }; @@ -173,12 +173,12 @@ pub fn bottomRight(self: Selection, s: *const Screen) Pin { .reverse => self.start(), .mirrored_forward => pin: { var p = self.end(); - p.x = self.start().x; + p.x = @min(self.start().x, p.node.cols() - 1); break :pin p; }, .mirrored_reverse => pin: { var p = self.start(); - p.x = self.end().x; + p.x = @min(self.end().x, p.node.cols() - 1); break :pin p; }, }; @@ -326,6 +326,7 @@ pub fn containedRowCached( br: point.Coordinate, p: point.Coordinate, ) ?Selection { + _ = s; if (p.y < tl.y or p.y > br.y) return null; // Rectangle case: we can return early as the x range will always be the @@ -333,12 +334,12 @@ pub fn containedRowCached( if (self.rectangle) return init( start: { var copy: Pin = pin; - copy.x = tl.x; + copy.x = @min(tl.x, copy.node.cols() - 1); break :start copy; }, end: { var copy: Pin = pin; - copy.x = br.x; + copy.x = @min(br.x, copy.node.cols() - 1); break :end copy; }, true, @@ -355,7 +356,7 @@ pub fn containedRowCached( tl_pin, end: { var copy: Pin = pin; - copy.x = s.pages.cols - 1; + copy.x = copy.node.cols() - 1; break :end copy; }, false, @@ -387,7 +388,7 @@ pub fn containedRowCached( }, end: { var copy: Pin = pin; - copy.x = s.pages.cols - 1; + copy.x = copy.node.cols() - 1; break :end copy; }, false, @@ -1054,6 +1055,32 @@ test "Selection: order, standard" { } } +test "Selection: rectangle corners clamp across mixed-width pages" { + const testing = std.testing; + var s = try Screen.init(testing.allocator, .{ + .cols = 4, + .rows = 2, + .max_scrollback = 0, + }); + defer s.deinit(); + + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 1 }); + const second = first.next.?; + second.page().size.cols = 2; + + const sel = Selection.init( + .{ .node = first, .x = 3 }, + .{ .node = second, .x = 1 }, + true, + ); + try testing.expectEqual(.mirrored_forward, sel.order(&s)); + + const bottom_right = sel.bottomRight(&s); + _ = bottom_right.rowAndCell(); + try testing.expect((Pin{ .node = second, .x = 1 }).eql(bottom_right)); +} + test "Selection: order, rectangle" { const testing = std.testing; const alloc = testing.allocator; @@ -1602,3 +1629,46 @@ test "Selection: containedRow" { ).?); } } + +test "Selection: containedRow clamps mixed-width pages" { + const testing = std.testing; + var s = try Screen.init(testing.allocator, .{ + .cols = 4, + .rows = 3, + .max_scrollback = 0, + }); + defer s.deinit(); + + const first = s.pages.pages.first.?; + try s.pages.split(.{ .node = first, .y = 2 }); + try s.pages.split(.{ .node = first, .y = 1 }); + const middle = first.next.?; + const last = middle.next.?; + middle.page().size.cols = 2; + + const linear = Selection.init( + .{ .node = first, .x = 1 }, + .{ .node = last, .x = 1 }, + false, + ); + const linear_row = linear.containedRow( + &s, + .{ .node = middle }, + ).?; + _ = linear_row.end().rowAndCell(); + try testing.expect((Pin{ .node = middle }).eql(linear_row.start())); + try testing.expect((Pin{ .node = middle, .x = 1 }).eql(linear_row.end())); + + const rectangle = Selection.init( + .{ .node = first, .x = 1 }, + .{ .node = last, .x = 3 }, + true, + ); + const rectangle_row = rectangle.containedRow( + &s, + .{ .node = middle }, + ).?; + _ = rectangle_row.end().rowAndCell(); + try testing.expect((Pin{ .node = middle, .x = 1 }).eql(rectangle_row.start())); + try testing.expect((Pin{ .node = middle, .x = 1 }).eql(rectangle_row.end())); +} diff --git a/src/terminal/SelectionGesture.zig b/src/terminal/SelectionGesture.zig index f0e11f63e..2be1e8aa3 100644 --- a/src/terminal/SelectionGesture.zig +++ b/src/terminal/SelectionGesture.zig @@ -228,7 +228,8 @@ pub fn validatedLeftClickPin( } pub const Press = struct { - /// The time when the press event occurred. Use a monotonic timer. + /// The time when the press event occurred. Prefer a monotonic timer; + /// backwards timestamps reset the repeat sequence. /// This can be null if you're on a system that doesn't support /// time for some reason. In that case, we only support single clicks. time: ?Time, @@ -387,8 +388,8 @@ pub fn drag( .cell => dragSelection( click_pin.*, d.pin, - @intFromFloat(@max(0, self.left_click_xpos)), - @intFromFloat(@max(0, d.xpos)), + pixelFromFloat(self.left_click_xpos), + pixelFromFloat(d.xpos), d.rectangle, d.geometry, ), @@ -630,7 +631,8 @@ fn pressRepeat( const time = p.time orelse return error.PressRequiresReset; { const prev_time = self.left_click_time orelse return error.PressRequiresReset; - const since = timeSince(time, prev_time); + const since = timeSince(time, prev_time) orelse + return error.PressRequiresReset; if (since > p.repeat_interval) return error.PressRequiresReset; } @@ -664,11 +666,29 @@ fn pressRepeat( self.left_click_behavior = p.behaviors[self.left_click_count - 1]; } -fn timeSince(time: Time, prev_time: Time) u64 { - if (comptime freestanding_wasm) return time -| prev_time; +fn timeSince(time: Time, prev_time: Time) ?u64 { + if (comptime freestanding_wasm) { + if (time < prev_time) return null; + return time - prev_time; + } + + if (time.order(prev_time) == .lt) return null; return time.since(prev_time); } +/// Convert a caller-provided floating-point position to a pixel coordinate. +/// Negative and NaN values clamp to the origin, matching the drag behavior +/// outside the left edge of the surface. +fn pixelFromFloat(value: f64) u32 { + if (std.math.isNan(value) or value <= 0) return 0; + + // @intFromFloat requires a value representable by the destination type. + // Saturate first so positive infinity and oversized coordinates are safe. + const max: f64 = @floatFromInt(std.math.maxInt(u32)); + if (value >= max) return std.math.maxInt(u32); + return @intFromFloat(value); +} + fn pressSelection( self: *const SelectionGesture, screen: *Screen, @@ -713,6 +733,8 @@ fn dragSelection( // Rectangular selections are handled similarly, except that // entire columns are considered rather than individual cells. + if (geometry.columns == 0 or geometry.cell_width == 0) return null; + // We only include cells in the selection if the threshold point lies // between the start and end points of the selection. A threshold of // 60% of the cell width was chosen empirically because it felt good. @@ -721,7 +743,12 @@ fn dragSelection( )); // We use this to clamp the pixel positions below. - const max_x = geometry.columns * geometry.cell_width - 1; + const pixel_span = std.math.mul( + u32, + geometry.columns, + geometry.cell_width, + ) catch std.math.maxInt(u32); + const max_x = pixel_span - 1; // We need to know how far across in the cell the drag pos is, so // we subtract the padding and then take it modulo the cell width. @@ -935,6 +962,16 @@ fn testPress(t: *Terminal, x: u16, y: u32, time: ?std.time.Instant) Press { }; } +fn testInstant(ns: u64) std.time.Instant { + return switch (builtin.os.tag) { + .windows, .uefi, .wasi => .{ .timestamp = ns }, + else => .{ .timestamp = .{ + .sec = @intCast(ns / std.time.ns_per_s), + .nsec = @intCast(ns % std.time.ns_per_s), + } }, + }; +} + fn testDrag(t: *Terminal, x: u16, y: u32, xpos: f64, ypos: f64) Drag { return .{ .pin = t.screens.active.pages.pin(.{ .active = .{ @@ -1532,6 +1569,58 @@ test "SelectionGesture drag returns selection and records autoscroll" { try testing.expectEqual(.down, gesture.left_drag_autoscroll); } +test "SelectionGesture drag clamps unrepresentable positions" { + var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); + defer t.deinit(testing.allocator); + + var gesture: SelectionGesture = .init; + defer gesture.deinit(&t); + + var press_event = testPress(&t, 1, 1, try std.time.Instant.now()); + press_event.xpos = 10; + _ = try gesture.press(&t, press_event); + + const positive = gesture.drag( + &t, + testDrag(&t, 1, 1, std.math.inf(f64), 50), + ).?; + try testing.expect((testPin(&t, 1, 1)).eql(positive.start())); + try testing.expect((testPin(&t, 1, 1)).eql(positive.end())); + + try testing.expectEqual(null, gesture.drag( + &t, + testDrag(&t, 1, 1, std.math.nan(f64), 50), + )); +} + +test "SelectionGesture drag saturates overflowing geometry" { + var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); + defer t.deinit(testing.allocator); + + var gesture: SelectionGesture = .init; + defer gesture.deinit(&t); + + _ = try gesture.press(&t, testPress(&t, 1, 1, try std.time.Instant.now())); + var drag_event = testDrag(&t, 1, 1, 10, 50); + drag_event.geometry.columns = std.math.maxInt(u32); + drag_event.geometry.cell_width = std.math.maxInt(u32); + try testing.expectEqual(null, gesture.drag(&t, drag_event)); +} + +test "SelectionGesture drag rejects empty geometry" { + var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); + defer t.deinit(testing.allocator); + + var gesture: SelectionGesture = .init; + defer gesture.deinit(&t); + + _ = try gesture.press(&t, testPress(&t, 1, 1, try std.time.Instant.now())); + var drag_event = testDrag(&t, 1, 1, 10, 50); + drag_event.geometry.columns = 0; + drag_event.geometry.cell_width = 0; + try testing.expectEqual(null, gesture.drag(&t, drag_event)); +} + test "SelectionGesture release clears autoscroll and records drag" { var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); defer t.deinit(testing.allocator); @@ -1965,6 +2054,22 @@ test "SelectionGesture expired repeat resets click count" { try testing.expectEqual(@as(u3, 1), gesture.left_click_count); } +test "SelectionGesture backwards repeat time resets click count" { + var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); + defer t.deinit(testing.allocator); + + var gesture: SelectionGesture = .init; + defer gesture.deinit(&t); + + const earlier = testInstant(std.time.ns_per_s); + const later = testInstant(2 * std.time.ns_per_s); + _ = try gesture.press(&t, testPress(&t, 1, 1, later)); + _ = try gesture.press(&t, testPress(&t, 1, 1, earlier)); + + try testing.expectEqual(@as(u3, 1), gesture.left_click_count); + try testing.expectEqual(.eq, gesture.left_click_time.?.order(earlier)); +} + test "SelectionGesture screen switch resets click count" { var t = try Terminal.init(testing.allocator, .{ .cols = 5, .rows = 5 }); defer t.deinit(testing.allocator); diff --git a/src/terminal/Tabstops.zig b/src/terminal/Tabstops.zig index 68138cbf8..8dbe67893 100644 --- a/src/terminal/Tabstops.zig +++ b/src/terminal/Tabstops.zig @@ -170,11 +170,11 @@ pub fn reset(self: *Tabstops, interval: usize) void { @memset(&self.prealloc_stops, 0); @memset(self.dynamic_stops, 0); - if (interval > 0) { - var i: usize = interval; - while (i < self.cols - 1) : (i += interval) { - self.set(i); - } + if (interval == 0 or self.cols <= 1) return; + + var i: usize = interval; + while (i < self.cols - 1) : (i += interval) { + self.set(i); } } @@ -230,6 +230,13 @@ test "Tabstops: interval" { try testing.expect(t.get(8)); } +test "Tabstops: interval with zero columns" { + var t: Tabstops = try init(testing.allocator, 0, 8); + defer t.deinit(testing.allocator); + + try testing.expectEqual(@as(usize, 0), t.cols); +} + test "Tabstops: count on 80" { // https://superuser.com/questions/710019/why-there-are-11-tabstops-on-a-80-column-console diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 3bcfdf5d9..e5bfd93b7 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -952,7 +952,13 @@ pub fn print(self: *Terminal, c: u21) !void { const cps = self.screens.active.cursor.page_pin.node.page().lookupGrapheme(prev.cell).?; for (cps) |cp2| { // log.debug("cp1={x} cp2={x}", .{ previous_codepoint, cp2 }); - assert(!unicode.graphemeBreak(previous_codepoint, cp2, &state)); + // With mode 2027 disabled, zero-width codepoints are + // attached without applying grapheme boundary rules. If + // the mode is enabled later, an existing cell can + // therefore contain one or more breaks. Feed those breaks + // into the state machine so it can reset its context and + // determine the boundary for the new codepoint. + _ = unicode.graphemeBreak(previous_codepoint, cp2, &state); previous_codepoint = cp2; } } @@ -3512,8 +3518,13 @@ pub fn resize( // Resize our tabstops if (self.cols != cols) { + const tabstops: Tabstops = try .init( + alloc, + cols, + TABSTOP_INTERVAL, + ); self.tabstops.deinit(alloc); - self.tabstops = try .init(alloc, cols, 8); + self.tabstops = tabstops; } // Resize primary screen, which supports reflow @@ -3548,13 +3559,33 @@ pub fn resize( }; } +test "Terminal: resize preserves tabstops on allocation failure" { + var failing = testing.FailingAllocator.init(testing.allocator, .{}); + const alloc = failing.allocator(); + var t = try init(alloc, .{ .cols = 10, .rows = 1 }); + defer t.deinit(alloc); + + failing.fail_index = failing.alloc_index; + try testing.expectError(error.OutOfMemory, t.resize(alloc, 513, 1)); + + try testing.expectEqual(@as(size.CellCountInt, 10), t.cols); + try testing.expect(t.tabstops.get(8)); +} + /// Set the pwd for the terminal. pub fn setPwd(self: *Terminal, pwd: []const u8) !void { - self.pwd.clearRetainingCapacity(); - if (pwd.len > 0) { - try self.pwd.appendSlice(self.gpa(), pwd); - try self.pwd.append(self.gpa(), 0); + if (pwd.len == 0) { + self.pwd.clearRetainingCapacity(); + return; } + + const capacity = std.math.add(usize, pwd.len, 1) catch + return error.OutOfMemory; + try self.pwd.ensureTotalCapacity(self.gpa(), capacity); + + self.pwd.items.len = capacity; + std.mem.copyForwards(u8, self.pwd.items[0..pwd.len], pwd); + self.pwd.items[pwd.len] = 0; } /// Returns the pwd for the terminal, if any. The memory is owned by the @@ -3564,13 +3595,41 @@ pub fn getPwd(self: *const Terminal) ?[:0]const u8 { return self.pwd.items[0 .. self.pwd.items.len - 1 :0]; } +test "Terminal: setPwd preserves a sentinel on allocation failure" { + var failing = testing.FailingAllocator.init(testing.allocator, .{}); + const alloc = failing.allocator(); + var t = try init(alloc, .{ .cols = 5, .rows = 1 }); + defer t.deinit(alloc); + + try t.pwd.ensureTotalCapacityPrecise(alloc, 3); + failing.fail_index = failing.alloc_index; + try testing.expectError(error.OutOfMemory, t.setPwd("pwd")); + try testing.expect(t.getPwd() == null); +} + +test "Terminal: setPwd accepts its current value" { + var t = try init(testing.allocator, .{ .cols = 5, .rows = 1 }); + defer t.deinit(testing.allocator); + + try t.setPwd("file:///tmp"); + try t.setPwd(t.getPwd().?); + try testing.expectEqualStrings("file:///tmp", t.getPwd().?); +} + /// Set the title for the terminal, as set by escape sequences (e.g. OSC 0/2). pub fn setTitle(self: *Terminal, t: []const u8) !void { - self.title.clearRetainingCapacity(); - if (t.len > 0) { - try self.title.appendSlice(self.gpa(), t); - try self.title.append(self.gpa(), 0); + if (t.len == 0) { + self.title.clearRetainingCapacity(); + return; } + + const capacity = std.math.add(usize, t.len, 1) catch + return error.OutOfMemory; + try self.title.ensureTotalCapacity(self.gpa(), capacity); + + self.title.items.len = capacity; + std.mem.copyForwards(u8, self.title.items[0..t.len], t); + self.title.items[t.len] = 0; } /// Returns the title for the terminal, if any. The memory is owned by the @@ -3580,6 +3639,27 @@ pub fn getTitle(self: *const Terminal) ?[:0]const u8 { return self.title.items[0 .. self.title.items.len - 1 :0]; } +test "Terminal: setTitle preserves a sentinel on allocation failure" { + var failing = testing.FailingAllocator.init(testing.allocator, .{}); + const alloc = failing.allocator(); + var t = try init(alloc, .{ .cols = 5, .rows = 1 }); + defer t.deinit(alloc); + + try t.title.ensureTotalCapacityPrecise(alloc, 5); + failing.fail_index = failing.alloc_index; + try testing.expectError(error.OutOfMemory, t.setTitle("title")); + try testing.expect(t.getTitle() == null); +} + +test "Terminal: setTitle accepts its current value" { + var t = try init(testing.allocator, .{ .cols = 5, .rows = 1 }); + defer t.deinit(testing.allocator); + + try t.setTitle("Ghostty"); + try t.setTitle(t.getTitle().?); + try testing.expectEqualStrings("Ghostty", t.getTitle().?); +} + /// Switch to the given screen type (alternate or primary). /// /// This does NOT handle behaviors such as clearing the screen, @@ -4297,6 +4377,22 @@ test "Terminal: print multicodepoint grapheme, disabled mode 2027" { try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } })); } +test "Terminal: enabling grapheme mode handles stored breaks" { + var t = try init(testing.allocator, .{ .cols = 5, .rows = 1 }); + defer t.deinit(testing.allocator); + + t.modes.set(.grapheme_cluster, false); + try t.print('a'); + try t.print(0x200B); // Zero width space is stored on the prior cell. + + t.modes.set(.grapheme_cluster, true); + try t.print(0x0301); + + const str = try t.plainString(testing.allocator); + defer testing.allocator.free(str); + try testing.expectEqualStrings("a\xE2\x80\x8B\xCC\x81", str); +} + // Terminal.print receives one codepoint at a time, so it can't use // unicode.graphemeWidth directly; that API requires a complete buffered // cluster or string end. This keeps the streaming printer's cursor advance @@ -12491,13 +12587,6 @@ fn testPrintSliceDifferential( }, 15 => { const v = rand.boolean(); - // Erase the display first: grapheme clusters created - // while mode 2027 was off can trip a pre-existing - // debug assert in print()'s cluster walk when the mode - // is toggled on (unrelated to printSlice; it reproduces - // with per-codepoint print alone). - t1.eraseDisplay(.complete, false); - t2.eraseDisplay(.complete, false); t1.modes.set(.grapheme_cluster, v); t2.modes.set(.grapheme_cluster, v); }, diff --git a/src/terminal/search/Thread.zig b/src/terminal/search/Thread.zig index fa09af5f0..d7f0d5625 100644 --- a/src/terminal/search/Thread.zig +++ b/src/terminal/search/Thread.zig @@ -121,7 +121,11 @@ pub fn deinit(self: *Thread) void { // Nothing can possibly access the mailbox anymore, destroy it. self.mailbox.destroy(self.alloc); - if (self.search) |*s| s.deinit(); + if (self.search) |*s| { + self.opts.mutex.lock(); + defer self.opts.mutex.unlock(); + s.deinit(self.opts.terminal); + } } /// The main entrypoint for the thread. @@ -252,11 +256,15 @@ fn drainMailbox(self: *Thread) !void { fn select(self: *Thread, sel: ScreenSearch.Select) !void { const s = if (self.search) |*s| s else return; - const screen_search = s.screens.getPtr(s.last_screen.key) orelse return; self.opts.mutex.lock(); defer self.opts.mutex.unlock(); + // A screen can be removed or replaced between refresh ticks. Reconcile + // while holding the terminal lock before touching any ScreenSearch pins. + s.feed(self.alloc, self.opts.terminal); + const screen_search = s.screens.getPtr(s.last_screen.key) orelse return; + // Make the selection. Ignore the result because we don't // care if the selection didn't change. _ = try screen_search.select(sel); @@ -308,7 +316,11 @@ fn changeNeedle(self: *Thread, needle: []const u8) !void { // If our search is unchanged, do nothing. if (std.ascii.eqlIgnoreCase(s.viewport.needle(), needle)) return; - s.deinit(); + { + self.opts.mutex.lock(); + defer self.opts.mutex.unlock(); + s.deinit(self.opts.terminal); + } self.search = null; // When the search changes then we need to emit that it stopped. @@ -497,6 +509,11 @@ const Search = struct { /// The searchers for all the screens. screens: std.EnumMap(ScreenSet.Key, ScreenSearch), + /// ScreenSet generations captured when each searcher was initialized. + /// Allocators may reuse a destroyed Screen address, so pointer equality + /// alone cannot distinguish replacement screens from stale handles. + screen_generations: std.EnumMap(ScreenSet.Key, usize), + /// All state related to screen switches, collected so that when /// we switch screens it makes everything related stale, too. last_screen: ScreenState, @@ -537,16 +554,35 @@ const Search = struct { return .{ .viewport = vp, .screens = .init(.{}), + .screen_generations = .init(.{}), .last_screen = .{ .key = .primary }, .last_complete = false, .stale_viewport_matches = true, }; } - pub fn deinit(self: *Search) void { + pub fn deinit(self: *Search, t: *Terminal) void { self.viewport.deinit(); var it = self.screens.iterator(); - while (it.next()) |entry| entry.value.deinit(); + while (it.next()) |entry| { + if (self.screenIsValid(&t.screens, entry.key, entry.value)) { + entry.value.deinit(); + } else { + entry.value.deinitScreenInvalid(); + } + } + } + + fn screenIsValid( + self: *const Search, + screens: *const ScreenSet, + key: ScreenSet.Key, + search: *const ScreenSearch, + ) bool { + const generation = self.screen_generations.get(key) orelse return false; + if (generation != screens.generation(key)) return false; + const actual = screens.get(key) orelse return false; + return actual == search.screen; } /// Returns true if all searches on all screens are complete. @@ -629,18 +665,16 @@ const Search = struct { // Remove screens we have that no longer exist or changed. var it = self.screens.iterator(); while (it.next()) |entry| { - const remove: bool = remove: { - // If the screen doesn't exist at all, remove it. - const actual = t.screens.all.get(entry.key) orelse break :remove true; - - // If the screen pointer changed, remove it, the screen - // was totally reinitialized. - break :remove actual != entry.value.screen; - }; + const remove = !self.screenIsValid( + &t.screens, + entry.key, + entry.value, + ); if (remove) { - entry.value.deinit(); + entry.value.deinitScreenInvalid(); _ = self.screens.remove(entry.key); + _ = self.screen_generations.remove(entry.key); } } } @@ -649,7 +683,7 @@ const Search = struct { var it = t.screens.all.iterator(); while (it.next()) |entry| { if (self.screens.contains(entry.key)) continue; - self.screens.put(entry.key, ScreenSearch.init( + const screen_search = ScreenSearch.init( alloc, entry.value.*, self.viewport.needle(), @@ -664,7 +698,12 @@ const Search = struct { ); continue; }, - }); + }; + self.screens.put(entry.key, screen_search); + self.screen_generations.put( + entry.key, + t.screens.generation(entry.key), + ); } } @@ -903,3 +942,32 @@ test { } }, t.screens.active.pages.pointFromPin(.screen, sel.end).?); } } + +test "select after active screen removal" { + const alloc = testing.allocator; + var mutex: std.Thread.Mutex = .{}; + var t: Terminal = try .init(alloc, .{ .cols = 20, .rows = 2 }); + defer t.deinit(alloc); + + _ = try t.switchScreen(.alternate); + + var search: Search = try .init(alloc, "needle"); + search.feed(alloc, &t); + try testing.expectEqual(ScreenSet.Key.alternate, search.last_screen.key); + try testing.expect(search.screens.contains(.alternate)); + + var thread: Thread = undefined; + thread.search = search; + thread.opts = .{ + .mutex = &mutex, + .terminal = &t, + }; + defer if (thread.search) |*active| active.deinit(&t); + + _ = try t.switchScreen(.primary); + t.screens.remove(alloc, .alternate); + + try thread.select(.next); + try testing.expectEqual(ScreenSet.Key.primary, thread.search.?.last_screen.key); + try testing.expect(!thread.search.?.screens.contains(.alternate)); +} diff --git a/src/terminal/search/pagelist.zig b/src/terminal/search/pagelist.zig index e8354d799..98a24c187 100644 --- a/src/terminal/search/pagelist.zig +++ b/src/terminal/search/pagelist.zig @@ -85,6 +85,13 @@ pub const PageListSearch = struct { self.list.untrackPin(self.pin); } + /// Release owned search buffers after the PageList itself has already + /// been destroyed. Its pin belonged to the PageList pool and was freed + /// with that list, so it must not be untracked here. + pub fn deinitListInvalid(self: *PageListSearch) void { + self.window.deinit(); + } + /// Return the next match in the loaded page nodes. If this returns /// null then the PageList search needs to be fed the next node(s). /// Call, `feed` to do this. @@ -126,6 +133,9 @@ pub const PageListSearch = struct { // Move our tracked pin to the new node. self.pin.node = node; + self.pin.y = node.rows() - 1; + self.pin.x = node.cols() - 1; + self.pin.garbage = false; if (rem == 0) break; } @@ -510,3 +520,31 @@ test "feed with pruned page" { // Feed should still do nothing try testing.expect(!try search.feed()); } + +test "feed keeps its tracked pin within a shorter page" { + const alloc = testing.allocator; + var pages: PageList = try .init(alloc, 10, 2, null); + defer pages.deinit(); + + const first = pages.pages.first.?; + while (first.rows() < first.capacity().rows) _ = try pages.grow(); + + const second = (try pages.grow()).?; + while (second.rows() < second.capacity().rows) _ = try pages.grow(); + + try pages.split(.{ .node = first, .y = 1, .x = 0 }); + const shorter = first.next.?; + try testing.expect(shorter.rows() < second.rows()); + + var search: PageListSearch = try .init( + alloc, + "x", + &pages, + second, + ); + defer search.deinit(); + + try testing.expect(try search.feed()); + try testing.expectEqual(shorter, search.pin.node); + try testing.expect(pages.pinIsValid(search.pin.*)); +} diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index 96f996d59..448bb98c3 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -157,10 +157,29 @@ pub const ScreenSearch = struct { } pub fn deinit(self: *ScreenSearch) void { + self.deinitInternal(true); + } + + /// Release owned search state after the underlying Screen has already + /// been destroyed. Tracked pins belonged to that Screen's PageList pool + /// and were freed with it, so only independently owned memory is freed. + pub fn deinitScreenInvalid(self: *ScreenSearch) void { + self.deinitInternal(false); + } + + fn deinitInternal(self: *ScreenSearch, screen_valid: bool) void { const alloc = self.allocator(); self.active.deinit(); - if (self.history) |*h| h.deinit(self.screen); - if (self.selected) |*m| m.deinit(self.screen); + if (self.history) |*h| { + if (screen_valid) { + h.deinit(self.screen); + } else { + h.searcher.deinitListInvalid(); + } + } + if (screen_valid) { + if (self.selected) |*m| m.deinit(self.screen); + } for (self.active_results.items) |*hl| hl.deinit(alloc); self.active_results.deinit(alloc); for (self.history_results.items) |*hl| hl.deinit(alloc); @@ -327,20 +346,54 @@ pub const ScreenSearch = struct { fn pruneHistory(self: *ScreenSearch) void { // Go through our history results in order (newest to oldest) to find - // any result that contains an invalid serial. Prune up to that - // point. - for (0..self.history_results.items.len) |i| { + // any result that contains an invalid serial. + var i: usize = 0; + while (i < self.history_results.items.len) { const hl = &self.history_results.items[i]; - const serials = hl.chunks.items(.serial); + const chunks = hl.chunks.slice(); + const serials = chunks.items(.serial); const lowest = serials[0]; if (lowest < self.screen.pages.page_serial_min) { // Everything from here forward we assume is invalid because // our history results only get older. const alloc = self.allocator(); + if (self.selected) |*m| { + const first_pruned = self.active_results.items.len + i; + if (m.idx >= first_pruned) { + m.deinit(self.screen); + self.selected = null; + } + } for (self.history_results.items[i..]) |*prune_hl| prune_hl.deinit(alloc); self.history_results.shrinkAndFree(alloc, i); return; } + + // A page can also be replaced in place, such as by compaction. + // In that case its old serial remains above page_serial_min, so + // validate the captured pointer and serial against the live list. + const nodes = chunks.items(.node); + for (nodes, serials) |node, serial| { + if (!self.screen.pages.nodeIsValid(node, serial)) break; + } else { + i += 1; + continue; + } + + // Only this result is known to be invalid. Older results may live + // on unrelated pages and remain usable, so remove it individually. + const result_idx = self.active_results.items.len + i; + if (self.selected) |*m| { + if (m.idx == result_idx) { + m.deinit(self.screen); + self.selected = null; + } else if (m.idx > result_idx) { + m.idx -= 1; + } + } + + var removed = self.history_results.orderedRemove(i); + removed.deinit(self.allocator()); } } @@ -1497,6 +1550,72 @@ test "select after all matches disappear drops the selection" { try testing.expectEqual(0, search.history_results.items.len); } +test "select after partial history erase drops a pruned selection" { + const alloc = testing.allocator; + var t: Terminal = try .init(alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + const list: *PageList = &t.screens.active.pages; + + var stream = t.vtStream(); + defer stream.deinit(); + + stream.nextSlice("error\r\n"); + const first = list.pages.first.?; + while (list.totalPages() < 3) stream.nextSlice("\r\n"); + const first_rows = first.rows(); + + var search: ScreenSearch = try .init(alloc, t.screens.active, "error"); + defer search.deinit(); + try search.searchAll(); + try testing.expectEqual(0, search.active_results.items.len); + try testing.expectEqual(1, search.history_results.items.len); + try testing.expect(try search.select(.next)); + + list.eraseHistory(.{ .history = .{ .y = first_rows - 1 } }); + try testing.expect(!search.selected.?.highlight.start.garbage); + + try testing.expect(!try search.select(.next)); + try testing.expect(search.selected == null); +} + +test "select after history compaction ignores replaced results" { + const alloc = testing.allocator; + var t: Terminal = try .init(alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + const list: *PageList = &t.screens.active.pages; + + var stream = t.vtStream(); + defer stream.deinit(); + + stream.nextSlice("error\r\n"); + const first = list.pages.first.?; + while (list.totalPages() < 3) stream.nextSlice("\r\n"); + try list.split(.{ + .node = first, + .y = first.rows() / 2, + .x = 0, + }); + + var search: ScreenSearch = try .init(alloc, t.screens.active, "error"); + defer search.deinit(); + try search.searchAll(); + try testing.expectEqual(1, search.history_results.items.len); + + const replacement = (try list.compact(first)).?; + try testing.expect(replacement != first); + + try testing.expect(!try search.select(.next)); + try testing.expect(search.selected == null); +} + test "screen search no scrollback has no history" { const alloc = testing.allocator; var t: Terminal = try .init(alloc, .{