diff --git a/builtin/keymaps/default.lua b/builtin/keymaps/default.lua index e94c033..7dfb9db 100644 --- a/builtin/keymaps/default.lua +++ b/builtin/keymaps/default.lua @@ -49,7 +49,7 @@ bind("C-v", "cursor.page-down") bind("BS", "buffer.delete-backward") bind("DEL", "buffer.delete-forward") bind("C-d", "buffer.delete-forward") -bind("RET", "buffer.newline") +bind("RET", "edit.newline-and-indent") bind("TAB", "buffer.tab") -- Incremental search --------------------------------------------------------- diff --git a/builtin/runtime/indent.lua b/builtin/runtime/indent.lua new file mode 100644 index 0000000..ef66f84 --- /dev/null +++ b/builtin/runtime/indent.lua @@ -0,0 +1,130 @@ +-- indent.lua --- auto-indent on newline (Arc 2). +-- +-- RET (`edit.newline-and-indent`) inserts a newline plus the current +-- line's leading whitespace, verbatim, clipped at the split point +-- (Q#AI3): copying bytes is the only policy that cannot be wrong about +-- tabs-vs-spaces, and the clip keeps a split inside the indent from +-- double-indenting the carried text. The whole thing is ONE edit — one +-- undo step, one CRDT op. With a region it is one `buf:replace` (CUA +-- type-over, Q#AI4); the selection is cleared after every successful +-- edit, region or not (a zero-length selection would otherwise go live +-- the moment the cursor moves off the anchor). `buffer.newline` stays +-- bound-free as the plain-newline escape hatch (Q#AI2). +-- +-- Framing: docs/auto-indent-framing.md. + +pmacs.indent = pmacs.indent or {} + +local ed = pmacs.editor + +-- Start of the line containing `pos`: chunked backward scan for the +-- last newline strictly before it (comment.lua's scan — there is no +-- line-access API on buffers; giant lines stay safe). +local function line_start_before(buf, pos) + local p = pos + while p > 0 do + local from = math.max(0, p - 4096) + local chunk = buf:slice(from, p) + local nl = chunk:match("()\n[^\n]*$") + if nl then return from + nl end + p = from + end + return 0 +end + +-- The indent to carry over a split at `split` (Q#AI3): +-- bytes[line_start .. min(first_non_ws, split)]. Forward chunked scan +-- from the line start, stopping at the first non-whitespace byte — +-- never materializing more of the line than the indent itself plus +-- one chunk (Enter at the end of a giant minified line must not copy +-- the whole line just to produce an empty indent). `[ \t]` rather +-- than `%s` so a CR on a CRLF line never counts as indent. +local function indent_before(buf, split) + local start = line_start_before(buf, split) + local parts = {} + local p = start + while p < split do + local chunk_to = math.min(p + 4096, split) + local chunk = buf:slice(p, chunk_to) + local ws = chunk:match("^[ \t]*") + table.insert(parts, ws) + if #ws < #chunk then break end + p = chunk_to + end + return table.concat(parts) +end + +-- Right-gravity translation of `pos` through the effective edit +-- (Q#AI5; the daemon optimistic-arm shape). `estop` is the PRE-edit +-- end of the replaced range; an insert has estart == estop. +local function translate(pos, estart, estop, einserted) + if pos < estart then return pos end + if pos > estop then return pos - (estop - estart) + einserted end + return estart + einserted +end + +-- edit.newline-and-indent body. +function pmacs.indent.newline() + local buf = pmacs.window.buffer() + if not buf then + ed.set_status("no buffer") + return false + end + + -- Snapshot the context BEFORE the edit (Q#AI5): intercepts run with + -- the registry borrow released and may switch window or buffer; the + -- fix-up below must never touch whatever is active afterwards. + local win0 = pmacs.window.current() + local cursor0 = ed.cursor() + + local region = ed.region() + local has_region = region ~= nil and region["end"] > region.start + local rstart, rstop + if has_region then + rstart, rstop = region.start, region["end"] + else + rstart, rstop = cursor0, cursor0 + end + local text = "\n" .. indent_before(buf, rstart) + + -- One edit = one undo step, one CRDT op. Same intercept discipline + -- as killring/comment: a rejection reports rather than throws and + -- leaves no state behind. + local ok, estart, estop, einserted = pcall(function() + if has_region then + return buf:replace(rstart, rstop, text) + end + return buf:insert(rstart, text) + end) + if not ok then + ed.set_status("newline-and-indent rejected by buffer intercept") + return false + end + + -- Context guard (Q#AI5): fix up only the window that made the edit. + if pmacs.window.current() ~= win0 or pmacs.window.buffer() ~= buf then + ed.set_status("newline-and-indent: context changed during edit") + return false + end + + -- A deviating effective edit means an intercept rewrote it — the + -- interceptor's positional result stands (M6.4: kind and payload + -- are immutable). Cursor repair uses ONE formula for the clean and + -- transformed paths alike: translate the pre-edit cursor through + -- the effective edit, then goto_byte (which clamps). The clean + -- insert-at-cursor case lands at estart + einserted — right after + -- the carried indent. + local deviated = estart ~= rstart or estop ~= rstop or einserted ~= #text + if deviated then + ed.set_status("newline-and-indent altered by buffer intercept") + end + ed.goto_byte(translate(cursor0, estart, estop, einserted)) + ed.clear_selection() + return not deviated +end + +pmacs.command.define { + name = "edit.newline-and-indent", + description = "Insert a newline carrying the current line's indentation.", + fn = function() pmacs.indent.newline() end, +} diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index f78cf8f..2bd5302 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -1,7 +1,7 @@ # Agent handoff — cross-machine continuity -**Last updated: 2026-07-10, on the desktop, by the session that shipped -PR #107.** This file is the bridge between development machines. If you +**Last updated: 2026-07-10, on the laptop, by the auto-indent +session.** This file is the bridge between development machines. If you are an agent reading this on a fresh clone: this document plus the `docs/*-framing.md` files ARE your memory. Read this fully before taking on work, seed your persistent memory from it, and **update this @@ -10,20 +10,26 @@ next machine reads it the way you just did. ## 1. Where the project stands (2026-07-10) -- `main` @ `2dde4b8`, protocol **v15** (`SUPPORTED=[6..15]`). -- **PR #107 OPEN**: comment/uncomment toggle on `M-;` (Arc 2). Awaiting - the user's review findings. If it's merged by the time you read this, - Arc 2 has only auto-indent and auto-pairing left. Check - `gh pr list --state open` first thing. +- `main` @ `efa41cb`, protocol **v15** (`SUPPORTED=[6..15]`). +- **Auto-indent on newline (Arc 2) in flight on this branch** — + framing `docs/auto-indent-framing.md` is at revision 6 (five + pre-branch review rounds plus PR #109 round 1). RET now binds + `edit.newline-and-indent`; plain Enter is no longer GPU-optimistic + (round-trips like the TUI). Rode along: Q#AI8 search invalidation is + shared by dispatch, direct notification, undo, and redo (stale + step/summary fail closed; live origins translate through edits), and + Q#AI9 clears empty selections only after successful core inserts and + in the daemon's optimistic CRDT source arm for both frontends. The + TUI's missing nonempty-selection optimistic type-over gate and + generated-buffer search invalidation remain named deferrals. - Roadmap: `docs/roadmap-2026-07.md` (ranked arcs). Position: - **Arc 1 (LSP utility surface) COMPLETE** — completion popup (#92/#93), panels/references/outline/hover (#94–#96), plus hardening follow-ups (#102, #105, #106). - - **Arc 2 (editing table stakes) NEARLY COMPLETE** — query-replace - (#97), kill ring + `M-y` (#103/#105/#106), comment-toggle (#107). - **Remaining: auto-indent on newline, then auto-pairing.** These are - the agreed next work items, in that order, each as its own small - framing + PR. + - **Arc 2 (editing table stakes)** — query-replace (#97), kill ring + + `M-y` (#103/#105/#106), comment-toggle (#107), auto-indent (this + branch). **Remaining after this merges: auto-pairing**, as its own + small framing + PR. - **Arc 3 (persistence) COMPLETE** — saveplace/recentf (#98), desktop-save (#99), autosave/crash-recovery (#100), save-clobber fix (#101). @@ -70,12 +76,15 @@ cargo test --workspace -- --skip basedpyright # full sweep git diff --check ``` -Machine-specific caveats that were true on the DESKTOP — re-verify on -this machine before trusting them: +Machine-specific caveats — re-verify on a machine you haven't used +before trusting them: -- **basedpyright**: the desktop's local binary is broken and HANGS the - `m4_5_basedpyright` tests — hence the `--skip`. If this machine has a - working basedpyright, the skip may be droppable (verify once). +- **basedpyright**: the DESKTOP's local binary is broken and HANGS the + `m4_5_basedpyright` tests — hence the `--skip` there. The LAPTOP has + a working basedpyright 1.39.9 (verified 2026-07-10: the m4_5 test + passes in 0.18s), so the skip is droppable on the laptop. +- **GPU on the laptop**: AMD Radeon 780M (RADV) — native Vulkan, + `PMACS_REQUIRE_GPU=1` works without lavapipe. - **m8 daemon tests are FLAKY** (timing). A lone m8 failure → rerun before investigating. - **GPU tests** need a Vulkan device. `PMACS_REQUIRE_GPU=1` makes diff --git a/docs/auto-indent-framing.md b/docs/auto-indent-framing.md new file mode 100644 index 0000000..f787dfc --- /dev/null +++ b/docs/auto-indent-framing.md @@ -0,0 +1,518 @@ +# Auto-indent on newline — framing (Arc 2, editing table stakes) + +Pressing RET in pmacs inserts a bare `\n`; every indented line means +re-typing the indentation by hand. This adds the table-stakes behavior: +RET carries the current line's indentation onto the new line, +language-agnostic, as one undoable edit. Second-to-last Arc 2 item; +auto-pairing follows as its own framing + PR. + +Roadmap: `docs/roadmap-2026-07.md` Arc 2 ("auto-indent on newline"). +Revision 6 (PR #109 round 1): Q#AI8's invalidation is one helper +invoked from all four edit paths — dispatch, direct notification, +undo, redo — and the empty-anchor clear moved daemon-side, closing +the optimistic case for BOTH frontends (the residual was never +GPU-only: the TUI mirror tracks no selection state). The acceptance +matrix now matches what the suites actually pin. Earlier revisions: +success-gated Q#AI9, live-origin translation, M6.4 same-kind +intercept contract, modal-context corrections. + +## Ground truth (as of `efa41cb`) + +- **RET → `buffer.newline` → `ed.insert_char_over_region(10)`** + (`builtin/keymaps/default.lua:52`, + `builtin/commands/default.lua:110`): one `Replace` when a region is + active (CUA type-over, one undo step), plain insert otherwise. The + no-region path does **not** clear an empty selection, and + `Window::region()` returns `None` when anchor == cursor while the + selection object persists (`src/window.rs:244-255`). So a + zero-length selection (e.g. `S-Left` at BOF) survives the edit, and + the moment the edit advances the cursor away from the anchor the + region is **already nonempty** — the very next self-insert + type-overs the just-inserted text. The same no-region arm serves + `buffer.self-insert` and `buffer.tab`, so ordinary typing has the + identical bug today (`S-Left` at BOF, `x`, `y` → the `y` type-overs + the `x`) — it is not RET-specific. `insert_char` catches the + intercept rejection internally — status message, return `()` + (`src/editor_core.rs:1681-1692`) — so its caller cannot observe + failure; and `insert_char_over_region`'s doc comment already claims + "any selection is cleared" while only the region arm does it + (`src/editor_core.rs:1698`, `:1717`). +- **Runtime modules are individually embedded** — each + `builtin/runtime/*.lua` has an explicit `include_str!` + eval entry + in `src/editor.rs` (comment.lua's at `src/editor.rs:336`). There is + no discovery mechanism; a new module ships with a loader entry or it + never loads. +- **The two frontends' Enter paths diverge.** The TUI attach client + round-trips Enter to the daemon keymap by design — + `src/optimistic.rs:90-93` names "indentation, newline-with-indent" + as the reason, locked by `classify_enter_and_tab_are_round_trip`. + The GPU frontend treats plain Enter as optimistic-eligible + (`optimistic_insert_text`, `pmacs-gpu/src/main.rs:1520-1530`) and + ships a raw `"\n"` CRDT op that never touches the keymap — justified + in its doc comment by "`buffer.newline` reduces to plain + `insert_char(10)`", the exact premise this feature invalidates. The + GPU already round-trips Enter when a selection is active, the + completion popup is open, or the dispatcher is busy/stale. The + selection gate reads `DecorationKind::Selection` decorations + (`pmacs-gpu/src/main.rs:2054-2060`); an **empty** selection paints + no decoration, so it does not gate — GPU optimistic typing proceeds + over an empty anchor. +- **Consequence of the GPU bypass: RET rebindings are dead in the GPU + frontend.** The classic buffer list binds RET buffer-locally to + `editor.buffer-list-visit` (`builtin/commands/default.lua:407-411`) + through normal dispatch; unlike listview it is not marked for + round-trip input, so GPU RET there inserts a raw newline instead of + visiting. Any user rebind of RET is bypassed the same way. +- **Dispatch supplies an empty active-mode list** — the keymap stack + is resolved with `&[]` for modes (`src/editor.rs:710`), so + mode-scope bindings do not resolve anywhere today, on any frontend. +- **GPU test seams**: `optimistic_insert_text` is a free function + with an existing in-crate unit test + (`pmacs-gpu/src/main.rs:7058-7104`), and it is the sole gate at the + top of `optimistic_crdt_insert` (`:2077-2079`). The + eligibility/routing layer above it lives in private `App`/`State` + methods (`:2050`, `:2077`) with no constructible unit seam — the + winit handler's fall-through to `send_key` is not unit-reachable. + `pmacs-gpu` declares **no cargo features** + (`pmacs-gpu/Cargo.toml`); `crdt` is a root-package feature only. +- **Modal contexts that consume Enter before the keymap**: isearch + accept (`src/editor.rs:1831`), context-menu invoke + (`src/editor.rs:1934`), completion-popup accept + (`src/editor.rs:1984`), minibuffer accept + (`src/minibuffer.rs:482`), query-replace prompt + (`src/editor.rs:968`). These never reach the global keymap; a RET + rebind cannot touch them. Acceptance pins them anyway. +- **Search staleness is asymmetric across edit paths, and staleness + is only half-honored by consumers.** Accepted isearch matches + deliberately stay highlighted "until the next edit marks them + stale" (`src/editor_core.rs:861-865`). `apply_active_edit` — the + path today's `buffer.newline` takes — honors that via + `SearchStore::mark_stale` (`src/editor_core.rs:1184-1193`), and + highlight producers suppress stale matches. But `search_step` + navigates stored ranges without checking `is_stale` + (`src/editor_core.rs:844`), and `search_match_summary` exposes + stale counts to the n/m prompt (`src/editor_core.rs:719`). + `SearchStore::set` clears staleness (`src/search.rs:100`), so any + pattern re-run refreshes. Meanwhile direct buffer edits notify + through `notify_buffer_edit` (`src/editor_core.rs:1207`), which + refreshes views/overlays only — it never marks search state stale. + Its **three** callers: the CRDT-op apply path (`src/daemon.rs:2133`), + the general Lua mutator path (`notify_buffer_edit_to_windows`, + `src/lua_bindings/mod.rs:1390-1395`), and the LuaHost + errors-buffer append (`src/lua.rs:442`). A pre-existing + stale-highlight/stale-step bug for all three, which a + Lua-implemented RET would inherit on the most common keystroke. +- **A live search's origin is a raw byte offset.** `SearchSession` + stores `origin: (BufferId, byte)` at `search_begin` + (`src/editor_core.rs:740-753`); every recompute focuses from that + unchanged offset (`src/editor_core.rs:803-810`), and cancel + restores the cursor to it directly (`src/editor_core.rs:866`). No + edit path translates it — an insert or delete strictly before the + origin skews both the recompute focus and the cancel restore even + when the match set is fresh. (`apply_active_edit` marks stale right + there yet leaves the origin alone, so the other-frontend dispatch + path carries the same pre-existing skew.) +- **Direct buffer edits do not reconcile window state** — + `notify_buffer_edit` adjusts no cursors and no selections. An + intercept that expands a replace past the cursor can leave + `cursor > buf:len()` and a dangling selection; nothing downstream + clamps. The daemon's optimistic-CRDT arm shows the canonical + repair: right-gravity cursor translation through the effective edit + (`src/daemon.rs:2103-2130` — `pos < start` → unchanged; + `pos > pre-edit end` → `pos - old_len + inserted_len`; within → + `start + inserted_len`). Note the mutators' returned triple + `(start, end, inserted_len)` has `end` = the **pre-edit** + replaced-range end (`src/lua_bindings/mod.rs:1246`); the post-edit + end is `start + inserted_len`. +- **Intercepts may only move an edit, never change what it does.** + M6.4 forbids kind-changing transforms and keeps the payload bytes + immutable; same-kind position/range overrides are the entire + surface (`src/lua_bindings/mod.rs:1061-1074`, `:1653-1657`). + Consequences: an intercepted `insert` can only be **relocated** — + the buffer always grows by the payload — while only an intercepted + `replace` can **shrink** the buffer, by expanding its replaced + range past the payload length. +- **Intercepts can switch buffers and windows.** Phase 2 of + `run_managed_edit` runs the intercept chain with the registry + borrow released (`src/lua_bindings/mod.rs:1287-1324`); an intercept + body may legally change the active window or buffer. Any post-edit + fix-up that blindly targets "the active window" can corrupt + unrelated state. `pmacs.window.current()` exposes the active window + id to Lua (`src/lua_bindings/mod.rs:10617-10622`). +- **No indent infrastructure exists.** No indent command; TAB inserts + a literal `\t` (`buffer.tab`); there is no tabs-vs-spaces or width + setting anywhere (config registry is a standing deferral); grammars + drive highlighting only — no `indents.scm` in the repo. Hardcoded + tab widths exist at **five** divergent sites: `TAB_WIDTH = 8` in + four core renderers (`src/text_view.rs:35`, `src/highlight.rs:228`, + `src/diag.rs:364`, `src/completion.rs:594`) and the GPU minimap's + own leading-whitespace scan at width **4** + (`pmacs-gpu/src/main.rs:5510`, `:5529-5531`; frontend-local, not + reusable for editing). +- **The comment.lua pattern covers the Lua mechanics**: line scans + over `buf:slice` (`line_start_before`, + `builtin/runtime/comment.lua:47-57` — there is no `buf:line()` API; + the scan is the idiom), `ed.cursor()/region()/goto_byte()/ + clear_selection()`, pcall'd mutators with the exact effective-edit + triple check. `ed.goto_byte` clamps to buffer length. +- `C-j` is bound nowhere in `builtin/`. + +## Decisions + +### Q#AI1 — Lua feature + four named Rust touches + +`builtin/runtime/indent.lua` (new), following the comment.lua module +shape. Rust changes, each deliberate and small: + +1. **Loader entry** in `src/editor.rs` — `include_str!` + eval after + the comment.lua entry (`:336`). Without it the module never loads + and RET would resolve to an undefined command. (Defining the + command in `builtin/commands/default.lua` was the alternative; a + feature with helpers gets its own module per the comment.lua + precedent.) +2. **Search-stale substrate fix** (Q#AI8): mark stale in the shared + notify path, make the two stale-blind consumers fail closed, and + translate the live origin. +3. **GPU eligibility fix**: remove the `ProtocolKey::Enter` arm from + `optimistic_insert_text` so plain Enter round-trips through the + daemon keymap, as the TUI already does. The arm's own documented + justification (byte-identical to what the daemon would do) stops + being true the moment RET means newline-and-indent. Tab keeps its + arm: `buffer.tab` is still a plain `insert_char(9)`; the doc + comment is rewritten Tab-only. +4. **Empty-selection core fix** (Q#AI9): the no-region arm of + `insert_char_over_region` clears a lingering selection. + +### Q#AI2 — Command + binding + +One new command, `edit.newline-and-indent` (the Emacs name), and the +default keymap's RET line changes to it. `buffer.newline` keeps its +command definition and plain-newline role — the escape hatch via +`M-x` or a rebind — unchanged, with one deliberate exception: its +empty-selection behavior improves through the shared primitive +(the Q#AI9 fix). +No new default chord for plain newline in v1: the natural candidate +`C-j` is LF itself — the same terminal-ambiguity class as the C-/ +undo note in the comment-toggle framing — so it's a named deferral, +not a casual bind. + +### Q#AI3 — Indent = verbatim copy, clipped at the split point + +On the line containing the split point: + + indent = bytes[line_start .. min(first_non_ws, split_point)] + +where whitespace is space/tab only. Insert `"\n" .. indent` as ONE +edit; cursor lands after the indent. + +- **Verbatim copy** (not width math): with no tabs-vs-spaces or width + config in the editor, reproducing the existing bytes is the only + policy that cannot be wrong about the file's convention. Tabs, + spaces, and mixed runs all round-trip untouched. +- **Clip at the split point**: splitting *inside* the leading + whitespace would otherwise double-indent the carried text. Clipping + preserves the carried text's total indentation exactly — + `··|··foo` → `··` / `····foo` (split after 2 of 4 spaces: new line + still starts at column 4). +- **No language awareness in v1**: no extra indent after `{`, no + dedent of `}`. Electric indent needs a per-language width opinion + pmacs has no surface for (deferral, below). +- Whitespace-only lines copy what's before the split point; the + abandoned line keeps its trailing whitespace (cleanup is a named + deferral, not an accident). + +### Q#AI4 — Region type-over preserved; selection always cleared + +Region active → one `buf:replace(start, end, "\n" .. indent)`, indent +computed on the line containing the region start (clipped at region +start); cursor after the indent. Matches `insert_char_over_region`'s +single-`Replace` / one-undo-step / one-CRDT-op contract that the +GPU's selection round-trip relies on. + +**The selection is cleared after every successful edit, region or +not.** A zero-length selection (anchor == cursor) reports no region, +so the edit takes the no-region path — but the edit itself advances +the cursor off the anchor, making the region nonempty **immediately**: +the very next self-insert type-overs the newline (`S-Left` at BOF, +RET, `x` → the `x` replaces the newline). Unconditional +`clear_selection()` closes that for this command. (Q#AI9 fixes the +same lingering-anchor bug in the core command family; this command +edits via `buf:insert`/`buf:replace`, not `insert_char_over_region`, +so it must clear its own.) + +### Q#AI5 — Edit discipline: comment.lua contract + window-state safety + +Snapshot first, edit second, guarded fix-up third: + +1. **Snapshot** `pmacs.window.current()` and the target buffer id + before the mutator runs. +2. `pcall`'d mutator; a rejecting intercept → status message, no + edit, no throw, no dangling state. +3. **Guard**: intercepts run with the registry borrow released and + may switch the active window or buffer (ground truth). If the + active window or its buffer no longer matches the snapshot, skip + all fix-up — report only; touching whatever window is now active + would corrupt unrelated state. +4. **Cursor repair, one formula for both paths**: translate the + pre-edit cursor through the *effective* edit with the established + right-gravity shape (`src/daemon.rs:2103-2130`): `pos < start` → + unchanged; `pos > pre-edit end` → `pos - old_len + inserted_len`; + within the replaced range → `start + inserted_len`. Then + `goto_byte` (which clamps). For the clean insert-at-cursor case + this lands exactly at `start + inserted_len` — the normal + after-the-indent position — so there is no special-cased "clean" + placement to drift from the repaired one. Never jump to a fixed + edit endpoint: an intercept that relocates the edit elsewhere in + the buffer must not teleport the cursor there. +5. The effective triple is compared EXACTLY against the request; on + deviation → status *"newline-and-indent altered by buffer + intercept"*; the interceptor's **positional** result stands — + kind and payload are immutable under the M6.4 contract (ground + truth). Concretely: the plain path's `buf:insert` can only be + relocated, so the buffer always grows and the `"\n" .. indent` + payload lands wherever the intercept moved it; only the region + path's `buf:replace` can shrink the buffer, via a same-kind range + expansion. This is a named deviation from the comment.lua + precedent (which skips fix-up entirely): skipping here would + bless `cursor > buf:len()` whenever an expanded replace shrinks + the buffer past the cursor. +6. `clear_selection()` (Q#AI4), under the same context guard. + +### Q#AI6 — Consequences of the GPU change, named + +- Plain Enter in the GPU costs one daemon round-trip before echo — + parity with every TUI keypress and with the GPU's own + selection/popup/busy Enter today. +- **Global and buffer-local RET rebindings become effective in the + GPU frontend.** Concretely: buffer-list RET now visits the selected + buffer instead of inserting a raw newline into the list (today's + behavior — the optimistic path bypasses the buffer-local binding). + This is an incidental bug fix, named and tested, not a side effect. + **Mode-scope bindings are excluded from this claim**: dispatch + resolves the keymap stack with an empty mode list + (`src/editor.rs:710`), so they stay unresolved on every frontend + until the mode system is wired. +- `this_command` after Enter in the GPU becomes + `edit.newline-and-indent` (was `buffer.self-insert` via the + exact-decode CRDT classification) — now consistent across + frontends; kill chains break across a newline in both (already true + in TUI). +- GPU unit tests locking Enter's optimistic eligibility flip to lock + the round-trip. + +### Q#AI7 — Chain/hook plumbing: nothing to build + +Keybound RET rotates the command boundary and `buffer.after-edit` +fires from dispatch's revision check; `M-x edit.newline-and-indent` +gets both via `invoke_interactive`. Same Arc 2 substrate as +comment-toggle; the acceptance suite asserts the hook fires once +anyway. + +### Q#AI8 — Search staleness: mark it, honor it, and keep the origin true + +Three parts, all required: + +1. **Mark**: one helper, `search_invalidate_for_edit` (mark stale + + translate the origin), invoked from **all four edit paths**: + `apply_active_edit` (dispatch), `notify_buffer_edit` (applied CRDT + ops `src/daemon.rs:2133`, general Lua mutator edits + `src/lua_bindings/mod.rs:1390-1395`, and the errors-buffer append + `src/lua.rs:442` — normally a no-op, unless the *errors* buffer + itself carries accepted search state, in which case marking it + stale is exactly right), and — round-1 finding — `undo` / `redo`, + which receive precise `Edit` values but previously invalidated + nothing. Generated-buffer rebuilds via `rebuild_views_for` remain + a named lower-frequency bypass (deferral). +2. **Honor (fail closed)**: `SearchStore::step` returns `None` while + stale — C-s / `search.next` stops navigating byte ranges that no + longer exist instead of teleporting the cursor to them — and + `search_match_summary` reports `(None, 0)` while stale, so the + TUI/GPU n/m prompt cannot show counts for suppressed highlights. + The consumers then match what the highlight producers already do. + A stale **live** search un-sticks on the next pattern keystroke: + the re-run calls `SearchStore::set`, which clears staleness + (`src/search.rs:100`). Auto-recomputing from the stored query on + step (instead of failing closed) is a named deferral — nicer UX, + separate change. +3. **Translate the live origin**: a small `EditorCore` helper + right-gravity-translates `SearchSession::origin` through the + effective edit, invoked alongside `mark_stale` in **both** + `apply_active_edit` and `notify_buffer_edit` (the formula is the + `src/daemon.rs:2103-2130` shape; the dispatch path has the same + pre-existing skew, and the two call sites already mirror each + other for `mark_stale`). Without it, a fresh recompute focuses + from a skewed offset and cancel restores the cursor to the wrong + place whenever an external edit lands before the origin. + +Together these close a pre-existing bug family — lingering +highlights, stale stepping/counts, and skewed origins after any +direct Lua edit or optimistic GPU edit — that a Lua-implemented RET +would otherwise put on the most common keystroke in the editor. + +### Q#AI9 — Empty-selection type-over: fix the core arm, name the GPU residual + +The lingering-anchor bug is not RET's alone (ground truth): the +no-region arm of `insert_char_over_region` leaves an empty selection +armed, so ordinary typing already type-overs its own previous +keystroke (`S-Left` at BOF, `x`, `y` → the `y` replaces the `x`). + +**In scope**: `insert_char` returns success — today it catches the +intercept rejection and returns `()` (ground truth), so the caller +cannot distinguish a landed edit from a rejected one — and the +no-region arm clears the lingering selection **only on success**. +Clearing unconditionally would mutate state on a *failed* +self-insert/Tab/plain-newline, which the rejecting-intercept contract +forbids. This fixes `buffer.self-insert`, `buffer.tab`, and the +retained `buffer.newline` escape hatch for all daemon-dispatched +input, on both frontends' round-trip paths, and makes +`insert_char_over_region`'s existing "any selection is cleared" doc +claim true. + +**Also in scope (round 1: the residual was never GPU-only)**: the +TUI attach mirror tracks buffers and cursors but no selection state +(`src/buffer_mirror.rs:113`), and its optimistic gate checks cursor +freshness and EOL only (`src/optimistic.rs:250`) — so both +frontends' optimistic paths could re-arm the type-over. The fix +lives in the daemon's CRDT source arm (`handle_remote_crdt_op`): +before applying the source cursor update, a selection whose anchor +equals the pre-edit cursor — i.e. one that was EMPTY — is cleared. +Nonempty selections stand untouched. + +**Out of scope, named**: the TUI optimistic gate performs no +type-over check at all — a NONEMPTY selection ending at EOL +optimistically inserts instead of consuming the region (the GPU +gates on Selection decorations, which nonempty selections do paint, +so it round-trips correctly there). That is a pre-existing TUI gate +gap, deferred with the substrate reconciliation work. + +## Bets + +1. **Verbatim copy is the right v1 policy** — no tabs-vs-spaces + complaints are possible when we only ever reproduce what's already + on the line. +2. **GPU Enter round-trip latency is imperceptible** — it matches the + TUI's every keypress and the GPU's existing non-optimistic paths. +3. **Clip-at-split matches muscle memory** — mid-indent splits + preserving total indentation is what vi/Emacs users expect; nobody + files "my line double-indented". +4. **Fail-closed staleness has no workflow regressions** — a stale + step no-op (until the pattern re-runs) surprises nobody, because + the highlights it would have navigated are already suppressed; + nothing legitimate consumes stale offsets once step and summary + are guarded. +5. **Clearing a lingering empty selection on the core insert arm + breaks nothing** — no workflow depends on an anchor surviving + plain typing; mainstream editors deactivate the mark on edit. + +## Deferred (named) + +- Language-aware indent (electric `{`/`}`, tree-sitter `indents.scm`) + — blocked on width/style config, which is blocked on the config + registry deferral. +- TAB as reindent (`indent-for-tab-command`) — TAB stays a literal + tab, and stays GPU-optimistic. +- Doc-comment continuation on newline — re-deferred from the + comment-toggle framing; needs in-comment detection + (grammar/comment-span work); its own framing after auto-pairing. +- Strip the abandoned line's trailing whitespace on split. +- A plain-newline default chord (`C-j` is LF — terminal-ambiguous). +- `C-o` open-line-with-indent. +- Substrate-level window-state reconciliation for direct buffer edits + (cursor clamp / selection repair in the notify path, rather than + per-command — including windows other than the acting one), and + aligning comment.lua's transformed-intercept fix-up with Q#AI5's + translate-and-clamp discipline. +- **TUI optimistic type-over gate gap** (Q#AI9 round 1): the TUI + attach gate consults no selection state, so a NONEMPTY selection + ending at EOL optimistically inserts instead of type-over. (The + empty-anchor half was fixed daemon-side in round 1.) +- **Generated-buffer rebuilds** (`rebuild_views_for`) bypass search + invalidation — a lower-frequency Q#AI8 gap, named not handled. +- Auto-recompute of a stale search from the stored query on step + (Q#AI8 fails closed instead). +- Unifying the five hardcoded tab-width sites (4× core `TAB_WIDTH=8`, + GPU minimap `4`) behind a real setting — config-registry work. +- Mode-scope keybinding resolution (dispatch passes `&[]` today) — + the mode system's wiring, not this feature's. +- A constructible unit seam for the GPU routing layer above + `optimistic_insert_text` (private `App`/`State` today). + +## Acceptance (`tests/auto_indent_acceptance.rs`, dispatch-driven) + +- RET at EOL of a space-indented line: new line carries the indent, + cursor after it; tab-indented and mixed `\t··` lines round-trip + verbatim. +- Mid-line split: `····foo|bar` → `····foo` / `····bar`, cursor + before `bar`. +- Split inside leading whitespace: `··|··foo` → `··` / `····foo` + (clip rule — no double indent). +- Zero-indent line and empty buffer: byte-identical to old + `buffer.newline`. +- Whitespace-only line: new line copies it; abandoned line keeps it. +- Region active: ONE `Replace` — region replaced by `"\n" .. indent`, + selection cleared; a single `buffer.undo` restores exactly. +- **Zero-length selection, command level**: `S-Left` at BOF, RET, + `x` → `x` inserts plainly, the newline survives (selection cleared + unconditionally). **Core level (Q#AI9)**: `S-Left` at BOF, `x`, + `y` → the `y` does not replace the `x`; likewise via + `M-x buffer.newline`. **Failure regression**: empty selection + armed, self-insert rejected by an intercept → the anchor REMAINS + (no state mutation on a failed edit). The GPU-optimistic variant + is the named residual, not claimed here. +- One undo step in the plain case too. +- Intercept discipline: rejecting intercept → status, buffer + unchanged, no throw; transforming intercept → reported, positional + result stands. **Relocating transform** (plain path: the insert's + `pos` moved — the only transform an insert admits) → payload + inserted at the new site, cursor translated, NOT teleported. + **Shrinking transform** (region path: the replace's range expanded + past the payload, shrinking the buffer below the cursor) → cursor + right-gravity-translated and clamped within `buf:len()`, selection + cleared — the tests assert validity, not immobility. + **Context-switching intercept** (switches window or buffer + mid-edit) → fix-up skipped; this proves the *new* context is + untouched — the original window's validity after such an intercept + belongs to the substrate-reconciliation deferral, and the test does + not claim it. +- **Search staleness** (Q#AI8; acceptance + lib split, per what each + seam can reach): + - Acceptance: isearch, accept, RET → post-accept `search_step` + no-op; the same via a direct `buf:insert` (notify path). + - Lib (editor_core): edit during **active** isearch → step no-op + and summary `(None, 0)`, then the next pattern keystroke + refreshes and stepping resumes; origin translation through + **inserts and deletes on both edit paths** (dispatch and + notify), pinning recompute focus and cancel restore; **undo and + redo** stale accepted matches and translate the live origin. + - Lib (search store): stale `step` fails closed; a fresh `set` + un-sticks it. + - Lib (daemon, `--features crdt`): the optimistic source arm + clears an EMPTY anchor and leaves a NONEMPTY selection alone. +- `after-edit` fires exactly once per RET (keybound and `M-x`). +- Kill-chain break: `C-k`, RET, `C-k` → two ring entries. +- Modal contexts: isearch accept and minibuffer accept pinned here + (plus buffer-list RET below). Query-replace and completion-popup RET + are pinned by `tests/query_replace_acceptance.rs` and + `tests/completion_popup_acceptance.rs`; context-menu RET is pinned by + `editor::tests::menu_enter_invokes_command_and_closes`. Those tests + run in the gates and are not duplicated here. +- Giant minified line (64 KiB, unindented and indented): splits + correctly — pins the forward-chunked indent scan's behavior + (boundedness is by construction). +- Buffer-list RET still visits via dispatch (now also reachable from + the GPU frontend). +- `this_command()` after RET is `edit.newline-and-indent`. +- **GPU coverage, two named seams (not "end to end")**: + - In-crate (`pmacs-gpu`): the existing + `optimistic_insert_text_covers_plain_chars_enter_and_tab` test + flips — Enter returns `None`, Tab still optimistic. This is the + sole gate at the top of `optimistic_crdt_insert` (`:2079`), so + classifier-`None` forces the handler's round-trip branch; the + handler itself has no constructible unit seam (named deferral). + - Root package (`--features crdt`): a synthetic attached replica + plus the root TUI optimistic orchestrator drive pending + optimistic self-inserts followed by Enter on an indented line, + asserting the daemon dispatches `edit.newline-and-indent` and + the resulting multi-byte CRDT op reaches the replica with + correct final text — the daemon side of the wire path the GPU + takes once it round-trips. diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index d9955ff..8be9030 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -1068,9 +1068,10 @@ impl ApplicationHandler for App { if let Err(e) = client.send_crdt_op(op.buffer_id, op.op) { eprintln!("pmacs-gpu: send_crdt_op failed: {e}"); } - // An optimistic Enter near the bottom edge can - // scroll; re-declare the scoped viewport so - // the producer styles the newly visible lines. + // An optimistic edit near the viewport edge can + // scroll (a wrap-inducing insert, a Backspace + // above the top); re-declare the scoped viewport + // so the producer styles the newly visible lines. if let Some(vp) = op.viewport && let Err(e) = client.send_viewport(vp.buffer_id, vp.visible, vp.generation) @@ -1508,22 +1509,25 @@ fn optimistic_delete_range( /// The literal text `key` inserts when handled optimistically, or /// `None` for keys that must round-trip through the daemon. /// -/// `Enter` and `Tab` qualify alongside printable chars because their -/// default bindings (`buffer.newline` / `buffer.tab`) reduce to plain -/// `insert_char(10)` / `insert_char(9)` — byte-identical to a -/// self-insert, so the local application cannot diverge from what the -/// daemon will do with the same op. Two caveats are the caller's job: +/// `Tab` qualifies alongside printable chars because its default +/// binding (`buffer.tab`) reduces to a plain `insert_char(9)` — +/// byte-identical to a self-insert, so the local application cannot +/// diverge from what the daemon will do with the same op. `Enter` +/// does NOT: since Q#AI1 (docs/auto-indent-framing.md) RET binds +/// `edit.newline-and-indent`, whose inserted text depends on the +/// current line's indentation — and round-tripping is also what makes +/// RET rebindings (e.g. the buffer list's visit binding) reachable +/// from this frontend at all. Two caveats are the caller's job: /// `optimistic_crdt_insert` round-trips when an own-window selection /// is active (the daemon commands consume the region first — CUA -/// type-over — which a raw op can't), and modified variants (`S-RET`, -/// `C-TAB`, …) return `None` here: a keymap may bind them to anything. +/// type-over — which a raw op can't), and modified variants (`C-TAB`, +/// …) return `None` here: a keymap may bind them to anything. fn optimistic_insert_text(key: ProtocolKey, mods: Modifiers, chbuf: &mut [u8; 4]) -> Option<&str> { if !is_plain_text_modifiers(mods) { return None; } match key { ProtocolKey::Char(ch) if !ch.is_control() => Some(ch.encode_utf8(chbuf)), - ProtocolKey::Enter if mods.is_empty() => Some("\n"), ProtocolKey::Tab if mods.is_empty() => Some("\t"), _ => None, } @@ -2195,10 +2199,10 @@ impl State { self.optimistic_cursor_floor = Some(predicted); self.optimistic_floor_set_at = Some(std::time::Instant::now()); // Follow the caret NOW rather than when the daemon's - // `CursorByte` confirms — an optimistic Enter on the bottom - // visible line (or a Backspace pulling the caret above the - // top) moves it outside the slice, and waiting a round trip - // to scroll reads as a hitch. + // `CursorByte` confirms — an optimistic edit on the bottom + // visible line that wraps (or a Backspace pulling the caret + // above the top) moves it outside the slice, and waiting a + // round trip to scroll reads as a hitch. let viewport = if self.scroll_to_cursor() { self.rebuild_lines_reusing_scroll(); self.viewport_send_if_changed(predicted.buffer_id) @@ -7055,7 +7059,7 @@ mod tests { } #[test] - fn optimistic_insert_text_covers_plain_chars_enter_and_tab() { + fn optimistic_insert_text_covers_plain_chars_and_tab_but_not_enter() { let mut buf = [0u8; 4]; let none = Modifiers::NONE; let shift = Modifiers::SHIFT; @@ -7072,8 +7076,10 @@ mod tests { ); assert_eq!( optimistic_insert_text(ProtocolKey::Enter, none, &mut buf), - Some("\n"), - "RET is bound to buffer.newline = insert_char(10): identical to a self-insert" + None, + "RET binds edit.newline-and-indent (Q#AI1): the inserted text depends \ + on the current line, so plain Enter must round-trip — this is also \ + what makes RET rebindings reachable from the GPU frontend" ); assert_eq!( optimistic_insert_text(ProtocolKey::Tab, none, &mut buf), diff --git a/src/daemon.rs b/src/daemon.rs index 0a87188..dea3493 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -2111,6 +2111,19 @@ fn handle_remote_crdt_op( continue; } if Some(*wid) == source_active_window_id { + // Q#AI9 (PR #109 round 1): an empty anchor armed at + // the pre-edit cursor must not survive the cursor + // moving off it — otherwise the optimistic paths + // (GPU always; TUI mirror, which tracks no selection + // state) re-arm the type-over that + // `insert_char_over_region`'s no-region clear fixed + // on the dispatch path. Nonempty selections stand: + // the TUI gate's missing type-over check is a named + // deferral, and guessing here would destroy a real + // selection. + if win.selection.map(|sel| sel.anchor) == Some(win.cursor) { + win.selection = None; + } // Source window: set directly to optimistic post-edit // position (matches the source frontend's mirror // cursor after advance/retreat). @@ -2551,6 +2564,114 @@ mod tests { ); } + /// Q#AI9 (PR #109 round 1): the optimistic-apply arm clears an + /// EMPTY anchor on the source window — the GPU always takes this + /// path, and the TUI attach mirror tracks no selection state, so + /// neither frontend's gate stops an armed-empty-anchor sequence + /// from re-creating the type-over that + /// `insert_char_over_region`'s no-region clear fixed on the + /// dispatch path. A NONEMPTY selection must survive untouched + /// (the TUI gate's missing type-over check is a named deferral). + #[cfg(feature = "crdt")] + #[test] + fn handle_remote_crdt_op_clears_only_an_empty_source_anchor() { + use crate::editor::EditorState; + use crate::protocol::FrontendId; + use crate::window::Selection; + + // Shared fixture: CRDT-backed active buffer + a peer doc that + // produces the optimistic op, sourced from LOCAL (which has a + // registered view, so the source-window arm runs). + fn apply_peer_insert(editor: &mut EditorState, buffer_id: crate::buffer::BufferId) { + let snapshot_bytes = { + let core = editor.core.borrow(); + let reg = core.registry.borrow(); + let buf = reg.get(buffer_id).expect("buffer"); + buf.crdt_state() + .expect("crdt-backed") + .export_snapshot() + .expect("export snapshot") + }; + let peer = loro::LoroDoc::new(); + peer.set_peer_id(u64::from(FrontendId::LOCAL.0)) + .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"); + super::handle_remote_crdt_op( + editor, + FrontendId::LOCAL, + buffer_id, + crate::rope::CrdtOp { + peer_id: FrontendId::LOCAL.0, + bytes: op_bytes, + }, + ); + } + + // Case 1: empty anchor at the cursor (S-Left-at-BOF shape) — + // cleared by the optimistic apply. + let mut editor = EditorState::new(); + 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 mut core = editor.core.borrow_mut(); + let at = core.active_window().cursor; + core.active_window_mut().selection = Some(Selection { anchor: at }); + } + apply_peer_insert(&mut editor, buffer_id); + { + let core = editor.core.borrow(); + assert!( + core.active_window().selection.is_none(), + "an empty anchor must not survive an optimistic source edit" + ); + assert_eq!( + core.active_window().cursor, + 1, + "cursor at post-edit position" + ); + } + + // Case 2: nonempty selection — the arm must not touch it. + let mut editor = EditorState::new(); + let buffer_id = editor.core.borrow().active_window().buffer_id; + editor.core.borrow_mut().insert_char('a'); + editor.core.borrow_mut().insert_char('b'); + { + 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 mut core = editor.core.borrow_mut(); + core.active_window_mut().selection = Some(Selection { anchor: 0 }); + // cursor is at 2 after the two inserts: nonempty region. + } + apply_peer_insert(&mut editor, buffer_id); + { + let core = editor.core.borrow(); + assert_eq!( + core.active_window().selection, + Some(Selection { anchor: 0 }), + "a nonempty selection survives the optimistic source edit" + ); + } + } + /// Kill ring Q#KR2 — GPU typing arrives here without touching /// dispatch_key, so it must update the source frontend's command /// boundary or `C-k x C-k` on the GPU would append across the typed diff --git a/src/editor.rs b/src/editor.rs index 98ae44b..0e07d4e 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -339,6 +339,12 @@ impl EditorState { include_str!("../builtin/runtime/comment.lua"), ) .expect("load comment builtin chunk"); + lua_host + .eval( + Some("@pmacs/builtin/runtime/indent.lua"), + include_str!("../builtin/runtime/indent.lua"), + ) + .expect("load indent 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 @@ -2856,6 +2862,82 @@ mod tests { assert_eq!(s.core.borrow().active_buffer_len(), 1); } + #[test] + fn empty_selection_is_cleared_by_a_landed_self_insert() { + // Q#AI9: an armed anchor at the cursor reports no region, so + // 'x' inserts plainly — but the insert moves the cursor off + // the anchor, and without the clear the region goes live and + // 'y' type-overs the 'x'. + let mut s = fresh_with(b""); + s.lua_host + .lua() + .load("pmacs.editor.begin_selection(0)") + .exec() + .unwrap(); + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('x'), KeyModifiers::NONE), + ); + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('y'), KeyModifiers::NONE), + ); + let core = s.core.borrow(); + assert_eq!( + core.active_buffer_len(), + 2, + "'y' must append, not type-over the freshly inserted 'x'" + ); + assert!( + core.active_window().selection.is_none(), + "a landed self-insert clears the lingering anchor" + ); + } + + #[test] + fn rejected_self_insert_leaves_the_empty_selection_anchor() { + // Q#AI9 failure regression: a rejecting intercept means NO + // state mutation — the armed anchor must survive. + let mut s = fresh_with(b""); + s.lua_host + .lua() + .load( + 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) + pmacs.editor.begin_selection(0) + "#, + ) + .exec() + .unwrap(); + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('x'), KeyModifiers::NONE), + ); + { + let core = s.core.borrow(); + assert_eq!(core.active_buffer_len(), 0, "the insert was rejected"); + assert!( + core.active_window().selection.is_some(), + "a rejected insert must not clear the anchor" + ); + } + // The next (allowed) insert lands and clears it. + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('x'), KeyModifiers::NONE), + ); + let core = s.core.borrow(); + assert_eq!(core.active_buffer_len(), 1); + assert!(core.active_window().selection.is_none()); + } + #[test] fn backspace_deletes_previous_char() { let mut s = fresh_with(b""); diff --git a/src/editor_core.rs b/src/editor_core.rs index 4dcccf9..8a69c40 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -714,7 +714,9 @@ impl EditorCore { /// `(active_index, total)` for the active buffer's matches, for the /// prompt's "n/m" readout. `active_index` is 0-based and `None` - /// when there are no matches. + /// when there are no matches. Stale matches read as absent (Q#AI8 + /// fail-closed): the highlights they count are already suppressed, + /// so the prompt must not advertise them either. #[must_use] pub fn search_match_summary(&self) -> (Option, usize) { let bid = self.active_buffer_id(); @@ -722,6 +724,9 @@ impl EditorCore { .search_store .lock() .expect("search store mutex poisoned"); + if guard.is_stale(bid) { + return (None, 0); + } guard .for_buffer(bid) .map_or((None, 0), |s| (s.active_index(), s.len())) @@ -1156,17 +1161,22 @@ impl EditorCore { /// Returns a stringified error on buffer or view failure. pub fn apply_active_edit(&mut self, op: EditOp<'_>) -> Result { let buffer_id = self.active_buffer_id(); - let mut reg = self.registry.borrow_mut(); - let buffer = reg.get_mut(buffer_id).map_err(|e| e.to_string())?; - let edit = buffer.apply_edit(op).map_err(|e| e.to_string())?; - for win in self.windows.values_mut() { - if win.buffer_id == buffer_id { - let _ = win.text_view.on_edit(buffer, &edit); - for overlay in &mut win.overlays { - let _ = overlay.on_edit(buffer, &edit); + // Scope the registry borrow: the origin translation below needs + // `&mut self` after the views have been notified. + let edit = { + let mut reg = self.registry.borrow_mut(); + let buffer = reg.get_mut(buffer_id).map_err(|e| e.to_string())?; + let edit = buffer.apply_edit(op).map_err(|e| e.to_string())?; + for win in self.windows.values_mut() { + if win.buffer_id == buffer_id { + let _ = win.text_view.on_edit(buffer, &edit); + for overlay in &mut win.overlays { + let _ = overlay.on_edit(buffer, &edit); + } } } - } + edit + }; // T M10.8 Day 4 — capture CRDT op (if the buffer was in // CRDT mode and produced one) for the dispatcher to // broadcast on the next tick. @@ -1182,16 +1192,53 @@ impl EditorCore { .push((CrdtOpOrigin::DaemonKey, buffer_id, (**crdt_op).clone())); } // Search matches were computed against the pre-edit text, so - // their byte positions are now wrong. Mark the buffer's matches - // stale (M11.8): the producer / TUI overlay suppress them until - // a fresh search re-runs against the current content. No-op for - // a buffer with no search state. The headline isearch bet — - // "stale-after-edit linger" — is closed here. + // their byte positions are now wrong (M11.8): the producer / + // TUI overlay suppress them until a fresh search re-runs. The + // headline isearch bet — "stale-after-edit linger" — is + // closed here. + self.search_invalidate_for_edit(buffer_id, &edit); + Ok(edit.new_rope.len()) + } + + /// Q#AI8 search invalidation for a landed edit: mark the buffer's + /// matches stale (no-op without search state) and right-gravity- + /// translate the live session origin. ONE helper so every edit + /// path — dispatch ([`Self::apply_active_edit`]), direct + /// notification ([`Self::notify_buffer_edit`]), and history + /// ([`Self::undo`] / [`Self::redo`]) — invalidates identically; + /// a path that skips this leaves highlights, step targets, and + /// the n/m count pointing at pre-edit offsets. + fn search_invalidate_for_edit(&mut self, buffer_id: BufferId, edit: &Edit) { self.search_store .lock() .expect("search store mutex poisoned") .mark_stale(buffer_id); - Ok(edit.new_rope.len()) + self.translate_search_origin(buffer_id, edit); + } + + /// Right-gravity-translate the live search origin through an edit + /// to `buffer_id` (Q#AI8; the `src/daemon.rs` optimistic-arm + /// shape). The origin is a raw byte offset captured at + /// [`Self::search_begin`]; without translation an insert/delete + /// before it skews every later recompute focus and the cancel + /// restore, even when the match set itself is fresh. + fn translate_search_origin(&mut self, buffer_id: BufferId, edit: &Edit) { + let Some(session) = self.search.as_mut() else { + return; + }; + if session.origin.0 != buffer_id { + return; + } + let start = edit.range.start; + let end = edit.range.end; + let pos = session.origin.1; + session.origin.1 = if pos < start { + pos + } else if pos > end { + pos - (end - start) + edit.inserted_len + } else { + start + edit.inserted_len + }; } /// Notify every window displaying `buffer_id` that the buffer was @@ -1204,7 +1251,14 @@ impl EditorCore { /// edited buffer would keep a stale [`crate::text_view::TextView`] /// line cache, causing later cursor motions to land at offsets the /// view cannot map back to display coordinates. + /// + /// Q#AI8: direct edits must also invalidate search state exactly + /// like [`Self::apply_active_edit`] does — mark the matches stale + /// and translate the live origin — otherwise accepted-match + /// highlights and the session origin survive at pre-edit offsets + /// for every Lua mutator edit and applied CRDT op. pub fn notify_buffer_edit(&mut self, buffer_id: BufferId, edit: &Edit) { + self.search_invalidate_for_edit(buffer_id, edit); let reg = self.registry.borrow(); let Ok(buffer) = reg.get(buffer_id) else { return; @@ -1677,8 +1731,11 @@ impl EditorCore { aw.goal_col = None; } - /// Insert a single character at the cursor. - pub fn insert_char(&mut self, ch: char) { + /// Insert a single character at the cursor. Returns `true` iff the + /// edit landed: a rejecting buffer intercept reports via the status + /// line and returns `false`, and callers must not mutate dependent + /// state (e.g. selection anchors) on a failed insert (Q#AI9). + pub fn insert_char(&mut self, ch: char) -> bool { self.active_window_mut().goal_col = None; let mut buf = [0u8; 4]; let s = ch.encode_utf8(&mut buf); @@ -1686,9 +1743,10 @@ impl EditorCore { let pos = self.active_window().cursor; if let Err(e) = self.apply_active_edit(EditOp::Insert { pos, bytes }) { self.status = format!("insert failed: {e}"); - return; + return false; } self.active_window_mut().cursor += bytes.len() as u64; + true } /// CUA type-over: insert `ch`, replacing the active region if one @@ -1699,7 +1757,14 @@ impl EditorCore { /// lands just past the inserted bytes and any selection is cleared. pub fn insert_char_over_region(&mut self, ch: char) { let Some((lo, hi)) = self.active_region() else { - self.insert_char(ch); + // Q#AI9: an empty selection (anchor == cursor) reports no + // region yet stays armed — the insert moves the cursor off + // the anchor and the very NEXT key type-overs the fresh + // text. Clear it, but only when the edit landed: a + // rejecting intercept must leave the anchor untouched. + if self.insert_char(ch) { + self.active_window_mut().selection = None; + } return; }; self.active_window_mut().goal_col = None; @@ -1839,6 +1904,9 @@ impl EditorCore { } } drop(reg); + // Q#AI8 (PR #109 round 1): history edits move bytes + // like any other edit — invalidate search state. + self.search_invalidate_for_edit(buffer_id, &edit); // Post-audit-round-5 F27: undo on a CRDT-backed // buffer produces a crdt_op that must broadcast to // every replica frontend (including the one whose @@ -1877,6 +1945,8 @@ impl EditorCore { } } drop(reg); + // Q#AI8 — same as undo above. + self.search_invalidate_for_edit(buffer_id, &edit); // Post-audit-round-5 F27 — same as undo above. self.queue_daemon_origin_crdt_op(buffer_id, &edit); } @@ -3669,6 +3739,237 @@ mod tests { ); } + #[test] + fn stale_matches_fail_closed_for_step_and_summary() { + // Q#AI8: once an edit marks matches stale, the highlights are + // suppressed — stepping and the n/m prompt must fail closed + // with them instead of navigating/advertising dead offsets. + let mut s = from_bytes(b"foo bar foo"); + s.active_window_mut().cursor = 0; + s.search_begin(true, false); + type_query(&mut s, "foo"); + s.search_finish(true); // accept keeps the matches + assert_eq!(s.search_match_summary(), (Some(0), 2)); + s.active_window_mut().cursor = 0; + assert!(s.insert_char('x'), "plain insert lands"); + assert_eq!( + s.search_match_summary(), + (None, 0), + "stale counts must not reach the prompt" + ); + let before = s.cursor(); + s.search_step(true); + assert_eq!(s.cursor(), before, "stale step is a no-op"); + } + + #[test] + fn live_search_origin_translates_through_local_edits() { + // Q#AI8: the session origin is a raw byte offset; an edit + // before it must shift it (right-gravity) so cancel restores + // the same TEXT position, not the same number. + let mut s = from_bytes(b"foo bar foo"); + s.active_window_mut().cursor = 5; + s.search_begin(true, false); // origin byte 5 + type_query(&mut s, "foo"); + assert_eq!(s.cursor(), 8, "focused the match after the origin"); + s.active_window_mut().cursor = 0; + assert!(s.insert_char('x')); + assert!(s.insert_char('y')); + s.search_finish(false); // cancel + assert_eq!( + s.cursor(), + 7, + "cancel restores the translated origin (5 + 2 inserted bytes)" + ); + } + + #[test] + fn live_search_recompute_focuses_from_the_translated_origin() { + let mut s = from_bytes(b"foo bar foo"); + s.active_window_mut().cursor = 1; + s.search_begin(true, false); // origin byte 1 + type_query(&mut s, "fo"); + assert_eq!(s.cursor(), 8, "first match at/after the origin"); + s.active_window_mut().cursor = 0; + assert!(s.insert_char('x')); + assert!(s.insert_char('y')); + assert!(s.insert_char('z')); + // "xyzfoo bar foo": origin 1 -> 4. Growing the query recomputes + // and must focus from the TRANSLATED origin: the match at 11, + // not the pre-edit offset 1's neighbor at 3. + type_query(&mut s, "o"); + assert_eq!( + s.cursor(), + 11, + "recompute focuses the first match at/after the translated origin" + ); + } + + #[test] + fn notify_buffer_edit_marks_stale_and_translates_the_origin() { + // Q#AI8 at the direct-edit seam (Lua mutators / applied CRDT + // ops): notify_buffer_edit must invalidate matches and shift + // the live origin exactly like apply_active_edit does. + let mut s = from_bytes(b"foo bar foo"); + let bid = s.active_buffer_id(); + s.active_window_mut().cursor = 1; + s.search_begin(true, false); // origin byte 1 + type_query(&mut s, "foo"); + let edit = { + let mut reg = s.registry.borrow_mut(); + let buffer = reg.get_mut(bid).expect("buffer"); + buffer + .apply_edit(crate::buffer::EditOp::Insert { + pos: 0, + bytes: b"zz", + }) + .expect("direct insert") + }; + s.notify_buffer_edit(bid, &edit); + assert!( + s.search_store.lock().expect("store").is_stale(bid), + "direct edits mark the matches stale" + ); + s.search_finish(false); // cancel + assert_eq!( + s.cursor(), + 3, + "cancel restores the origin translated through the direct edit" + ); + } + + #[test] + fn undo_and_redo_stale_accepted_search_navigation() { + // Q#AI8 (PR #109 round 1): history edits move bytes like any + // other edit — undo/redo must invalidate search state. + let mut s = from_bytes(b"foo bar foo"); + s.active_window_mut().cursor = 0; + assert!(s.insert_char('x')); // the history entry: "xfoo bar foo" + s.search_begin(true, false); + type_query(&mut s, "foo"); + s.search_finish(true); // accept + assert_eq!(s.search_match_summary(), (Some(0), 2)); + s.undo(); // back to "foo bar foo": offsets moved + assert_eq!( + s.search_match_summary(), + (None, 0), + "undo stales the accepted matches" + ); + let before = s.cursor(); + s.search_step(true); + assert_eq!(s.cursor(), before, "stale step is a no-op after undo"); + + // Redo the same way: refresh the matches first (a fresh set + // clears staleness), then redo must stale them again. + s.search_begin(true, false); + type_query(&mut s, "foo"); + s.search_finish(true); + assert_eq!(s.search_match_summary().1, 2); + s.redo(); // forward to "xfoo bar foo" again + assert_eq!( + s.search_match_summary(), + (None, 0), + "redo stales the accepted matches" + ); + } + + #[test] + fn undo_translates_the_live_search_origin() { + let mut s = from_bytes(b"foo bar foo"); + s.active_window_mut().cursor = 0; + assert!(s.insert_char('x')); // "xfoo bar foo" + s.active_window_mut().cursor = 5; + s.search_begin(true, false); // origin 5 ('b' of "bar") + type_query(&mut s, "foo"); + s.undo(); // removes the 'x' at 0: origin must shift to 4 + s.search_finish(false); // cancel + assert_eq!( + s.cursor(), + 4, + "cancel lands on the origin translated through the undo" + ); + } + + #[test] + fn origin_translates_through_deletes_on_both_paths() { + // Dispatch path (apply_active_edit): backspace before the + // origin. + let mut s = from_bytes(b"foo bar foo"); + s.active_window_mut().cursor = 5; + s.search_begin(true, false); // origin 5 + type_query(&mut s, "foo"); + s.active_window_mut().cursor = 2; + s.backspace(); // deletes byte 1: origin 5 -> 4 + s.search_finish(false); + assert_eq!(s.cursor(), 4, "origin shifted left by the deleted byte"); + + // Direct-notification path: a delete edit through + // notify_buffer_edit. + let mut s = from_bytes(b"foo bar foo"); + let bid = s.active_buffer_id(); + s.active_window_mut().cursor = 5; + s.search_begin(true, false); // origin 5 + type_query(&mut s, "foo"); + let edit = { + let mut reg = s.registry.borrow_mut(); + let buffer = reg.get_mut(bid).expect("buffer"); + buffer + .apply_edit(crate::buffer::EditOp::Delete { + range: Range::new(0, 2), + }) + .expect("direct delete") + }; + s.notify_buffer_edit(bid, &edit); + s.search_finish(false); + assert_eq!( + s.cursor(), + 3, + "origin shifted left by the two directly deleted bytes" + ); + } + + #[test] + fn active_search_fails_closed_while_stale_and_recovers_on_retype() { + // Q#AI8 during a LIVE session: an external edit mid-search + // makes step and summary fail closed; the next pattern + // keystroke recomputes (set clears staleness) and resumes. + let mut s = from_bytes(b"foo bar foo"); + let bid = s.active_buffer_id(); + s.active_window_mut().cursor = 0; + s.search_begin(true, false); + type_query(&mut s, "fo"); + assert_eq!(s.search_match_summary().1, 2); + let edit = { + let mut reg = s.registry.borrow_mut(); + let buffer = reg.get_mut(bid).expect("buffer"); + buffer + .apply_edit(crate::buffer::EditOp::Insert { + pos: 0, + bytes: b"zz", + }) + .expect("direct insert") + }; + s.notify_buffer_edit(bid, &edit); + assert_eq!( + s.search_match_summary(), + (None, 0), + "summary fails closed mid-search" + ); + let before = s.cursor(); + s.search_step(true); + assert_eq!(s.cursor(), before, "step fails closed mid-search"); + // Growing the query recomputes against the current text. + type_query(&mut s, "o"); + assert_eq!( + s.search_match_summary().1, + 2, + "the next pattern keystroke refreshes the match set" + ); + assert_eq!(s.cursor(), 2, "focus lands from the translated origin"); + s.search_step(true); + assert_eq!(s.cursor(), 10, "stepping resumes after the refresh"); + } + #[test] fn search_backspace_widens_the_match_set() { let mut s = from_bytes(b"fo foo food"); diff --git a/src/search.rs b/src/search.rs index bda13db..fbe7f2b 100644 --- a/src/search.rs +++ b/src/search.rs @@ -145,8 +145,15 @@ impl SearchStore { /// Step the active match forward or backward, wrapping. Returns /// the new active match's range, or `None` when the buffer has no - /// matches. + /// matches — or when they are stale (Q#AI8 fail-closed): stale + /// ranges were computed against pre-edit text, and stepping + /// through them would teleport the cursor to offsets that no + /// longer exist. A live search un-sticks on the next pattern + /// keystroke ([`Self::set`] clears staleness). pub fn step(&mut self, buffer_id: BufferId, forward: bool) -> Option { + if self.stale.contains(&buffer_id) { + return None; + } let s = self.by_buffer.get_mut(&buffer_id)?; let n = s.matches.len(); if n == 0 { @@ -830,4 +837,20 @@ mod tests { s.mark_stale(other); assert!(!s.is_stale(other)); } + + #[test] + fn step_fails_closed_while_stale() { + // Q#AI8: stale ranges were computed against pre-edit text; + // stepping through them would teleport the cursor to offsets + // that no longer exist. + let mut s = SearchStore::new(); + let bid = BufferId::next(); + s.set(bid, "x", vec![r(0, 1), r(4, 5)]); + assert!(s.step(bid, true).is_some(), "fresh matches step"); + s.mark_stale(bid); + assert!(s.step(bid, true).is_none(), "stale matches do not"); + // A re-run (`set`) clears staleness and stepping resumes. + s.set(bid, "x", vec![r(0, 1), r(4, 5)]); + assert!(s.step(bid, true).is_some(), "fresh set un-sticks stepping"); + } } diff --git a/tests/auto_indent_acceptance.rs b/tests/auto_indent_acceptance.rs new file mode 100644 index 0000000..5d40590 --- /dev/null +++ b/tests/auto_indent_acceptance.rs @@ -0,0 +1,505 @@ +//! Auto-indent acceptance (Arc 2, docs/auto-indent-framing.md). +//! +//! Dispatch-driven: RET through `dispatch_key`, `M-x` through the real +//! minibuffer. Auto-indent is language-agnostic (Q#AI3 copies bytes), +//! so buffers are plain in-memory scratch buffers — no files, no +//! language detection, no `StateDir`. + +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 m_x(s: &mut EditorState, name: &str) { + alt(s, 'x'); + type_str(s, name); + press(s, KeyCode::Enter); +} + +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 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 cursor(s: &EditorState) -> i64 { + eval(s, "return pmacs.editor.cursor()") +} + +fn status(s: &EditorState) -> String { + s.core.borrow().status.clone() +} + +/// Fresh editor whose active scratch buffer holds `body`, cursor at 0. +fn editor_with(body: &str) -> EditorState { + let s = EditorState::new(); + if !body.is_empty() { + exec(&s, &format!("pmacs.window.buffer():insert(0, {body:?})")); + } + exec(&s, "pmacs.editor.goto_byte(0)"); + s +} + +// --------------------------------------------------------------------------- +// The indent copy (Q#AI3) +// --------------------------------------------------------------------------- + +#[test] +fn ret_at_eol_carries_the_space_indent() { + let mut s = editor_with(" foo\nbar\n"); + exec(&s, "pmacs.editor.goto_byte(7)"); // end of " foo" + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), " foo\n \nbar\n"); + assert_eq!(cursor(&s), 12, "cursor lands after the carried indent"); +} + +#[test] +fn tab_and_mixed_indents_round_trip_verbatim() { + let mut s = editor_with("\tfoo"); + exec(&s, "pmacs.editor.goto_byte(4)"); + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), "\tfoo\n\t", "a tab indent copies as a tab"); + assert_eq!(cursor(&s), 6); + + let mut s = editor_with("\t foo"); + exec(&s, "pmacs.editor.goto_byte(6)"); + press(&mut s, KeyCode::Enter); + assert_eq!( + buffer_text(&s), + "\t foo\n\t ", + "mixed tab+space indents copy byte-for-byte" + ); + assert_eq!(cursor(&s), 10); +} + +#[test] +fn mid_line_split_carries_the_tail_onto_the_indented_line() { + let mut s = editor_with(" foobar"); + exec(&s, "pmacs.editor.goto_byte(7)"); // between "foo" and "bar" + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), " foo\n bar"); + assert_eq!(cursor(&s), 12, "cursor sits before the carried tail"); +} + +#[test] +fn split_inside_the_leading_whitespace_does_not_double_indent() { + // Q#AI3 clip rule: `··|··foo` → `··` / `····foo` — the carried + // text keeps its TOTAL indentation (4), instead of gaining the + // full 4-wide indent on top of its remaining 2 spaces. + let mut s = editor_with(" foo"); + exec(&s, "pmacs.editor.goto_byte(2)"); + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), " \n foo"); + assert_eq!(cursor(&s), 5, "cursor after the clipped indent"); +} + +#[test] +fn zero_indent_and_empty_buffer_match_plain_newline() { + let mut s = editor_with(""); + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), "\n"); + assert_eq!(cursor(&s), 1); + + let mut s = editor_with("foo"); + exec(&s, "pmacs.editor.goto_byte(3)"); + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), "foo\n"); + assert_eq!(cursor(&s), 4); +} + +#[test] +fn giant_minified_line_splits_correctly() { + // PR #109 round 1 finding 4: the indent scan is forward-chunked + // and stops at the first non-whitespace byte, so Enter at the end + // of a huge unindented line never materializes the line. This + // pins the behavior; boundedness is by construction. + let long = "x".repeat(64 * 1024); + let mut s = editor_with(&long); + exec(&s, &format!("pmacs.editor.goto_byte({})", long.len())); + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), format!("{long}\n")); + assert_eq!(cursor(&s) as usize, long.len() + 1); + + // And an indented giant line still carries exactly its indent. + let body = format!(" {long}"); + let mut s = editor_with(&body); + exec(&s, &format!("pmacs.editor.goto_byte({})", body.len())); + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), format!("{body}\n ")); +} + +#[test] +fn whitespace_only_line_copies_and_the_abandoned_line_keeps_its_whitespace() { + // Named non-goal (Q#AI3): no trailing-whitespace cleanup on the + // line being left behind. + let mut s = editor_with(" "); + exec(&s, "pmacs.editor.goto_byte(4)"); + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), " \n "); + assert_eq!(cursor(&s), 9); +} + +// --------------------------------------------------------------------------- +// Region type-over + selections (Q#AI4) +// --------------------------------------------------------------------------- + +#[test] +fn region_ret_is_one_replace_one_undo_step_and_clears_the_selection() { + let mut s = editor_with(" hello world"); + exec( + &s, + "pmacs.editor.begin_selection(4); pmacs.editor.goto_byte(15)", + ); + press(&mut s, KeyCode::Enter); + assert_eq!( + buffer_text(&s), + " \n ", + "the region is replaced by newline+indent in one edit" + ); + assert_eq!(cursor(&s), 9); + let region_active: bool = eval(&s, "return pmacs.editor.region() ~= nil"); + assert!(!region_active, "selection clears after a region RET"); + ctrl(&mut s, '/'); // buffer.undo, exactly once + assert_eq!( + buffer_text(&s), + " hello world", + "one undo restores the whole type-over" + ); +} + +#[test] +fn plain_ret_is_one_undo_step() { + let mut s = editor_with(" ab"); + exec(&s, "pmacs.editor.goto_byte(4)"); + press(&mut s, KeyCode::Enter); + assert_eq!(buffer_text(&s), " ab\n "); + ctrl(&mut s, '/'); + assert_eq!(buffer_text(&s), " ab"); +} + +#[test] +fn zero_length_selection_does_not_type_over_the_fresh_newline() { + // Q#AI4: an armed anchor at the cursor reports no region; the RET + // moves the cursor off it, so without the unconditional clear the + // next self-insert would replace the newline ("S-Left at BOF, + // RET, x" → "x"). + let mut s = editor_with(""); + exec(&s, "pmacs.editor.begin_selection(0)"); + press(&mut s, KeyCode::Enter); + type_str(&mut s, "x"); + assert_eq!(buffer_text(&s), "\nx", "the newline survives the 'x'"); + + // Q#AI9: the retained plain-newline escape hatch (through the + // fixed core arm) behaves the same. + let mut s = editor_with(""); + exec(&s, "pmacs.editor.begin_selection(0)"); + m_x(&mut s, "buffer.newline"); + type_str(&mut s, "x"); + assert_eq!(buffer_text(&s), "\nx"); +} + +// --------------------------------------------------------------------------- +// Intercept discipline (Q#AI5) +// --------------------------------------------------------------------------- + +#[test] +fn rejecting_intercept_reports_without_throwing_or_mutating() { + let mut s = editor_with(" a"); + exec(&s, "pmacs.editor.goto_byte(3)"); + 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) + "#, + ); + press(&mut s, KeyCode::Enter); + assert!(status(&s).contains("rejected"), "got: {:?}", status(&s)); + assert_eq!(buffer_text(&s), " a"); + assert_eq!(cursor(&s), 3, "no cursor motion on a rejected RET"); + press(&mut s, KeyCode::Enter); // allowed again: works + assert_eq!(buffer_text(&s), " a\n "); +} + +#[test] +fn relocating_intercept_moves_the_payload_but_does_not_teleport_the_cursor() { + // The only transform an insert admits (M6.4): moving its `pos`. + // The payload lands where the intercept sent it; the cursor is + // translated through the edit, NOT jumped to the remote site. + let mut s = editor_with(" abc"); + exec(&s, "pmacs.editor.goto_byte(5)"); + exec( + &s, + r#" + pmacs.buffer.add_intercept(pmacs.window.buffer(), function(op) + if op.kind == "insert" then + return { kind = "insert", pos = 0, bytes = op.bytes } + end + return nil + end) + "#, + ); + press(&mut s, KeyCode::Enter); + assert!(status(&s).contains("altered"), "got: {:?}", status(&s)); + assert_eq!( + buffer_text(&s), + "\n abc", + "the newline+indent payload landed at the intercept's position" + ); + assert_eq!( + cursor(&s), + 8, + "cursor shifted right by the inserted length (5+3), not teleported to the edit" + ); +} + +#[test] +fn shrinking_intercept_leaves_a_valid_cursor_and_no_selection() { + // Only a replace can shrink the buffer (M6.4): the intercept + // expands the replaced range past the payload. The cursor must be + // right-gravity-translated into the shrunken buffer — validity, + // not immobility — and the selection cleared. + let mut s = editor_with(" hello world wide"); + exec( + &s, + "pmacs.editor.begin_selection(2); pmacs.editor.goto_byte(7)", + ); + 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"] + 8, + bytes = op.bytes, + } + end + return nil + end) + "#, + ); + press(&mut s, KeyCode::Enter); + assert!(status(&s).contains("altered"), "got: {:?}", status(&s)); + assert_eq!(buffer_text(&s), " \n ide", "the expanded replace stands"); + let len: i64 = eval(&s, "return pmacs.window.buffer():len()"); + assert_eq!(cursor(&s), 5, "cursor translated to the edit's new end"); + assert!(cursor(&s) <= len, "cursor within the shrunken buffer"); + let region_active: bool = eval(&s, "return pmacs.editor.region() ~= nil"); + assert!(!region_active, "selection cleared under the same guard"); +} + +#[test] +fn context_switching_intercept_skips_fixup_and_leaves_the_new_context_alone() { + // Q#AI5 guard: an intercept may switch the active window/buffer + // (the registry borrow is released). The fix-up must not touch + // whatever is active afterwards. This proves the NEW context is + // untouched; the original window's state after such an intercept + // is the substrate-reconciliation deferral's territory. + let mut s = editor_with(" a"); + exec(&s, "pmacs.editor.goto_byte(3)"); + exec( + &s, + r#" + _G.orig = pmacs.window.buffer() + _G.other = pmacs.buffer.create("*other*") + pmacs.buffer.add_intercept(_G.orig, function(_op) + pmacs.window.switch_buffer(_G.other) + return nil + end) + "#, + ); + press(&mut s, KeyCode::Enter); + assert!( + status(&s).contains("context changed"), + "got: {:?}", + status(&s) + ); + let name: String = eval(&s, "return pmacs.window.buffer():name()"); + assert_eq!(name, "*other*", "the intercept's buffer switch stands"); + assert_eq!(cursor(&s), 0, "the new context's cursor is untouched"); + let orig: mlua::String = eval(&s, "return _G.orig:slice(0, _G.orig:len())"); + assert_eq!( + String::from_utf8_lossy(&orig.as_bytes()), + " a\n ", + "the edit itself landed in the original buffer" + ); +} + +// --------------------------------------------------------------------------- +// Search staleness through RET (Q#AI8) +// --------------------------------------------------------------------------- + +#[test] +fn accepted_search_navigation_fails_closed_after_ret() { + let mut s = editor_with("ind ind ind"); + ctrl(&mut s, 's'); + type_str(&mut s, "ind"); + press(&mut s, KeyCode::Enter); // accept: matches stay until an edit + exec(&s, "pmacs.editor.goto_byte(11)"); + press(&mut s, KeyCode::Enter); // auto-indent RET marks them stale + assert_eq!(buffer_text(&s), "ind ind ind\n"); + let at = cursor(&s); + exec(&s, "pmacs.editor.search_step(true)"); + assert_eq!( + cursor(&s), + at, + "post-accept navigation is a no-op once RET staled the matches" + ); +} + +#[test] +fn direct_lua_edit_stales_accepted_search_navigation() { + let mut s = editor_with("foo foo"); + ctrl(&mut s, 's'); + type_str(&mut s, "foo"); + press(&mut s, KeyCode::Enter); // accept + exec(&s, "pmacs.window.buffer():insert(0, \"zz\")"); // notify path + let at = cursor(&s); + exec(&s, "pmacs.editor.search_step(true)"); + assert_eq!( + cursor(&s), + at, + "a direct buf:insert must stale the matches like any edit" + ); +} + +// --------------------------------------------------------------------------- +// Substrate plumbing (Q#AI7) +// --------------------------------------------------------------------------- + +#[test] +fn after_edit_fires_exactly_once_per_ret_keybound_and_m_x() { + let mut s = editor_with(" a"); + exec(&s, "pmacs.editor.goto_byte(3)"); + exec( + &s, + "_G.ae = 0; pmacs.hook.add('buffer.after-edit', function() _G.ae = _G.ae + 1 end)", + ); + press(&mut s, KeyCode::Enter); + let n: i64 = eval(&s, "return _G.ae"); + assert_eq!(n, 1, "keybound RET fires after-edit once"); + m_x(&mut s, "edit.newline-and-indent"); + assert_eq!(buffer_text(&s), " a\n \n "); + let n: i64 = eval(&s, "return _G.ae"); + assert_eq!(n, 2, "M-x RET fires after-edit exactly once more"); +} + +#[test] +fn ret_between_kills_breaks_the_kill_chain() { + let mut s = editor_with("one\ntwo\nthree\n"); + ctrl(&mut s, 'k'); // kills "one"; line now blank, cursor 0 + press(&mut s, KeyCode::Enter); // rotates the command boundary + ctrl(&mut s, 'k'); // kills "\n" — must push fresh, not append + let ring: Vec = eval(&s, "return pmacs.killring.list()"); + assert_eq!( + ring, + vec!["\n", "one"], + "C-k, RET, C-k yields two ring entries (chain broken)" + ); +} + +#[test] +fn this_command_during_ret_is_the_new_command() { + let mut s = editor_with(""); + exec( + &s, + "pmacs.hook.add('buffer.after-edit', function() _G.tc = pmacs.editor.this_command() end)", + ); + press(&mut s, KeyCode::Enter); + let tc: String = eval(&s, "return _G.tc"); + assert_eq!(tc, "edit.newline-and-indent"); +} + +// --------------------------------------------------------------------------- +// Contexts RET must not disturb (ground truth: consumed before the keymap) +// --------------------------------------------------------------------------- + +#[test] +fn minibuffer_and_buffer_list_ret_are_unaffected() { + // The m_x helper itself proves minibuffer accept (used throughout + // this suite). The classic buffer list's RET is a buffer-local + // binding through normal dispatch (ground truth) — it must visit, + // not newline-and-indent into the list. + let mut s = editor_with("hello"); + ctrl(&mut s, 'x'); + ctrl(&mut s, 'b'); + let name: String = eval(&s, "return pmacs.window.buffer():name()"); + assert_eq!(name, "*buffer-list*"); + let listed = buffer_text(&s); + exec(&s, "_G.list = pmacs.window.buffer()"); + press(&mut s, KeyCode::Enter); // buffer-local RET: visit + let name: String = eval(&s, "return pmacs.window.buffer():name()"); + assert_ne!(name, "*buffer-list*", "RET visits instead of inserting"); + let list_after: String = eval( + &s, + "if not _G.list:is_valid() then return \"\" end \ + return _G.list:slice(0, _G.list:len())", + ); + assert!( + list_after == "" || list_after == listed, + "no newline landed in the buffer list" + ); +} + +#[test] +fn isearch_ret_accepts_instead_of_inserting() { + let mut s = editor_with("abc abc"); + ctrl(&mut s, 's'); + type_str(&mut s, "abc"); + press(&mut s, KeyCode::Enter); // isearch accept, consumed pre-keymap + assert_eq!( + buffer_text(&s), + "abc abc", + "RET during isearch accepts; no newline is inserted" + ); +} diff --git a/tests/auto_indent_crdt_acceptance.rs b/tests/auto_indent_crdt_acceptance.rs new file mode 100644 index 0000000..718f927 --- /dev/null +++ b/tests/auto_indent_crdt_acceptance.rs @@ -0,0 +1,152 @@ +// auto_indent_crdt_acceptance.rs --- RET's daemon CRDT round trip. + +//! Auto-indent daemon-side wire acceptance (Q#AI6, the second of the +//! two named GPU seams in docs/auto-indent-framing.md): a synthetic +//! attached replica sends pending optimistic self-inserts followed by +//! a round-tripped Enter, and the daemon must dispatch +//! `edit.newline-and-indent` and broadcast the resulting multi-byte +//! CRDT op back to the source replica. This is the daemon side of the +//! wire path the GPU frontend takes now that plain Enter is no longer +//! optimistic-eligible; the in-crate classifier test in `pmacs-gpu` +//! covers the frontend side of the seam. + +#![cfg(feature = "crdt")] + +use std::time::Duration; + +use pmacs::crdt::CrdtState; +use pmacs::protocol::{FrontendEvent, FrontendId, Key, KeyEvent, Modifiers}; +use pmacs::rope::CrdtOp as RopeCrdtOp; +use pmacs::transport::write_message; + +mod common; +use common::daemon::{TestDaemon, attach_multi}; + +/// Read the daemon's initial `BufferSnapshot` for a freshly-attached +/// replica stream (the daemon always emits it first). +fn read_initial_snapshot( + stream: &mut std::os::unix::net::UnixStream, +) -> (pmacs::buffer::BufferId, Vec) { + match pmacs::transport::read_message::(stream) + .expect("read initial BufferSnapshot") + { + pmacs::protocol::InstanceMessage::BufferSnapshot { + buffer_id, + crdt_snapshot, + } => (buffer_id, crdt_snapshot), + other => panic!("expected initial BufferSnapshot, got {other:?}"), + } +} + +/// Mutate the local replica, export the delta, and ship it as an +/// optimistic `FrontendEvent::CrdtOp` (the m10_11 idiom). +fn send_optimistic_op_from( + stream: &mut std::os::unix::net::UnixStream, + replica: &CrdtState, + frontend_id: FrontendId, + buffer_id: pmacs::buffer::BufferId, + mutate: F, +) where + F: FnOnce(&CrdtState), +{ + let v = replica.version(); + mutate(replica); + let op_bytes = replica + .export_updates_since(&v) + .expect("export updates after local mutation"); + write_message( + stream, + &FrontendEvent::CrdtOp { + frontend_id, + buffer_id, + op: RopeCrdtOp { + peer_id: frontend_id.0, + bytes: op_bytes, + }, + }, + ) + .expect("write CrdtOp"); +} + +/// Pump broadcast messages into the replica until it materializes +/// `expected` or the deadline passes. +fn pump_until( + stream: &mut std::os::unix::net::UnixStream, + replica: &CrdtState, + buffer_id: pmacs::buffer::BufferId, + expected: &str, + timeout: Duration, +) -> Result<(), String> { + let deadline = std::time::Instant::now() + timeout; + while std::time::Instant::now() < deadline { + if replica.materialize_string() == expected { + return Ok(()); + } + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + stream + .set_read_timeout(Some(remaining.min(Duration::from_millis(100)))) + .ok(); + match pmacs::transport::read_message::(stream) { + Ok(pmacs::protocol::InstanceMessage::CrdtOp { buffer_id: b, op }) if b == buffer_id => { + let _ = replica.import_updates(&op.bytes); + } + Ok(_) | Err(_) => {} + } + } + Err(format!( + "expected materialize {expected:?}, got {observed:?} after {timeout:?}", + observed = replica.materialize_string() + )) +} + +/// Pending optimistic self-inserts (`"··x"`, one op per keystroke, +/// mirroring GPU typing), then Enter as a round-tripped Key. The +/// daemon's dispatch must run `edit.newline-and-indent` — carrying +/// the two-space indent — and the multi-byte op must come back to the +/// source replica. A plain-newline dispatch would converge to +/// `" x\n"` instead and fail the assertion. +#[test] +fn round_tripped_enter_after_pending_optimistic_input_auto_indents() { + let daemon = TestDaemon::spawn(); + let (hello, mut stream) = attach_multi(&daemon); + let fid = hello.assigned_frontend_id; + + let (buffer_id, snap) = read_initial_snapshot(&mut stream); + let replica = CrdtState::new(fid.0).expect("CrdtState::new"); + replica.import_snapshot(&snap).expect("import_snapshot"); + + // Three pending optimistic self-inserts, ahead of the Enter. + send_optimistic_op_from(&mut stream, &replica, fid, buffer_id, |r| { + r.insert(0, " ").expect("insert space"); + }); + send_optimistic_op_from(&mut stream, &replica, fid, buffer_id, |r| { + r.insert(1, " ").expect("insert space"); + }); + send_optimistic_op_from(&mut stream, &replica, fid, buffer_id, |r| { + r.insert(2, "x").expect("insert x"); + }); + + // Enter round-trips (never optimistic since Q#AI1): the daemon + // applies the pending ops first — its cursor for this frontend + // tracks the optimistic post-edit position — then dispatches the + // keymap's RET binding. + write_message( + &mut stream, + &FrontendEvent::Key(KeyEvent { + frontend_id: fid, + key: Key::Enter, + mods: Modifiers::NONE, + timestamp_ns: 0, + }), + ) + .expect("send Enter"); + + pump_until( + &mut stream, + &replica, + buffer_id, + " x\n ", + Duration::from_secs(5), + ) + .expect("replica converges to the auto-indented text"); +}