From 8435c7fe0113525f6df8420fa6fef45362bd65ab Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Thu, 3 Sep 2026 16:10:37 -0300 Subject: chords: Look and Exec act once per selection, and say when they cannot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both acme chords read the PRIMARY range and dropped every other cursor on the floor — the one thing a multi-cursor editor must not do with a command the user aimed at all of them. They are also structurally outside the machinery that would have handled it: the Enter/Tab chord is intercepted before `handleNormal` so it never becomes an Action with a scope, and `runBuiltin` bails on `multiOnce` anyway, because a builtin is per-keystroke rather than per-cursor. So `chordEachSel` takes the `submitPipe` shape instead: `paneRanges` once, forward in document order, every range's bytes COPIED before the first builtin runs. The copy is not caution — a `Look` opens panes and an `Exec` can run a builtin that edits or closes the pane those offsets point into, and a selection whose text is `Del` is a legal Exec. The loop re-checks the slot and its serial between iterations, the same guard `replaySels` makes for the same reason. With one cursor it returns false on the first line and the old path runs untouched. FOCUS FOLLOWS THE PRIMARY. `lookAt` sets `p.active` for every target it opens, so `Look` over four selections used to leave you at whichever one happened to sort last — an accident rather than an answer. ...AND A LOOK WITH NOWHERE TO PUT ITS ANSWER SAYS SO. All four slot checks in `lookAt` were a bare `orelse return`: with one selection that merely felt like a dead key, and with several it means "I opened nine of your fourteen and told you nothing". They report `NoPaneSlots` now, through the channel output_pane.zig already raises it on and a test already pins. The three openers report their own failure too, so a file that will not open says whether it was permission, a pipe, or size — which `look.readFile` only started distinguishing this week. Known and left: N selections that all resolve to nothing run N searches over one +Search buffer. Wasteful, converges, and worth its own change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016Q4RATpafkwahrovHQLKRf --- src/pardes.zig | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 112 insertions(+), 8 deletions(-) (limited to 'src/pardes.zig') diff --git a/src/pardes.zig b/src/pardes.zig index d9a77c31..00226f3c 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -700,6 +700,31 @@ test "gj/gk step the wrapped rows a body draws while j/k keep the file's lines" try std.testing.expectEqual(@as(i32, 1), pane.cur_row); } +test "the acme chords act once per selection, not once on the primary" { + const gpa = std.testing.allocator; + const p = try Pardes.init(gpa, .{ .tty_only = true, .cols = 100, .rows = 30 }); + defer p.deinit(); + while (p.nextEffect()) |_| {} + // Two selections, each naming a DIFFERENT builtin, so what ran is visible + // in the layout rather than in a shell nobody can read from a test. + const pane = try p.hxOpenFileContent("Newcol\nNewcol\n"); + const pl = try p.paneCursorLines(pane); + const ranges = [_]modal.HxRange{ + .{ .anchor = 0, .head = 6 }, + .{ .anchor = 7, .head = 13 }, + }; + Pardes.setPaneRanges(pane, pl, pane.file.?.content, &ranges, &.{}, 0, true); + try std.testing.expectEqual(@as(u8, 1), pane.nsel); + + const before = p.ncol; + p.update(.{ .key = .{ .cp = Key.tab } }); // config.exec_key: Exec + // BOTH ran. Before this the chord read the primary range and dropped the + // other cursor, so one keystroke over two cursors made one column. + try std.testing.expectEqual(before + 2, p.ncol); + // ...and the chord consumed the selection exactly as it does with one. + try std.testing.expectEqual(@as(u8, 0), p.panes[0].?.nsel); +} + test "selection pipe replaces all ranges atomically and undo restores them" { const gpa = std.testing.allocator; const p = try Pardes.init(gpa, .{ .tty_only = true }); @@ -8309,6 +8334,65 @@ pub const Pardes = struct { return p.yankRows(pane, @min(pane.msel.r0, pane.msel.r1), @max(pane.msel.r0, pane.msel.r1)); } + /// Run one acme chord (`Look`/`Exec`) once per EXPLICIT selection, in + /// document order. False when there is only the primary range, which is + /// the caller's cue to take its own single-selection path unchanged. + /// + /// The bytes of every range are copied BEFORE the first builtin runs, for + /// the reason `submitPipe` copies too: a `Look` opens panes and an `Exec` + /// can run a builtin that edits or closes the very pane these offsets are + /// into. After that the loop owns nothing of the pane but its slot, and + /// re-checks even that. + /// + /// FOCUS FOLLOWS THE PRIMARY, not the last range. `lookAt` sets `p.active` + /// for every target it opens, so without this the pane you end up looking + /// at is whichever selection happened to sort last — an accident rather + /// than an answer. `Exec` moves focus for neither, so this costs it + /// nothing. + fn chordEachSel(p: *Pardes, pane: *Pane, cmd: Builtin) bool { + if (pane.nsel == 0) return false; + const pl = p.paneCursorLines(pane) catch return false; + const text = p.flatSurface(pane, pl) catch return false; + var ranges: [MAX_SELS]modal.HxRange = undefined; + const got = paneRanges(pane, text, pl.row0, &ranges); + if (got.n < 2) return false; + + var texts: [MAX_SELS][]u8 = undefined; + var made: usize = 0; + defer for (texts[0..made]) |t| p.gpa.free(t); + for (ranges[0..got.n]) |range| { + const lo = @min(range.anchor, range.head); + const hi = @max(range.anchor, range.head); + if (hi > text.len) break; + texts[made] = p.gpa.dupe(u8, text[lo..hi]) catch break; + made += 1; + } + // All or nothing, like the pipe: half a chord is not a chord. + if (made != got.n) return false; + + const id = p.active; + const serial = pane.serial; + pane.vsel.active = false; + pane.msel.active = false; + pane.select = false; + pane.nsel = 0; // the chord consumed them, exactly as it consumes one + + var primary_active: ?usize = null; + for (texts[0..made], 0..) |txt, i| { + p.runBuiltin(cmd, id, "", txt); + if (i == got.pri) primary_active = p.active; + // The pane this loop is standing on can be closed by what it just + // ran — a selection whose text is `Del` is a legal Exec. Same + // slot-and-serial re-check `replaySels` makes for the same reason. + const still = p.panes[id] orelse break; + if (still.serial != serial) break; + } + if (primary_active) |a| if (p.panes[a] != null) { + p.active = a; + }; + return true; + } + const PointerOperand = struct { /// Absolute body position corresponding to the pointed screen cell. row: i32, @@ -8590,6 +8674,15 @@ pub const Pardes = struct { const explicit = (p.native_images and hasPdfSelection(pane)) or (pane.vsel.active and pane.vsel.explicit) or pane.msel.active; if (explicit) { + // ONE ACTION PER SELECTION. Both chords used to read the + // PRIMARY range and drop the other cursors on the floor, which + // is the one thing a multi-cursor editor must not do with a + // command the user aimed at every cursor. `chordEachSel` is + // the `submitPipe` shape — `paneRanges` once, forward, copies + // taken before anything runs — and returns false when there is + // nothing multi about this keystroke, which is every keystroke + // with one cursor and therefore the unchanged path below. + if (p.chordEachSel(pane, cmd)) return; if (p.currentSelText(pane)) |txt| { pane.vsel.active = false; pane.msel.active = false; @@ -14407,8 +14500,14 @@ pub const Pardes = struct { return; }; } - const free = p.freeSlot() orelse return; - const nt = p.newShell(free, dir) catch return; + // A LOOK WITH NOWHERE TO PUT THE ANSWER SAYS SO. All four + // slot checks in this function were a bare `orelse return`, + // which with one selection merely felt like a dead key and with + // several means "I opened nine of your fourteen and told you + // nothing". `NoPaneSlots` is the error output_pane.zig already + // raises for this, through the channel a test already pins. + const free = p.freeSlot() orelse return p.reportError(id, "look", error.NoPaneSlots); + const nt = p.newShell(free, dir) catch |err| return p.reportError(id, "look", err); nt.greet = true; const src = p.splitParent(id); const f = p.layoutFindTerm(src).?; @@ -14419,15 +14518,19 @@ pub const Pardes = struct { .file => |target| { if (comptime pdf_enabled) if (target.kind == .pdf) { if (p.focusPaneByPath(target.path, target.at)) return; - const free = p.freeSlot() orelse return; - const nt = pdf_pane.openPane(p, free, target.path, target.at.line) catch return; + const free = p.freeSlot() orelse return p.reportError(id, "look", error.NoPaneSlots); + const nt = pdf_pane.openPane(p, free, target.path, target.at.line) catch |err| + return p.reportError(id, "look", err); p.placeDoc(id, free, nt); return; }; // focus an existing pane on this path (rescrolled), else open if (p.focusPaneByPath(target.path, target.at)) return; - const free = p.freeSlot() orelse return; - const nt = file_pane.open(p, free, target.path, target.at.line) catch return; + const free = p.freeSlot() orelse return p.reportError(id, "look", error.NoPaneSlots); + // ...and WHY a file would not open, which `look.readFile` now + // distinguishes: permission denied, a pipe, too large. + const nt = file_pane.open(p, free, target.path, target.at.line) catch |err| + return p.reportError(id, "look", err); if (target.at.col > 0) nt.cur_col = @intCast(target.at.col - 1); p.placeDoc(id, free, nt); // center the target line: the pane's real body height only @@ -14448,8 +14551,9 @@ pub const Pardes = struct { }, .image => |target| { if (p.focusPaneByPath(target.path, .{})) return; - const free = p.freeSlot() orelse return; - const nt = image_pane.create(p, free, target.path, &.{}) catch return; + const free = p.freeSlot() orelse return p.reportError(id, "look", error.NoPaneSlots); + const nt = image_pane.create(p, free, target.path, &.{}) catch |err| + return p.reportError(id, "look", err); p.placeDoc(id, free, nt); }, } -- cgit v1.3