From db073cc6689c98e6757f3626a8b2ff362da38b39 Mon Sep 17 00:00:00 2001
From: Levi Neuwirth <ln@levineuwirth.org>
Date: Mon, 15 Jun 2026 20:00:53 -0400
Subject: [PATCH] CUA type-over is a single undo step (Q#U1)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Typing over a selection composed two edits in Lua —
delete_region() + insert_char() — so it recorded two undo steps:
one undo left the half-replaced text, two restored the original.
Undo granularity is per apply_edit in both modes (v0.1 pushes one
UndoEntry per edit; CRDT commits per edit via export and groups by
commit, with record_checkpoint unused), so the fix is to make
type-over one edit.

New core EditorCore::insert_char_over_region emits a single
EditOp::Replace when a region is active (cursor past the inserted
bytes, selection cleared) and delegates to insert_char otherwise.
The three type-over commands (buffer.newline / tab / self-insert)
call it via a new Lua binding instead of the delete+insert pair.
delete_region and insert_char are unchanged for their other
callers; region-aware backspace/delete already emit one op.

Verified one undo unit in BOTH modes (dual_mode
replace_is_a_single_undo_step covers v01 + crdt — a CRDT Replace is
delete-then-insert internally but one commit) plus an end-to-end
acceptance test through the key-dispatch path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 builtin/commands/default.lua      | 16 ++++----
 docs/cua-typeover-undo-framing.md | 66 +++++++++++++++++++++++++++++++
 src/buffer.rs                     | 21 ++++++++++
 src/editor_core.rs                | 26 ++++++++++++
 src/lua_bindings.rs               | 11 ++++++
 tests/cua_region_acceptance.rs    | 28 +++++++++++++
 6 files changed, 161 insertions(+), 7 deletions(-)
 create mode 100644 docs/cua-typeover-undo-framing.md

diff --git a/builtin/commands/default.lua b/builtin/commands/default.lua
index f210f39..8b4c992 100644
--- a/builtin/commands/default.lua
+++ b/builtin/commands/default.lua
@@ -101,19 +101,21 @@ cmd { name = "cursor.select-line-start",
 cmd { name = "cursor.select-line-end",
       description = "Extend selection to end of line.",
       fn = function() ensure_anchor(); ed.move_line_end() end }
--- CUA type-over: inserting with an active selection replaces it
--- (`delete_region` is a no-op without one). The pmacs-gpu frontend
--- relies on this: its optimistic-insert path detects an own-window
--- selection and round-trips the key so these commands run.
+-- CUA type-over: inserting with an active selection replaces it in a
+-- SINGLE edit (one undo step — `insert_char_over_region` emits one
+-- `Replace`, not a `delete` + `insert` pair). Without a selection it
+-- is a plain insert. The pmacs-gpu frontend relies on this: its
+-- optimistic-insert path detects an own-window selection and
+-- round-trips the key so these commands run daemon-side.
 cmd { name = "buffer.newline",
       description = "Insert a newline at the cursor, replacing the active region.",
-      fn = function() ed.delete_region(); ed.insert_char(10) end }
+      fn = function() ed.insert_char_over_region(10) end }
 cmd { name = "buffer.tab",
       description = "Insert a tab at the cursor, replacing the active region.",
-      fn = function() ed.delete_region(); ed.insert_char(9) end }
+      fn = function() ed.insert_char_over_region(9) end }
 cmd { name = "buffer.self-insert",
       description = "Insert the codepoint argument at the cursor, replacing the active region.",
-      fn = function(codepoint) ed.delete_region(); ed.insert_char(codepoint) end }
+      fn = function(codepoint) ed.insert_char_over_region(codepoint) end }
 
 -- History --------------------------------------------------------------------
 
