From 06178eeaad7648216a0da5eb73537e9dba2266cb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 2 Sep 2026 08:09:59 -0700 Subject: [PATCH] terminal/search: resume a complete search when history is prepended A search that had already exhausted a screen's PageList never picked up history pages prepended afterwards by incremental snapshot restore. The lower level PageListSearch and so on could already handle this, we just needed to let it know that more history existed to search. This fixes that. --- src/terminal/search/pagelist.zig | 57 +++++++++++++ src/terminal/search/screen.zig | 140 ++++++++++++++++++++++++++++++- src/terminal/search/terminal.zig | 112 +++++++++++++++++++++++++ 3 files changed, 305 insertions(+), 4 deletions(-) diff --git a/src/terminal/search/pagelist.zig b/src/terminal/search/pagelist.zig index 519d1ce76..a5ee402e8 100644 --- a/src/terminal/search/pagelist.zig +++ b/src/terminal/search/pagelist.zig @@ -562,3 +562,60 @@ test "feed keeps its tracked pin within a shorter page" { try testing.expectEqual(shorter, search.pin.node); try testing.expect(pages.pinIsValid(search.pin.*)); } + +test "feed discovers pages prepended after exhaustion" { + const alloc = testing.allocator; + const io = testing.io; + var t: Terminal = try .init(io, alloc, .{ + .cols = 10, + .rows = 10, + .max_scrollback_bytes = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + + var s = t.vtStream(); + defer s.deinit(); + s.nextSlice("Fizz"); + + // Exhaust the list: one match on the only page and nothing left to feed. + const pages = &t.screens.active.pages; + var search: PageListSearch = try .init( + alloc, + "Fizz", + pages, + pages.pages.last.?, + ); + defer search.deinit(); + try testing.expect(search.next() != null); + try testing.expect(search.next() == null); + try testing.expect(!try search.feed()); + + // Prepend an older page the way incremental snapshot history restore + // does. The frontier pin names the page that used to be first, and + // finalize keeps that node's identity, so the pin now has a `prev`. + const old_first = pages.pages.first.?; + { + var allocation = try pages.allocatePage(.{ .cols = 10, .rows = 2 }); + defer allocation.deinit(); + const page = allocation.page(); + page.size.rows = 2; + for ("Fizz", 0..) |c, x| page.getRowAndCell(x, 1).cell.* = .init(c); + try allocation.finalize(.prepend); + } + const prepended = pages.pages.first.?; + try testing.expect(prepended != old_first); + try testing.expectEqual(old_first, search.pin.node); + + // Feeding resumes into the restored history and finds its match. + try testing.expect(try search.feed()); + try testing.expectEqual(prepended, search.pin.node); + const h = search.next().?; + const sel = h.untracked(); + try testing.expectEqual(prepended, sel.start.node); + try testing.expectEqual(prepended, sel.end.node); + try testing.expectEqual(@as(size.CellCountInt, 1), sel.start.y); + try testing.expectEqual(@as(size.CellCountInt, 0), sel.start.x); + try testing.expectEqual(@as(size.CellCountInt, 3), sel.end.x); + try testing.expect(search.next() == null); + try testing.expect(!try search.feed()); +} diff --git a/src/terminal/search/screen.zig b/src/terminal/search/screen.zig index cbda5a388..ac6d2361b 100644 --- a/src/terminal/search/screen.zig +++ b/src/terminal/search/screen.zig @@ -124,7 +124,9 @@ pub const ScreenSearch = struct { .history_feed => true, // Not obvious but complete search states will prune - // stale history results on feed. + // stale history results on feed, and pick up pages + // prepended below the searched frontier since the last + // feed (incremental snapshot history restore). .complete => true, else => false, @@ -305,6 +307,11 @@ pub const ScreenSearch = struct { /// /// Feed on a complete screen search will perform some cleanup of /// potentially stale history results (pruned) and reclaim some memory. + /// + /// If pages were prepended to the PageList since the search completed + /// (incremental snapshot history restore), the feed picks them up and + /// the search resumes, since "complete" only ever means caught up with + /// the PageList as of the last feed. pub fn feed(self: *ScreenSearch) Allocator.Error!void { // Resize/reflow invalidates every flattened result, not just search // state that needs another history feed. @@ -338,9 +345,13 @@ pub const ScreenSearch = struct { // Feed goes back to searching history. .history_feed => self.state = .history, - // If we're complete then the feed call above should always - // return false and we can't reach this. - .complete => unreachable, + // A complete search had exhausted the PageList as of its last + // feed, but pages can still be prepended below that afterward + // (incremental snapshot history restore). The history searcher's + // tracked pin keeps its node identity through a prepend, so it + // just discovered those older pages and loaded them. Resume + // searching history. + .complete => self.state = .history, } } @@ -1992,3 +2003,124 @@ test "select after clearing scrollback" { _ = try search.select(.next); _ = try search.select(.prev); } + +test "feed after complete discovers prepended history pages" { + const alloc = testing.allocator; + const io = testing.io; + var t: Terminal = try .init(io, alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback_bytes = std.math.maxInt(usize), + }); + defer t.deinit(alloc); + const list: *PageList = &t.screens.active.pages; + + var s = t.vtStream(); + defer s.deinit(); + + // The screen begins with the tail of the needle so a restored page which + // soft-wraps into it can complete a match across the restore boundary. + s.nextSlice("st\r\nTest\r\n"); + while (list.totalPages() < 2) s.nextSlice("\r\n"); + for (0..list.rows) |_| s.nextSlice("\r\n"); + s.nextSlice("Test"); + const old_first = list.pages.first.?; + + // Search to completion, then select the oldest match so we can verify + // that restored (older) results never move an existing selection. + var search: ScreenSearch = try .init(alloc, t.screens.active, "Test"); + defer search.deinit(); + try search.searchAll(); + try testing.expect(search.state.isComplete()); + try testing.expectEqual(1, search.active_results.items.len); + try testing.expectEqual(1, search.history_results.items.len); + try testing.expect(try search.select(.prev)); + const selected_idx = search.selected.?.idx; + try testing.expectEqual(1, selected_idx); + const selected_before = search.selectedMatch().?.untracked(); + try testing.expectEqual(old_first, selected_before.start.node); + + // Restore the newest history page: a full match on its first row and a + // soft-wrapped last row ending in "Te" which joins the screen's "st". + { + var allocation = try list.allocatePage(.{ .cols = 10, .rows = 2 }); + defer allocation.deinit(); + const page = allocation.page(); + page.size.rows = 2; + for ("Test", 0..) |c, x| page.getRowAndCell(x, 0).cell.* = .init(c); + for ("xxxxxxxxTe", 0..) |c, x| page.getRowAndCell(x, 1).cell.* = .init(c); + page.getRow(1).wrap = true; + old_first.page().getRow(0).wrap_continuation = true; + try allocation.finalize(.prepend); + } + const newest = list.pages.first.?; + + // The completed search is fed like any periodic refresh. It must pick + // the page up and go back to searching rather than staying complete. + try search.feed(); + try testing.expect(!search.state.isComplete()); + try search.searchAll(); + try testing.expectEqual(1, search.active_results.items.len); + try testing.expectEqual(3, search.history_results.items.len); + + // New results are older than everything cached, so they append in + // newest-to-oldest order: the boundary match first, then the row above. + { + const boundary = search.history_results.items[1].untracked(); + try testing.expectEqual(newest, boundary.start.node); + try testing.expectEqual(old_first, boundary.end.node); + try testing.expectEqual(@as(size.CellCountInt, 1), boundary.start.y); + try testing.expectEqual(@as(size.CellCountInt, 8), boundary.start.x); + try testing.expectEqual(@as(size.CellCountInt, 0), boundary.end.y); + try testing.expectEqual(@as(size.CellCountInt, 1), boundary.end.x); + const str = try t.screens.active.selectionString(alloc, .{ + .sel = .init(boundary.start, boundary.end, false), + }); + defer alloc.free(str); + try testing.expectEqualStrings("Test", str); + } + { + const oldest = search.history_results.items[2].untracked(); + try testing.expectEqual(newest, oldest.start.node); + try testing.expectEqual(@as(size.CellCountInt, 0), oldest.start.y); + try testing.expectEqual(@as(size.CellCountInt, 0), oldest.start.x); + } + + // The selection kept both its index and its target. + try testing.expectEqual(selected_idx, search.selected.?.idx); + { + const selected_after = search.selectedMatch().?.untracked(); + try testing.expect(selected_before.start.eql(selected_after.start)); + try testing.expect(selected_before.end.eql(selected_after.end)); + } + + // Restore an even older page. Each incremental step is discovered from + // the frontier left behind by the previous one. + { + var allocation = try list.allocatePage(.{ .cols = 10, .rows = 2 }); + defer allocation.deinit(); + const page = allocation.page(); + page.size.rows = 2; + for ("Test", 0..) |c, x| page.getRowAndCell(x, 0).cell.* = .init(c); + try allocation.finalize(.prepend); + } + const oldest_node = list.pages.first.?; + try search.feed(); + try search.searchAll(); + try testing.expectEqual(4, search.history_results.items.len); + try testing.expectEqual( + oldest_node, + search.history_results.items[3].untracked().start.node, + ); + try testing.expectEqual(selected_idx, search.selected.?.idx); + { + const selected_after = search.selectedMatch().?.untracked(); + try testing.expect(selected_before.start.eql(selected_after.start)); + try testing.expect(selected_before.end.eql(selected_after.end)); + } + + // With nothing new, a feed on the complete search stays complete. + try search.feed(); + try testing.expect(search.state.isComplete()); + try testing.expectEqual(5, search.matchesLen()); +} diff --git a/src/terminal/search/terminal.zig b/src/terminal/search/terminal.zig index 062cc0049..19f059c63 100644 --- a/src/terminal/search/terminal.zig +++ b/src/terminal/search/terminal.zig @@ -653,3 +653,115 @@ test "no matches selects nothing" { try testing.expect(!try search.select(&t, .next, .if_needed)); try testing.expect(!try search.select(&t, .prev, .if_needed)); } + +test "feed after complete discovers prepended snapshot history" { + const alloc = testing.allocator; + const io = testing.io; + const snapshot = @import("../snapshot/main.zig"); + + // A source terminal with several pages of scrollback where every line + // is a match, so the expected total is simply the number of lines. + var source: Terminal = try .init(io, alloc, .{ + .cols = 10, + .rows = 2, + .max_scrollback_bytes = std.math.maxInt(usize), + }); + defer source.deinit(alloc); + var needle_count: usize = 0; + { + var stream = source.vtStream(); + defer stream.deinit(); + const list = &source.screens.active.pages; + while (list.totalPages() < 4) : (needle_count += 1) { + stream.nextSlice("needle\r\n"); + } + } + + var encoded: std.Io.Writer.Allocating = .init(alloc); + defer encoded.deinit(); + try snapshot.encode(alloc, &encoded.writer, &source, .{ + .continuation = .ground, + }); + + // Restore only through READY. The terminal is usable while its history + // pages are still in flight. + var reader: std.Io.Reader = .fixed(encoded.written()); + var decoder: snapshot.Decoder = .init(&reader); + var decoded = try decoder.ready(alloc, io, .{ + .max_continuation_bytes = 0, + }); + defer decoded.deinit(alloc); + var t = decoded.toOwned(); + defer t.deinit(alloc); + const pages_at_ready = t.screens.active.pages.totalPages(); + + const Pump = struct { + fn run(search: *TerminalSearch, term: *Terminal) void { + while (search.status() != .complete) { + switch (search.status()) { + .feed_required => search.feed(term, true), + .running => _ = search.tick(), + .complete => unreachable, + } + } + } + }; + + // Search the READY state to completion and select the oldest match. + var search: TerminalSearch = try .init(alloc, "needle"); + defer search.deinit(&t); + Pump.run(&search, &t); + const partial = search.activeScreenSearch().?.matchesLen(); + try testing.expect(partial > 0); + try testing.expect(partial < needle_count); + try testing.expect(try search.select(&t, .prev, .none)); + const selected_idx = search.activeScreenSearch().?.selected.?.idx; + try testing.expectEqual(partial - 1, selected_idx); + const selected_before = search.activeScreenSearch().?.selectedMatch().?.untracked(); + + // Restore every history page below the live terminal. + var restored_pages: usize = 0; + while (try decoder.next(alloc, &t)) |progress| : (restored_pages += 1) { + try testing.expect(progress.rows > 0); + } + try testing.expect(restored_pages > 0); + try testing.expectEqual( + pages_at_ready + restored_pages, + t.screens.active.pages.totalPages(), + ); + + // Refresh the existing search the way ghostty_search_run does: feed, + // then tick to completion. It must now cover the restored history. + search.feed(&t, true); + Pump.run(&search, &t); + const screen_search = search.activeScreenSearch().?; + try testing.expectEqual(needle_count, screen_search.matchesLen()); + + // Restored results are older than everything already cached, so the + // selection keeps both its index and its target. + try testing.expectEqual(selected_idx, screen_search.selected.?.idx); + const selected_after = screen_search.selectedMatch().?.untracked(); + try testing.expect(selected_before.start.eql(selected_after.start)); + try testing.expect(selected_before.end.eql(selected_after.end)); + + // Results stay ordered newest to oldest, ending at the very first line. + const matches = try screen_search.matches(alloc); + defer alloc.free(matches); + const pages = &t.screens.active.pages; + var prev_y: ?usize = null; + for (matches) |hl| { + const pt = pages.pointFromPin(.screen, hl.startPin()).?; + if (prev_y) |y| try testing.expect(pt.screen.y < y); + prev_y = pt.screen.y; + } + try testing.expectEqual(0, prev_y.?); + + // A search created after the restore agrees with the refreshed one. + var fresh: TerminalSearch = try .init(alloc, "needle"); + defer fresh.deinit(&t); + Pump.run(&fresh, &t); + try testing.expectEqual( + needle_count, + fresh.activeScreenSearch().?.matchesLen(), + ); +}