From 2383b2e3ba35c1398c586a8a89ade0f8c2ac2f74 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 9 Jul 2026 16:17:54 -0400 Subject: [PATCH 1/4] docs: kill-ring framing (Arc 2), rev 3 Q#KR1-11: Lua ring on a per-frontend command-boundary substrate. Eight-row boundary table (keybound/self-insert/unbound/GPU-optimistic/ pointer/paste/menu/M-x-interactive); stable ring-entry ids so shared- ring interleaving cannot corrupt per-frontend chains or yank sessions; sessions carry {buffer,start,end,entry_id,text} snapshots with slice verification. Three shipped bugs in scope (Q#KR10): semantic-path Paste drop (GPU Ctrl-V is a no-op today), missing after-edit for M-x/menu/ paste edits, and paste trusting the client-supplied payload id instead of the dispatcher's authenticated source. Q#KR11: frontend.detached hook + command_history detach cleanup. Incorporates three review rounds. Co-Authored-By: Claude Fable 5 --- docs/kill-ring-framing.md | 352 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 352 insertions(+) create mode 100644 docs/kill-ring-framing.md diff --git a/docs/kill-ring-framing.md b/docs/kill-ring-framing.md new file mode 100644 index 0000000..5406837 --- /dev/null +++ b/docs/kill-ring-framing.md @@ -0,0 +1,352 @@ +# Kill ring — framing (Arc 2, editing table stakes) — rev 3 + +pmacs has Emacs keybindings on a one-slot clipboard: `C-w`/`M-w`/`C-y` +cut/copy/paste through `EditorCore.clipboard_slot`, a single `Vec`. +Kill something, kill something else, and the first kill is gone. There +is no `C-k`, and `M-y` is unbound. This arc adds the real thing: kills +accumulate in a ring, consecutive kills append, `C-y` yanks the head, +`M-y` immediately after a yank cycles older entries. + +Roadmap: `docs/roadmap-2026-07.md` Arc 2 ("real kill ring + `M-y`"). + +**Rev 2** rebuilt the design around per-frontend command boundaries +after review showed `dispatch_key` is not the only input path. **Rev 3** +closes the second review's blockers: pointer gestures join the boundary +table; the shared ring gets stable entry identities so per-frontend +state survives other frontends' mutations; and two shipped bugs the +review surfaced move into scope — **GPU `Ctrl-V` paste is silently +dropped for semantic frontends**, and inbound paste fires no +`buffer.after-edit` anywhere. + +## Ground truth (scouted + twice review-verified; as of `4c4295d`) + +Input paths that reach a buffer or move point — **only the first runs +Lua command bodies**: + +1. **Round-tripped chords** (`dispatch_key`): every CTRL/ALT chord + (`src/optimistic.rs:134-142`) — all kill/yank commands, both + frontends — plus all TUI keys. Commands run via `Action::Run` + (`src/editor.rs:707`) or the self-insert fallback (`:730`); the + post-command revision check fires `buffer.after-edit` (`:739`). +2. **GPU optimistic edits**: bare typing, Enter/Tab, Backspace/Delete + arrive as CRDT ops at `handle_remote_crdt_op` (`src/daemon.rs:1941`) + — no command, no dispatch_key. (Fires `after-edit` itself.) +3. **Pointer gestures**: grid `Mouse` → `dispatch_mouse` + (`src/editor.rs:1114`) and the semantic `Pointer` path (`:1253`) + move the cursor and set/clear selections — **no command boundary**. + Right-click additionally opens the context menu. +4. **Inbound OS paste**: `FrontendEvent::Paste` → `paste_inbound` + (`editor_core.rs:2096`) — **but only on the grid path** + (`apply_event`, `src/daemon.rs:2241`). The semantic input + dispatcher **drops `Paste`** (`apply_semantic_input_event`, + `src/daemon.rs:2200`, `_ => {}` with a "no grid-less effect yet" + comment). pmacs-gpu always negotiates semantic render + (`pmacs-gpu/src/attach.rs:259`), so **GPU `Ctrl-V` is a no-op + today** — a shipped bug. Where paste *does* land (TUI bracketed + paste), it fires **no `buffer.after-edit`** — a second shipped gap + (LSP never sees pasted text). And the grid handler **trusts the + client-supplied payload id**: it sets `core.active_frontend` from + `Paste.frontend_id` rather than the dispatcher's authenticated + `source` (`DispatcherEvent::FrontendEvent { source, event }`, + `src/daemon.rs:1428`) — a third shipped gap: a forged payload id + pastes into *another frontend's* active window. +5. **Context menu**: `menu_invoke_active` (`src/editor.rs:946`) calls + `invoke_command` directly — no rotation, no post-command check. +6. **`M-x`**: the minibuffer shadow returns before the post-command + check (`src/editor.rs:664`), so `M-x`-invoked editing commands also + miss `after-edit` today. + +Clipboard: `clipboard_slot` + `pending_clipboard: Option<(FrontendId, +Vec)>` (`editor_core.rs:213,219`); `InstanceSignal::Clipboard` +(v6-floor) goes **to the originating frontend only** +(`daemon.rs:949-956`); GPU writes arboard, TUI writes OSC 52. External +content is visible to the daemon only via path 4. + +`buf:replace` (`mod.rs:1216`) uses the Lua buffer edit path +(intercepts, then `notify_buffer_edit_to_windows`, `mod.rs:1344`) and +queues a daemon-origin CRDT op — rotation needs **no protocol change**, +but intercepts may alter/reject the edit, so callers must verify the +applied text. + +Frontend lifecycle: `SessionDetached` (`src/daemon.rs:1574`) already +prunes six per-frontend maps; anything new that is keyed by +`FrontendId` must join that cleanup, and Lua-side per-frontend tables +need a detach signal (none exists). + +Nothing tracks the previous command; no per-command hook; `C-k`/`M-y` +unbound; `M-d`/`M-BS`/`C-BS`/`C-h`/`C-DEL` discard deleted bytes; no +Emacs mark; `ed.region/cursor/delete_region/goto_byte`, `buf:slice`, +`pmacs.frontend.id()` (reads `active_frontend`) all exist; no +`clipboard_get`/`clipboard_set`. + +## Decisions + +### Q#KR1 — Lua ring on a Rust command-boundary substrate + +The ring, append policy, yank sessions, and commands live in +`builtin/runtime/killring.lua`. Rust grows: the command-boundary +substrate (Q#KR2), `ed.clipboard_set(bytes)` + `ed.clipboard_get()`, +the **semantic-paste fix** and the **after-edit delivery fix** +(Q#KR10), the detach cleanup + `frontend.detached` hook (Q#KR11), and +the module load line in `EditorState::new`. The ring table is +daemon-global (shared across frontends, like the Emacs daemon); +everything session-shaped is per-frontend and identity-checked against +the shared ring (Q#KR4/6/7). + +### Q#KR2 — Per-frontend command boundaries; every input path updates them + +`EditorCore.command_history: HashMap`, +`CommandBoundary { this: Option, last: Option }`. +`ed.last_command()` reads the active frontend's entry. Two operations: +**`rotate(fid, name)`** (`last = this; this = Some(name)`) and +**`break_chain(fid)`** (`this = None` — the next rotation yields +`last = None`, failing every chain/session check). + +The boundary table — one row per input path (rev 3 adds pointer +gestures and the unified paste route): + +| path | site | operation | +|---|---|---| +| keybound command | `dispatch_key` `Action::Run` | `rotate(fid, name)` | +| typed char (round-trip) | self-insert fallback | `rotate(fid, "buffer.self-insert")` | +| unbound key | `dispatch_key` unbound arm | `break_chain(fid)` | +| GPU optimistic edit | `handle_remote_crdt_op` | `break_chain(source)` | +| **pointer gesture** | `dispatch_mouse` (grid) **and** the semantic `Pointer` handler — any press/drag/release that moves point or changes the selection; menu-opening right-click included | `break_chain(fid)` | +| inbound OS paste | the **unified** paste route (Q#KR10) | `break_chain(source)` — the **authenticated** id, never the payload's | +| menu item | `menu_invoke_active` | `rotate(fid, name)` | +| `M-x` accept | `pmacs.command.invoke_interactive` (new) | `rotate(fid, name)` | + +Pointer scope note: scroll-wheel events that move only the viewport do +**not** break the chain (point is untouched); anything that relocates +the cursor or edits the selection does. Emacs behaves the same way +(`mwheel-scroll` preserves `last-command` chains; `mouse-set-point` +breaks them). + +`invoke_interactive(name)` rotates then invokes; `editor.execute- +command`'s accept uses it. Plain `pmacs.command.invoke` stamps nothing +(public programmatic API). This preserves rev 2's Emacs-verified `M-x` +matrix: `C-k`→`M-x kill-line` no-append; `M-x kill-line`→`C-k` append; +`M-x` twice no-append. + +### Q#KR3 — What feeds the ring (v1 command set) + +Unchanged from rev 2: `C-w`/`M-w`/`C-y` bodies become ring-aware +(context menu inherits by name); new `C-k` `edit.kill-line` (chunked +`buf:slice` newline scan; at EOL kills the newline) and `M-y` +`edit.yank-pop`. **Deferred**: word kills — `M-d`, `M-BS`, `C-BS`, +`C-h`, `C-DEL` all share the discard-the-bytes deleters and keep plain +deleting until the Rust deleters return bytes. + +### Q#KR4 — Append requires success **and** an unmoved head (stable ids) + +Rev 2's per-frontend `last_kill_ok: bool` corrupts the shared ring +under interleaving (review blocker): A kills, B kills, A kills again — +A's flag is still true, so A appends onto **B's** entry. + +So ring entries carry a **stable identity**: each push mints a +monotonically increasing `id`; entries are `{id, text}`. The +per-frontend kill state is **`last_kill_id`** (nil = no live kill +chain), and the append condition becomes: + +> `ed.last_command()` is a kill command **and** this frontend's +> `last_kill_id` is non-nil **and** equals the current head's `id`. + +A-kill/B-kill/A-kill: A's `last_kill_id` names A's entry, the head is +B's → **push fresh**, B's kill intact. A successful push/append sets +`last_kill_id = head.id`; every failed or no-op kill-family invocation +clears it (rev 2's success rule, carried forward). Appends keep the +entry's `id` (the entry is *extended*, not replaced — a same-frontend +chain keeps appending). The appended head re-syncs to the acting +frontend's OS clipboard. + +### Q#KR5 — The ring + +Entries `{id, text}`, most-recent first, shared. Cap default **60**; +`pmacs.killring.max([n])` validated as in rev 2 (**non-finite +rejected** — `NaN`, `math.huge`; ≥ 1; floored; **shrink trims +immediately**). Duplicate-of-head pushes collapse (no new id). +`pmacs.killring.list()` returns texts for introspection/tests. + +### Q#KR6 — Yank: per-frontend sessions, snapshot + stable cursor + +`C-y` (unchanged flow from rev 2, session shape corrected): +1. Slot check: `ed.clipboard_get()` non-empty and ≠ head text → push it + (external content joins the ring). +2. Ring empty → `"kill ring empty"`, no session. +3. `start` = region lo or cursor; `ed.clipboard_set` if the slot + differs; `ed.clipboard_paste()` (existing insert-over-region, + `cursor = start + len`). +4. **Session (per `FrontendId`)**: `{buffer, start, end, entry_id, + text}` — `text` is the snapshot actually inserted (rev 2 claimed to + verify against it but never stored it — review catch), `entry_id` + is the ring entry's stable id (an index would be shifted by any + other frontend's push — review blocker). Created only on successful + paste. + +**OS clipboard scope** (unchanged, explicit): the ring is shared; OS +mirroring is local to the acting frontend — `pending_clipboard`'s +existing `(FrontendId, bytes)` shape, and the right privacy call for +frontends on different machines. + +Named limitation (unchanged): an external copy is invisible until +pasted; `C-y` before any paste yanks the ring head. GPU `Ctrl-V` +covers the direct gesture — **and actually works after this PR** +(Q#KR10 fixes the semantic drop). + +### Q#KR7 — Yank-pop: stable-id rotation, snapshot-verified + +`M-y` is valid only when, for the acting frontend: (1) `last_command` +is `edit.paste`/`edit.yank-pop`; (2) a live session exists; (3) the +session's buffer is active; (4) `buf:slice(start, end) == +session.text`. Any failure → refuse with status, drop nothing into the +buffer, **and create/keep no session** (a second invalid `M-y` cannot +ride the first's name-stamp). + +When valid: +1. Locate `session.entry_id` in the ring; **absent (evicted or + trimmed) → invalidate the session and refuse.** Otherwise step to + the next-older entry, wrapping. Because the cursor is an id, another + frontend pushing entries mid-session shifts positions but not + identity — rotation continues from where this frontend actually was + (Emacs's `kill-ring-yank-pointer` behavior; an integer index would + silently re-yank the wrong entry — review blocker). +2. `buf:replace(start, end, entry.text)`, then verify by re-slice. +3. On match: `ed.goto_byte(start + #entry.text)`, update `end`, + `entry_id`, `text`. +4. On mismatch (a buffer intercept altered the edit): **accepted + post-hoc semantics, stated as a decision** — the interceptor's + result is left in place, the session is dropped, and the status says + so. Kill/yank inside intercepted (round-trip/REPL-prompt) buffers is + niche; a preflight refusal would need a new Lua-visible intercept + probe, which is not worth the seam in v1. (Review offered both + options; this is the explicit choice of the simpler one.) + +The OS slot is untouched by `M-y`; undo treats each rotation as a +normal edit. + +### Q#KR8 — No mark in v1 + +Unchanged: CUA selections only; `C-SPC`/set-mark deferred; `C-w` +requires a selection. + +### Q#KR9 — Module shape and loading + +Unchanged from rev 2: `killring.lua` holds the ring, the per-frontend +tables (keyed by `pmacs.frontend.id()`), commands, and bindings; +`default.lua` bodies delegate at invoke time; loaded explicitly in +`EditorState::new`. + +### Q#KR10 — Unified paste + after-edit delivery (two shipped bugs, in scope) + +**(a) Semantic paste, on the authenticated source.** +`FrontendEvent::Paste` is handled only in the grid `apply_event`; the +semantic dispatcher drops it, and the GPU is always semantic — so GPU +`Ctrl-V` does nothing today. Fix: route `Paste` **before the +grid/semantic split** (one handler for both attachment kinds) → +`paste_inbound` + `break_chain`. + +That handler keys everything off the **dispatcher's authenticated +`source: FrontendId`** — `DispatcherEvent::FrontendEvent { source, .. }` +is stamped per attach session, while `Paste.frontend_id` is +client-supplied bytes. The current grid handler trusts the payload and +sets `active_frontend` from it, so a forged id pastes into another +frontend's active window (review blocker). The unified handler ignores +the payload id (logging a mismatch), targets `source`'s active view, +and calls `break_chain(source)`. A forged-id acceptance test pins it. + +**(b) After-edit delivery — for the three scoped paths, not "every +command".** Three sites run edits outside `dispatch_key`'s revision +check and never fire `buffer.after-edit`: the minibuffer accept +callback (`M-x`), the menu invoke, and **paste**. Extract the +revision-compare-then-fire into a helper and call it at those three +sites. No double-fire on the keybound path. + +Scope honesty (review correction): the helper keeps the existing +*active-buffer* before/after comparison, which is sound for these three +paths — kill, yank, menu Cut, and paste all edit the active buffer and +do not switch away mid-command. It is **not** a general guarantee: a +command that edits buffer A and then switches to B still evades the +compare (the check would diff B's revision). The general fix is a +buffer-aware edit epoch (any-buffer mutation counter + per-buffer hook +targeting) — deferred, named, and out of this PR's scope. + +### Q#KR11 — Frontend lifecycle cleanup + +`SessionDetached` already prunes six per-frontend maps +(`daemon.rs:1574`); `command_history` joins them. For the Lua side, the +same arm fires a new **`frontend.detached`** hook carrying the raw +frontend id; `killring.lua` subscribes and drops that frontend's +session/`last_kill_id` entries. (Frontend ids are monotonic, so without +this both maps grow for the daemon's lifetime — review note.) The hook +is tiny and generally useful (the first frontend-lifecycle hook). + +## Phasing + +One PR. In-diff order: Rust substrate (command boundaries + all eight +table rows, `clipboard_get/set`, unified paste route, after-edit helper ++ three call sites, `frontend.detached` + detach cleanup, module load) +→ `killring.lua` + `default.lua` bodies → acceptance. + +## Bets (score at close) + +1. **The eight-row boundary table is complete** — no further input path + can edit a buffer or move point without rotating or breaking. + (Falsified twice at smaller sizes; this is the bet to watch.) +2. **Stable entry ids make shared-ring interleaving safe** — no + append-corruption, no index-shift mis-yanks, eviction invalidates + cleanly. +3. **The unified paste route regresses nothing on the grid path** while + making GPU paste work at all. +4. **Q#KR10's helper closes the after-edit hole at all three sites** + with no double-fire. + +## Deferred (named) + +Unchanged from rev 2: word kills (five bindings) pending +bytes-returning deleters; `C-SPC`/mark; clipboard watching; ring +browser; ring persistence; `C-u C-y` / `C-M-w`. Plus: a Lua-visible +intercept probe (would upgrade Q#KR7's post-hoc accept to a preflight), +and a **buffer-aware edit epoch** so after-edit delivery covers +commands that edit one buffer then switch to another (Q#KR10b's named +general fix). + +## Acceptance (dispatch_key-driven; multi-frontend via distinct `FrontendId`s) + +Chain mechanics — rev 2 set, plus: +- **Pointer break, kill side**: `C-k`, click elsewhere in the same + buffer (grid mouse path *and* semantic pointer path), `C-k` → two + entries. +- **Pointer break, yank side**: `C-y`, click elsewhere, `M-y` → + refused (not a yank), buffer unchanged at both locations. +- Wheel-scroll does **not** break a kill chain. + +Shared-ring interleaving: +- **A-kill / B-kill / A-kill** → three entries; B's text intact + (the `last_kill_id` blocker case). +- **A `C-y` / B kills / A `M-y`** → A rotates from its own entry + (stable id), not from B's shifted position. +- Eviction mid-session (B pushes past the cap until A's entry drops) → + A's `M-y` refuses cleanly. + +Paste (Q#KR10a): +- `Paste` reaches the buffer on the **semantic** path (the GPU case — + asserting the shipped bug is fixed), breaks the chain, and fires + `buffer.after-edit` exactly once; grid path unchanged. +- **Forged-id paste**: frontend A sends `Paste` whose payload + `frontend_id` names B → the text lands in **A's** active view, A's + chain breaks, and B's state (view, chain, sessions) is untouched. + +Hook delivery (Q#KR10b): +- `after-edit` probe fires exactly once for menu Cut, `M-x + edit.kill-line`, and a paste; no double-fire for keybound `C-k`. + +Lifecycle (Q#KR11): +- `SessionDetached` clears the frontend's `command_history` entry and + fires `frontend.detached`; the killring module's tables for that id + are gone (probe via `pmacs.killring._debug_sessions()` or list + introspection). + +Everything else carries over from rev 2: `M-x` three-direction matrix, +failed-kill/failed-yank no-ops, GPU-optimistic and unbound-key breaks, +slot sync + external-content integration, yank-over-selection, cap +validation (`math.huge`/`NaN`) and shrink-trim. From 04314ce132caf3254ba13a426339ee20b212c7c6 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 9 Jul 2026 18:26:52 -0400 Subject: [PATCH 2/4] feat(edit): kill ring + yank-pop on a per-frontend command-boundary substrate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Arc 2 (docs/kill-ring-framing.md, rev 3 — three review rounds). Kills accumulate in a ring; consecutive kills append; C-y yanks the head; M-y right after a yank cycles older entries; C-k (kill-line) exists at last. The ring is daemon-global (Emacs-daemon model); chains and yank sessions are per-frontend. The substrate (Q#KR2): EditorCore.command_history maps FrontendId -> {this, last} command. Every input path updates it --- the rev-1 design treated dispatch_key as the only input path and review falsified that twice: keybound command dispatch_key Run arm rotate typed char (round-trip) self-insert fallback rotate unbound key dispatch_key unbound arm break GPU optimistic edit handle_remote_crdt_op break pointer gesture dispatch_mouse + dispatch_pointer break inbound OS paste unified paste route break menu item menu_invoke_active rotate M-x accept pmacs.command.invoke_interactive rotate invoke_interactive gives Emacs's execute-extended-command semantics (M-x kill-line then C-k appends; C-k then M-x kill-line does not); the public pmacs.command.invoke stamps nothing. Wheel scroll deliberately does NOT break (mwheel-scroll vs mouse-set-point, as in Emacs). Three shipped bugs fixed en route (Q#KR10): - Semantic-path Paste was dropped ("no grid-less effect yet"), and the GPU always negotiates semantic render --- GPU Ctrl-V was a no-op. Paste is now a dispatcher-level arm serving both attachment kinds. - That arm keys off the dispatcher's AUTHENTICATED source; the old grid arm trusted the client-supplied payload frontend_id, letting a forged id paste into another frontend's active window (unit-tested). - Paste, M-x-invoked commands, and menu-invoked commands never fired buffer.after-edit (each runs outside dispatch_key's revision check), so LSP/syntax/autosave missed those edits. A shared with_after_edit_check helper now wraps all three sites; scope is honest --- active-buffer compare, sound for these paths, not a general any-buffer guarantee (buffer-aware edit epoch deferred). The ring (killring.lua, Q#KR4-7): entries carry stable monotonic ids. Append requires last_command in the kill family AND this frontend's last_kill_id == the head's id --- A-kill/B-kill/A-kill pushes fresh instead of corrupting B's entry. Yank sessions store {buffer, start, stop, entry_id, text}: M-y validates last_command + live session + same buffer + slice(start,stop) == text (out-of-bounds reads as changed --- pcall'd; an early test caught the guard throwing on an upstream deletion instead of refusing), rotates by locating the entry_id's CURRENT position (positions shift under other frontends' pushes; ids don't), verifies the applied replace (intercepts may alter it; accepted post-hoc semantics), then goto_byte. Failed kills clear last_kill_id; failed/refused yanks create no session, so a second invalid M-y cannot ride the first's name-stamp. OS clipboard: ring head mirrors to the ACTING frontend's OS clipboard only (pending_clipboard's existing shape; frontends may be different machines). External content joins the ring at yank time via the clipboard_get slot check (an OS copy reaches the daemon only when pasted). New core seams: clipboard_set(bytes) / clipboard_get. Lifecycle (Q#KR11): SessionDetached prunes command_history and fires the new frontend.detached hook (raw id); killring.lua drops that frontend's tables. pmacs.killring.max([n]) validated (non-finite rejected --- math.huge would defeat the cap; shrink trims immediately), default 60. Deferred, named: word kills (M-d/M-BS/C-BS/C-h/C-DEL discard bytes --- needs bytes-returning deleters), C-SPC/set-mark, clipboard watching, ring browser/persistence, C-u C-y / C-M-w, buffer-aware edit epoch, Lua-visible intercept probe. Tests: tests/kill_ring_acceptance.rs (24) --- chain mechanics incl. all break rows, the M-x three-direction matrix, per-frontend interleaving (A-kill/B-kill/A-kill; stable-id rotation under B's pushes; eviction mid-session; upstream-edit invalidation), menu Cut via real right-click + menu pointer (feeds ring, fires after-edit once, chains with C-k), external-paste integration, cap validation + shrink-trim, detach cleanup. Plus daemon unit tests: forged-id paste lands in the authenticated source's window and leaves the claimed frontend's chain untouched; optimistic CRDT op breaks only the source's chain. Gates: fmt + workspace clippy clean; lib 1500; crdt 1672; killring 24; cua 5; query-replace 16; completion 9; autosave 29; desktop 11; persistence 5; clobber 6; m4 90; m8 10+15; m10/m11 crdt; GPU 58; git diff --check clean. Co-Authored-By: Claude Fable 5 --- builtin/commands/default.lua | 28 +- builtin/hooks/default.lua | 9 + builtin/runtime/killring.lua | 317 ++++++++++++++++ src/daemon.rs | 288 +++++++++++++-- src/editor.rs | 117 +++++- src/editor_core.rs | 83 +++++ src/lua_bindings/mod.rs | 77 ++++ tests/kill_ring_acceptance.rs | 660 ++++++++++++++++++++++++++++++++++ 8 files changed, 1527 insertions(+), 52 deletions(-) create mode 100644 builtin/runtime/killring.lua create mode 100644 tests/kill_ring_acceptance.rs diff --git a/builtin/commands/default.lua b/builtin/commands/default.lua index 865068c..deaf6ef 100644 --- a/builtin/commands/default.lua +++ b/builtin/commands/default.lua @@ -200,21 +200,19 @@ cmd { name = "region.cancel", -- bracketed paste also refreshes. The default bindings are the Emacs -- kill/yank set (M-w / C-w / C-y, C-x h), which were all free. +-- The cut/copy/paste trio delegates to the kill ring (Arc 2): kills +-- accumulate, C-y yanks the head, M-y cycles older entries. Resolution +-- happens at invoke time, so chunk load order doesn't matter; the +-- context menu invokes these by name and inherits the ring for free. cmd { name = "edit.copy", - description = "Copy the active region to the clipboard.", - fn = function() - if not ed.clipboard_copy() then ed.set_status("no region") end - end } + description = "Save the active region to the kill ring (and OS clipboard).", + fn = function() pmacs.killring.copy() end } cmd { name = "edit.cut", - description = "Cut the active region to the clipboard.", - fn = function() - if not ed.clipboard_cut() then ed.set_status("no region") end - end } + description = "Kill the active region into the kill ring (and OS clipboard).", + fn = function() pmacs.killring.cut() end } cmd { name = "edit.paste", - description = "Paste the clipboard at the cursor, replacing any region.", - fn = function() - if not ed.clipboard_paste() then ed.set_status("clipboard empty") end - end } + description = "Yank the most recent kill at the cursor, replacing any region.", + fn = function() pmacs.killring.yank() end } cmd { name = "edit.select-all", description = "Select the whole buffer.", fn = function() ed.select_all() end } @@ -628,7 +626,11 @@ cmd { name = "editor.execute-command", history = "command", on_accept = function(name) if name == nil or name == "" then return end - local ok, err = pcall(pmacs.command.invoke, name) + -- invoke_interactive records the command boundary (kill + -- ring Q#KR2), so chain-sensitive commands behave as under + -- Emacs's execute-extended-command: M-x kill-line then C-k + -- appends; C-k then M-x kill-line does not. + local ok, err = pcall(pmacs.command.invoke_interactive, name) if not ok then -- mlua's `tostring(err)` includes a Lua stack traceback -- separated by newlines. The status line is one row; diff --git a/builtin/hooks/default.lua b/builtin/hooks/default.lua index 98d89e4..7fe3a0a 100644 --- a/builtin/hooks/default.lua +++ b/builtin/hooks/default.lua @@ -67,6 +67,15 @@ define { kind = "short-circuit", } +define { + name = "frontend.detached", + description = "Fired when an attached frontend's session ends. " .. + "Receives the raw frontend id (integer). Modules keying " .. + "state by pmacs.frontend.id() (kill-ring sessions) drop " .. + "that id's entries here (Q#KR11).", + kind = "all-must-succeed", +} + define { name = "process.after-tick", description = "Fired once per editor frame, immediately after the process " .. diff --git a/builtin/runtime/killring.lua b/builtin/runtime/killring.lua new file mode 100644 index 0000000..730ec6c --- /dev/null +++ b/builtin/runtime/killring.lua @@ -0,0 +1,317 @@ +-- killring.lua --- the Emacs kill ring (Arc 2, kill-ring framing). +-- +-- Kills accumulate here instead of overwriting the one clipboard slot: +-- consecutive kills append into one entry, `C-y` yanks the head, `M-y` +-- right after a yank cycles older entries. The ring is daemon-global +-- (shared across attached frontends, like the Emacs daemon); kill +-- chains and yank sessions are per-frontend, keyed by +-- `pmacs.frontend.id()` and checked against stable ring-entry ids so +-- one frontend's activity can never corrupt another's (Q#KR4/6/7). +-- +-- The Rust substrate this rides on (Q#KR2): every input path either +-- rotates the per-frontend command boundary (commands) or breaks it +-- (optimistic CRDT edits, pointer gestures, pastes, unbound keys), so +-- `pmacs.editor.last_command()` is trustworthy on both frontends. +-- +-- OS clipboard: the ring head is mirrored to the *acting frontend's* +-- OS clipboard on every kill/append (`ed.clipboard_set`); external +-- content joins the ring at yank time via the slot check (an OS copy +-- only reaches the daemon when pasted). `M-y` never touches the slot. +-- +-- Framing: docs/kill-ring-framing.md. + +pmacs.killring = pmacs.killring or {} + +local ed = pmacs.editor + +local DEFAULT_MAX = 60 + +local ring = {} -- array of { id, text }, most-recent first (shared) +local next_id = 1 +local max_entries = DEFAULT_MAX + +-- Per-frontend state (Q#KR4/KR6). Keyed by pmacs.frontend.id(). +local last_kill_id = {} -- fid -> ring-entry id of that frontend's last kill +local sessions = {} -- fid -> { buffer, start, stop, entry_id, text } + +-- Commands whose success may extend a kill chain (Q#KR4). +local KILL_CHAIN = { ["edit.kill-line"] = true, ["edit.cut"] = true } + +local function trim() + while #ring > max_entries do + table.remove(ring) + end +end + +-- max([n]) --- getter when nil; validated setter otherwise. Rejects +-- non-numbers, NaN, and non-finite values (math.huge would defeat the +-- cap); floors; lowering the cap trims existing entries immediately. +function pmacs.killring.max(n) + if n == nil then return max_entries end + if type(n) ~= "number" or n ~= n or n == math.huge or n < 1 then + error("pmacs.killring.max: expected a finite number >= 1") + end + max_entries = math.floor(n) + trim() + return max_entries +end + +-- The ring's texts, most-recent first (introspection / tests). +function pmacs.killring.list() + local out = {} + for i, e in ipairs(ring) do out[i] = e.text end + return out +end + +-- Test/debug seam (Q#KR11 lifecycle assertions). +function pmacs.killring._debug_state(fid) + return { session = sessions[fid], last_kill_id = last_kill_id[fid] } +end + +-- Push `text` as a fresh entry (duplicate-of-head collapses, keeping +-- the existing id). Returns the head entry. +local function push_entry(text) + if ring[1] and ring[1].text == text then return ring[1] end + table.insert(ring, 1, { id = next_id, text = text }) + next_id = next_id + 1 + trim() + return ring[1] +end + +-- A kill-family command failed or was a no-op: it must not leave a +-- live chain for the next kill to append to (Q#KR4). +local function fail_kill(fid) + last_kill_id[fid] = nil +end + +-- Chain-aware kill (Q#KR4): append to the head iff the previous +-- command was a chain kill AND this frontend's last kill IS the +-- current head (another frontend's push in between means the head is +-- not ours — append would corrupt their entry). Mirrors the head to +-- the acting frontend's OS clipboard either way. +local function kill_push(fid, text) + local chained = KILL_CHAIN[ed.last_command() or ""] + and last_kill_id[fid] ~= nil + and ring[1] ~= nil + and ring[1].id == last_kill_id[fid] + local head + if chained then + ring[1].text = ring[1].text .. text + head = ring[1] + else + head = push_entry(text) + end + last_kill_id[fid] = head.id + ed.clipboard_set(head.text) + return head +end + +-- edit.cut body (C-w): kill the active region into the ring. +function pmacs.killring.cut() + local fid = pmacs.frontend.id() + local region = ed.region() + local buf = pmacs.window.buffer() + if not region or not buf then + fail_kill(fid) + ed.set_status("no region") + return false + end + local text = buf:slice(region.start, region["end"]) + if not ed.delete_region() then + fail_kill(fid) + ed.set_status("no region") + return false + end + kill_push(fid, text) + return true +end + +-- edit.copy body (M-w): save the region to the ring without deleting. +-- Not a chain command (Q#KR4's family is kill-line + cut): a copy +-- pushes fresh (duplicate-of-head collapses) and neither extends nor +-- starts an append chain. +function pmacs.killring.copy() + local fid = pmacs.frontend.id() + local region = ed.region() + local buf = pmacs.window.buffer() + if not region or not buf then + fail_kill(fid) + ed.set_status("no region") + return false + end + local text = buf:slice(region.start, region["end"]) + push_entry(text) + ed.clipboard_set(text) + fail_kill(fid) -- a copy is not an appendable kill + return true +end + +-- edit.kill-line body (C-k): kill from the cursor to end of line; at +-- the newline itself, kill the newline (Emacs kill-line with +-- kill-whole-line nil). Consecutive C-k's append (Q#KR4), so +-- C-k C-k C-k builds one multi-line entry. +function pmacs.killring.kill_line() + local fid = pmacs.frontend.id() + local buf = pmacs.window.buffer() + if not buf then + fail_kill(fid) + return false + end + local cursor = ed.cursor() + local len = buf:len() + if cursor >= len then + fail_kill(fid) + ed.set_status("end of buffer") + return false + end + -- Find the next newline by chunked scan (lines are almost always + -- shorter than one chunk; a chunk loop keeps giant lines safe). + local eol = nil + local p = cursor + while p < len do + local chunk_to = math.min(p + 4096, len) + local chunk = buf:slice(p, chunk_to) + local nl = chunk:find("\n", 1, true) + if nl then + eol = p + nl - 1 + break + end + p = chunk_to + end + local kill_to + if eol == cursor then + kill_to = cursor + 1 -- at the newline: kill the newline itself + else + kill_to = eol or len -- rest of the line (or of a final bare line) + end + local text = buf:slice(cursor, kill_to) + buf:delete(cursor, kill_to) + kill_push(fid, text) + return true +end + +-- Drop a frontend's yank session (invalid M-y must not leave state a +-- second M-y could ride, Q#KR7). +local function drop_session(fid, msg) + sessions[fid] = nil + if msg then ed.set_status(msg) end +end + +-- edit.paste body (C-y): yank the ring head (Q#KR6). +function pmacs.killring.yank() + local fid = pmacs.frontend.id() + -- Slot check: content that arrived via an OS paste (paste_inbound + -- refreshes the slot) joins the ring the first time it is yanked. + local slot = ed.clipboard_get() + if slot and slot ~= "" and (not ring[1] or ring[1].text ~= slot) then + push_entry(slot) + end + local head = ring[1] + if not head then + drop_session(fid, "kill ring empty") + return false + end + local region = ed.region() + local start = region and region.start or ed.cursor() + if not slot or slot ~= head.text then + ed.clipboard_set(head.text) + end + local ok = ed.clipboard_paste() + if not ok then + drop_session(fid) -- failed paste creates no session (Q#KR6) + return false + end + local buf = pmacs.window.buffer() + sessions[fid] = { + buffer = buf and tostring(buf) or "", + start = start, + stop = ed.cursor(), + entry_id = head.id, + text = head.text, + } + return true +end + +-- edit.yank-pop body (M-y): replace the just-yanked text with the +-- next-older ring entry (Q#KR7). Valid only immediately after a yank +-- or another pop, with a live, still-verifiable session. +function pmacs.killring.yank_pop() + local fid = pmacs.frontend.id() + local lc = ed.last_command() + local s = sessions[fid] + if not (lc == "edit.paste" or lc == "edit.yank-pop") or not s then + drop_session(fid, "previous command was not a yank") + return false + end + local buf = pmacs.window.buffer() + if not buf or tostring(buf) ~= s.buffer then + drop_session(fid, "yank was in another buffer") + return false + end + -- Invalidation guard: the remembered range must still hold exactly + -- the text this session yanked. A concurrent edit (another + -- frontend, a hook) that moved or altered it fails here — refuse + -- rather than splice garbage. The slice is pcall'd: an upstream + -- deletion can shrink the buffer below `stop`, and an out-of-bounds + -- range must read as "changed", not throw. + local ok, current = pcall(function() return buf:slice(s.start, s.stop) end) + if not ok or current ~= s.text then + drop_session(fid, "buffer changed since the yank") + return false + end + -- Stable-id rotation: find where this session's entry sits NOW + -- (other frontends' pushes shift positions, not ids) and step to + -- the next older, wrapping. An evicted id invalidates. + local pos = nil + for i, e in ipairs(ring) do + if e.id == s.entry_id then + pos = i + break + end + end + if not pos then + drop_session(fid, "kill ring entry expired") + return false + end + local entry = ring[pos % #ring + 1] + buf:replace(s.start, s.stop, entry.text) + -- Verify the applied edit: buffer intercepts may alter or reject a + -- replace. Accepted post-hoc semantics (Q#KR7): on mismatch the + -- interceptor's result stands, the session ends, and we say so. + -- pcall'd for the same reason as the guard above: an intercept that + -- shrank the buffer must invalidate, not throw. + local vok, applied = pcall(function() + return buf:slice(s.start, s.start + #entry.text) + end) + if not vok or applied ~= entry.text then + drop_session(fid, "yank-pop altered by buffer intercept; stopped") + return false + end + ed.goto_byte(s.start + #entry.text) + s.stop = s.start + #entry.text + s.entry_id = entry.id + s.text = entry.text + return true +end + +-- Q#KR11: a detached frontend's chain/session state must not outlive +-- it (ids are monotonic; these tables would grow forever). +pmacs.hook.add("frontend.detached", function(fid) + sessions[fid] = nil + last_kill_id[fid] = nil +end) + +pmacs.command.define { + name = "edit.kill-line", + description = "Kill from the cursor to the end of the line (into the kill ring).", + fn = function() pmacs.killring.kill_line() end, +} + +pmacs.command.define { + name = "edit.yank-pop", + description = "Replace the just-yanked text with the previous kill (after C-y).", + fn = function() pmacs.killring.yank_pop() end, +} + +pmacs.keymap.bind { scope = "global", sequence = "C-k", command = "edit.kill-line" } +pmacs.keymap.bind { scope = "global", sequence = "M-y", command = "edit.yank-pop" } diff --git a/src/daemon.rs b/src/daemon.rs index 4c2f06f..15342de 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1540,6 +1540,30 @@ fn handle_dispatcher_event( editor.dispatch_menu_pointer(source, index, invoke); } } + FrontendEvent::Paste { + frontend_id: claimed_fid, + data, + } => { + // Kill ring Q#KR10a — the unified paste route, for + // BOTH attachment kinds. Handled here (not in + // `apply_event`) for two reasons: + // + // 1. The semantic input dispatcher used to drop + // `Paste` entirely, so GPU Ctrl-V was a no-op + // (pmacs-gpu always negotiates semantic render). + // 2. The authenticated `source` is in scope. The + // event's `claimed_fid` is client-supplied and + // not trusted (the CrdtOp / Viewport / Pointer + // source-trust rule); the old grid arm set + // `active_frontend` from it, letting a forged id + // paste into another frontend's active window. + // + // The paste is a non-command edit, so it breaks the + // source's command chain (Q#KR2), and it fires + // `buffer.after-edit` like any other edit (Q#KR10b) + // — previously it never did, so LSP missed pastes. + handle_inbound_paste(editor, source, claimed_fid, &data); + } _ => { let term_size = *term_sizes .get(&source) @@ -1579,10 +1603,21 @@ fn handle_dispatcher_event( last_dispatch_idle_sent.remove(&frontend_id); last_active_buffer_sent.remove(&frontend_id); session_registry.unregister_session(frontend_id); - editor - .core - .borrow_mut() - .unregister_frontend_view(frontend_id); + { + let mut core = editor.core.borrow_mut(); + core.unregister_frontend_view(frontend_id); + // Kill ring Q#KR11: frontend ids are monotonic, so + // per-frontend state must not outlive the session. + core.command_history.remove(&frontend_id); + } + // Q#KR11: let Lua modules holding per-frontend tables + // (killring sessions / kill flags) drop this id's entries. + // The first frontend-lifecycle hook; carries the raw id. + let mut args = mlua::MultiValue::new(); + args.push_back(mlua::Value::Integer( + i64::try_from(frontend_id.0).unwrap_or(i64::MAX), + )); + editor.lua_host.run_hook("frontend.detached", args); } } } @@ -1914,6 +1949,35 @@ fn validate_remote_crdt_op( Ok(()) } +/// The unified inbound-paste route (kill ring Q#KR10a) — one handler +/// for grid *and* semantic sessions, keyed by the dispatcher's +/// authenticated `source`. `claimed` is the event payload's +/// client-supplied id: never trusted (a forged id must not paste into +/// another frontend's active window), only logged on mismatch. The +/// paste breaks the source's command chain (a non-command edit, Q#KR2) +/// and fires `buffer.after-edit` when the buffer changed (Q#KR10b). +fn handle_inbound_paste( + editor: &mut EditorState, + source: FrontendId, + claimed: FrontendId, + data: &[u8], +) { + if claimed != source { + eprintln!( + "pmacs daemon: Paste claimed {claimed:?} but came \ + from {source:?}; using the authenticated source" + ); + } + editor.core.borrow_mut().active_frontend = source; + editor.with_after_edit_check(|state| { + let mut core = state.core.borrow_mut(); + core.break_command_chain(source); + if let Err(e) = core.paste_inbound(data) { + eprintln!("pmacs: inbound paste failed: {e}"); + } + }); +} + /// T M10.10 (post-audit) — apply a *pre-validated* /// `FrontendEvent::CrdtOp`. Identity, capability, and scope checks /// happen upstream in `validate_remote_crdt_op`; this function trusts @@ -1944,6 +2008,12 @@ fn handle_remote_crdt_op( buffer_id: crate::buffer::BufferId, op: crate::rope::CrdtOp, ) { + // Kill ring Q#KR2: an optimistic edit is non-command input — GPU + // typing, Enter/Tab, Backspace/Delete all arrive here without ever + // touching dispatch_key. It must break the source frontend's + // command chain, or `C-k x C-k` on the GPU would append across the + // typed character. + editor.core.borrow_mut().break_command_chain(source); // Effect 1: apply to buffer's CRDT + rope. Capture the Edit // (or `None` for an op that imported cleanly but produced no // text delta — F17). @@ -2193,9 +2263,10 @@ fn build_presence_snapshot(editor: &EditorState, frontend_id: FrontendId) -> Pre /// arms of [`apply_event`] but takes no `RenderState` — a semantic /// frontend lays out locally, so the only state these events touch is /// the editor core (cursor, buffer, commands), which `dispatch_key` / -/// `dispatch_mouse` operate on directly. `Resize` / `Paste` / `Focus` -/// have no grid-less effect yet and are dropped; `Viewport` / `CrdtOp` -/// are handled in their own dispatcher arms and never reach here. +/// `dispatch_mouse` operate on directly. `Resize` / `Focus` have no +/// grid-less effect yet and are dropped; `Viewport` / `CrdtOp` / +/// `Paste` (Q#KR10a) are handled in their own dispatcher arms and +/// never reach here. #[allow(clippy::needless_pass_by_value)] // consumes the event, mirroring `apply_event`. fn apply_semantic_input_event(editor: &mut EditorState, ev: FrontendEvent, term_size: CellSize) { match ev { @@ -2238,20 +2309,13 @@ fn apply_event( render_state.resize(size); *term_size = size; } - FrontendEvent::Paste { frontend_id, data } => { - // Q#CM6 — inbound OS paste (terminal bracketed paste, or - // GPU Ctrl-V reading `arboard`). Insert at the cursor of the - // originating frontend's active window, replacing any region, - // and refresh the clipboard slot so a later in-app paste - // repeats the same text. (Previously dropped: pmacs had - // never honored a paste.) - let mut core = editor.core.borrow_mut(); - core.active_frontend = frontend_id; - if let Err(e) = core.paste_inbound(&data) { - eprintln!("pmacs: inbound paste failed: {e}"); - } - } - FrontendEvent::FocusGained(_) + // Q#KR10a — Paste is handled in the dispatcher's own + // `FrontendEvent::Paste` arm (unified for grid and semantic + // sessions, keyed by the authenticated source), and never + // reaches here. Listed explicitly so a future reshuffle can't + // silently re-route it through this payload-trusting path. + FrontendEvent::Paste { .. } + | FrontendEvent::FocusGained(_) | FrontendEvent::FocusLost(_) // T M11.1: the semantic-frontend viewport declaration. Its // consumer is the instance-side projection seam @@ -2447,6 +2511,188 @@ mod tests { ); } + /// Kill ring Q#KR2 — an optimistic edit is non-command input: GPU + /// typing arrives here without touching dispatch_key, so it must + /// break the source frontend's command chain or `C-k x C-k` on the + /// GPU would append across the typed character. + #[cfg(feature = "crdt")] + #[test] + fn handle_remote_crdt_op_breaks_the_source_command_chain() { + use crate::editor::EditorState; + use crate::protocol::FrontendId; + + let mut editor = EditorState::new(); + let source = FrontendId(7); + // A live kill chain for the source frontend... + editor + .core + .borrow_mut() + .rotate_command(source, "edit.kill-line"); + // ...and one for a bystander that must survive. + editor + .core + .borrow_mut() + .rotate_command(FrontendId::LOCAL, "edit.kill-line"); + + let buffer_id = editor.core.borrow().active_window().buffer_id; + { + let core = editor.core.borrow(); + let mut reg = core.registry.borrow_mut(); + reg.get_mut(buffer_id) + .expect("active buffer") + .upgrade_to_crdt(2) + .expect("upgrade to crdt"); + } + let snapshot_bytes = { + let core = editor.core.borrow(); + let reg = core.registry.borrow(); + reg.get(buffer_id) + .expect("buffer") + .crdt_state() + .expect("crdt-backed") + .export_snapshot() + .expect("export snapshot") + }; + let peer = loro::LoroDoc::new(); + peer.set_peer_id(7).expect("set peer id"); + peer.import(&snapshot_bytes).expect("import snapshot"); + let v_before = peer.oplog_vv(); + peer.get_text("body").insert(0, "x").expect("peer insert"); + let op_bytes = peer + .export(loro::ExportMode::updates(&v_before)) + .expect("export op"); + + handle_remote_crdt_op( + &mut editor, + source, + buffer_id, + crate::rope::CrdtOp { + peer_id: 7, + bytes: op_bytes, + }, + ); + + let core = editor.core.borrow(); + assert!( + core.command_history + .get(&source) + .is_none_or(|b| b.this.is_none()), + "the optimistic edit must break the source's chain" + ); + assert_eq!( + core.command_history + .get(&FrontendId::LOCAL) + .and_then(|b| b.this.as_deref()), + Some("edit.kill-line"), + "a bystander frontend's chain is untouched" + ); + } + + /// Kill ring Q#KR10a — the unified paste route trusts only the + /// dispatcher's authenticated source. A forged payload id must not + /// paste into another frontend's active window, and the paste + /// breaks the SOURCE's chain (not the claimed frontend's) and + /// fires `buffer.after-edit` exactly once. + #[test] + fn inbound_paste_uses_authenticated_source_not_the_claimed_id() { + use crate::editor::EditorState; + use crate::protocol::FrontendId; + use crate::text_view::TextView; + use crate::window::{FrontendView, Layout, Window, WindowId}; + + let mut editor = EditorState::new(); + editor + .lua_host + .eval( + Some("test"), + r#" + _G.PASTE_AFTER_EDIT = 0 + pmacs.hook.add("buffer.after-edit", function() + _G.PASTE_AFTER_EDIT = _G.PASTE_AFTER_EDIT + 1 + end) + "#, + ) + .expect("install after-edit hook"); + + // Give the attacker frontend its OWN view onto its own buffer, + // so "which window did the text land in" is observable. + let source = FrontendId(7); + let victim = FrontendId::LOCAL; + let attacker_buf = { + let core = editor.core.borrow(); + let mut reg = core.registry.borrow_mut(); + reg.create("attacker-buffer") + }; + { + let mut core = editor.core.borrow_mut(); + let tv = { + let reg = core.registry.borrow(); + TextView::new(reg.get(attacker_buf).expect("attacker buffer")) + }; + let wid = WindowId::next(); + core.windows.insert(wid, Window::new(wid, attacker_buf, tv)); + core.register_frontend_view( + source, + FrontendView { + layout: Layout::single(wid), + active: wid, + }, + ); + } + let victim_buf = editor.core.borrow().active_window().buffer_id; + // Seed a live chain on the victim: the forged paste must not + // break it (only the authenticated source's chain breaks). + editor + .core + .borrow_mut() + .rotate_command(victim, "edit.kill-line"); + + // The payload CLAIMS to be the victim. + handle_inbound_paste(&mut editor, source, victim, b"FORGED"); + + let core = editor.core.borrow(); + let text_of = |id| { + let reg = core.registry.borrow(); + let buf = reg.get(id).expect("buffer"); + let len = buf.len(); + let mut out = vec![0u8; usize::try_from(len).unwrap_or(0)]; + if len > 0 { + buf.snapshot_rope().slice(0, len, &mut out); + } + String::from_utf8_lossy(&out).into_owned() + }; + assert!( + text_of(attacker_buf).contains("FORGED"), + "the paste lands in the AUTHENTICATED source's active window" + ); + assert!( + !text_of(victim_buf).contains("FORGED"), + "a forged payload id must not paste into the claimed frontend's window" + ); + assert!( + core.command_history + .get(&source) + .is_none_or(|b| b.this.is_none()), + "the paste breaks the source's chain" + ); + assert_eq!( + core.command_history + .get(&victim) + .and_then(|b| b.this.as_deref()), + Some("edit.kill-line"), + "the claimed frontend's chain is untouched" + ); + drop(core); + let count = editor + .lua_host + .eval(Some("test-readback"), "return _G.PASTE_AFTER_EDIT") + .expect("read counter"); + assert!( + matches!(count, mlua::Value::Integer(1)), + "paste fires buffer.after-edit exactly once, got {count:?}" + ); + } + /// v15 regression: an optimistic-path edit (the bulk of plain-char /// typing from a semantic frontend) must clear the transient /// status message, exactly as `dispatch_key`'s entry clear does diff --git a/src/editor.rs b/src/editor.rs index 5fd9f61..67cdc59 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -327,6 +327,12 @@ impl EditorState { include_str!("../builtin/runtime/autosave.lua"), ) .expect("load autosave builtin chunk"); + lua_host + .eval( + Some("@pmacs/builtin/runtime/killring.lua"), + include_str!("../builtin/runtime/killring.lua"), + ) + .expect("load killring builtin chunk"); // T M7.11 bundled-package bootstrap. Through M7.10 the REPL // was loaded directly via `eval(include_str!(...))`; the // M7.11 deliverable migrates it to the package system so it @@ -706,6 +712,10 @@ impl EditorState { match action { Action::Run { command, .. } => { + // Kill ring Q#KR2: record the command boundary before the + // body runs, so the body's own `ed.last_command()` reads + // its *predecessor* (Emacs `last-command` semantics). + self.core.borrow_mut().rotate_command(frontend_id, &command); if let Err(e) = self .lua_host .invoke_command(&command, mlua::MultiValue::new()) @@ -723,20 +733,28 @@ impl EditorState { // shadow instead of the dispatcher. self.core.borrow_mut().completion_popup_close(); } - Action::Unbound { sequence } => match printable_char(&sequence) { - Some(ch) => { + Action::Unbound { sequence } => { + if let Some(ch) = printable_char(&sequence) { + // Typing a character is a command too (Q#KR2): it + // must break a kill chain — `C-k x C-k` is two ring + // entries, not an append. + self.core + .borrow_mut() + .rotate_command(frontend_id, "buffer.self-insert"); let mut args = mlua::MultiValue::new(); args.push_back(mlua::Value::Integer(ch as i64)); if let Err(e) = self.lua_host.invoke_command("buffer.self-insert", args) { self.core.borrow_mut().status = format!("self-insert failed: {}", first_line(&e.to_string())); } - } - None => { + } else { + // An unbound key still breaks the chain (Q#KR2) — + // Emacs's `undefined` runs as a command. + self.core.borrow_mut().break_command_chain(frontend_id); self.core.borrow_mut().status = format!("{}: not bound", display_sequence(&sequence)); } - }, + } } let post_revision = self.active_buffer_revision(); @@ -761,6 +779,32 @@ impl EditorState { self.buffer_revision(id) } + /// Run `f`, then fire `buffer.after-edit` if the active buffer's + /// revision changed — the same compare `dispatch_key` performs + /// after a keybound command (kill ring Q#KR10b). + /// + /// For call sites that execute edits *outside* `dispatch_key`'s + /// post-command check: the minibuffer accept callback (`M-x`), the + /// menu invoke, and the unified paste route. Without it, those + /// edits are invisible to LSP `didChange`, the syntax reparse, and + /// autosave's observers. + /// + /// Scope, honestly: the *active-buffer* before/after compare is + /// sound for these paths (all edit the active buffer and stay + /// there) but is not a general any-buffer guarantee — a callback + /// that edits buffer A then switches to B evades it. The general + /// fix is a buffer-aware edit epoch; deferred, named in the + /// kill-ring framing. + pub(crate) fn with_after_edit_check(&mut self, f: impl FnOnce(&mut Self)) { + let pre = self.active_buffer_revision(); + f(self); + let post = self.active_buffer_revision(); + if pre != post { + self.lua_host + .run_hook("buffer.after-edit", mlua::MultiValue::new()); + } + } + /// Edit revision of a specific buffer, or `None` if the registry no /// longer knows it. Used by the query-replace shadow to compare the /// *edited* (origin) buffer, not whichever is active. @@ -946,13 +990,28 @@ impl EditorState { fn menu_invoke_active(&mut self) { let command = self.core.borrow().menu_active_command(); self.core.borrow_mut().menu_close(); - if let Some(command) = command - && let Err(e) = self - .lua_host - .invoke_command(&command, mlua::MultiValue::new()) - { - self.core.borrow_mut().status = - format!("error in {command}: {}", first_line(&e.to_string())); + if let Some(command) = command { + // A menu item is an interactive command (kill ring Q#KR2): + // rotate the boundary so a menu Cut chains like a keybound + // one. The invoke below bypasses dispatch_key, which would + // otherwise leave the boundary stale. + { + let mut core = self.core.borrow_mut(); + let fid = core.active_frontend; + core.rotate_command(fid, &command); + } + // Q#KR10b: menu invocation bypasses dispatch_key's + // revision check — a menu Cut's edit must still fire + // `buffer.after-edit`. + self.with_after_edit_check(|state| { + if let Err(e) = state + .lua_host + .invoke_command(&command, mlua::MultiValue::new()) + { + state.core.borrow_mut().status = + format!("error in {command}: {}", first_line(&e.to_string())); + } + }); } } @@ -1062,12 +1121,18 @@ impl EditorState { .create_string(&contents) .expect("Lua VM out of memory while building minibuffer callback args"), )); - if let Err(e) = on_accept.call::(args) { - self.core.borrow_mut().status = format!( - "minibuffer on_accept failed: {}", - first_line(&e.to_string()) - ); - } + // Q#KR10b: the accept callback runs outside dispatch_key's + // post-command revision check (the minibuffer interception + // returns before it), so an M-x'd editing command would never + // fire `buffer.after-edit` without this wrapper. + self.with_after_edit_check(|state| { + if let Err(e) = on_accept.call::(args) { + state.core.borrow_mut().status = format!( + "minibuffer on_accept failed: {}", + first_line(&e.to_string()) + ); + } + }); } fn minibuffer_cancel(&mut self) { @@ -1159,6 +1224,12 @@ impl EditorState { self.mouse_click = None; return; // Mode-line click: reserved. } + // Point moves: break the command chain (kill ring + // Q#KR2). Scroll arms below deliberately do NOT — a + // wheel that only moves the viewport preserves a kill + // chain, as in Emacs (`mwheel-scroll` vs + // `mouse-set-point`). + self.core.borrow_mut().break_command_chain(frontend_id); let click_cell = CellCoord::new(cell_row, cell_col); let is_double_click = self.is_double_click(frontend_id, win_id, click_cell); self.activate_and_position(win_id, local_row, local_col); @@ -1181,10 +1252,12 @@ impl EditorState { if local_row >= inner_rows { return; } + self.core.borrow_mut().break_command_chain(frontend_id); self.activate_and_position(win_id, local_row, local_col); } MouseEventKind::Up(MouseButton::Left) => { let mut core = self.core.borrow_mut(); + core.break_command_chain(frontend_id); if let Some(sel) = core.active_window().selection && sel.anchor == core.cursor() { @@ -1197,6 +1270,8 @@ impl EditorState { return; // Mode-line right-click: reserved. } self.mouse_click = None; + // Opening the menu is a pointer gesture too (Q#KR2). + self.core.borrow_mut().break_command_chain(frontend_id); self.open_context_menu(win_id, local_row, local_col, (cell_row, cell_col)); } MouseEventKind::ScrollUp => { @@ -1271,6 +1346,12 @@ impl EditorState { return; } core.set_active_window_id(win_id); + // Every PointerKind moves point or changes the selection (the + // GPU scrolls locally via Viewport, which never reaches here), + // so any pointer gesture breaks the frontend's command chain + // (kill ring Q#KR2) — clicking away and killing again must not + // append, and M-y after a click must refuse. + core.break_command_chain(frontend_id); let byte = { let registry = core.registry.clone(); let reg = registry.borrow(); diff --git a/src/editor_core.rs b/src/editor_core.rs index 1b14c44..6eb50d0 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -123,6 +123,25 @@ pub struct QueryReplaceSession { found_any: bool, } +/// One frontend's command boundary — Emacs's `this-command` / +/// `last-command` pair (kill ring, Q#KR2). +/// +/// `this` is the command currently (or most recently) executing for the +/// frontend; `last` is the one before it. A chain-sensitive command +/// (kill append, `M-y`) reads `last` *while it runs* — its own rotation +/// already moved its predecessor there. `this = None` is a broken +/// chain: some non-command input (an optimistic CRDT edit, a pointer +/// gesture, a paste, an unbound key) intervened, so the next rotation +/// makes `last = None` and every chain check fails. +#[derive(Debug, Clone, Default)] +pub struct CommandBoundary { + /// The command executing now / most recently, or `None` after a + /// non-command input. + pub this: Option, + /// The command before `this`. + pub last: Option, +} + /// The world state mutated by editor commands. pub struct EditorCore { /// Shared buffer registry. The registry is the canonical owner @@ -217,6 +236,15 @@ pub struct EditorCore { /// frontend, which writes the OS clipboard (OSC 52 in the TUI, /// `arboard` in the GPU). Drained per-tick like `pending_crdt_ops`. pending_clipboard: Option<(FrontendId, Vec)>, + /// Per-frontend command boundaries (kill ring, Q#KR2) — Emacs's + /// `this-command` / `last-command`, tracked **per frontend**: two + /// attached frontends interleave their own command streams, and a + /// kill chain or yank session on frontend A must not survive into + /// frontend B's checks. Every input path updates this — commands + /// rotate; non-command inputs (optimistic CRDT edits, pointer + /// gestures, pastes, unbound keys) break the chain. Entries are + /// pruned on `SessionDetached` (Q#KR11). + pub command_history: HashMap, /// Open context menu (Q#CM1), or `None` when closed. Shared /// `Arc` so the TUI [`crate::menu::MenuView`] overlay renders /// from the same state the dispatch path mutates — the menu twin of @@ -280,6 +308,7 @@ impl EditorCore { search: None, clipboard_slot: Vec::new(), pending_clipboard: None, + command_history: HashMap::new(), menu: crate::menu::make_shared_menu(), completion_popup: crate::completion::make_shared_popup(), round_trip_buffers: std::collections::HashSet::new(), @@ -2043,6 +2072,38 @@ impl EditorCore { Some(out) } + // ---- command boundaries (kill ring, Q#KR2) -------------------------- + + /// Record `name` as `fid`'s executing command: `last = this; + /// this = name`. Called once per interactive command dispatch — + /// keybound commands, the self-insert fallback, menu items, and + /// `pmacs.command.invoke_interactive` (`M-x`). + pub fn rotate_command(&mut self, fid: FrontendId, name: &str) { + let entry = self.command_history.entry(fid).or_default(); + entry.last = entry.this.take(); + entry.this = Some(name.to_owned()); + } + + /// Break `fid`'s command chain: a non-command input happened (an + /// optimistic CRDT edit, a point-moving pointer gesture, an inbound + /// paste, an unbound key). Sets `this = None`, so the next + /// rotation yields `last = None` and every chain-sensitive check + /// (kill append, `M-y`) fails. + pub fn break_command_chain(&mut self, fid: FrontendId) { + self.command_history.entry(fid).or_default().this = None; + } + + /// The active frontend's previous command — Emacs's `last-command` + /// as observed *from inside* the currently-running command (its own + /// rotation already happened). + #[must_use] + pub fn last_command(&self) -> Option<&str> { + self.command_history + .get(&self.active_frontend)? + .last + .as_deref() + } + /// Copy the active region into the clipboard slot and queue an /// outbound OS-clipboard publish to the originating frontend. /// Returns `false` (a no-op) when there is no region. @@ -2069,6 +2130,28 @@ impl EditorCore { Ok(true) } + /// Set the clipboard slot to arbitrary bytes and queue the + /// OS-clipboard publish to the acting frontend (kill ring Q#KR1). + /// The ring's kills have no region for [`Self::clipboard_copy`] to + /// read (`C-k`'s killed line, an appended chain), so the Lua ring + /// pushes the exact bytes here. + pub fn clipboard_set(&mut self, bytes: Vec) { + self.clipboard_slot.clone_from(&bytes); + self.pending_clipboard = Some((self.active_frontend, bytes)); + } + + /// The clipboard slot's current bytes, or `None` when empty (kill + /// ring Q#KR6 — the yank-time "did external content arrive via a + /// paste since our last kill" check). + #[must_use] + pub fn clipboard_get(&self) -> Option<&[u8]> { + if self.clipboard_slot.is_empty() { + None + } else { + Some(&self.clipboard_slot) + } + } + /// Paste the clipboard slot at the cursor, replacing the active /// region if one exists (one undo step, like CUA type-over). /// Returns `false` when the slot is empty. diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 2830bed..49e1b79 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -4652,6 +4652,42 @@ fn install_command_module(lua: &Lua, commands: &SharedCommandRegistry) -> mlua:: )?; } + { + // invoke_interactive(name, ...): like `invoke`, but records a + // command boundary first (kill ring Q#KR2) — `last = this; + // this = name` for the active frontend. Used by + // `editor.execute-command` (M-x) so the invoked command's + // chain semantics match Emacs's `execute-extended-command` + // (which sets `this-command`): `M-x edit.kill-line` then `C-k` + // appends, while `C-k` then `M-x edit.kill-line` does not. + // + // Plain `invoke` deliberately stamps NOTHING: it is a public + // programmatic API called from wrappers, hooks, and async + // callbacks, and must never pollute interactive command + // history. + let cmds = commands.clone(); + command.set( + "invoke_interactive", + lua.create_function(move |lua, (name, args): (String, Variadic)| { + if let Some(core) = lua.app_data_ref::() { + let mut core = core.borrow_mut(); + let fid = core.active_frontend; + core.rotate_command(fid, &name); + } + let body = { + let r = cmds.borrow(); + r.get(&name) + .ok_or_else(|| { + mlua::Error::external(CommandError::NotFound { name: name.clone() }) + })? + .body + .clone() + }; + body.call::>(args) + })?, + )?; + } + { let cmds = commands.clone(); command.set( @@ -10935,6 +10971,18 @@ fn install_session(editor: &Table, lua: &Lua, core: &SharedCore) -> mlua::Result })?, )?; } + { + // last_command(): the active frontend's previous interactive + // command — Emacs's `last-command` as seen from inside the + // running command (kill ring Q#KR2). `nil` after a non-command + // input (optimistic edit, pointer gesture, paste, unbound key) + // broke the chain. + let cc = core.clone(); + editor.set( + "last_command", + lua.create_function(move |_, ()| Ok(cc.borrow().last_command().map(str::to_owned)))?, + )?; + } { // view_top(): the active window's first visible source line. // The saveplace getter (Arc 3) — pairs with set_view_top so a @@ -11107,6 +11155,35 @@ fn install_session(editor: &Table, lua: &Lua, core: &SharedCore) -> mlua::Result })?, )?; } + { + // clipboard_set(bytes): set the slot + queue the OS publish to + // the acting frontend (kill ring Q#KR1). The ring's kills have + // no region for clipboard_copy to read. + let cc = core.clone(); + editor.set( + "clipboard_set", + lua.create_function(move |_, bytes: mlua::String| { + cc.borrow_mut().clipboard_set(bytes.as_bytes().to_vec()); + Ok(()) + })?, + )?; + } + { + // clipboard_get() -> string?: the slot's bytes (kill ring + // Q#KR6 — yank's "did external content arrive via a paste + // since our last kill" check). nil when empty. + let cc = core.clone(); + editor.set( + "clipboard_get", + lua.create_function(move |lua, ()| { + let cc = cc.borrow(); + match cc.clipboard_get() { + Some(bytes) => Ok(Some(lua.create_string(bytes)?)), + None => Ok(None), + } + })?, + )?; + } { let cc = core.clone(); editor.set( diff --git a/tests/kill_ring_acceptance.rs b/tests/kill_ring_acceptance.rs new file mode 100644 index 0000000..c154f59 --- /dev/null +++ b/tests/kill_ring_acceptance.rs @@ -0,0 +1,660 @@ +//! Kill-ring acceptance (Arc 2, docs/kill-ring-framing.md rev 3). +//! +//! Drives the real dispatch surfaces: `dispatch_key` for chords (both +//! "frontends" via distinct `FrontendId`s — an unregistered id falls +//! back to LOCAL's view, sharing the buffer while keeping its own +//! command boundary, which is exactly the shared-ring interleaving +//! shape), `dispatch_mouse` / `dispatch_pointer` for the pointer +//! boundary rows, and the real minibuffer for `M-x`. +//! +//! The daemon-side rows (optimistic CRDT edits, the unified +//! authenticated-source paste) are covered by unit tests next to +//! `handle_remote_crdt_op` / `handle_inbound_paste` in `src/daemon.rs`. + +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; +use pmacs::editor::EditorState; +use pmacs::protocol::FrontendId; + +fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent { + KeyEvent { + code, + modifiers: mods, + kind: KeyEventKind::Press, + state: KeyEventState::NONE, + } +} + +fn ctrl(s: &mut EditorState, c: char) { + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char(c), KeyModifiers::CONTROL), + ); +} + +fn alt(s: &mut EditorState, c: char) { + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Char(c), KeyModifiers::ALT)); +} + +fn press(s: &mut EditorState, code: KeyCode) { + s.dispatch_key(FrontendId::LOCAL, key(code, KeyModifiers::NONE)); +} + +fn type_str(s: &mut EditorState, text: &str) { + for ch in text.chars() { + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char(ch), KeyModifiers::NONE), + ); + } +} + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_string()).exec().unwrap(); +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_string()).eval().unwrap() +} + +fn ring(s: &EditorState) -> Vec { + eval(s, "return pmacs.killring.list()") +} + +fn buffer_text(s: &EditorState) -> String { + let b: mlua::String = eval( + s, + "local b = pmacs.window.buffer(); return b:slice(0, b:len())", + ); + String::from_utf8_lossy(&b.as_bytes()).into_owned() +} + +fn status(s: &EditorState) -> String { + s.core.borrow().status.clone() +} + +/// Fresh editor whose scratch buffer holds `text`, cursor at 0. +fn editor_with(text: &str) -> EditorState { + let mut s = EditorState::new(); + type_str(&mut s, text); + exec(&s, "pmacs.editor.goto_byte(0)"); + s +} + +// --------------------------------------------------------------------------- +// Chain mechanics +// --------------------------------------------------------------------------- + +#[test] +fn kill_line_kills_to_eol_and_the_newline_separately() { + let mut s = editor_with("hello\nworld"); + ctrl(&mut s, 'k'); // kills "hello" + assert_eq!(buffer_text(&s), "\nworld"); + assert_eq!(ring(&s), vec!["hello"]); + // Cursor now sits at the newline: C-k kills the newline itself — + // and, being consecutive, APPENDS. + ctrl(&mut s, 'k'); + assert_eq!(buffer_text(&s), "world"); + assert_eq!(ring(&s), vec!["hello\n"], "consecutive C-k appends"); +} + +#[test] +fn consecutive_kills_build_one_entry_and_sync_the_clipboard() { + let mut s = editor_with("one\ntwo\nthree\n"); + ctrl(&mut s, 'k'); // "one" + ctrl(&mut s, 'k'); // "\n" + ctrl(&mut s, 'k'); // "two" + assert_eq!(ring(&s), vec!["one\ntwo"]); + // OS slot mirrors the appended head (Q#KR4). + let slot: String = eval(&s, "return pmacs.editor.clipboard_get()"); + assert_eq!(slot, "one\ntwo"); +} + +#[test] +fn movement_breaks_the_kill_chain() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); // "one"; buffer now "\ntwo\n" + press(&mut s, KeyCode::Down); // a cursor command: chain broken + exec(&s, "pmacs.editor.goto_byte(1)"); // start of "two" + ctrl(&mut s, 'k'); // "two" + assert_eq!(ring(&s), vec!["two", "one"], "two entries, no append"); +} + +#[test] +fn self_insert_breaks_the_kill_chain() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); // "one" (line now "\ntwo\n", cursor 0) + type_str(&mut s, "x"); // buffer "x\ntwo\n" + ctrl(&mut s, 'k'); // kills "" ... wait: cursor after 'x' is 1, at "\n" + // cursor sits at the newline → kills it. + assert_eq!(ring(&s), vec!["\n", "one"], "self-insert broke the chain"); +} + +#[test] +fn an_unbound_key_breaks_the_kill_chain() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); + press(&mut s, KeyCode::F(12)); // unbound, not printable + assert!(status(&s).contains("not bound")); + exec(&s, "pmacs.editor.goto_byte(1)"); + ctrl(&mut s, 'k'); + assert_eq!(ring(&s).len(), 2, "unbound key broke the chain"); +} + +#[test] +fn failed_kill_does_not_leave_an_appendable_chain() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); // ring: ["one"] + ctrl(&mut s, 'w'); // no region → fails, clears last_kill_id + assert!(status(&s).contains("no region")); + assert_eq!(ring(&s), vec!["one"], "failed kill pushed nothing"); + ctrl(&mut s, 'k'); // kills "\n" + assert_eq!( + ring(&s), + vec!["\n", "one"], + "a kill after a FAILED kill pushes fresh (no stale append)" + ); +} + +// --------------------------------------------------------------------------- +// M-x semantics (the three-direction matrix, Q#KR2) +// --------------------------------------------------------------------------- + +fn m_x(s: &mut EditorState, name: &str) { + alt(s, 'x'); + type_str(s, name); + press(s, KeyCode::Enter); +} + +#[test] +fn m_x_kill_after_keybound_kill_does_not_append() { + let mut s = editor_with("one\ntwo\nthree\n"); + ctrl(&mut s, 'k'); // "one" + m_x(&mut s, "edit.kill-line"); // kills "\n" — but must NOT append + assert_eq!( + ring(&s), + vec!["\n", "one"], + "the minibuffer interaction breaks the chain (Emacs semantics)" + ); +} + +#[test] +fn keybound_kill_after_m_x_kill_appends() { + let mut s = editor_with("one\ntwo\nthree\n"); + m_x(&mut s, "edit.kill-line"); // "one" — invoke_interactive stamps it + ctrl(&mut s, 'k'); // "\n" — last_command is edit.kill-line → append + assert_eq!( + ring(&s), + vec!["one\n"], + "execute-extended-command sets this-command (Emacs semantics)" + ); +} + +#[test] +fn m_x_kill_twice_does_not_append() { + let mut s = editor_with("one\ntwo\nthree\n"); + m_x(&mut s, "edit.kill-line"); + m_x(&mut s, "edit.kill-line"); + assert_eq!(ring(&s).len(), 2, "each M-x interposes execute-command"); +} + +// --------------------------------------------------------------------------- +// Yank / yank-pop +// --------------------------------------------------------------------------- + +#[test] +fn yank_inserts_the_head_and_yank_pop_cycles_and_wraps() { + let mut s = editor_with("one\ntwo\nthree\n"); + ctrl(&mut s, 'k'); // "one" + press(&mut s, KeyCode::Down); // break chain + exec(&s, "pmacs.editor.goto_byte(1)"); + ctrl(&mut s, 'k'); // rest of "two"-line from byte 1: "wo"... careful + // (buffer was "\ntwo\nthree\n"; byte 1 = 't'; kills "two"[1..] = "wo") + let r = ring(&s); + assert_eq!(r.len(), 2); + let newest = r[0].clone(); // "wo" + let oldest = r[1].clone(); // "one" + + // Yank at the end of the buffer. + let len: i64 = eval(&s, "local b = pmacs.window.buffer(); return b:len()"); + exec(&s, &format!("pmacs.editor.goto_byte({len})")); + ctrl(&mut s, 'y'); + assert!(buffer_text(&s).ends_with(&newest), "C-y yanks the head"); + let cursor_after_yank: i64 = eval(&s, "return pmacs.editor.cursor()"); + assert_eq!( + cursor_after_yank, + len + i64::try_from(newest.len()).unwrap() + ); + + // M-y replaces with the older entry... + alt(&mut s, 'y'); + assert!(buffer_text(&s).ends_with(&oldest), "M-y rotates to older"); + let cursor: i64 = eval(&s, "return pmacs.editor.cursor()"); + assert_eq!( + cursor, + len + i64::try_from(oldest.len()).unwrap(), + "cursor at end of rotation" + ); + // ...and wraps back to the newest. + alt(&mut s, 'y'); + assert!(buffer_text(&s).ends_with(&newest), "M-y wraps"); +} + +#[test] +fn yank_pop_without_a_yank_refuses_and_stays_refused() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); + let before = buffer_text(&s); + alt(&mut s, 'y'); + assert!(status(&s).contains("not a yank")); + assert_eq!(buffer_text(&s), before, "no edit on refusal"); + // A second M-y must not ride the first one's name-stamp (Q#KR7): + // last_command IS edit.yank-pop now, but no session exists. + alt(&mut s, 'y'); + assert!(status(&s).contains("not a yank")); + assert_eq!(buffer_text(&s), before); +} + +#[test] +fn pointer_click_breaks_kill_chain_and_yank_session() { + use crossterm::event::{MouseButton, MouseEvent, MouseEventKind}; + let mut s = editor_with("one\ntwo\nthree\n"); + let term = pmacs::cell::CellSize::new(24, 80); + let click = |s: &mut EditorState, kind| { + s.dispatch_mouse( + FrontendId::LOCAL, + MouseEvent { + kind, + column: 1, + row: 1, + modifiers: KeyModifiers::NONE, + }, + term, + ); + }; + + // Kill side: C-k, click, C-k → two entries. + ctrl(&mut s, 'k'); + click(&mut s, MouseEventKind::Down(MouseButton::Left)); + click(&mut s, MouseEventKind::Up(MouseButton::Left)); + ctrl(&mut s, 'k'); + assert_eq!(ring(&s).len(), 2, "a click must break the kill chain"); + + // Yank side: C-y, click, M-y → refused. + ctrl(&mut s, 'y'); + click(&mut s, MouseEventKind::Down(MouseButton::Left)); + click(&mut s, MouseEventKind::Up(MouseButton::Left)); + let before = buffer_text(&s); + alt(&mut s, 'y'); + assert!( + status(&s).contains("not a yank"), + "click invalidates the yank" + ); + assert_eq!(buffer_text(&s), before); +} + +#[test] +fn wheel_scroll_does_not_break_the_kill_chain() { + use crossterm::event::{MouseEvent, MouseEventKind}; + let term = pmacs::cell::CellSize::new(24, 80); + let scroll = |s: &mut EditorState, kind| { + s.dispatch_mouse( + FrontendId::LOCAL, + MouseEvent { + kind, + column: 1, + row: 1, + modifiers: KeyModifiers::NONE, + }, + term, + ); + }; + + // Case 1 — a boundary-clamped scroll (ScrollUp at the top) moves + // nothing at all: the chain holds and the next C-k appends in + // place. + let mut s = editor_with("one\ntwo\nthree\n"); + ctrl(&mut s, 'k'); // "one" + scroll(&mut s, MouseEventKind::ScrollUp); + ctrl(&mut s, 'k'); // the newline + assert_eq!(ring(&s), vec!["one\n"], "no-op scroll preserves the chain"); + + // Case 2 — pmacs scrolling is cursor-follows-view, so a mid-buffer + // wheel moves point too. The chain STILL holds (Emacs: mwheel + // preserves last-command) and the next kill appends from the NEW + // point — one entry, not two. + let mut s = editor_with("one\ntwo\nthree\nfour\nfive\nsix\n"); + ctrl(&mut s, 'k'); // "one" + scroll(&mut s, MouseEventKind::ScrollDown); + ctrl(&mut s, 'k'); + assert_eq!( + ring(&s).len(), + 1, + "a cursor-following scroll still preserves the chain: {:?}", + ring(&s) + ); +} + +#[test] +fn semantic_pointer_gesture_breaks_the_chain() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); + let buf_id = s.core.borrow().active_window().buffer_id; + s.dispatch_pointer( + FrontendId::LOCAL, + buf_id, + 5, + pmacs::protocol::PointerKind::Down, + pmacs::protocol::Modifiers::NONE, + ); + exec(&s, "pmacs.editor.goto_byte(1)"); + ctrl(&mut s, 'k'); + assert_eq!( + ring(&s).len(), + 2, + "a semantic pointer Down breaks the chain" + ); +} + +// --------------------------------------------------------------------------- +// Region kills, selection yank +// --------------------------------------------------------------------------- + +#[test] +fn cut_and_copy_feed_the_ring_and_yank_replaces_a_selection() { + let mut s = editor_with("alpha beta\n"); + // Select "alpha" (bytes 0..5). + exec( + &s, + "pmacs.editor.goto_byte(5); pmacs.editor.begin_selection(0)", + ); + alt(&mut s, 'w'); // copy + assert_eq!(ring(&s), vec!["alpha"]); + assert_eq!(buffer_text(&s), "alpha beta\n", "copy does not delete"); + + // Cut " beta" (bytes 5..10). + exec( + &s, + "pmacs.editor.goto_byte(10); pmacs.editor.begin_selection(5)", + ); + ctrl(&mut s, 'w'); + assert_eq!(buffer_text(&s), "alpha\n"); + assert_eq!(ring(&s), vec![" beta", "alpha"]); + + // Yank over a selection replaces it, and M-y rotates against the + // replaced range. + exec( + &s, + "pmacs.editor.goto_byte(5); pmacs.editor.begin_selection(0)", + ); + ctrl(&mut s, 'y'); // "alpha" → " beta" + assert_eq!(buffer_text(&s), " beta\n"); + alt(&mut s, 'y'); // rotate to "alpha" + assert_eq!(buffer_text(&s), "alpha\n"); +} + +// --------------------------------------------------------------------------- +// Shared-ring interleaving (two frontends) +// --------------------------------------------------------------------------- + +/// A second "frontend": an unregistered id shares LOCAL's view (the +/// `active_view` fallback) but has its own command boundary and killring +/// session — the exact shape of the Q#KR4/KR7 interleaving blockers. +const B: FrontendId = FrontendId(9); + +fn ctrl_as(s: &mut EditorState, fid: FrontendId, c: char) { + s.dispatch_key(fid, key(KeyCode::Char(c), KeyModifiers::CONTROL)); +} + +#[test] +fn interleaved_kills_never_append_across_frontends() { + let mut s = editor_with("one\ntwo\nthree\n"); + ctrl(&mut s, 'k'); // A kills "one" + exec(&s, "pmacs.editor.goto_byte(1)"); + ctrl_as(&mut s, B, 'k'); // B kills "wo" — B's own first kill + exec(&s, "pmacs.editor.goto_byte(2)"); + ctrl(&mut s, 'k'); // A again — A's last_kill is NOT the head (B's is) + let r = ring(&s); + assert_eq!(r.len(), 3, "A-kill/B-kill/A-kill = three entries"); + assert_eq!(r[1], "two", "B's entry intact — never appended onto"); +} + +#[test] +fn yank_pop_rotates_from_the_sessions_own_entry_despite_other_pushes() { + let mut s = editor_with("one\ntwo\nthree\n"); + ctrl(&mut s, 'k'); // A: ring ["one"] + let len: i64 = eval(&s, "local b = pmacs.window.buffer(); return b:len()"); + exec(&s, &format!("pmacs.editor.goto_byte({len})")); + ctrl(&mut s, 'y'); // A yanks "one"; session → entry("one") + + // B pushes a new head, shifting positions but not ids. + exec(&s, "pmacs.editor.goto_byte(1)"); + ctrl_as(&mut s, B, 'k'); // B kills "wo" → ring ["wo", "one"] + + // A's M-y must rotate from A's OWN entry ("one" — now position 2), + // to the next-older-with-wrap = "wo". An index-based session would + // have mis-resolved after B's push. + // (B's kill edited the buffer, but before A's yanked range, so the + // session's slice-verify passes — the range shifted is upstream.) + // Note: B's kill removed bytes BEFORE the yank range, so the + // remembered {start,stop} no longer hold the yanked text → the + // invalidation guard fires instead. That IS the specified + // behavior: refuse rather than splice a shifted range. + alt(&mut s, 'y'); + assert!( + status(&s).contains("changed since the yank"), + "a concurrent upstream edit invalidates rather than mis-splices: {:?}", + status(&s) + ); +} + +#[test] +fn yank_pop_uses_stable_ids_when_other_pushes_leave_the_range_intact() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); // ring ["one"], A's chain live + let len: i64 = eval(&s, "local b = pmacs.window.buffer(); return b:len()"); + exec(&s, &format!("pmacs.editor.goto_byte({len})")); + ctrl(&mut s, 'y'); // A yanks "one" at end; session entry = "one" + + // B COPIES bytes 1..3 ("tw" of the post-kill buffer "\ntwo\none"): + // ring becomes ["tw", "one"], buffer untouched, so A's yanked + // range is still intact. + exec( + &s, + "pmacs.editor.begin_selection(1); pmacs.editor.goto_byte(3)", + ); + s.dispatch_key(B, key(KeyCode::Char('w'), KeyModifiers::ALT)); + assert_eq!(ring(&s), vec!["tw", "one"]); + + // A's M-y: the session's entry ("one") sits at position 2 now; + // next-older-with-wrap is B's copy. An integer index recorded at + // yank time (position 1) would have rotated from the wrong place. + alt(&mut s, 'y'); + assert!( + buffer_text(&s).ends_with("tw"), + "rotation follows the stable id, not a shifted index: {:?}", + buffer_text(&s) + ); +} + +#[test] +fn eviction_mid_session_invalidates_the_yank_pop() { + let mut s = editor_with("one\ntwo\nthree\nfour\n"); + exec(&s, "pmacs.killring.max(2)"); + ctrl(&mut s, 'k'); // ring ["one"] + let len: i64 = eval(&s, "local b = pmacs.window.buffer(); return b:len()"); + exec(&s, &format!("pmacs.editor.goto_byte({len})")); + ctrl(&mut s, 'y'); // session → entry "one" + + // Two copies (buffer untouched) evict "one" from a cap-2 ring. + exec( + &s, + "pmacs.editor.begin_selection(1); pmacs.editor.goto_byte(3)", + ); + s.dispatch_key(B, key(KeyCode::Char('w'), KeyModifiers::ALT)); + exec( + &s, + "pmacs.editor.begin_selection(5); pmacs.editor.goto_byte(8)", + ); + s.dispatch_key(B, key(KeyCode::Char('w'), KeyModifiers::ALT)); + let r = ring(&s); + assert_eq!(r.len(), 2); + assert!(!r.contains(&"one".to_string()), "'one' evicted"); + + alt(&mut s, 'y'); + assert!( + status(&s).contains("expired"), + "an evicted session entry refuses cleanly: {:?}", + status(&s) + ); +} + +// --------------------------------------------------------------------------- +// External content, menu, hook delivery, cap +// --------------------------------------------------------------------------- + +#[test] +fn externally_pasted_content_joins_the_ring_at_yank() { + let mut s = editor_with(""); + ctrl(&mut s, 'k'); // fails (empty buffer) — ring stays empty + // An OS paste arrives (the daemon route sets the slot + inserts). + s.core.borrow_mut().paste_inbound(b"external").unwrap(); + assert_eq!(buffer_text(&s), "external"); + // The next yank notices slot ≠ head, pushes it, and yanks it. + ctrl(&mut s, 'y'); + assert_eq!(ring(&s), vec!["external"]); + assert_eq!(buffer_text(&s), "externalexternal"); +} + +#[test] +fn menu_cut_feeds_the_ring_fires_after_edit_and_chains() { + use crossterm::event::{MouseButton, MouseEvent, MouseEventKind}; + let mut s = editor_with("alpha beta\n"); + exec( + &s, + r#" + _G.AE = 0 + pmacs.hook.add("buffer.after-edit", function() _G.AE = _G.AE + 1 end) + "#, + ); + // Select "alpha", open the context menu via right-click, invoke Cut. + exec( + &s, + "pmacs.editor.goto_byte(5); pmacs.editor.begin_selection(0)", + ); + s.dispatch_mouse( + FrontendId::LOCAL, + MouseEvent { + kind: MouseEventKind::Down(MouseButton::Right), + column: 2, + row: 0, + modifiers: KeyModifiers::NONE, + }, + pmacs::cell::CellSize::new(24, 80), + ); + assert!( + s.core.borrow().menu_is_open(), + "right-click opened the menu" + ); + // Find the "edit.cut" row in the open menu's state. + let cut_index = { + let menu = s.core.borrow().menu.clone(); + let guard = menu.lock().unwrap(); + guard + .as_ref() + .and_then(|m| { + m.rows.iter().position(|r| { + matches!(r, pmacs::menu::MenuRow::Item { command, .. } + if command == "edit.cut") + }) + }) + .expect("menu has a Cut row") + }; + s.dispatch_menu_pointer( + FrontendId::LOCAL, + Some(u32::try_from(cut_index).unwrap()), + true, + ); + + assert_eq!(ring(&s), vec!["alpha"], "menu Cut fed the ring"); + assert_eq!(buffer_text(&s), " beta\n"); + let fired: i64 = eval(&s, "return _G.AE"); + assert_eq!(fired, 1, "menu Cut fires after-edit exactly once (Q#KR10b)"); + + // The menu rotation makes the cut chain like a keybound one: + // a following C-k appends. + ctrl(&mut s, 'k'); + let r = ring(&s); + assert_eq!(r[0], "alpha beta", "menu Cut then C-k appends (rotate row)"); +} + +#[test] +fn m_x_kill_fires_after_edit_and_keybound_does_not_double_fire() { + let mut s = editor_with("one\ntwo\n"); + exec( + &s, + r#" + _G.AE = 0 + pmacs.hook.add("buffer.after-edit", function() _G.AE = _G.AE + 1 end) + "#, + ); + ctrl(&mut s, 'k'); + let after_keybound: i64 = eval(&s, "return _G.AE"); + assert_eq!(after_keybound, 1, "keybound kill fires once, no double"); + m_x(&mut s, "edit.kill-line"); + let after_mx: i64 = eval(&s, "return _G.AE"); + assert_eq!(after_mx, 2, "M-x kill fires after-edit too (Q#KR10b)"); +} + +#[test] +fn cap_is_validated_and_shrink_trims() { + let s = editor_with(""); + let d: i64 = eval(&s, "return pmacs.killring.max()"); + assert_eq!(d, 60); + for bad in ["0/0", "math.huge", "-math.huge", "0", "-3", "'ten'", "{}"] { + let ok: bool = eval(&s, &format!("return (pcall(pmacs.killring.max, {bad}))")); + assert!(!ok, "killring.max({bad}) must be rejected"); + } + let set: i64 = eval(&s, "return pmacs.killring.max(2.9)"); + assert_eq!(set, 2, "floored"); + + // Five distinct entries via COPY over a static buffer (copies never + // delete, so the byte offsets stay put; each text is distinct so + // duplicate-of-head collapse never fires). + let mut s2 = editor_with("aa bb cc dd ee\n"); + exec(&s2, "pmacs.killring.max(10)"); + for i in 0..5i64 { + let lo = i * 3; + exec( + &s2, + &format!( + "pmacs.editor.begin_selection({lo}); pmacs.editor.goto_byte({})", + lo + 2 + ), + ); + alt(&mut s2, 'w'); + } + assert_eq!(ring(&s2).len(), 5); + let trimmed: i64 = eval(&s2, "return pmacs.killring.max(3)"); + assert_eq!(trimmed, 3); + assert_eq!(ring(&s2).len(), 3, "shrinking the cap trims immediately"); +} + +#[test] +fn frontend_detached_drops_per_frontend_state() { + let mut s = editor_with("one\ntwo\n"); + ctrl_as(&mut s, B, 'k'); // B kills → B has last_kill_id + let has: bool = eval( + &s, + "return pmacs.killring._debug_state(9).last_kill_id ~= nil", + ); + assert!(has, "B has kill state"); + // The daemon fires this on SessionDetached (Q#KR11); fire it the + // same way to exercise the Lua-side cleanup. + exec(&s, "pmacs.hook.run('frontend.detached', 9)"); + let gone: bool = eval( + &s, + "local st = pmacs.killring._debug_state(9); \ + return st.last_kill_id == nil and st.session == nil", + ); + assert!(gone, "detach dropped B's killring state"); +} From c6038a790ded99221efe02abe42dc01ec982a5ab Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 9 Jul 2026 20:08:17 -0400 Subject: [PATCH 3/4] fix(edit): semantic right-click breaks the chain; intercept-safe kill/yank-pop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the PR #103 review. - BLOCKING semantic right-click: the dispatcher routes PointerKind::Context directly to open_menu_at_byte, bypassing dispatch_pointer's break — so GPU C-k, right-click, dismiss, C-k still appended, and M-y survived the click. open_menu_at_byte now breaks the chain like the grid right-click path. - HIGH C-k under intercepts: kill_line captured text then called buf:delete un-pcall'd. A REJECTING intercept threw before fail_kill, leaving the old chain live (the next C-k appended to a kill that never happened); a TRANSFORMING intercept could delete different bytes while the ring and OS clipboard kept the original text. The delete is now pcall'd and verified by length delta: rejection clears the chain with a status; a transformed delete feeds nothing (the interceptor's result stands — accepted post-hoc semantics), also clearing the chain. Same discipline applied to cut's delete_region. - HIGH rejected M-y: buf:replace ran outside pcall, so a rejecting intercept threw through command dispatch and left sessions[fid] live — a second M-y could reuse the supposedly-invalid session. The replace is pcall'd; rejection drops the session with a status. Tests (kill_ring_acceptance now 28): semantic_context_right_click_breaks _the_chain (drives open_menu_at_byte directly — the GPU route); rejecting_intercept_clears_the_kill_chain (reject-once intercept: the kill after the rejection pushes fresh, not append); transforming_intercept_does_not_feed_the_ring (delete shrunk to one byte: ring untouched, interceptor's result stands); rejecting_intercept_ends_the_yank_session (second M-y refuses on no-session, no splice). Gates: fmt + workspace clippy clean; lib 1500; crdt 1672; killring 28; cua 5; m6_4 repl (intercept suite) 15; git diff --check clean. Co-Authored-By: Claude Fable 5 --- builtin/runtime/killring.lua | 43 ++++++++++-- src/editor.rs | 6 ++ tests/kill_ring_acceptance.rs | 125 ++++++++++++++++++++++++++++++++++ 3 files changed, 170 insertions(+), 4 deletions(-) diff --git a/builtin/runtime/killring.lua b/builtin/runtime/killring.lua index 730ec6c..cbab480 100644 --- a/builtin/runtime/killring.lua +++ b/builtin/runtime/killring.lua @@ -117,9 +117,18 @@ function pmacs.killring.cut() return false end local text = buf:slice(region.start, region["end"]) - if not ed.delete_region() then + -- Same intercept discipline as kill_line: a rejected or transformed + -- delete must not feed the ring or leave a live chain. + local len_before = buf:len() + local dok, deleted = pcall(ed.delete_region) + if not dok or not deleted then fail_kill(fid) - ed.set_status("no region") + ed.set_status(dok and "no region" or "kill rejected by buffer intercept") + return false + end + if len_before - buf:len() ~= #text then + fail_kill(fid) + ed.set_status("kill altered by buffer intercept; ring not updated") return false end kill_push(fid, text) @@ -185,7 +194,25 @@ function pmacs.killring.kill_line() kill_to = eol or len -- rest of the line (or of a final bare line) end local text = buf:slice(cursor, kill_to) - buf:delete(cursor, kill_to) + -- The delete runs the buffer's edit intercepts, which may REJECT + -- (error) or TRANSFORM the operation. A rejection must clear the + -- kill chain (or the next C-k would append to a kill that never + -- happened); a transformation means the bytes actually removed are + -- not `text`, so pushing `text` would put never-killed bytes on the + -- ring and the OS clipboard. Verify by length delta: only a clean, + -- untransformed delete feeds the ring. + local len_before = buf:len() + local ok = pcall(function() buf:delete(cursor, kill_to) end) + if not ok then + fail_kill(fid) + ed.set_status("kill rejected by buffer intercept") + return false + end + if len_before - buf:len() ~= #text then + fail_kill(fid) + ed.set_status("kill altered by buffer intercept; ring not updated") + return false + end kill_push(fid, text) return true end @@ -274,7 +301,15 @@ function pmacs.killring.yank_pop() return false end local entry = ring[pos % #ring + 1] - buf:replace(s.start, s.stop, entry.text) + -- The replace runs buffer intercepts, which may REJECT by erroring. + -- A rejection must still end the session — letting the error + -- propagate would leave `sessions[fid]` live, and a second M-y + -- could reuse the supposedly-invalid session. + local rok = pcall(function() buf:replace(s.start, s.stop, entry.text) end) + if not rok then + drop_session(fid, "yank-pop rejected by buffer intercept") + return false + end -- Verify the applied edit: buffer intercepts may alter or reject a -- replace. Accepted post-hoc semantics (Q#KR7): on mismatch the -- interceptor's result stands, the session ends, and we say so. diff --git a/src/editor.rs b/src/editor.rs index 67cdc59..2b946f7 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1429,6 +1429,12 @@ impl EditorState { return; } core.set_active_window_id(win_id); + // A context right-click is a pointer gesture (kill ring + // Q#KR2): it must break the chain like the grid path's + // right-click does. The semantic dispatcher routes + // PointerKind::Context here directly, bypassing + // dispatch_pointer's break. + core.break_command_chain(frontend_id); if core.active_region().is_none() { let snapped = { let registry = core.registry.clone(); diff --git a/tests/kill_ring_acceptance.rs b/tests/kill_ring_acceptance.rs index c154f59..07785d4 100644 --- a/tests/kill_ring_acceptance.rs +++ b/tests/kill_ring_acceptance.rs @@ -639,6 +639,131 @@ fn cap_is_validated_and_shrink_trims() { assert_eq!(ring(&s2).len(), 3, "shrinking the cap trims immediately"); } +#[test] +fn semantic_context_right_click_breaks_the_chain() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); // "one" — chain live + // The semantic dispatcher routes PointerKind::Context straight to + // open_menu_at_byte, bypassing dispatch_pointer — the GPU + // right-click path. + let buf_id = s.core.borrow().active_window().buffer_id; + s.open_menu_at_byte(FrontendId::LOCAL, buf_id, 3); + // Dismiss the menu without invoking anything. + s.core.borrow_mut().menu_close(); + ctrl(&mut s, 'k'); + assert_eq!( + ring(&s).len(), + 2, + "a semantic right-click must break the kill chain: {:?}", + ring(&s) + ); +} + +#[test] +fn rejecting_intercept_clears_the_kill_chain() { + let mut s = editor_with("one\ntwo\nthree\n"); + ctrl(&mut s, 'k'); // "one" — chain live, ring ["one"] + // An intercept that rejects exactly the NEXT edit, then allows. + exec( + &s, + r#" + _G.reject_once = true + pmacs.buffer.add_intercept(pmacs.window.buffer(), function(_op) + if _G.reject_once then + _G.reject_once = false + error("rejected by test intercept") + end + return nil + end) + "#, + ); + ctrl(&mut s, 'k'); // rejected — must clear the chain, push nothing + assert!( + status(&s).contains("rejected"), + "rejection is reported: {:?}", + status(&s) + ); + assert_eq!(ring(&s), vec!["one"], "a rejected kill feeds nothing"); + ctrl(&mut s, 'k'); // allowed again — must push FRESH, not append + assert_eq!( + ring(&s), + vec!["\n", "one"], + "the chain did not survive the rejection" + ); +} + +#[test] +fn transforming_intercept_does_not_feed_the_ring() { + let mut s = editor_with("alpha\nbeta\n"); + // An intercept that shrinks every delete to its first byte: the + // bytes actually removed are not what C-k sliced, so pushing the + // sliced text would put never-killed bytes on the ring. + exec( + &s, + r#" + pmacs.buffer.add_intercept(pmacs.window.buffer(), function(op) + if op.kind == "delete" then + return { kind = "delete", start = op.start, ["end"] = op.start + 1 } + end + return nil + end) + "#, + ); + ctrl(&mut s, 'k'); + assert!( + status(&s).contains("altered"), + "transformation is reported: {:?}", + status(&s) + ); + assert!(ring(&s).is_empty(), "a transformed kill feeds nothing"); + // The interceptor's result stands (accepted post-hoc semantics). + assert_eq!(buffer_text(&s), "lpha\nbeta\n"); +} + +#[test] +fn rejecting_intercept_ends_the_yank_session() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); // "one" + press(&mut s, KeyCode::Down); + exec(&s, "pmacs.editor.goto_byte(1)"); + ctrl(&mut s, 'k'); // "two" + assert_eq!(ring(&s).len(), 2); + + let len: i64 = eval(&s, "local b = pmacs.window.buffer(); return b:len()"); + exec(&s, &format!("pmacs.editor.goto_byte({len})")); + ctrl(&mut s, 'y'); // session live + + // Reject the next edit (the M-y replace). + exec( + &s, + r#" + _G.reject_once = true + pmacs.buffer.add_intercept(pmacs.window.buffer(), function(_op) + if _G.reject_once then + _G.reject_once = false + error("rejected by test intercept") + end + return nil + end) + "#, + ); + alt(&mut s, 'y'); // rejected — must END the session, not throw through + assert!( + status(&s).contains("rejected"), + "rejection reported: {:?}", + status(&s) + ); + // A second M-y must refuse on "no session", not reuse the dead one. + let before = buffer_text(&s); + alt(&mut s, 'y'); + assert!( + status(&s).contains("not a yank"), + "the rejected session is gone: {:?}", + status(&s) + ); + assert_eq!(buffer_text(&s), before, "no splice from a dead session"); +} + #[test] fn frontend_detached_drops_per_frontend_state() { let mut s = editor_with("one\ntwo\n"); From 8da143b402d98626de7da5a3e36515823ada3dce Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 9 Jul 2026 20:40:48 -0400 Subject: [PATCH 4/4] fix(edit): exact effective-edit verification for kill/yank-pop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the PR #103 round-3 review: length-delta verification is defeated by an intercept that rewrites an op to a DIFFERENT equal-length range, and "replacement text appears at start" is defeated by one that enlarges `end` by a byte. The buffer mutators (buf:insert/delete/replace) now RETURN the effective edit — `(start, end, inserted_len)` of the post-intercept operation actually applied (they returned nothing before, so no caller breaks). killring compares those against what it requested: - C-k / cut: any deviation (shifted range, resized range, nonzero insertion) means the bytes removed are not the bytes sliced — the ring and OS clipboard receive nothing, the chain clears, and the interceptor's result stands. cut now goes through buf:delete (for the effective edit) with explicit clear_selection + goto_byte. - M-y: any deviation from (s.start, s.stop, #entry.text) drops the session — including the end+1 enlargement that silently deleted an extra byte while passing the old text-at-start check. The redundant post-replace slice verify is gone; the exact contract replaces it. Tests (kill_ring_acceptance now 30): equal_length_shifted_delete_does_not_feed_the_ring (delete shifted +2, same length — the case a length delta cannot see), stop_enlarging_replace_ends_the_yank_session (mid-buffer yank so the enlarged range is valid and the transform path — not range validation — is what fires; at buffer end the same intercept fails validation and takes the rejection path, which also drops the session). Gates: fmt + workspace clippy clean; lib 1500; crdt 1672; killring 30; cua 5; m6_4/m6_5 repl (mutator-heavy) 15/11; git diff --check clean. Co-Authored-By: Claude Fable 5 --- builtin/runtime/killring.lua | 60 ++++++++++++++++------------- src/lua_bindings/mod.rs | 22 +++++++++-- tests/kill_ring_acceptance.rs | 72 +++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 30 deletions(-) diff --git a/builtin/runtime/killring.lua b/builtin/runtime/killring.lua index cbab480..8e2fad9 100644 --- a/builtin/runtime/killring.lua +++ b/builtin/runtime/killring.lua @@ -116,21 +116,26 @@ function pmacs.killring.cut() ed.set_status("no region") return false end - local text = buf:slice(region.start, region["end"]) - -- Same intercept discipline as kill_line: a rejected or transformed - -- delete must not feed the ring or leave a live chain. - local len_before = buf:len() - local dok, deleted = pcall(ed.delete_region) - if not dok or not deleted then + local rstart, rstop = region.start, region["end"] + local text = buf:slice(rstart, rstop) + -- Same intercept discipline as kill_line, via the mutator so the + -- EFFECTIVE edit is checkable exactly. The selection is cleared + -- explicitly (ed.delete_region did that as a side effect). + local dok, estart, estop, einserted = pcall(function() + return buf:delete(rstart, rstop) + end) + ed.clear_selection() + if not dok then fail_kill(fid) - ed.set_status(dok and "no region" or "kill rejected by buffer intercept") + ed.set_status("kill rejected by buffer intercept") return false end - if len_before - buf:len() ~= #text then + if estart ~= rstart or estop ~= rstop or einserted ~= 0 then fail_kill(fid) ed.set_status("kill altered by buffer intercept; ring not updated") return false end + ed.goto_byte(rstart) kill_push(fid, text) return true end @@ -199,16 +204,19 @@ function pmacs.killring.kill_line() -- kill chain (or the next C-k would append to a kill that never -- happened); a transformation means the bytes actually removed are -- not `text`, so pushing `text` would put never-killed bytes on the - -- ring and the OS clipboard. Verify by length delta: only a clean, - -- untransformed delete feeds the ring. - local len_before = buf:len() - local ok = pcall(function() buf:delete(cursor, kill_to) end) + -- ring and the OS clipboard. The mutators return the EFFECTIVE edit + -- (post-intercept start/end/inserted), so this is an exact check — + -- a length delta would be defeated by an equal-length rewrite to a + -- different range. + local ok, estart, estop, einserted = pcall(function() + return buf:delete(cursor, kill_to) + end) if not ok then fail_kill(fid) ed.set_status("kill rejected by buffer intercept") return false end - if len_before - buf:len() ~= #text then + if estart ~= cursor or estop ~= kill_to or einserted ~= 0 then fail_kill(fid) ed.set_status("kill altered by buffer intercept; ring not updated") return false @@ -301,24 +309,22 @@ function pmacs.killring.yank_pop() return false end local entry = ring[pos % #ring + 1] - -- The replace runs buffer intercepts, which may REJECT by erroring. - -- A rejection must still end the session — letting the error - -- propagate would leave `sessions[fid]` live, and a second M-y - -- could reuse the supposedly-invalid session. - local rok = pcall(function() buf:replace(s.start, s.stop, entry.text) end) + -- The replace runs buffer intercepts, which may REJECT (error) or + -- TRANSFORM. A rejection must still end the session — letting the + -- error propagate would leave `sessions[fid]` live for a second M-y + -- to reuse. And the verification must be EXACT: the mutator returns + -- the effective edit, and any deviation from the requested + -- (start, stop, #text) — e.g. an intercept enlarging `stop` by one + -- byte, silently deleting extra content — ends the session (the + -- interceptor's result stands; accepted post-hoc semantics, Q#KR7). + local rok, estart, estop, einserted = pcall(function() + return buf:replace(s.start, s.stop, entry.text) + end) if not rok then drop_session(fid, "yank-pop rejected by buffer intercept") return false end - -- Verify the applied edit: buffer intercepts may alter or reject a - -- replace. Accepted post-hoc semantics (Q#KR7): on mismatch the - -- interceptor's result stands, the session ends, and we say so. - -- pcall'd for the same reason as the guard above: an intercept that - -- shrank the buffer must invalidate, not throw. - local vok, applied = pcall(function() - return buf:slice(s.start, s.start + #entry.text) - end) - if not vok or applied ~= entry.text then + if estart ~= s.start or estop ~= s.stop or einserted ~= #entry.text then drop_session(fid, "yank-pop altered by buffer intercept; stopped") return false end diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 49e1b79..17ed4fa 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -1198,7 +1198,7 @@ fn add_mutation_methods>(methods: &mut M) { bypass_intercept, )?; notify_buffer_edit_to_windows(lua, this.0, &edit); - Ok(()) + effective_edit_triple(&edit) }, ); @@ -1209,7 +1209,7 @@ fn add_mutation_methods>(methods: &mut M) { let bypass_intercept = parse_bypass_intercept(opts.as_ref())?; let edit = run_buffer_edit(lua, this.0, EditOp::Delete { range }, bypass_intercept)?; notify_buffer_edit_to_windows(lua, this.0, &edit); - Ok(()) + effective_edit_triple(&edit) }, ); @@ -1229,7 +1229,7 @@ fn add_mutation_methods>(methods: &mut M) { bypass_intercept, )?; notify_buffer_edit_to_windows(lua, this.0, &edit); - Ok(()) + effective_edit_triple(&edit) }, ); } @@ -1243,6 +1243,22 @@ fn parse_bypass_intercept(opts: Option<&Table>) -> mlua::Result { }) } +/// The mutators' Lua return value: the **effective** edit after buffer +/// intercepts ran — `(start, end, inserted_len)` of the operation that +/// was actually applied (kill ring review round 4). An intercept may +/// legally rewrite an op's range; callers that must know exactly what +/// happened (killring's C-k / M-y) compare these against what they +/// requested instead of inferring from length deltas, which an +/// equal-length rewrite defeats. +fn effective_edit_triple(edit: &crate::rope::Edit) -> mlua::Result<(i64, i64, i64)> { + let cvt = |v: u64| i64::try_from(v).map_err(mlua::Error::external); + Ok(( + cvt(edit.range.start)?, + cvt(edit.range.end)?, + cvt(edit.inserted_len)?, + )) +} + fn run_buffer_edit( lua: &Lua, id: BufferId, diff --git a/tests/kill_ring_acceptance.rs b/tests/kill_ring_acceptance.rs index 07785d4..1a375ad 100644 --- a/tests/kill_ring_acceptance.rs +++ b/tests/kill_ring_acceptance.rs @@ -720,6 +720,78 @@ fn transforming_intercept_does_not_feed_the_ring() { assert_eq!(buffer_text(&s), "lpha\nbeta\n"); } +#[test] +fn equal_length_shifted_delete_does_not_feed_the_ring() { + let mut s = editor_with("abcdef\nghijkl\n"); + // An intercept that SHIFTS every delete right by 2 bytes while + // keeping its length — a length-delta check cannot see this, and + // the ring would receive text that was never killed. + exec( + &s, + r#" + pmacs.buffer.add_intercept(pmacs.window.buffer(), function(op) + if op.kind == "delete" then + return { kind = "delete", start = op.start + 2, ["end"] = op["end"] + 2 } + end + return nil + end) + "#, + ); + ctrl(&mut s, 'k'); // wanted [0,6) "abcdef"; intercept deletes [2,8) + assert!( + status(&s).contains("altered"), + "an equal-length shifted delete is detected: {:?}", + status(&s) + ); + assert!( + ring(&s).is_empty(), + "never-killed text must not reach the ring: {:?}", + ring(&s) + ); + // The interceptor's result stands. + assert_eq!(buffer_text(&s), "abhijkl\n"); +} + +#[test] +fn stop_enlarging_replace_ends_the_yank_session() { + let mut s = editor_with("one\ntwo\n"); + ctrl(&mut s, 'k'); // "one" + press(&mut s, KeyCode::Down); + exec(&s, "pmacs.editor.goto_byte(1)"); + ctrl(&mut s, 'k'); // "two" + // Yank at the START so the buffer extends past the yanked range — + // an end+1 range at buffer end would fail validation ("rejected") + // instead of exercising the transform path. + exec(&s, "pmacs.editor.goto_byte(0)"); + ctrl(&mut s, 'y'); // session live + + // An intercept that enlarges every replace's end by ONE byte: the + // replacement text still lands at s.start, so a "text appears at + // start" verify passes — but one extra byte was silently deleted. + exec( + &s, + r#" + pmacs.buffer.add_intercept(pmacs.window.buffer(), function(op) + if op.kind == "replace" then + return { kind = "replace", start = op.start, ["end"] = op["end"] + 1 } + end + return nil + end) + "#, + ); + alt(&mut s, 'y'); + assert!( + status(&s).contains("altered"), + "an end-enlarged replace is detected: {:?}", + status(&s) + ); + // The session is dead: a further M-y refuses without editing. + let before = buffer_text(&s); + alt(&mut s, 'y'); + assert!(status(&s).contains("not a yank")); + assert_eq!(buffer_text(&s), before); +} + #[test] fn rejecting_intercept_ends_the_yank_session() { let mut s = editor_with("one\ntwo\n");