diff --git a/docs/cua-typeover-undo-framing.md b/docs/cua-typeover-undo-framing.md
new file mode 100644
index 0000000..e9f834d
--- /dev/null
+++ b/docs/cua-typeover-undo-framing.md
@@ -0,0 +1,66 @@
+# CUA type-over undo grouping — framing pass
+
+Date: 2026-06-15. The region-aware type-over (PR #60-era) replaces a
+selection by composing two ops in Lua —
+`ed.delete_region(); ed.insert_char(cp)` (builtin/commands/default.lua
+`buffer.{newline,tab,self-insert}`). That's two `apply_edit` calls,
+hence two undo steps: undo once leaves the half-replaced text, undo
+twice restores the original. We want one atomic undo step.
+
+## Survey facts
+
+- Undo granularity is **per `apply_edit`** in both modes. v0.1 pushes
+  one `UndoEntry` per edit (buffer.rs:1162). CRDT mode bypasses that
+  stack and lets loro's `UndoManager` group ops; each `apply_edit`
+  commits via `export_updates_since` (buffer.rs:1061), and nothing
+  calls `record_checkpoint` (it's dead code) — so the undo unit is
+  effectively the commit, i.e. one per `apply_edit`.
+- `EditOp::Replace { range, bytes }` already exists end to end:
+  `rope.replace` in v0.1 (buffer.rs:1111), delete-then-insert on the
+  loro text in CRDT (buffer.rs:1028) — but **one** `apply_edit`, so
+  one undo unit either way.
+- Type-over always runs daemon-side: with a selection, pmacs-gpu
+  round-trips the key rather than optimistically applying (the comment
+  at default.lua:104), so the daemon's command does the edit and
+  broadcasts the result. No frontend-undo coordination needed.
+
+## Q#U1 — mechanism
+
+**Stance: model type-over as a single `EditOp::Replace`, not a
+delete+insert pair.** This is the *correct* representation (a
+type-over is one replace, not two edits) and gives one undo step in
+both modes by construction — no undo-group / transaction machinery,
+no reliance on loro's time-based merge interval (which is unset). A
+general `begin/end_undo_group` boundary was considered and rejected
+as over-built for the one concrete case; it can be inducted later if a
+third multi-edit command needs it.
+
+## Q#U2 — surface
+
+**Stance: a core `insert_char_over_region(ch)` + one Lua binding.**
+Region active → one `Replace(region, ch_bytes)`, cursor → region
+start + len, selection cleared. No region → delegate to the existing
+`insert_char` (unchanged Insert path). The three type-over commands
+call the single binding with their codepoint (10 / 9 / arg).
+`delete_region` and `insert_char` stay (other callers); only the
+type-over composition moves into the core. Region-aware
+backspace/delete already emit one `delete_region` op, so they're
+already one undo step — untouched.
+
+## Predicted findings (categorical bets)
+
+1. **CRDT undo-unit assumption**: the per-commit grouping reasoning is
+   inferred from the code, not loro docs — the dual-mode test is the
+   proof; if CRDT still shows two steps, loro is checkpointing between
+   the internal delete and insert and an explicit boundary is needed.
+2. **Cursor/selection after replace**: an off-by-the-inserted-length
+   cursor or a lingering selection in some multi-byte / empty-insert
+   edge — pinned by an editor_core test.
+
+## Session plan
+
+One commit: core `insert_char_over_region`, Lua binding, rewire the
+three commands, a dual-mode buffer test (Replace = one undo step) and
+an editor_core / acceptance test (type-over over a selection = one
+undo step, cursor + selection correct). Manual validation deferred to
+the user as usual.
diff --git a/src/buffer.rs b/src/buffer.rs
index e01a683..c90578a 100644
--- a/src/buffer.rs
+++ b/src/buffer.rs
@@ -2071,6 +2071,27 @@ mod tests {
         assert!(matches!(b.undo(), Err(BufferError::NothingToUndo)));
     });
 
+    dual_mode_test!(replace_is_a_single_undo_step, |_make, make_bytes| {
+        // CUA type-over models "replace the selection with the typed
+        // char" as one `EditOp::Replace`, so it must undo in ONE step
+        // in both modes — including CRDT, where a Replace is a
+        // delete-then-insert on the loro text but still a single
+        // `apply_edit` (one commit, one undo unit). If it were two
+        // units, the first undo would leave "hel" and the second
+        // would be needed to reach "hello".
+        let mut b = make_bytes("test", b"hello");
+        b.apply_edit(EditOp::Replace {
+            range: Range::new(2, 5),
+            bytes: b"X",
+        })
+        .unwrap();
+        assert_eq!(collect(&b), b"heX");
+
+        b.undo().unwrap();
+        assert_eq!(collect(&b), b"hello", "type-over undoes in one step");
+        assert!(matches!(b.undo(), Err(BufferError::NothingToUndo)));
+    });
+
     dual_mode_test!(redo_replays_undone_edit, |_make, make_bytes| {
         let mut b = make_bytes("test", b"abc");
         b.apply_edit(EditOp::Insert {
diff --git a/src/editor_core.rs b/src/editor_core.rs
index 2ccc500..5ad0bd9 100644
--- a/src/editor_core.rs
+++ b/src/editor_core.rs
@@ -954,6 +954,32 @@ impl EditorCore {
         self.active_window_mut().cursor += bytes.len() as u64;
     }
 
+    /// CUA type-over: insert `ch`, replacing the active region if one
+    /// exists. With a region this is a *single* `EditOp::Replace` — one
+    /// undo step — rather than the former `delete_region()` +
+    /// `insert_char()` pair, which recorded two. With no region it
+    /// delegates to [`Self::insert_char`] (a plain insert). The cursor
+    /// 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);
+            return;
+        };
+        self.active_window_mut().goal_col = None;
+        let mut buf = [0u8; 4];
+        let bytes = ch.encode_utf8(&mut buf).as_bytes();
+        if let Err(e) = self.apply_active_edit(EditOp::Replace {
+            range: Range { start: lo, end: hi },
+            bytes,
+        }) {
+            self.status = format!("replace failed: {e}");
+            return;
+        }
+        let aw = self.active_window_mut();
+        aw.cursor = lo + bytes.len() as u64;
+        aw.selection = None;
+    }
+
     /// Delete the codepoint immediately before the cursor.
     pub fn backspace(&mut self) {
         self.active_window_mut().goal_col = None;
diff --git a/src/lua_bindings.rs b/src/lua_bindings.rs
index 2f82183..16910dd 100644
--- a/src/lua_bindings.rs
+++ b/src/lua_bindings.rs
@@ -11411,6 +11411,17 @@ fn install_editing(editor: &Table, lua: &Lua, core: &SharedCore) -> mlua::Result
             })?,
         )?;
     }
