From 27aaf0ef30015459fe34ceeba9108c5114e8bdae Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 8 Jul 2026 15:51:33 -0400 Subject: [PATCH 1/3] docs: query-replace framing (Arc 2 interleave) Q#QR1-10: zero-protocol-change (reuses v15 StatusFacts.message band, SearchMatchActive decorations, dispatch_idle search gate); distinct QueryReplaceSession + 5th dispatcher shadow with the after-edit hook precedent; Emacs search-forward-after-each-replace (cached regex, zero-width filter, invalid-at-start refusal); chained minibuffer.read with separate from/to history buckets + empty-to=deletion; per-match core.status prompt; SearchMatchActive highlight-and-reveal; quit keeps replacements (only nothing-matched restores origin). Incorporates the 2026-07-08 review findings. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/query-replace-framing.md | 267 ++++++++++++++++++++++++++++++++++ 1 file changed, 267 insertions(+) create mode 100644 docs/query-replace-framing.md diff --git a/docs/query-replace-framing.md b/docs/query-replace-framing.md new file mode 100644 index 0000000..e2a454e --- /dev/null +++ b/docs/query-replace-framing.md @@ -0,0 +1,267 @@ +# Query-replace — framing (Arc 2 interleave) + +pmacs has incremental search, substring and regex, in both frontends — +and no replace at all. `search.rs` finds matches and never substitutes; +there is no `M-%`. This is the highest-value missing editing table-stake +(you reach for it hourly), and it sits right on top of isearch. + +The happy discovery from scouting: **the whole feature is +zero-protocol-change.** It reuses three things already on the wire — +the `StatusFacts.message` band (added v15 for exactly this class of +transient prompt), the `SearchMatch`/`SearchMatchActive` decorations +(store-driven, both frontends), and the `dispatch_idle`-false gate that +already makes semantic frontends round-trip keys during a search. The +entire arc is core + Lua; no v16. + +Roadmap context: `docs/roadmap-2026-07.md` Arc 2 (the editing +table-stakes interleave, promised after Arc 1's panels). + +## What already exists (verified) + +- **Match store** (`src/search.rs`): `SearchStore` is per-buffer, + keyed `BufferId → SearchState { query, matches: Vec, + active }`; `SharedSearchStore = Arc>` on the core. `set` + replaces query+matches, `focus_from(byte)` points `active` at the + first match `≥ byte`, `step` advances with wrap. Matchers are free + functions over `&[u8]`: `find_all` (smart-case substring, + non-overlapping) and `find_all_regex` (`None` iff the pattern fails + to compile). **No replace API — greenfield.** +- **Highlights are store-driven, not session-driven** + (`semantic_render.rs`): the producer emits `SearchMatchActive` for + the match equal to `active_match()`, `SearchMatch` for the rest, + reading only the store. So writing a match into `search_store` and + pointing `active` at it renders in both frontends with **zero new + rendering code**. +- **The isearch shadow is the template** (`src/editor.rs`): + `dispatch_key` routes to `dispatch_search_key` while + `search_active()`; `SearchKey::from_chord` maps a fixed key + vocabulary; an active search eats every key. `dispatch_idle()` + returns false while `search_active()`, so the GPU round-trips keys + instead of optimistically self-inserting (test + `isearch_flips_dispatch_idle_so_gpu_round_trips`). +- **The transient prompt band** (`StatusFacts.message`, v15): setting + `core.status` shows an echo-area string in both frontends (TUI + bottom row, GPU band). `dispatch_key` clears `core.status` at entry, + so a handler that re-sets it at the end owns the band cleanly. +- **Region replace** is `EditOp::Replace { range, bytes }` via + `apply_active_edit` — one undo step (the `insert_char_over_region` + precedent). Every edit marks the search store stale; nothing + auto-recomputes matches (Q#QR2 owns this). + +## Decisions + +### Q#QR1 — A distinct `QueryReplaceSession` + a 5th dispatcher shadow + +Not an overload of the isearch session — the lifecycles differ (isearch +is one string, cancel-restores-origin; query-replace is from+to plus an +interactive y/n phase). Mirror the structure instead: +`QueryReplaceSession` on `EditorCore`, `query_replace_active()`, and a +`QueryReplaceKey::from_chord` + `dispatch_query_replace_key` in +`editor.rs` — the fifth member of the shadow family (minibuffer, +search, menu, completion, **query-replace**). Add +`query_replace_active()` to the `dispatch_idle()` disjunction and the +modal-close guard, exactly as the others. + +**`buffer.after-edit` must fire from inside the shadow (P1).** A modal +shadow `return`s before `dispatch_key`'s normal post-command edit +check, so `apply_active_edit` from `dispatch_query_replace_key` would +not notify LSP `didChange` / syntax reparse / anything on the edit +chain — the replaced text would silently keep stale styling and +diagnostics. Mirror `dispatch_completion_key` (`editor.rs:797` +precedent): snapshot `active_buffer_revision()` before handling the +key, and `run_hook("buffer.after-edit", …)` if it changed. `!` +(replace-all) applies many edits in one keypress — fire the hook +**once** after the batch (revision compared across the whole handler), +not per replacement, so the debounced `didChange` coalesces naturally. + +### Q#QR2 — Search-forward-after-each-replace (Emacs's algorithm), not precompute-all + +The load-bearing correctness decision. Do **not** precompute the whole +match list and walk it — replacing `a`→`aa` (or `foo`→`foobar`) would +re-match the replacement text and loop, and precomputed offsets go +stale after the first edit. Instead, hold a `next_from` byte cursor; +each step finds the *next* match at/after `next_from` in the **current** +buffer: + +- **replace** (`y`/`SPC`): `Replace` the match with the to-bytes; set + `next_from = match.start + to.len()` (past the replacement, so it's + never re-matched); advance. +- **skip** (`n`/`DEL`): set `next_from = match.end`; advance. +- **advance**: find the next match from `next_from`; none ⇒ finish. + +Offset shift is handled by construction (every search is on the live +buffer from a byte past the last edit), and replacements are never +re-matched. Add `find_first_from(haystack, query, start)` (literal + +regex) to `search.rs` so each step is one bounded forward scan, not an +`O(buffer)` `find_all` filtered — keeps `!` (replace-all) linear. + +**Cursor reveal (P2).** Highlighting a match that's off-screen is +useless — like isearch's `search_place_cursor` after each step, move +point so the frontend scrolls it into view: **advance** sets the cursor +to the current match's `start`; **replace** sets it to the *end of the +inserted replacement* (`match.start + to.len()`), which is also +`next_from`; **natural finish** (ran out of matches) leaves point +there. Quit semantics are Q#QR10. + +**Regex specifics (P2).** Compile the pattern **once** at session start +and store the `regex::bytes::Regex` in the session; the regex +`find_first_from` scans from `next_from` using that cached engine +(`Regex::find_at`) — recompiling per step would make `!` quadratic and +defeat the "linear" claim. **Invalid pattern at start**: if the regex +fails to compile, don't begin the session — set a status +(`"Invalid regex: …"`) and return, the same clean refusal isearch's +`invalid` flag gives (there's no mid-session recompile since the +pattern is fixed once entered). **Zero-width matches**: the regex +first-match path filters them exactly as `find_all_regex` does (a +zero-width match would never advance `next_from` and would loop) — skip +forward past a zero-width hit. + +### Q#QR3 — Two entry strings via chained `minibuffer.read` + +The Lua command collects the from-string, then the to-string in its +`on_accept`, then calls `ed.query_replace_start(from, to, regex)` which +begins the core session. Both prompts ride `minibuffer.read` +(dual-frontend since v12); the minibuffer is closed by the time the +second `on_accept` starts the session, so the handoff into the +query-replace shadow is clean. Emacs's "Query replace: X Query replace +X with: Y" flow, faithfully. + +**Separate history buckets (P3):** `history = "query-replace-from"` and +`history = "query-replace-to"` — one shared bucket would mix search +patterns and replacement text in both dropdowns. + +**Empty-string rules (P3):** an empty *from* string is rejected (the +from-prompt's `on_accept` returns early with a status, like other +minibuffer flows) — there's nothing to search for. An empty *to* string +is **valid** and means deletion (replace each match with nothing); the +to-prompt must not copy the reject-empty pattern. + +### Q#QR4 — Per-match prompt via `core.status` (StatusFacts.message) + +No new wire message. Each prompt sets +`core.status = "Query replacing FROM with TO (SPC/y, n, !, ., q)"` — +shown in both frontends via the v15 band. This is exactly the v15 +rider's purpose (transient echo-area content), and it's the Emacs +behavior (query-replace prompts live in the echo area = the status +line). A running count (`… — 3 replaced`) can ride the same string. + +### Q#QR5 — Current-match highlight reuses `SearchMatchActive` + +Write just the current match into `search_store` +(`set(buffer, from, [current])`, active = 0); the producer renders it +as `SearchMatchActive` (amber) in both frontends. Clear the store on +finish (isearch's cancel discipline). Store contention with a lingering +isearch is moot: shadows are modal and mutually exclusive, and the +first write overwrites whatever isearch left. v1 highlights only the +current match (Emacs's default prompt highlight); lazy-highlighting all +remaining matches is deferred. + +### Q#QR6 — Key vocabulary (v1) + +`y` / `SPC` replace-and-advance; `n` / `DEL` skip-and-advance; `!` +replace this and all remaining without prompting; `.` replace this then +quit; `q` / `RET` / `Esc` / `C-g` quit. Unrecognized keys are eaten +(the isearch precedent). Deferred: `,` (replace-but-stay), `^` (back +up), `?` (help). Finish/quit semantics are Q#QR10. + +### Q#QR7 — Undo granularity + +Each replacement is one `EditOp::Replace` = one undo step, so an +N-match query-replace is N undo steps. Simple and correct; a single +undo-group for the whole run is deferred (it's the same +`begin/end_undo_group` mechanism the CUA type-over framing Q#U1 parked +— induct it when a second caller wants it). + +### Q#QR8 — Scope + +Forward, from point to buffer end (Emacs's default). Matches before the +cursor are not touched. Cursor placement is Q#QR2 (per-step) and Q#QR10 +(on finish). Whole-buffer and backward query-replace are deferred. + +### Q#QR9 — Regex replacement is literal in v1 + +`C-M-%` (`query-replace-regexp`) matches via `find_all_regex`/the regex +`find_first_from`, but the replacement string is inserted literally — +no `\1` capture-group references. Capture-group substitution is +deferred (it needs the regex engine's capture API threaded through the +replace step). + +### Q#QR10 — Finish / quit semantics (NOT isearch's) + +The load-bearing difference from isearch: query-replace has usually +**already mutated the buffer** by the time it ends, so "cancel" cannot +mean "restore." Precisely: + +- **Quit** (`q`/`RET`/`Esc`/`C-g`, or `.` after its replace, or running + out of matches): does **not** roll back any replacement already made; + clears the highlight (`search_store.clear`); leaves point at the + current/last-inspected match (Q#QR2's cursor rule already put it + there); sets a status count (`"Replaced N occurrence(s)"`). `C-g` + behaves the same as `q` — Emacs's query-replace `C-g` exits and keeps + the replacements; it is *not* an undo. +- **Nothing matched** (the session never found a first match): a + distinct case — restore the origin cursor (isearch's cancel + discipline, since nothing was touched) and status `"No matches for + 'FROM'"`. This is the only path that restores point. + +So the session records `origin` (for the nothing-matched restore only) +and whether any replacement happened; every other exit leaves point at +the inspected match. + +## Phasing + +One implementation pass (the feature is small and cohesive), validated +in both frontends: + +1. **Core + Lua + literal & regex.** `QueryReplaceSession` + methods + + the shadow (with the Q#QR1 after-edit hook) + `find_first_from` + (literal & regex, cached engine) + the status prompt + the + highlight-and-reveal; Lua `query-replace` / `query-replace-regexp` + commands (chained `minibuffer.read`, Q#QR3 empty-string + bucket + rules) bound `M-%` / `C-M-%`. Acceptance tests through `dispatch_key` + (hermetic, like `completion_popup_acceptance`): replace/skip/`!`/`.`, + the three quit paths + nothing-matched-restores-origin (Q#QR10), + empty-to deletion, offset-shift correctness (`a`→`aa` doesn't loop), + regex incl. invalid-at-start and zero-width, buffer.after-edit fires + (an LSP/syntax observer sees the replaced text), and the + `dispatch_idle`-false gate. **Explicit binding tests for both + `M-%` and `C-M-%`** — control-meta-shifted punctuation is exactly + the chord that can parse differently across the TUI and GPU key + paths (the `C-c H` lesson from Arc 1b), so assert `from_chord` + resolves each and that it fires through `dispatch_key`. GPU + validation scores bets #1/#3. + +If the interactive-phase key handling or the offset bookkeeping proves +fiddlier than expected, phase 2 splits out regex + `!`/`.`; but the +plan is a single PR. + +## Categorical bets (score at close) + +1. **Zero protocol change holds.** Status band + `SearchMatchActive` + + `dispatch_idle` gate carry the whole feature to the GPU with no v16 + and no GPU code. (The panels arc's bet-#3 lesson makes me watch the + GPU path specifically — "the mechanism exists" bit us there.) +2. **Search-forward-after-replace is correct where precompute-all + loops.** `a`→`aa`, `foo`→`foobar`, and a replacement that would form + a new downstream match all terminate correctly because each search + starts past the replacement on the live buffer. +3. **The 5th shadow drops in cleanly** — `dispatch_idle`, modal-close + guard, and GPU round-trip all generalize like the completion popup + did (Arc 1a). +4. **A store-contention or status-clear edge** — some interaction where + a lingering isearch highlight, or `dispatch_key`'s entry + status-clear, briefly shows the wrong band/highlight during the + interactive phase. + +## Deferred (named, not silently dropped) + +- Capture-group references (`\1`) in regex replacements (Q#QR9). +- `,` (replace-but-stay), `^` (back up a match), `?` (help) keys. +- Backward and whole-buffer query-replace (Q#QR8). +- Single undo-group for a whole run (Q#QR7; shares CUA Q#U1's + `begin/end_undo_group`). +- Smart default from-string (word/region at point) + last-used default. +- Lazy-highlight of all remaining matches during the prompt (v1 shows + only the current match). +- `replace-string` / `replace-regexp` (non-interactive replace-all) — + trivial once the replace core exists; a thin non-prompting entry. From 7e7b3f2dcde0cd7b755dd4484bba5b2b338ce034 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 8 Jul 2026 16:05:23 -0400 Subject: [PATCH 2/3] =?UTF-8?q?feat(edit):=20query-replace=20(M-%=20/=20C-?= =?UTF-8?q?M-%)=20=E2=80=94=20Arc=202?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Emacs query-replace, built on isearch with zero protocol change (framing: docs/query-replace-framing.md). - search.rs: find_first_from (literal) + find_first_regex_from (cached engine, zero-width-skip) + compile_search_regex (shared smart-case compile). Q#QR2's forward-scan-past-replacement primitive. - QueryReplaceSession + core methods (editor_core.rs): begin (invalid regex refuses, Q#QR2), replace/skip/all/replace-and-quit/finish; matches run forward from next_from on the LIVE buffer, so offset shifts and never-re-matching-replacements (a->aa) fall out for free; current match highlighted via a single-element search_store set (SearchMatchActive, both frontends free) + cursor reveal; quit keeps replacements, only nothing-matched restores origin (Q#QR10). - Dispatcher shadow (editor.rs): QueryReplaceKey (y/SPC, n/DEL, !, ., q/RET/Esc/C-g) + dispatch_query_replace_key, the 5th modal shadow; added to dispatch_idle disjunction (GPU round-trips keys) and fires buffer.after-edit itself (Q#QR1 — a shadow returns before the normal post-command check; once per !-batch). - Lua: ed.query_replace_start/query_replace_active; query-replace / query-replace-regexp commands (chained minibuffer.read, separate from/to history buckets, empty-from reject / empty-to deletion); M-% / C-M-% bindings. - Per-match prompt via core.status → v15 StatusFacts.message band. Tests: 7 core unit + 11 dispatch_key acceptance (replace/skip/!/./quit, nothing-matched restore, empty-to deletion, a->aa non-loop, regex incl invalid, after-edit fires, dispatch_idle gate, explicit M-% AND C-M-% binding tests) + 5 search unit. Co-Authored-By: Claude Opus 4.8 (1M context) --- builtin/commands/default.lua | 34 ++++ builtin/keymaps/default.lua | 4 + src/editor.rs | 80 ++++++++ src/editor_core.rs | 322 ++++++++++++++++++++++++++++++ src/lua_bindings/mod.rs | 23 +++ src/search.rs | 138 ++++++++++++- tests/query_replace_acceptance.rs | 282 ++++++++++++++++++++++++++ 7 files changed, 876 insertions(+), 7 deletions(-) create mode 100644 tests/query_replace_acceptance.rs diff --git a/builtin/commands/default.lua b/builtin/commands/default.lua index 5f3678c..96049fe 100644 --- a/builtin/commands/default.lua +++ b/builtin/commands/default.lua @@ -140,6 +140,40 @@ cmd { name = "search.backward-regex", description = "Start an incremental regex search backward from the cursor.", fn = function() ed.search_start(false, true) end } +-- Query-replace (Arc 2). Two chained minibuffer prompts collect the +-- from/to strings (separate history buckets so search patterns and +-- replacement text don't mix), then ed.query_replace_start begins the +-- core interactive session (y/n/!/./q handled by a dispatcher shadow). +-- An empty FROM is rejected (nothing to search); an empty TO is valid +-- and means deletion (Q#QR3). +local function begin_query_replace(regex) + pmacs.minibuffer.read { + prompt = regex and "Query replace regexp: " or "Query replace: ", + history = "query-replace-from", + on_accept = function(from) + if from == nil or from == "" then + pmacs.editor.set_status("query-replace: empty search string") + return + end + pmacs.minibuffer.read { + prompt = string.format( + regex and "Query replace regexp %s with: " or "Query replace %s with: ", from), + history = "query-replace-to", + on_accept = function(to) + ed.query_replace_start(from, to or "", regex) + end, + } + end, + } +end + +cmd { name = "query-replace", + description = "Interactively replace a string from the cursor forward (M-%).", + fn = function() begin_query_replace(false) end } +cmd { name = "query-replace-regexp", + description = "Interactively replace a regexp from the cursor forward (C-M-%).", + fn = function() begin_query_replace(true) end } + -- History -------------------------------------------------------------------- cmd { name = "buffer.undo", description = "Undo the most recent edit.", diff --git a/builtin/keymaps/default.lua b/builtin/keymaps/default.lua index 0c9f612..e94c033 100644 --- a/builtin/keymaps/default.lua +++ b/builtin/keymaps/default.lua @@ -66,6 +66,10 @@ bind("C-r", "search.backward") bind("C-M-s", "search.forward-regex") bind("C-M-r", "search.backward-regex") +-- Query-replace (Arc 2): M-% literal, C-M-% regexp (Emacs bindings). +bind("M-%", "query-replace") +bind("C-M-%", "query-replace-regexp") + -- CUA-style word-level deletion (the same shortcuts users expect from -- IDEs, browsers, terminals on Linux/Windows). C-BS deletes back to -- the start of the previous word; C-DEL deletes forward through the diff --git a/src/editor.rs b/src/editor.rs index 5df243d..5d45d0a 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -524,6 +524,7 @@ impl EditorState { // optimistic local edit would do. !core.minibuffer.is_active() && !core.search_active() + && !core.query_replace_active() && !core.menu_is_open() && !core.active_buffer_round_trips() } @@ -578,6 +579,18 @@ impl EditorState { return; } + // Query-replace interception (Arc 2): the fifth modal shadow. + // While the interactive phase runs, every key drives it + // (y/n/!/./q), shadowing the global keymap like search. Both + // frontends reach this via the `FrontendEvent::Key` round-trip + // (`dispatch_idle` is false while it runs). The handler fires + // `buffer.after-edit` itself — a modal shadow returns before the + // normal post-command edit check below (Q#QR1). + if self.core.borrow().query_replace_active() { + self.dispatch_query_replace_key(chord); + return; + } + // Minibuffer interception: when a prompt is active, every key // routes through the minibuffer's hardcoded handler. The main // editor's keymap is bypassed; the user can still cancel with @@ -810,6 +823,32 @@ impl EditorState { } } + /// Drive an active query-replace from a keystroke (Arc 2, Q#QR6). + /// Fires `buffer.after-edit` itself when the key produced an edit + /// (Q#QR1): a modal shadow returns before `dispatch_key`'s normal + /// post-command edit check, so LSP `didChange` / syntax reparse + /// would otherwise never see the replaced text. `!` applies many + /// edits in one keypress; the single revision compare here fires + /// the hook once for the batch, which is what the debounced + /// `didChange` wants. + fn dispatch_query_replace_key(&mut self, chord: Chord) { + let pre_revision = self.active_buffer_revision(); + match QueryReplaceKey::from_chord(chord) { + QueryReplaceKey::Replace => self.core.borrow_mut().query_replace_replace(), + QueryReplaceKey::Skip => self.core.borrow_mut().query_replace_skip(), + QueryReplaceKey::All => self.core.borrow_mut().query_replace_all(), + QueryReplaceKey::ReplaceAndQuit => { + self.core.borrow_mut().query_replace_replace_and_quit(); + } + QueryReplaceKey::Quit => self.core.borrow_mut().query_replace_finish(), + QueryReplaceKey::Ignore => {} + } + if pre_revision != self.active_buffer_revision() { + self.lua_host + .run_hook("buffer.after-edit", mlua::MultiValue::new()); + } + } + /// Drive an open context menu from a keystroke (Q#CM1). fn dispatch_menu_key(&mut self, chord: Chord) { match MenuKey::from_chord(chord) { @@ -1629,6 +1668,47 @@ impl SearchKey { } } +/// Keys handled while a query-replace's interactive phase runs (Arc 2, +/// Q#QR6). A full modal shadow like [`SearchKey`]: an active +/// query-replace eats every key, and the same decode runs in both +/// frontends via the `FrontendEvent::Key` round-trip. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +enum QueryReplaceKey { + /// `y` / `SPC` — replace this match, advance. + Replace, + /// `n` / `DEL` — skip this match, advance. + Skip, + /// `!` — replace this and all remaining without prompting. + All, + /// `.` — replace this, then quit. + ReplaceAndQuit, + /// `q` / `RET` / `Esc` / `C-g` — quit (replacements are kept). + Quit, + /// Any other key — eaten (no-op), like an active isearch. + Ignore, +} + +impl QueryReplaceKey { + fn from_chord(chord: Chord) -> Self { + let ctrl = chord.modifiers.contains(KeyModifiers::CONTROL); + if ctrl { + // C-g quits; every other control chord is eaten. + return match chord.code { + KeyCode::Char('g') => Self::Quit, + _ => Self::Ignore, + }; + } + match chord.code { + KeyCode::Char('y' | ' ') => Self::Replace, + KeyCode::Char('n') | KeyCode::Backspace | KeyCode::Delete => Self::Skip, + KeyCode::Char('!') => Self::All, + KeyCode::Char('.') => Self::ReplaceAndQuit, + KeyCode::Char('q') | KeyCode::Enter | KeyCode::Esc => Self::Quit, + _ => Self::Ignore, + } + } +} + /// Keys handled while a context menu is open (Q#CM1). Like /// [`SearchKey`], this shadows the global keymap; the same decode runs /// in both frontends via the daemon's `FrontendEvent::Key` round-trip. diff --git a/src/editor_core.rs b/src/editor_core.rs index d1c4c59..35e163b 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -88,6 +88,41 @@ pub struct SearchSession { invalid: bool, } +/// Live state of an in-progress query-replace (Arc 2, Q#QR1). +/// +/// Present only while a query-replace's interactive phase is running +/// (`EditorCore::query_replace`); `None` otherwise. Unlike +/// [`SearchSession`], the buffer is usually already mutated by the +/// time this ends, so `origin` is used *only* for the nothing-matched +/// restore (Q#QR10); every other exit leaves point at the inspected +/// match. Matching runs forward from `next_from` on the *live* buffer +/// (Q#QR2), so offset shifts and never-re-matching-replacements fall +/// out for free. +#[derive(Clone, Debug)] +pub struct QueryReplaceSession { + /// The literal substring or regex source being replaced. + from: String, + /// The replacement text (may be empty — Q#QR3 deletion). + to: String, + /// Compiled regex engine when in regex mode (Q#QR9), cached for + /// the whole run so `!` stays linear; `None` = smart-case literal. + re: Option, + /// Buffer + cursor when the session began. Restored on cancel + /// *only* when nothing ever matched (Q#QR10). + origin: (BufferId, Position), + /// Byte offset the next forward search starts from — advanced past + /// each replacement so inserted text is never re-matched. + next_from: Position, + /// The match currently being prompted, or `None` before the first + /// advance / after finishing. + current: Option, + /// Number of replacements applied so far. + replaced: usize, + /// Whether any match was ever found (distinguishes "nothing + /// matched → restore origin" from "matched, then quit"). + found_any: bool, +} + /// The world state mutated by editor commands. pub struct EditorCore { /// Shared buffer registry. The registry is the canonical owner @@ -203,6 +238,10 @@ pub struct EditorCore { /// write would bypass the intercept chain entirely. Marked from /// Lua via `pmacs.buffer.set_round_trip_input`; pruned on kill. round_trip_buffers: std::collections::HashSet, + /// Live query-replace interactive session (Arc 2), or `None`. The + /// query-replace twin of `search`; drives the fifth dispatcher + /// shadow. + query_replace: Option, } impl EditorCore { @@ -244,6 +283,7 @@ impl EditorCore { menu: crate::menu::make_shared_menu(), completion_popup: crate::completion::make_shared_popup(), round_trip_buffers: std::collections::HashSet::new(), + query_replace: None, } } @@ -774,6 +814,206 @@ impl EditorCore { aw.goal_col = None; } + // ---- query-replace (Arc 2, Q#QR1-10) ----------------------------------- + + /// True while a query-replace interactive session is running (the + /// fifth dispatcher-shadow predicate; also drives `dispatch_idle` + /// and the modal-close guard). + #[must_use] + pub fn query_replace_active(&self) -> bool { + self.query_replace.is_some() + } + + /// Begin a query-replace from the cursor forward (Q#QR8). `regex` + /// selects `query-replace-regexp` (Q#QR9). An invalid regex refuses + /// to start (Q#QR2). Immediately advances to (and prompts on) the + /// first match, or finishes with "No matches" when there are none. + pub fn query_replace_begin(&mut self, from: String, to: String, regex: bool) { + if self.query_replace.is_some() || from.is_empty() { + return; + } + let re = if regex { + let Some(re) = crate::search::compile_search_regex(&from) else { + self.status = format!("Invalid regex: {from}"); + return; + }; + Some(re) + } else { + None + }; + let origin = (self.active_buffer_id(), self.cursor()); + // Reuse the isearch match-wash overlay to highlight the current + // match in the TUI; the GPU gets it via SearchMatch decorations. + self.ensure_search_overlay(); + self.query_replace = Some(QueryReplaceSession { + from, + to, + re, + origin, + next_from: origin.1, + current: None, + replaced: 0, + found_any: false, + }); + self.query_replace_advance(); + } + + /// Find the next match at/after `next_from` on the live buffer. On + /// a hit: highlight it, reveal it (cursor to its start, Q#QR2), and + /// prompt. On a miss: finish (natural end / nothing-matched). + fn query_replace_advance(&mut self) { + let Some(session) = self.query_replace.as_ref() else { + return; + }; + let bid = session.origin.0; + let start = session.next_from.min(self.active_buffer_len()) as usize; + let bytes = self.buffer_bytes(bid); + let found = match &session.re { + Some(re) => crate::search::find_first_regex_from(&bytes, re, start), + None => crate::search::find_first_from(&bytes, &session.from, start), + }; + let Some(range) = found else { + self.query_replace_finish(); + return; + }; + let from = session.from.clone(); + if let Some(session) = self.query_replace.as_mut() { + session.current = Some(range); + session.found_any = true; + } + // Highlight just this match: a single-element store set renders + // it as SearchMatchActive in both frontends (Q#QR5). + { + let mut guard = self + .search_store + .lock() + .expect("search store mutex poisoned"); + guard.set(bid, from, vec![range]); + } + self.search_place_cursor(range.start); + self.query_replace_set_prompt(); + } + + /// Set `core.status` to the per-match prompt (Q#QR4) — shown in + /// both frontends via the v15 `StatusFacts.message` band. + fn query_replace_set_prompt(&mut self) { + if let Some(session) = self.query_replace.as_ref() { + self.status = format!( + "Query replacing '{}' with '{}' (y/n, ! all, . last, q quit)", + session.from, session.to + ); + } + } + + /// Replace the current match with the to-string as a single edit + /// (Q#QR7), advancing `next_from` past the inserted text so it is + /// never re-matched (Q#QR2). Returns `true` when an edit was + /// applied. Does NOT advance to the next match — callers chain + /// `query_replace_advance` (or finish) as their flow needs. + fn query_replace_apply_current(&mut self) -> bool { + let Some(session) = self.query_replace.as_ref() else { + return false; + }; + let Some(range) = session.current else { + return false; + }; + let to = session.to.clone(); + if let Err(e) = self.apply_active_edit(EditOp::Replace { + range: Range { + start: range.start, + end: range.end, + }, + bytes: to.as_bytes(), + }) { + self.status = format!("query-replace: {e}"); + return false; + } + let new_next = range.start + to.len() as u64; + if let Some(session) = self.query_replace.as_mut() { + session.next_from = new_next; + session.current = None; + session.replaced += 1; + } + self.search_place_cursor(new_next); + true + } + + /// `y` / `SPC` — replace the current match, then advance to the next. + pub fn query_replace_replace(&mut self) { + if self.query_replace_apply_current() { + self.query_replace_advance(); + } + } + + /// `n` / `DEL` — leave the current match, advance past it to the next. + pub fn query_replace_skip(&mut self) { + if let Some(session) = self.query_replace.as_mut() + && let Some(range) = session.current + { + session.next_from = range.end; + session.current = None; + } + self.query_replace_advance(); + } + + /// `!` — replace the current match and all remaining without + /// prompting, then finish (Q#QR6). One `after-edit` hook fires for + /// the batch (the dispatcher compares revision across the handler). + pub fn query_replace_all(&mut self) { + while self.query_replace_apply_current() { + // Find the next match (mirrors advance's search, without the + // highlight/prompt work — we're not stopping to ask). + let Some(session) = self.query_replace.as_ref() else { + break; + }; + let bid = session.origin.0; + let start = session.next_from.min(self.active_buffer_len()) as usize; + let bytes = self.buffer_bytes(bid); + let found = match &session.re { + Some(re) => crate::search::find_first_regex_from(&bytes, re, start), + None => crate::search::find_first_from(&bytes, &session.from, start), + }; + match found { + Some(range) => { + if let Some(session) = self.query_replace.as_mut() { + session.current = Some(range); + } + } + None => break, + } + } + self.query_replace_finish(); + } + + /// `.` — replace the current match, then finish (Q#QR6). + pub fn query_replace_replace_and_quit(&mut self) { + self.query_replace_apply_current(); + self.query_replace_finish(); + } + + /// End the session (Q#QR10): clear the highlight, restore the origin + /// cursor *only* if nothing ever matched, and set the count status. + /// Every other exit leaves point where the last step put it. + pub fn query_replace_finish(&mut self) { + let Some(session) = self.query_replace.take() else { + return; + }; + let bid = session.origin.0; + self.search_store + .lock() + .expect("search store mutex poisoned") + .clear(bid); + if session.found_any { + let n = session.replaced; + self.status = format!("Replaced {n} occurrence{}", if n == 1 { "" } else { "s" }); + } else { + if self.active_buffer_id() == bid { + self.search_place_cursor(session.origin.1); + } + self.status = format!("No matches for '{}'", session.from); + } + } + /// Snapshot a buffer's full byte content (empty if the id is /// stale). O(1) rope snapshot + one copy; used to feed `find_all`. fn buffer_bytes(&self, buffer_id: BufferId) -> Vec { @@ -3441,4 +3681,86 @@ mod tests { "focus change closes the session even with the same buffer" ); } + + // ---- query-replace core (Arc 2) ---------------------------------------- + + #[test] + fn query_replace_all_replaces_and_counts() { + let mut s = from_bytes(b"foo foo foo\n"); + s.query_replace_begin("foo".into(), "bar".into(), false); + assert!(s.query_replace_active(), "session opens on the first match"); + s.query_replace_all(); + assert_eq!(text_of(&s), "bar bar bar\n"); + assert!(!s.query_replace_active(), "! finishes the session"); + assert_eq!(s.status, "Replaced 3 occurrences"); + } + + #[test] + fn query_replace_growing_replacement_does_not_loop() { + // The a→aa shape: replacing must not re-match the inserted text. + let mut s = from_bytes(b"a a a\n"); + s.query_replace_begin("a".into(), "aa".into(), false); + s.query_replace_all(); + assert_eq!(text_of(&s), "aa aa aa\n", "each 'a' replaced exactly once"); + assert_eq!(s.status, "Replaced 3 occurrences"); + } + + #[test] + fn query_replace_empty_to_deletes() { + let mut s = from_bytes(b"a-b-c\n"); + s.query_replace_begin("-".into(), String::new(), false); + s.query_replace_all(); + assert_eq!(text_of(&s), "abc\n", "empty replacement deletes matches"); + } + + #[test] + fn query_replace_skip_then_replace_is_selective() { + let mut s = from_bytes(b"x x x\n"); + s.query_replace_begin("x".into(), "y".into(), false); + s.query_replace_skip(); // leave the first x + s.query_replace_replace(); // replace the second x, advance to third + s.query_replace_replace_and_quit(); // replace the third, quit + assert_eq!(text_of(&s), "x y y\n", "first skipped, rest replaced"); + assert!(!s.query_replace_active()); + } + + #[test] + fn query_replace_nothing_matched_restores_origin() { + let mut s = from_bytes(b"hello world\n"); + s.active_window_mut().cursor = 6; // on "world" + s.query_replace_begin("zzz".into(), "q".into(), false); + assert!( + !s.query_replace_active(), + "no match → session never stays open" + ); + assert_eq!(text_of(&s), "hello world\n", "buffer untouched"); + assert_eq!(s.active_window().cursor, 6, "origin cursor restored"); + assert_eq!(s.status, "No matches for 'zzz'"); + } + + #[test] + fn query_replace_starts_from_cursor_forward() { + let mut s = from_bytes(b"k _ k\n"); + s.active_window_mut().cursor = 2; // between the two k's + s.query_replace_begin("k".into(), "K".into(), false); + s.query_replace_all(); + assert_eq!(text_of(&s), "k _ K\n", "only the match at/after point"); + } + + #[test] + fn query_replace_regex_replaces_and_invalid_refuses() { + let mut s = from_bytes(b"a1 b2 c3\n"); + s.query_replace_begin("[0-9]".into(), "#".into(), true); + s.query_replace_all(); + assert_eq!(text_of(&s), "a# b# c#\n", "regex matches digits"); + + // Invalid regex refuses to start and leaves a status. + let mut s2 = from_bytes(b"abc\n"); + s2.query_replace_begin("(unclosed".into(), "x".into(), true); + assert!( + !s2.query_replace_active(), + "invalid regex never opens a session" + ); + assert!(s2.status.starts_with("Invalid regex")); + } } diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 1f9f4a0..b6c97af 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -10601,6 +10601,29 @@ fn install_search(editor: &Table, lua: &Lua, core: &SharedCore) -> mlua::Result< lua.create_function(move |_, ()| Ok(cc.borrow().search_active()))?, )?; } + { + // query_replace_start(from, to, regex): begin an interactive + // query-replace from the cursor forward (Arc 2). The Lua + // `query-replace` command collects `from`/`to` via chained + // minibuffer prompts, then calls this; the interactive y/n/!/./q + // phase is a core dispatcher shadow from here on. + let cc = core.clone(); + editor.set( + "query_replace_start", + lua.create_function(move |_, (from, to, regex): (String, String, bool)| { + cc.borrow_mut().query_replace_begin(from, to, regex); + Ok(()) + })?, + )?; + } + { + // query_replace_active(): true during the interactive phase. + let cc = core.clone(); + editor.set( + "query_replace_active", + lua.create_function(move |_, ()| Ok(cc.borrow().query_replace_active()))?, + )?; + } Ok(()) } diff --git a/src/search.rs b/src/search.rs index 1e486ec..bda13db 100644 --- a/src/search.rs +++ b/src/search.rs @@ -255,13 +255,7 @@ pub fn find_all_regex(haystack: &[u8], pattern: &str) -> Option> if pattern.is_empty() { return Some(Vec::new()); } - let case_insensitive = !pattern.chars().any(char::is_uppercase); - let re = if case_insensitive { - regex::bytes::Regex::new(&format!("(?i){pattern}")) - } else { - regex::bytes::Regex::new(pattern) - } - .ok()?; + let re = compile_search_regex(pattern)?; let matches = re .find_iter(haystack) .filter(|m| m.end() > m.start()) @@ -273,6 +267,80 @@ pub fn find_all_regex(haystack: &[u8], pattern: &str) -> Option> Some(matches) } +/// Compile `pattern` with the same smart-case rule the search paths use +/// (case-insensitive unless the pattern has an uppercase letter, via a +/// `(?i)` prefix), or `None` if it fails to compile. Shared by +/// [`find_all_regex`] and the query-replace session (which caches the +/// compiled engine for the whole run — Q#QR2). +#[must_use] +pub fn compile_search_regex(pattern: &str) -> Option { + let case_insensitive = !pattern.chars().any(char::is_uppercase); + if case_insensitive { + regex::bytes::Regex::new(&format!("(?i){pattern}")) + } else { + regex::bytes::Regex::new(pattern) + } + .ok() +} + +/// The first smart-case literal match of `query` in `haystack` at or +/// after byte `start` (Q#QR2: query-replace's forward step). Same +/// case-folding as [`find_all`]. An empty query, or `start` past the +/// last possible match, yields `None`. +#[must_use] +pub fn find_first_from(haystack: &[u8], query: &str, start: usize) -> Option { + let q = query.as_bytes(); + if q.is_empty() || start > haystack.len() || haystack.len() - start < q.len() { + return None; + } + let case_sensitive = query.chars().any(char::is_uppercase); + let mut i = start; + while i + q.len() <= haystack.len() { + let hit = haystack[i..i + q.len()].iter().zip(q).all(|(&h, &n)| { + if case_sensitive { + h == n + } else { + h.eq_ignore_ascii_case(&n) + } + }); + if hit { + return Some(ByteRange { + start: i as u64, + end: (i + q.len()) as u64, + }); + } + i += 1; + } + None +} + +/// The first non-zero-width match of the pre-compiled `re` in +/// `haystack` at or after byte `start` (Q#QR2). Uses `find_at` so the +/// engine keeps look-around context (`\b`, `^`) correct at the seam, +/// and skips zero-width matches (`a*`, anchors) by advancing one byte — +/// a zero-width hit never moves `next_from`, so it would otherwise +/// loop. +#[must_use] +pub fn find_first_regex_from( + haystack: &[u8], + re: ®ex::bytes::Regex, + start: usize, +) -> Option { + let mut pos = start; + while pos <= haystack.len() { + let m = re.find_at(haystack, pos)?; + if m.end() > m.start() { + return Some(ByteRange { + start: m.start() as u64, + end: m.end() as u64, + }); + } + // Zero-width match: step past it to guarantee progress. + pos = m.start() + 1; + } + None +} + // --------------------------------------------------------------------------- // TUI view // --------------------------------------------------------------------------- @@ -519,6 +587,62 @@ mod tests { assert_eq!(find_all_regex(b"abc", ""), Some(vec![])); } + // ---- find_first_from (query-replace forward step, Q#QR2) --------------- + + #[test] + fn find_first_from_scans_forward() { + assert_eq!(find_first_from(b"a.a.a", "a", 0), Some(r(0, 1))); + // Start past the first hit → the next one. + assert_eq!(find_first_from(b"a.a.a", "a", 1), Some(r(2, 3))); + assert_eq!(find_first_from(b"a.a.a", "a", 3), Some(r(4, 5))); + // No match at/after start. + assert_eq!(find_first_from(b"a.a.a", "a", 5), None); + assert_eq!(find_first_from(b"abc", "z", 0), None); + // Empty query never matches. + assert_eq!(find_first_from(b"abc", "", 0), None); + } + + #[test] + fn find_first_from_is_smart_case() { + // Lowercase query folds case; uppercase query is exact. + assert_eq!(find_first_from(b"xFoo", "foo", 0), Some(r(1, 4))); + assert_eq!(find_first_from(b"xFoo", "Foo", 0), Some(r(1, 4))); + assert_eq!(find_first_from(b"xfoo", "Foo", 0), None); + } + + #[test] + fn find_first_from_does_not_reloop_on_growing_replacement() { + // The a→aa shape: after replacing the 'a' at 0 with "aa", the + // next search must start PAST the replacement (byte 2), not + // re-match the inserted text. Simulated here by starting the + // scan at the replacement end. + assert_eq!(find_first_from(b"aa_a", "a", 2), Some(r(3, 4))); + } + + #[test] + fn find_first_regex_from_scans_and_skips_zero_width() { + let re = compile_search_regex("a+").unwrap(); + assert_eq!(find_first_regex_from(b"_aa_a", &re, 0), Some(r(1, 3))); + assert_eq!(find_first_regex_from(b"_aa_a", &re, 3), Some(r(4, 5))); + assert_eq!(find_first_regex_from(b"_aa_a", &re, 5), None); + // Zero-width pattern `x*` never yields a match (all filtered), + // and crucially terminates rather than looping. + let z = compile_search_regex("x*").unwrap(); + assert_eq!(find_first_regex_from(b"abc", &z, 0), None); + } + + #[test] + fn compile_search_regex_smart_case_and_invalid() { + // Lowercase → case-insensitive. + let re = compile_search_regex("foo").unwrap(); + assert!(re.is_match(b"FOO")); + // Uppercase → case-sensitive. + let re = compile_search_regex("Foo").unwrap(); + assert!(!re.is_match(b"foo")); + // Invalid pattern → None (the session refuses to start). + assert!(compile_search_regex("(unclosed").is_none()); + } + #[test] fn store_set_clamps_active_and_clears_on_empty() { let mut s = SearchStore::new(); diff --git a/tests/query_replace_acceptance.rs b/tests/query_replace_acceptance.rs new file mode 100644 index 0000000..e9e0660 --- /dev/null +++ b/tests/query_replace_acceptance.rs @@ -0,0 +1,282 @@ +//! Query-replace acceptance (Arc 2) — the interactive phase end-to-end +//! through `dispatch_key`, exactly as a user (or a round-tripping GPU) +//! drives it: the `M-%` / `C-M-%` bindings, the y/n/!/./q vocabulary, +//! the three quit paths + nothing-matched-restores-origin, empty-to +//! deletion, offset-shift correctness, regex, the `buffer.after-edit` +//! hook firing on replaced text, and the `dispatch_idle`-false gate. +//! +//! Framing: docs/query-replace-framing.md. + +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::empty(), + } +} + +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() { + press(s, KeyCode::Char(ch)); + } +} + +/// Move the active cursor to buffer start (no default binding for it; +/// walk up then to line start, like lsp.lua's cursor-move). +fn goto_start(s: &EditorState) { + s.lua_host + .lua() + .load( + r" + while pmacs.editor.cursor_line() > 0 do pmacs.editor.move_up() end + pmacs.editor.move_line_start() + ", + ) + .exec() + .expect("move cursor to buffer start"); +} + +/// `(buffer text, active?, query-replace active?)` through the Lua +/// surface. +fn probe(s: &EditorState) -> (String, bool) { + s.lua_host + .lua() + .load( + r" + local b = pmacs.window.buffer() + return b:slice(0, b:len()), pmacs.editor.query_replace_active() + ", + ) + .eval() + .expect("probe query-replace state") +} + +/// Drive the two minibuffer prompts a `query-replace` command opens: +/// type `from`, RET, type `to`, RET. Leaves the session in its +/// interactive phase (or finished, if `!`/no-match). +fn start_query_replace(s: &mut EditorState, from: &str, to: &str, regex: bool) { + let cmd = if regex { + "query-replace-regexp" + } else { + "query-replace" + }; + s.lua_host + .lua() + .load(format!("pmacs.command.invoke('{cmd}')")) + .exec() + .expect("invoke query-replace command"); + type_str(s, from); + press(s, KeyCode::Enter); + type_str(s, to); + press(s, KeyCode::Enter); +} + +#[test] +fn replace_skip_and_quit_is_selective() { + let mut s = EditorState::new(); + type_str(&mut s, "x x x x"); + // Cursor to buffer start so all four are ahead of point. + goto_start(&s); + start_query_replace(&mut s, "x", "y", false); + let (_, active) = probe(&s); + assert!( + active, + "session is in its interactive phase on the first match" + ); + + press(&mut s, KeyCode::Char('y')); // replace 1st + press(&mut s, KeyCode::Char('n')); // skip 2nd + press(&mut s, KeyCode::Char('y')); // replace 3rd + press(&mut s, KeyCode::Char('q')); // quit before the 4th + let (text, active) = probe(&s); + assert_eq!(text, "y x y x", "y/n/y then quit"); + assert!(!active, "q ends the session"); +} + +#[test] +fn bang_replaces_all_remaining() { + let mut s = EditorState::new(); + type_str(&mut s, "a a a a"); + goto_start(&s); + start_query_replace(&mut s, "a", "b", false); + press(&mut s, KeyCode::Char('!')); + let (text, active) = probe(&s); + assert_eq!(text, "b b b b"); + assert!(!active, "! finishes the session"); +} + +#[test] +fn dot_replaces_current_then_quits() { + let mut s = EditorState::new(); + type_str(&mut s, "a a a"); + goto_start(&s); + start_query_replace(&mut s, "a", "z", false); + press(&mut s, KeyCode::Char('.')); // replace first, then quit + let (text, active) = probe(&s); + assert_eq!(text, "z a a", "only the first is replaced"); + assert!(!active); +} + +#[test] +fn growing_replacement_does_not_loop() { + // a → aa must not re-match the inserted text (offset-shift + the + // search-forward-past-replacement rule). + let mut s = EditorState::new(); + type_str(&mut s, "a a a"); + goto_start(&s); + start_query_replace(&mut s, "a", "aa", false); + press(&mut s, KeyCode::Char('!')); + let (text, _) = probe(&s); + assert_eq!(text, "aa aa aa", "each 'a' replaced exactly once"); +} + +#[test] +fn empty_to_deletes() { + let mut s = EditorState::new(); + type_str(&mut s, "a-b-c"); + goto_start(&s); + start_query_replace(&mut s, "-", "", false); + press(&mut s, KeyCode::Char('!')); + let (text, _) = probe(&s); + assert_eq!(text, "abc", "empty replacement deletes matches"); +} + +#[test] +fn regex_query_replace_via_binding() { + let mut s = EditorState::new(); + type_str(&mut s, "a1 b2 c3"); + goto_start(&s); + start_query_replace(&mut s, "[0-9]", "#", true); + press(&mut s, KeyCode::Char('!')); + let (text, _) = probe(&s); + assert_eq!(text, "a# b# c#"); +} + +#[test] +fn m_percent_binding_starts_query_replace() { + // The literal chord: M-% (Alt + Shift+5 → Char('%') with ALT). + let mut s = EditorState::new(); + type_str(&mut s, "cat cat"); + goto_start(&s); + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('%'), KeyModifiers::ALT), + ); + // The from-prompt minibuffer should now be active. + let mb: bool = s + .lua_host + .lua() + .load("return pmacs.minibuffer.is_active and pmacs.minibuffer.is_active() or false") + .eval() + .unwrap_or(false); + assert!(mb, "M-% opened the query-replace from-prompt"); + // Complete the flow and confirm it replaces. + type_str(&mut s, "cat"); + press(&mut s, KeyCode::Enter); + type_str(&mut s, "dog"); + press(&mut s, KeyCode::Enter); + press(&mut s, KeyCode::Char('!')); + let (text, _) = probe(&s); + assert_eq!(text, "dog dog", "M-% drove a full query-replace"); +} + +#[test] +fn c_m_percent_binding_starts_regexp_query_replace() { + // Control-meta-shifted punctuation — the chord most likely to parse + // differently across key paths (the C-c H lesson). + let mut s = EditorState::new(); + type_str(&mut s, "x1 x2"); + goto_start(&s); + s.dispatch_key( + FrontendId::LOCAL, + key( + KeyCode::Char('%'), + KeyModifiers::CONTROL | KeyModifiers::ALT, + ), + ); + let mb: bool = s + .lua_host + .lua() + .load("return pmacs.minibuffer.is_active and pmacs.minibuffer.is_active() or false") + .eval() + .unwrap_or(false); + assert!(mb, "C-M-% opened the query-replace-regexp from-prompt"); + type_str(&mut s, "x[0-9]"); + press(&mut s, KeyCode::Enter); + type_str(&mut s, "Q"); + press(&mut s, KeyCode::Enter); + press(&mut s, KeyCode::Char('!')); + let (text, _) = probe(&s); + assert_eq!(text, "Q Q", "C-M-% drove a regexp query-replace"); +} + +#[test] +fn nothing_matched_leaves_buffer_untouched() { + let mut s = EditorState::new(); + type_str(&mut s, "hello"); + start_query_replace(&mut s, "zzz", "q", false); + let (text, active) = probe(&s); + assert_eq!(text, "hello", "no match → buffer untouched"); + assert!(!active, "no match → session never stays open"); +} + +#[test] +fn query_replace_flips_dispatch_idle_so_gpu_round_trips() { + // While the interactive phase runs, dispatch_idle must be false so a + // semantic frontend round-trips y/n/etc. instead of self-inserting. + let mut s = EditorState::new(); + type_str(&mut s, "a a"); + goto_start(&s); + start_query_replace(&mut s, "a", "b", false); + assert!( + !s.dispatch_idle(), + "query-replace interactive phase forces key round-trip" + ); + press(&mut s, KeyCode::Char('!')); + assert!(s.dispatch_idle(), "idle again after the session finishes"); +} + +#[test] +fn replace_fires_after_edit_hook() { + // The Q#QR1 hook: an LSP/syntax observer must see replaced text. + let mut s = EditorState::new(); + s.lua_host + .lua() + .load( + r" + _G.EDITS = 0 + pmacs.hook.add('buffer.after-edit', function() _G.EDITS = _G.EDITS + 1 end) + ", + ) + .exec() + .expect("install after-edit counter"); + type_str(&mut s, "a a a"); + goto_start(&s); + let before: i64 = s + .lua_host + .lua() + .load("return _G.EDITS") + .eval() + .expect("read counter"); + start_query_replace(&mut s, "a", "b", false); + press(&mut s, KeyCode::Char('y')); // one replacement + let after: i64 = s + .lua_host + .lua() + .load("return _G.EDITS") + .eval() + .expect("read counter"); + assert!( + after > before, + "buffer.after-edit fired for the replacement (before {before}, after {after})" + ); +} From a118f0a3a2c43f0546401f7c2e4b3dcf7f0ac9b1 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 8 Jul 2026 17:07:08 -0400 Subject: [PATCH 3/3] fix(query-replace): pin the session to its origin buffer (wrong-buffer guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review High (merge blocker): query-replace searched the origin buffer's bytes but applied edits and moved the cursor through apply_active_edit / search_place_cursor, which target whatever is ACTIVE. Focus can drift mid-session — a click into another split, a key from another frontend, both changing the active buffer outside the shadow — so a match found in the origin buffer could be applied to an unrelated one. Buffer corruption. Fix: query_replace_on_origin() verifies the active buffer still equals the session's origin buffer before every edit; on mismatch it ABORTS without editing (clears the highlight, drops the session, status 'query-replace aborted: active buffer changed'), so an origin match can never land in a foreign buffer. Guards replace/skip/all/replace-and-quit. The dispatcher's after-edit revision compare now targets the ORIGIN buffer (query_replace_origin_buffer + buffer_revision) not the active one, so a drift-abort — which edits nothing — never spuriously fires the hook. The forward-search clamp uses the origin bytes' length, not active_buffer_len. Also (review Low/med): query_replace_active() added to the completion-popup modal-close guard, so a popup opened via the direct Lua start (ed.query_replace_start) can't linger rendered-but-unreachable while QR swallows keys. Tests: core drift-abort (both buffers untouched) + end-to-end focus-drift regression; ! fires after-edit exactly once for the batch; RET/Esc/C-g quit paths (keep replacements); DEL skips. Acceptance header corrected to match actual coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/query-replace-framing.md | 15 +++- src/editor.rs | 23 ++++- src/editor_core.rs | 89 +++++++++++++++++- tests/query_replace_acceptance.rs | 144 +++++++++++++++++++++++++++++- 4 files changed, 260 insertions(+), 11 deletions(-) diff --git a/docs/query-replace-framing.md b/docs/query-replace-framing.md index e2a454e..b05f419 100644 --- a/docs/query-replace-framing.md +++ b/docs/query-replace-framing.md @@ -60,7 +60,20 @@ interactive y/n phase). Mirror the structure instead: `editor.rs` — the fifth member of the shadow family (minibuffer, search, menu, completion, **query-replace**). Add `query_replace_active()` to the `dispatch_idle()` disjunction and the -modal-close guard, exactly as the others. +completion-popup modal-close guard, exactly as the others. + +**Pin to the origin buffer (as-built fix).** The session records its +origin buffer, but every edit and cursor move goes through the +*active* window/buffer — and focus can drift mid-session (a click into +another split, a key from another frontend, both of which change the +active buffer outside the shadow). Applying an origin-buffer match to +whatever became active is buffer corruption. Guard it: +`query_replace_on_origin()` checks the active buffer still equals the +origin buffer before every edit and **aborts the session without +editing** on mismatch (never corrupt an unrelated buffer). The +`buffer.after-edit` revision compare (above) targets the *origin* +buffer specifically, not the active one, so a focus-drift abort — which +edits nothing — never spuriously fires the hook. **`buffer.after-edit` must fire from inside the shadow (P1).** A modal shadow `return`s before `dispatch_key`'s normal post-command edit diff --git a/src/editor.rs b/src/editor.rs index 5d45d0a..7656365 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -553,7 +553,10 @@ impl EditorState { { let mut core = self.core.borrow_mut(); if core.completion_popup_is_open() - && (core.menu_is_open() || core.search_active() || core.minibuffer.is_active()) + && (core.menu_is_open() + || core.search_active() + || core.query_replace_active() + || core.minibuffer.is_active()) { core.completion_popup_close(); } @@ -692,6 +695,13 @@ impl EditorState { /// mid-dispatch). fn active_buffer_revision(&self) -> Option { let id = self.core.borrow().active_buffer_id(); + self.buffer_revision(id) + } + + /// 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. + fn buffer_revision(&self, id: crate::buffer::BufferId) -> Option { let reg = self.lua_host.registry().borrow(); reg.get(id).ok().map(crate::buffer::Buffer::revision) } @@ -832,7 +842,11 @@ impl EditorState { /// the hook once for the batch, which is what the debounced /// `didChange` wants. fn dispatch_query_replace_key(&mut self, chord: Chord) { - let pre_revision = self.active_buffer_revision(); + // Compare the *origin* buffer's revision (the one query-replace + // edits), not the active buffer's — they can differ if focus + // drifted, and the wrong-buffer guard may abort without editing. + let origin_buf = self.core.borrow().query_replace_origin_buffer(); + let pre = origin_buf.and_then(|id| self.buffer_revision(id)); match QueryReplaceKey::from_chord(chord) { QueryReplaceKey::Replace => self.core.borrow_mut().query_replace_replace(), QueryReplaceKey::Skip => self.core.borrow_mut().query_replace_skip(), @@ -843,7 +857,10 @@ impl EditorState { QueryReplaceKey::Quit => self.core.borrow_mut().query_replace_finish(), QueryReplaceKey::Ignore => {} } - if pre_revision != self.active_buffer_revision() { + // `!` applies many edits under one keypress; the single compare + // fires `buffer.after-edit` once for the batch (Q#QR1). + let post = origin_buf.and_then(|id| self.buffer_revision(id)); + if origin_buf.is_some() && pre != post { self.lua_host .run_hook("buffer.after-edit", mlua::MultiValue::new()); } diff --git a/src/editor_core.rs b/src/editor_core.rs index 35e163b..9eae480 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -824,6 +824,40 @@ impl EditorCore { self.query_replace.is_some() } + /// The buffer a running query-replace is pinned to, or `None`. The + /// dispatcher reads this so the `buffer.after-edit` revision compare + /// targets the *edited* buffer, not whichever is active. + #[must_use] + pub fn query_replace_origin_buffer(&self) -> Option { + self.query_replace.as_ref().map(|s| s.origin.0) + } + + /// Query-replace's wrong-buffer guard. Every edit and cursor move it + /// makes goes through the *active* window/buffer, but the session is + /// pinned to the buffer it started in — and focus can drift + /// mid-session (a click into another split, a key from another + /// frontend). Before touching the buffer, verify the active buffer + /// is still the origin buffer; if not, **abort without editing** so + /// a match found in the origin buffer can never be applied to an + /// unrelated one. Returns `true` when it is safe to proceed. + fn query_replace_on_origin(&mut self) -> bool { + let Some(origin_bid) = self.query_replace.as_ref().map(|s| s.origin.0) else { + return false; + }; + if self.active_buffer_id() == origin_bid { + return true; + } + // Focus moved off the origin buffer — abort, don't corrupt. + if let Some(session) = self.query_replace.take() { + self.search_store + .lock() + .expect("search store mutex poisoned") + .clear(session.origin.0); + } + self.status = "query-replace aborted: active buffer changed".into(); + false + } + /// Begin a query-replace from the cursor forward (Q#QR8). `regex` /// selects `query-replace-regexp` (Q#QR9). An invalid regex refuses /// to start (Q#QR2). Immediately advances to (and prompts on) the @@ -866,8 +900,8 @@ impl EditorCore { return; }; let bid = session.origin.0; - let start = session.next_from.min(self.active_buffer_len()) as usize; let bytes = self.buffer_bytes(bid); + let start = (session.next_from as usize).min(bytes.len()); let found = match &session.re { Some(re) => crate::search::find_first_regex_from(&bytes, re, start), None => crate::search::find_first_from(&bytes, &session.from, start), @@ -940,13 +974,16 @@ impl EditorCore { /// `y` / `SPC` — replace the current match, then advance to the next. pub fn query_replace_replace(&mut self) { - if self.query_replace_apply_current() { + if self.query_replace_on_origin() && self.query_replace_apply_current() { self.query_replace_advance(); } } /// `n` / `DEL` — leave the current match, advance past it to the next. pub fn query_replace_skip(&mut self) { + if !self.query_replace_on_origin() { + return; + } if let Some(session) = self.query_replace.as_mut() && let Some(range) = session.current { @@ -960,6 +997,9 @@ impl EditorCore { /// prompting, then finish (Q#QR6). One `after-edit` hook fires for /// the batch (the dispatcher compares revision across the handler). pub fn query_replace_all(&mut self) { + if !self.query_replace_on_origin() { + return; + } while self.query_replace_apply_current() { // Find the next match (mirrors advance's search, without the // highlight/prompt work — we're not stopping to ask). @@ -967,8 +1007,8 @@ impl EditorCore { break; }; let bid = session.origin.0; - let start = session.next_from.min(self.active_buffer_len()) as usize; let bytes = self.buffer_bytes(bid); + let start = (session.next_from as usize).min(bytes.len()); let found = match &session.re { Some(re) => crate::search::find_first_regex_from(&bytes, re, start), None => crate::search::find_first_from(&bytes, &session.from, start), @@ -987,6 +1027,9 @@ impl EditorCore { /// `.` — replace the current match, then finish (Q#QR6). pub fn query_replace_replace_and_quit(&mut self) { + if !self.query_replace_on_origin() { + return; + } self.query_replace_apply_current(); self.query_replace_finish(); } @@ -3747,6 +3790,46 @@ mod tests { assert_eq!(text_of(&s), "k _ K\n", "only the match at/after point"); } + #[test] + fn query_replace_aborts_when_active_buffer_changes() { + // The wrong-buffer merge-blocker: a session started in buffer X + // must never apply its match to a buffer that became active + // mid-session. Focus drifts (a click / cross-frontend key), then + // the next replace key aborts safely instead of corrupting. + let mut s = from_bytes(b"foo foo\n"); + let x = s.active_buffer_id(); + s.query_replace_begin("foo".into(), "bar".into(), false); + assert!(s.query_replace_active()); + + // Switch the active buffer to an unrelated one (focus drift). + let y = s.registry.borrow_mut().create("*other*"); + { + let reg = s.registry.borrow(); + let buf = reg.get(y).unwrap(); + let tv = crate::text_view::TextView::new(buf); + drop(reg); + let win = s.active_window_mut(); + win.buffer_id = y; + win.text_view = tv; + win.cursor = 0; + } + assert_eq!(s.active_buffer_id(), y); + + s.query_replace_replace(); // the y/replace key while drifted + assert!(!s.query_replace_active(), "drift aborts the session"); + assert_eq!(s.status, "query-replace aborted: active buffer changed"); + // Neither buffer was mutated by the aborted replace. + { + let reg = s.registry.borrow(); + let bx = reg.get(x).unwrap(); + let mut xb = vec![0u8; bx.len() as usize]; + bx.snapshot_rope().slice(0, bx.len(), &mut xb); + assert_eq!(&xb, b"foo foo\n", "origin buffer X untouched"); + let by = reg.get(y).unwrap(); + assert_eq!(by.len(), 0, "unrelated buffer Y untouched"); + } + } + #[test] fn query_replace_regex_replaces_and_invalid_refuses() { let mut s = from_bytes(b"a1 b2 c3\n"); diff --git a/tests/query_replace_acceptance.rs b/tests/query_replace_acceptance.rs index e9e0660..4fa69b3 100644 --- a/tests/query_replace_acceptance.rs +++ b/tests/query_replace_acceptance.rs @@ -1,9 +1,12 @@ //! Query-replace acceptance (Arc 2) — the interactive phase end-to-end //! through `dispatch_key`, exactly as a user (or a round-tripping GPU) -//! drives it: the `M-%` / `C-M-%` bindings, the y/n/!/./q vocabulary, -//! the three quit paths + nothing-matched-restores-origin, empty-to -//! deletion, offset-shift correctness, regex, the `buffer.after-edit` -//! hook firing on replaced text, and the `dispatch_idle`-false gate. +//! drives it: the `M-%` / `C-M-%` bindings; the full key vocabulary +//! (`y`/`SPC` replace, `n`/`DEL` skip, `!` all, `.` last); every quit +//! path (`q`, `RET`, `Esc`, `C-g`) keeping replacements, plus +//! nothing-matched-restores-origin; empty-to deletion; offset-shift +//! correctness (`a`→`aa` doesn't loop); regex; the `buffer.after-edit` +//! hook (once per `y`, and exactly once for an `!` batch); the +//! `dispatch_idle`-false gate; and the wrong-buffer/focus-drift abort. //! //! Framing: docs/query-replace-framing.md. @@ -280,3 +283,136 @@ fn replace_fires_after_edit_hook() { "buffer.after-edit fired for the replacement (before {before}, after {after})" ); } + +#[test] +fn bang_fires_after_edit_hook_once_for_the_batch() { + // Q#QR1: `!` applies many replacements under one keypress, but the + // debounced didChange wants a single after-edit — the shadow + // compares revision once across the whole handler. + let mut s = EditorState::new(); + s.lua_host + .lua() + .load( + r" + _G.EDITS = 0 + pmacs.hook.add('buffer.after-edit', function() _G.EDITS = _G.EDITS + 1 end) + ", + ) + .exec() + .expect("install after-edit counter"); + type_str(&mut s, "a a a a"); + goto_start(&s); + // Zero out the counter after the typing edits. + s.lua_host.lua().load("_G.EDITS = 0").exec().ok(); + start_query_replace(&mut s, "a", "b", false); + s.lua_host.lua().load("_G.EDITS = 0").exec().ok(); + press(&mut s, KeyCode::Char('!')); // four replacements in one keypress + let edits: i64 = s + .lua_host + .lua() + .load("return _G.EDITS") + .eval() + .expect("read counter"); + let (text, _) = probe(&s); + assert_eq!(text, "b b b b", "! replaced all four"); + assert_eq!(edits, 1, "after-edit fires exactly once for the ! batch"); +} + +#[test] +fn quit_via_ret_and_esc_keeps_replacements() { + // Q#QR10: RET and Esc both quit (keeping replacements), not just q. + for quit in [KeyCode::Enter, KeyCode::Esc] { + let mut s = EditorState::new(); + type_str(&mut s, "a a a"); + goto_start(&s); + start_query_replace(&mut s, "a", "b", false); + press(&mut s, KeyCode::Char('y')); // replace the first + press(&mut s, quit); // quit before the rest + let (text, active) = probe(&s); + assert_eq!(text, "b a a", "quit keeps the one replacement ({quit:?})"); + assert!(!active, "{quit:?} ends the session"); + } +} + +#[test] +fn ctrl_g_quits_keeping_replacements() { + // Q#QR10: C-g exits and KEEPS replacements (unlike isearch's C-g). + let mut s = EditorState::new(); + type_str(&mut s, "a a a"); + goto_start(&s); + start_query_replace(&mut s, "a", "b", false); + press(&mut s, KeyCode::Char('y')); + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('g'), KeyModifiers::CONTROL), + ); + let (text, active) = probe(&s); + assert_eq!(text, "b a a", "C-g keeps replacements (not an undo)"); + assert!(!active); +} + +#[test] +fn del_key_skips_like_n() { + let mut s = EditorState::new(); + type_str(&mut s, "a a a"); + goto_start(&s); + start_query_replace(&mut s, "a", "b", false); + press(&mut s, KeyCode::Backspace); // DEL/Backspace → skip first + press(&mut s, KeyCode::Char('y')); // replace second + press(&mut s, KeyCode::Char('q')); + let (text, _) = probe(&s); + assert_eq!( + text, "a b a", + "DEL skipped the first, y replaced the second" + ); +} + +#[test] +fn focus_drift_mid_session_aborts_without_touching_either_buffer() { + // The merge-blocker, end-to-end: a click into another buffer + // (simulated by switch_buffer, which the pointer path also uses) + // while query-replace is active. The next y must abort, not apply + // the origin-buffer match to the now-active unrelated buffer. + let mut s = EditorState::new(); + type_str(&mut s, "foo foo"); + goto_start(&s); + start_query_replace(&mut s, "foo", "bar", false); + assert!(probe(&s).1, "session active on the first match"); + + // Focus drifts to a fresh, unrelated buffer. + s.lua_host + .lua() + .load( + r" + _G.OTHER = pmacs.buffer.create('*drift*') + pmacs.window.switch_buffer(_G.OTHER) + ", + ) + .exec() + .expect("switch to another buffer"); + + press(&mut s, KeyCode::Char('y')); // the replace key, now drifted + assert!(!probe(&s).1, "drift aborts the session"); + let (drift_text, _) = probe(&s); // active buffer is *drift* + assert_eq!(drift_text, "", "the unrelated buffer was not edited"); + + // The origin buffer is also intact — switch back and check. + s.lua_host + .lua() + .load( + r" + for _, id in ipairs(pmacs.buffer.list()) do + if pmacs.describe.buffer(id).name == '*scratch*' then + pmacs.window.switch_buffer(id) + end + end + ", + ) + .exec() + .ok(); + assert_eq!( + probe(&s).0, + "foo foo", + "origin buffer untouched by the aborted replace" + ); +}