From c6038a790ded99221efe02abe42dc01ec982a5ab Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 9 Jul 2026 20:08:17 -0400 Subject: [PATCH] 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");