+    {
+        let cc = core.clone();
+        editor.set(
+            "insert_char_over_region",
+            lua.create_function(move |_, codepoint: i64| {
+                let ch = char_from_lua_codepoint(codepoint)?;
+                cc.borrow_mut().insert_char_over_region(ch);
+                Ok(())
+            })?,
+        )?;
+    }
     Ok(())
 }
 
diff --git a/tests/cua_region_acceptance.rs b/tests/cua_region_acceptance.rs
index e7e6676..c4c16d7 100644
--- a/tests/cua_region_acceptance.rs
+++ b/tests/cua_region_acceptance.rs
@@ -147,6 +147,34 @@ fn typing_replaces_the_shift_selected_region() {
     assert_eq!(cursor, 4);
 }
 
+#[test]
+fn type_over_is_a_single_undo_step() {
+    let mut s = EditorState::new();
+    type_str(&mut s, "hello");
+
+    // Select "llo" (region [2, 5)) and type 'X' → "heX".
+    for _ in 0..3 {
+        s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Left, KeyModifiers::SHIFT));
+    }
+    s.dispatch_key(
+        FrontendId::LOCAL,
+        key(KeyCode::Char('X'), KeyModifiers::SHIFT),
+    );
+    let (text, _, _) = probe(&s);
+    assert_eq!(text, "heX");
+
+    // A single undo restores the pre-type-over text — not the
+    // half-replaced "hel" the old delete-then-insert pair left after
+    // one undo.
+    s.lua_host
+        .lua()
+        .load("pmacs.editor.undo()")
+        .exec()
+        .expect("undo");
+    let (text, _, _) = probe(&s);
+    assert_eq!(text, "hello", "type-over undoes in one step");
+}
+
 #[test]
 fn delete_forward_deletes_the_shift_selected_region() {
     let mut s = EditorState::new();