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");