From 179863c0a7b011d5a4bd5e064f7d021d239aa96e Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Tue, 15 Sep 2026 20:17:50 -0300 Subject: Clear selections when navigating Back and Forward --- docs/helix-keys.md | 2 +- src/panes.zig | 4 +-- src/pardes.zig | 104 +++++++++++++++++++++++------------------------------ test/panes.zig | 62 +++++++++++++++++++++++++++++--- test/pdf.zig | 27 ++++++++++++++ 5 files changed, 132 insertions(+), 67 deletions(-) diff --git a/docs/helix-keys.md b/docs/helix-keys.md index 9dc86ee2..6907473a 100644 --- a/docs/helix-keys.md +++ b/docs/helix-keys.md @@ -134,7 +134,7 @@ language-backend queries, and the shell pipe. | `gd` `gD` `gy` `gi` `gr` | LSP definition / declaration / type-definition / implementation / references. ONE answer jumps straight there; several fill `+Search`, where n/N walk and Enter opens | in-process ZLS (`src/lsp/lsp_zls.zig`), `.zig` only — on a file the backend does not speak these do nothing at all, with no error row. Ctrl+left-click is the mouse spelling of `gd` | out of corpus | | `]d` `[d` / `]D` `[D` | step the diagnostics list / go to its last or first; if no list is up, asking the backend for one is part of the press | | out of corpus | | `=` | `format_selections` — writes a `- old` / `+ new` diff into `+Lsp` | deliberate divergence: the seam returns ROWS, not edits, so this SHOWS the formatting instead of applying it. Not in the corpus, so there is no waiver to name — the query leaves the core as an effect the headless harness has no shell to perform | out of corpus | -| `Ctrl-o` / `Ctrl-i` | jumplist back / forward (also Mouse4 / Mouse5 in SDL, macOS and web); selections remain unchanged (a selected cursor stays at its selection while the view visits the saved location); raw tty forwards both to the child — the `Back` / `Forward` builtins, also on `SPC j o` / `SPC j i`, with `SPC j l` rendering the stack as a buffer | helix binds both keys (`jump_backward` / `jump_forward`) but to a POSITION jumplist; pardes' stack is over panes and focus, so the keys agree and the semantics do not. `Ctrl-i` and Tab are the same byte under the legacy encoding; there Tab keeps meaning execute, and the pair only separates where the host speaks the kitty keyboard protocol | pardes-specific | +| `Ctrl-o` / `Ctrl-i` | jumplist back / forward (also Mouse4 / Mouse5 in SDL, macOS and web); successful navigation clears selections in both panes and places the cursor at the saved location; raw tty forwards both to the child — the `Back` / `Forward` builtins, also on `SPC j o` / `SPC j i`, with `SPC j l` rendering the stack as a buffer | helix binds both keys (`jump_backward` / `jump_forward`) but to a POSITION jumplist; pardes' stack is over panes and focus, so the keys agree and the semantics do not. `Ctrl-i` and Tab are the same byte under the legacy encoding; there Tab keeps meaning execute, and the pair only separates where the host speaks the kitty keyboard protocol | pardes-specific | | `\|` | pipe every selection through `/bin/sh -c`: its bytes in on stdin, its stdout replacing them, one undo across all cursors | helix's own key and meaning; the command is typed into the pane's tag after a bare `\|` marker rather than into a popup | out of corpus | | `A-\|` | the same, and the output is DISCARDED — the text is not touched at all | helix `shell_pipe_to`. For a command run for its effect. Marker `\|-` | out of corpus | | `!` | run with NO stdin, insert the output BEFORE each selection | helix `shell_insert_output`. Runs ONCE and every cursor gets that one answer, as helix does — ten cursors and `date` give ten identical stamps. Marker `!` | out of corpus | diff --git a/src/panes.zig b/src/panes.zig index c3fa732e..88e6a8ea 100644 --- a/src/panes.zig +++ b/src/panes.zig @@ -3604,7 +3604,7 @@ pub const Pdf = struct { return result; } - test "PDF selections retain their owning page across history navigation" { + test "PDF selections retain their owning page across page focus" { if (comptime !enabled) return; const gpa = std.testing.allocator; var state = try State.open(gpa, "docs/design.pdf", 1); @@ -5372,7 +5372,7 @@ pub const Pdf = struct { const destination = owner.state.sectionDestination(core.pdf_gpa, at.col - 1) orelse return true; switch (destination) { .internal => |internal| { - core.clearLookSelection(owner.pane); + core.clearNavigationSelection(owner.pane); revealOutlineDestination(core, owner.pane, internal); }, .external => |uri| if (uri.len <= 256) core.emit(.{ .open_link = .from(uri) }), diff --git a/src/pardes.zig b/src/pardes.zig index 2a8ae5cc..16343972 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -5961,7 +5961,6 @@ pub const Pardes = struct { jumps: [MAX_JUMPS]Loc = undefined, njumps: usize = 0, jcur: usize = 0, - jump_selection_cursor: ?struct { pane: usize, serial: u32, row: i32, col: i32 } = null, look_src: [MAX_PANES]u32 = undefined, n_look_src: usize = 0, look_walk_owner: ?u32 = null, @@ -12220,13 +12219,13 @@ pub const Pardes = struct { fn focusPaneByPath(p: *Pardes, path: []const u8, at: look.Spot) bool { const target = p.openPaneTarget(path, at) orelse return false; - p.clearLookSelection(p.panes[target.pane.id].?); + p.clearNavigationSelection(p.panes[target.pane.id].?); p.focusPaneLine(target.pane.id, target.pane.at, .center); return true; } - /// Look starts a new selection; history navigation keeps the old one. - pub fn clearLookSelection(p: *Pardes, pane: *Pane) void { + /// Navigation starts a new selection at the destination. + pub fn clearNavigationSelection(p: *Pardes, pane: *Pane) void { pane.vsel = .{}; pane.msel = .{}; pane.nsel = 0; @@ -12241,9 +12240,6 @@ pub const Pardes = struct { state.clearSelection(p.pdf_gpa); }; if (p.drag == .select and p.panes[p.drag.select.id] == pane) p.drag = .none; - if (p.jump_selection_cursor) |saved| if (saved.serial == pane.serial) { - p.jump_selection_cursor = null; - }; } /// Resolve without changing focus or falling back to search. PDF links use @@ -12333,7 +12329,7 @@ pub const Pardes = struct { const pane = p.panes[id] orelse return; const link = probe.link orelse return; const target = p.pdfLinkLocation(pane, link) orelse return; - p.clearLookSelection(pane); + p.clearNavigationSelection(pane); if (p.canonicalLookLocation(id, probe.text)) |visible| { if (!std.mem.eql(u8, visible, target)) { const content = std.fmt.allocPrint(p.gpa, "{s}\n{s}\n", .{ visible, target }) catch |err| @@ -12367,7 +12363,7 @@ pub const Pardes = struct { // The operand may borrow selected terminal/PDF text released below. const txt = p.scratch.allocator().dupe(u8, operand) catch return; const trimmed = std.mem.trim(u8, txt, " \t\r\n"); - p.clearLookSelection(pane); + p.clearNavigationSelection(pane); const pl = look.parsePathLine(trimmed); if (comptime pdf_enabled) if (panes.Pdf.lookSection(p, id, pl.path, pl.at)) return; var realbuf: [4096]u8 = undefined; @@ -12390,14 +12386,14 @@ pub const Pardes = struct { .pane => |t| { if (t.id >= MAX_PANES) return; const target = p.panes[t.id] orelse return; - p.clearLookSelection(target); + p.clearNavigationSelection(target); p.focusPaneLine(t.id, t.at, .center); }, .url => |u| if (u.len <= 256) p.emit(.{ .open_link = .from(u) }), .dir => |dir| { for (p.panes, 0..) |slot, i| { if (slot) |tt| if (std.mem.eql(u8, tt.cwdSlice(), dir) and p.takesCommandLine(i)) { - p.clearLookSelection(tt); + p.clearNavigationSelection(tt); p.active = i; p.emitWrite(i, "ls\r"); return; @@ -12973,14 +12969,6 @@ pub const Pardes = struct { p.jcur = @min(cur, w -| 1); const pane = p.panes[p.active] orelse return; - // After a history arrival, an unchanged selection head still belongs - // to the selection, not to the location we just showed. - if (p.jump_selection_cursor) |saved| { - if (saved.pane == p.active and saved.serial == pane.serial and - saved.row == pane.cur_row and saved.col == pane.cur_col and - (pane.vsel.active or pane.msel.active or pane.nsel > 0)) return; - p.jump_selection_cursor = null; - } const now: Loc = .{ .pane = @intCast(p.active), .serial = pane.serial, @@ -13012,25 +13000,14 @@ pub const Pardes = struct { pub fn jumpBy(p: *Pardes, delta: i32) void { const next = @as(i64, @intCast(p.jcur)) + delta; if (p.njumps == 0 or next < 0 or next >= p.njumps) return; - p.jcur = @intCast(next); - const j = p.jumps[p.jcur]; + const j = p.jumps[@intCast(next)]; + if (j.pane >= MAX_PANES) return; const pane = p.panes[j.pane] orelse return; if (pane.serial != j.serial) return; - // A modal selection stores its head in the cursor. History changes - // the viewed location, but must not stretch that selection to it. - for (0..pane.sel.len) |slot| p.capturePointerSelection(pane, slot) catch return; - const preserve_cursor = pane.vsel.active or pane.msel.active or pane.nsel > 0; - const row = pane.cur_row; - const col = pane.cur_col; - const pinned = pane.cur_pinned; + if (p.panes[p.active]) |source| p.clearNavigationSelection(source); + if (p.active != j.pane) p.clearNavigationSelection(pane); + p.jcur = @intCast(next); p.focusPaneLine(j.pane, .{ .line = j.line, .col = j.col }, .center); - p.jump_selection_cursor = null; - if (preserve_cursor) { - p.jump_selection_cursor = .{ .pane = j.pane, .serial = pane.serial, .row = row, .col = col }; - pane.cur_row = row; - pane.cur_col = col; - pane.cur_pinned = pinned; - } } /// Recompute geometry, push grid-size changes to each emulator + pty, fire @@ -15000,32 +14977,43 @@ test "Look ignores missing and out of bounds pane addresses" { } } -test "jump history preserves modal and mouse selections" { +test "jump history clears selections and lands at the recorded cursor" { const p = try Pardes.init(std.testing.allocator, .{ .cols = 60, .rows = 12 }); defer p.deinit(); const pane = try p.setTestFile("alpha beta\ngamma delta\nepsilon zeta\n"); - pane.setRange(pane.file.?.content, 0, .{ .anchor = 1, .head = 5 }, true); - pane.sel[0] = .{ .state = .done, .c0 = 2, .c1 = 5, .r0 = 1, .r1 = 1 }; - const selection = pane.primaryRange(pane.file.?.content, 0); - try p.capturePointerSelection(pane, 0); - const mouse = pane.sel; p.njumps = 2; p.jcur = 1; p.jumps[0] = .{ .pane = 0, .serial = pane.serial, .line = 3, .col = 8 }; p.jumps[1] = .{ .pane = 0, .serial = pane.serial, .line = 1, .col = 1 }; - p.update(.{ .key = .{ .cp = 'o', .ctrl = true } }); - try std.testing.expectEqual(@as(usize, 0), p.jcur); - try std.testing.expectEqual(@as(u32, 3), p.jumps[0].line); - try std.testing.expectEqualDeep(selection, pane.primaryRange(pane.file.?.content, 0)); - try std.testing.expectEqualDeep(mouse, pane.sel); - p.update(.{ .key = .{ .cp = 'i', .ctrl = true } }); - try std.testing.expectEqual(@as(usize, 1), p.jcur); - try std.testing.expectEqualDeep(selection, pane.primaryRange(pane.file.?.content, 0)); - try std.testing.expectEqualDeep(mouse, pane.sel); - pane.vsel.active = false; - p.jumpBy(-1); - try std.testing.expectEqual(@as(i32, 2), pane.cur_row); - try std.testing.expectEqual(@as(i32, 7), pane.cur_col); + for ([_]u21{ 'o', 'i' }, 0..) |key, index| { + pane.setRange(pane.file.?.content, 0, .{ .anchor = 1, .head = 5 }, true); + pane.sel[0] = .{ .state = .done, .c0 = 2, .c1 = 5, .r0 = 1, .r1 = 1 }; + try p.capturePointerSelection(pane, 0); + pane.select = true; + p.update(.{ .key = .{ .cp = key, .ctrl = true } }); + try std.testing.expectEqual(index, p.jcur); + try std.testing.expectEqual(@as(usize, 2), p.njumps); + try std.testing.expectEqual(@as(u32, 3), p.jumps[0].line); + try std.testing.expect(!pane.vsel.active and !pane.msel.active and !pane.select); + try std.testing.expectEqual(@as(u8, 0), pane.nsel); + try std.testing.expectEqual(.none, pane.sel[0].state); + try std.testing.expect(pane.pointer_selections[0] == null); + try std.testing.expectEqual(@as(i32, if (index == 0) 2 else 0), pane.cur_row); + try std.testing.expectEqual(@as(i32, if (index == 0) 7 else 0), pane.cur_col); + } + pane.setRange(pane.file.?.content, 0, .{ .anchor = 1, .head = 5 }, true); + const selected = pane.primaryRange(pane.file.?.content, 0); + p.jumpBy(1); // No forward entry: selection stays untouched. + try std.testing.expectEqualDeep(selected, pane.primaryRange(pane.file.?.content, 0)); + for ([_]Loc{ + .{ .pane = 15, .serial = pane.serial, .line = 1, .col = 1 }, + .{ .pane = 0, .serial = pane.serial + 1, .line = 1, .col = 1 }, + }) |missing| { + p.jumps[0] = missing; + p.jumpBy(-1); + try std.testing.expectEqual(@as(usize, 1), p.jcur); + try std.testing.expectEqualDeep(selected, pane.primaryRange(pane.file.?.content, 0)); + } } test "jump history terminal rectangle follows output and clears on reflow" { @@ -15044,17 +15032,15 @@ test "jump history terminal rectangle follows output and clears on reflow" { try std.testing.expectEqual(.none, pane.sel[0].state); } -test "mouse thumb buttons navigate once without disturbing selection gestures" { +test "mouse thumb buttons navigate once and clear selection gestures" { const p = try Pardes.init(std.testing.allocator, .{ .cols = 60, .rows = 12 }); defer p.deinit(); const pane = try p.setTestFile("alpha beta\ngamma delta\nepsilon zeta\n"); pane.setRange(pane.file.?.content, 0, .{ .anchor = 1, .head = 5 }, true); - const selection = pane.primaryRange(pane.file.?.content, 0); p.njumps = 3; p.jcur = 2; for (0..3) |i| p.jumps[i] = .{ .pane = 0, .serial = pane.serial, .line = @intCast(i + 1), .col = 1 }; p.drag = .{ .select = .{ .id = 0, .button = config.select_button } }; - const drag = p.drag; p.update(.{ .mouse = .{ .button = .back, .kind = .press, .col = 0, .row = 0 } }); try std.testing.expectEqual(@as(usize, 1), p.jcur); for ([_]Mouse.Kind{ .drag, .release, .motion }) |kind| @@ -15063,6 +15049,6 @@ test "mouse thumb buttons navigate once without disturbing selection gestures" { p.update(.{ .mouse = .{ .button = .forward, .kind = .press, .col = 0, .row = 0 } }); p.update(.{ .mouse = .{ .button = .forward, .kind = .release, .col = 0, .row = 0 } }); try std.testing.expectEqual(@as(usize, 2), p.jcur); - try std.testing.expectEqualDeep(selection, pane.primaryRange(pane.file.?.content, 0)); - try std.testing.expectEqualDeep(drag, p.drag); + try std.testing.expect(!pane.vsel.active); + try std.testing.expectEqual(.none, p.drag); } diff --git a/test/panes.zig b/test/panes.zig index cb2e8169..81015356 100644 --- a/test/panes.zig +++ b/test/panes.zig @@ -899,7 +899,7 @@ const JumpSelectionTests = struct { if (wrap) try std.testing.expect(std.mem.indexOf(u8, selected, "界") != null); // Seed two actual locations after the drag, so both history directions - // must scroll while retaining the rectangle's original source text. + // must scroll and discard the previous selection. p.njumps = 2; p.jcur = 1; p.jumps[0] = .{ .pane = 0, .serial = pane.serial, .line = 60, .col = 1 }; @@ -909,25 +909,77 @@ const JumpSelectionTests = struct { _ = try p.render(frame.allocator()); try std.testing.expectEqual(@as(usize, 0), p.jcur); try std.testing.expect(pane.file.?.scroll > 20); - try std.testing.expectEqualStrings(selected, pardes.test_api.heldSelection(p, 0) orelse return error.LostMouseSelection); + try LookResetTests.cleared(pane); + try std.testing.expect(!pane.vsel.active); + try std.testing.expect(pardes.test_api.heldSelection(p, 0) == null); + try std.testing.expectEqual(@as(i32, 59), pane.cur_row); + try std.testing.expectEqual(@as(i32, 0), pane.cur_col); + // A selection created after Back must also disappear on Forward. + drag(p, x + 1, y, x + 3, y + 1); + try std.testing.expect(pardes.test_api.heldSelection(p, 0) != null); p.update(.{ .key = .{ .cp = 'i', .ctrl = true } }); _ = frame.reset(.retain_capacity); _ = try p.render(frame.allocator()); try std.testing.expectEqual(@as(usize, 1), p.jcur); try std.testing.expectEqual(@as(usize, 0), pane.file.?.scroll); - try std.testing.expectEqualStrings(selected, pardes.test_api.heldSelection(p, 0) orelse return error.LostMouseSelection); + try LookResetTests.cleared(pane); + try std.testing.expect(!pane.vsel.active); + try std.testing.expect(pardes.test_api.heldSelection(p, 0) == null); + try std.testing.expectEqual(@as(i32, 0), pane.cur_row); + try std.testing.expectEqual(@as(i32, 0), pane.cur_col); } - test "jump selection retains copied rectangle text after scrolling away and back" { + test "jump selection clears copied rectangles in both history directions" { for ([_]bool{ false, true }) |reverse| try exercise("alpha\nbravo\n", false, reverse, 1, 3, "lph\nrav"); } - test "jump selection retains wrapped Unicode and tab rectangle text" { + test "jump selection clears wrapped Unicode and tab rectangles" { for ([_]bool{ false, true }) |reverse| try exercise("界\tabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789\n", true, reverse, 0, 8, null); } + + test "jump selection clears source and destination state across panes" { + const p = try Pardes.init(std.testing.allocator, .{ .tty_only = true, .cols = 100, .rows = 30 }); + defer p.deinit(); + const source = try p.setTestFile("alpha\nbravo\ncharlie\n"); + const id = p.freeSlot().?; + const destination = try p.newDocPane(id); + destination.file = .{ + .path = try p.gpa.dupe(u8, "/jump-destination.txt"), + .content = try p.gpa.dupe(u8, "first\nsecond\nthird\n"), + .history = try panes.File.History.create(p.gpa), + }; + layout.insert(p, 0, 1, id); + layout.compute(p); + try LookResetTests.seed(source, "source held text"); + try LookResetTests.seed(destination, "destination held text"); + p.active = 0; + p.njumps = 2; + p.jcur = 1; + p.jumps[0] = .{ .pane = @intCast(id), .serial = destination.serial, .line = 3, .col = 2 }; + p.jumps[1] = .{ .pane = 0, .serial = source.serial, .line = 2, .col = 1 }; + p.update(.{ .key = .{ .cp = 'o', .ctrl = true } }); + try std.testing.expectEqual(id, p.active); + try std.testing.expectEqual(@as(usize, 0), p.jcur); + try LookResetTests.cleared(source); + try LookResetTests.cleared(destination); + try std.testing.expect(!source.vsel.active and !destination.vsel.active); + try std.testing.expectEqual(@as(i32, 2), destination.cur_row); + try std.testing.expectEqual(@as(i32, 1), destination.cur_col); + + try LookResetTests.seed(source, "new source held text"); + try LookResetTests.seed(destination, "new destination held text"); + p.update(.{ .key = .{ .cp = 'i', .ctrl = true } }); + try std.testing.expectEqual(@as(usize, 0), p.active); + try std.testing.expectEqual(@as(usize, 1), p.jcur); + try LookResetTests.cleared(source); + try LookResetTests.cleared(destination); + try std.testing.expect(!source.vsel.active and !destination.vsel.active); + try std.testing.expectEqual(@as(i32, 1), source.cur_row); + try std.testing.expectEqual(@as(i32, 0), source.cur_col); + } }; const TagNameTintTests = struct { diff --git a/test/pdf.zig b/test/pdf.zig index 7a05447e..beeb0b40 100644 --- a/test/pdf.zig +++ b/test/pdf.zig @@ -2055,3 +2055,30 @@ test "Esc back into a PDF keeps the offset within its page" { // snap, so a test that watched the offset alone would not see it coming. try std.testing.expect(!pv.scroll_to_page_pending); } + +test "PDF Back and Forward clear selections and land on recorded pages" { + if (!pdf_enabled or platform == .web) return error.SkipZigTest; + const p = try Pardes.init(std.testing.allocator, .{ .file = "docs/design.pdf", .cols = 80, .rows = 28 }); + defer p.deinit(); + p.native_images = true; + p.presentation.enabled = false; + const pane = p.panes[0].?; + const state = &pane.pdf.?; + try std.testing.expect(state.page_count > 1); + try std.testing.expect(state.focusLocation(p.pdf_gpa, 2, 0)); + p.njumps = 2; + p.jcur = 1; + p.jumps[0] = .{ .pane = 0, .serial = pane.serial, .line = 1, .col = 0 }; + p.jumps[1] = .{ .pane = 0, .serial = pane.serial, .line = 2, .col = 0 }; + for ([_]u21{ 'o', 'i' }, 0..) |key, destination| { + try std.testing.expectEqual(panes.Pdf.SelectionUpdate.changed, state.setSelection(p.pdf_gpa, .{ .x = 0, .y = 0 }, .{ .x = 1, .y = 1 }, true)); + try std.testing.expect(state.selection != null and state.selection_text.len > 0); + p.update(.{ .key = .{ .cp = key, .ctrl = true } }); + try std.testing.expectEqual(destination, p.jcur); + try std.testing.expectEqual(destination, state.page); + try std.testing.expect(state.selection == null); + try std.testing.expect(state.selection_page == null); + try std.testing.expectEqual(@as(usize, 0), state.selection_text.len); + try std.testing.expect(state.drag_anchor == null and state.drag_head == null); + } +} -- cgit v1.3