From 35a2e3b667b2b1b0ef43d23b61045824d82fa5b9 Mon Sep 17 00:00:00 2001 From: Mohamed Hamidi Date: Sat, 13 Jun 2026 10:06:25 +0200 Subject: [PATCH] fix: undo counted changes in one step --- AGENTS.md | 6 ++-- CHANGELOG.md | 1 + src/index.ts | 30 ++++++++++------ src/vim.ts | 41 ++++++++++++--------- test/vim.test.ts | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 31 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0b5dacc..04b5a29 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,13 +57,13 @@ This API surface makes text objects (`ciw`, `di"`), direct cursor manipulation, ``` src/ - index.ts (342 lines) Plugin entry: intercept registration, action application - vim.ts (608 lines) Pure vim engine: state, handlers, command tables, types + index.ts (357 lines) Plugin entry: intercept registration, action application + vim.ts (615 lines) Pure vim engine: state, handlers, command tables, types leader.ts (73 lines) Leader key matching: matchesKeyLike, findMatchingLeader, leaderChar clipboard.ts (19 lines) writeClipboard() — cross-platform (pbcopy/xclip/xsel/wl-copy/clip.exe) version.ts (46 lines) Version constant, GitHub update check (cached daily) test/ - vim.test.ts (1268 lines) Characterization tests for all key handling branches + vim.test.ts (1300 lines) Characterization tests for all key handling branches leader.test.ts (125 lines) Unit tests for leader key matching functions ``` diff --git a/CHANGELOG.md b/CHANGELOG.md index 5003bd3..c215c9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Version ### Fixed +- Counted delete/change commands such as `3dw` now undo in one `u` press instead of one undo per repeated host command. - Leader key now works with modifier-based configurations like the default `ctrl+x`. Previously, `parseLeaderKey` expected vim-style `C-x` notation but OpenCode's keybinds API returns `ctrl+x` format, so the leader key was silently broken for any config with modifiers. - Leader detection supports all OpenCode modifier aliases (`control`, `alt`, `option`, `super`) and multiple leader bindings. - Optional chaining on `api.kv.set` to avoid crashes on older OpenCode versions. diff --git a/src/index.ts b/src/index.ts index ddbd5f8..59952ec 100644 --- a/src/index.ts +++ b/src/index.ts @@ -43,10 +43,10 @@ const plugin: TuiPluginModule = { let leaderPending = false; let leaderTimer: ReturnType | null = null; - // Snapshot for single-step undo of deleteRange operations. - // The host editor's undo system splits multi-line deletions into - // multiple entries, so we save/restore the buffer ourselves. - let undoSnapshot: { text: string; cursor: number } | null = null; + // Snapshots for single-step undo of vim changes. + // The host editor's undo system splits repeated commands into multiple + // entries, so we save/restore the buffer ourselves. + let undoSnapshots: Array<{ text: string; cursor: number }> = []; const prompt = { getLine: (n: number) => getInputText().split("\n")[n] ?? "", @@ -78,11 +78,12 @@ const plugin: TuiPluginModule = { } function applyActions(actions: Action[]) { + let keepUndoSnapshotForBatch = false; for (const action of actions) { // Any buffer-modifying action (other than our own deleteRange/undo) // invalidates the undo snapshot. - if (action.type === "cmd" || action.type === "insertText") { - undoSnapshot = null; + if ((action.type === "cmd" || action.type === "insertText") && !keepUndoSnapshotForBatch) { + undoSnapshots = []; } switch (action.type) { case "cmd": @@ -136,10 +137,6 @@ const plugin: TuiPluginModule = { const editor = api.renderer?.currentFocusedEditor; const eb = editor?.editBuffer; if (eb?.deleteRange) { - undoSnapshot = { - text: editor.plainText ?? "", - cursor: editor.cursorOffset ?? 0, - }; const text = editor.plainText ?? ""; const [sl, sc] = offsetToLineCol(text, action.start); const [el, ec] = offsetToLineCol(text, action.end + 1); @@ -147,7 +144,19 @@ const plugin: TuiPluginModule = { } break; } + case "saveUndoSnapshot": { + const editor = api.renderer?.currentFocusedEditor; + if (editor) { + undoSnapshots.push({ + text: editor.plainText ?? "", + cursor: editor.cursorOffset ?? 0, + }); + } + keepUndoSnapshotForBatch = true; + break; + } case "undo": { + const undoSnapshot = undoSnapshots.pop(); if (undoSnapshot) { const editor = api.renderer?.currentFocusedEditor; const eb = editor?.editBuffer; @@ -155,7 +164,6 @@ const plugin: TuiPluginModule = { eb.setText(undoSnapshot.text); editor.cursorOffset = undoSnapshot.cursor; } - undoSnapshot = null; } else { setTimeout(() => api.keymap.dispatchCommand("input.undo"), 0); } diff --git a/src/vim.ts b/src/vim.ts index 8543dc0..8370abe 100644 --- a/src/vim.ts +++ b/src/vim.ts @@ -10,6 +10,7 @@ export type Action = | { type: "yankSelection" } | { type: "clearSelection" } | { type: "deleteRange"; start: number; end: number } + | { type: "saveUndoSnapshot" } | { type: "undo" } | { type: "cursorTo"; offset: number } | { type: "selectRange"; start: number; end: number }; @@ -210,7 +211,7 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom pushN(actions, "input.delete", n); actions.push({ type: "insertText", text: key.repeat(n) }); state.pendingChar = null; - return { consume: true, actions }; + return finishUndoableChange(actions); } // Pending g prefix (gg, ge, etc.) @@ -282,12 +283,12 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom if (state.yankRegister) actions.push({ type: "yank", text: state.yankRegister }); actions.push({ type: "cmd", cmd: "prompt.paste" }); resetPending(state); - return { consume: true, actions }; + return finishUndoableChange(actions); } if (key === "X") { pushN(actions, "input.backspace", consumeCount(state)); - return { consume: true, actions }; + return finishUndoableChange(actions); } if (key === "J") { @@ -296,7 +297,7 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom actions.push({ type: "cmd", cmd: "input.line.end" }); actions.push({ type: "cmd", cmd: "input.delete" }); } - return { consume: true, actions }; + return finishUndoableChange(actions); } // Operators: d, c, y @@ -311,11 +312,13 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom state.yankRegister = text; actions.push({ type: "yank", text }); actions.push({ type: "toast", message: `${n} line${n > 1 ? "s" : ""} yanked`, duration: 1000 }); + resetPending(state); } else { pushN(actions, "input.delete.line", n); if (key === "c") enterInsert(state, actions); + else resetPending(state); + return finishUndoableChange(actions); } - state.pendingOp = null; return { consume: true, actions }; } state.pendingOp = key; @@ -325,13 +328,13 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom if (key === "D") { actions.push({ type: "cmd", cmd: "input.delete.to.line.end" }); resetPending(state); - return { consume: true, actions }; + return finishUndoableChange(actions); } if (key === "C") { actions.push({ type: "cmd", cmd: "input.delete.to.line.end" }); enterInsert(state, actions); - return { consume: true, actions }; + return finishUndoableChange(actions); } // Pending operator + e (end-of-word needs special handling) @@ -344,12 +347,12 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom state.yankRegister = text; actions.push({ type: "yank", text }); resetPending(state); - } else { - actions.push({ type: "deleteRange", start: offset, end: target }); - if (state.pendingOp === "c") enterInsert(state, actions); - else resetPending(state); + return { consume: true, actions }; } - return { consume: true, actions }; + actions.push({ type: "deleteRange", start: offset, end: target }); + if (state.pendingOp === "c") enterInsert(state, actions); + else resetPending(state); + return finishUndoableChange(actions); } // Pending operator + motion @@ -370,14 +373,14 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom pushN(actions, "input.delete.line", n + 1); if (state.pendingOp === "c") enterInsert(state, actions); else resetPending(state); - return { consume: true, actions }; + return finishUndoableChange(actions); } if (key === "k") { pushN(actions, "input.move.up", n); pushN(actions, "input.delete.line", n + 1); if (state.pendingOp === "c") enterInsert(state, actions); else resetPending(state); - return { consume: true, actions }; + return finishUndoableChange(actions); } if (key === "G") { consumeCount(state); @@ -386,7 +389,7 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom actions.push({ type: "deleteRange", start: offset, end: Math.max(0, text.length - 1) }); if (state.pendingOp === "c") enterInsert(state, actions); else resetPending(state); - return { consume: true, actions }; + return finishUndoableChange(actions); } const deleteCmd = DELETE_MOTION[key]; @@ -394,7 +397,7 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom pushN(actions, deleteCmd, n); if (state.pendingOp === "c") enterInsert(state, actions); else resetPending(state); - return { consume: true, actions }; + return finishUndoableChange(actions); } resetPending(state); @@ -429,7 +432,7 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom if (key === "x") { pushN(actions, "input.delete", consumeCount(state)); - return { consume: true, actions }; + return finishUndoableChange(actions); } if (key === "r") { @@ -574,6 +577,10 @@ function resetPending(state: VimState) { state.count = 0; } +function finishUndoableChange(actions: Action[]): HandlerResult { + return { consume: true, actions: [{ type: "saveUndoSnapshot" }, ...actions] }; +} + function consumeCount(state: VimState): number { const n = state.count || 1; state.count = 0; diff --git a/test/vim.test.ts b/test/vim.test.ts index 44b1926..be152d9 100644 --- a/test/vim.test.ts +++ b/test/vim.test.ts @@ -27,6 +27,10 @@ function deleteRanges(actions: Action[]): Array<{ start: number; end: number }> .map((a) => ({ start: a.start, end: a.end })); } +function saveUndoSnapshots(actions: Action[]): Action[] { + return actions.filter((a) => a.type === "saveUndoSnapshot"); +} + const ev = (name: string, opts?: { shift?: boolean; ctrl?: boolean; meta?: boolean; super?: boolean }) => ({ name, shift: opts?.shift ?? false, @@ -376,6 +380,9 @@ describe("handleNormalKey — e motion", () => { const r = handleNormalKey(state, "e", ev("e"), ePrompt); expect(deleteRanges(r.actions)).toEqual([{ start: 0, end: 4 }]); expect(state.mode).toBe("normal"); + // The deleteRange goes through finishUndoableChange, so the snapshot + // comes from a single source (the saveUndoSnapshot action), not index.ts. + expect(saveUndoSnapshots(r.actions)).toHaveLength(1); }); it("ce deletes from cursor to end of word and enters insert", () => { @@ -416,6 +423,18 @@ describe("handleNormalKey — operators", () => { expect(cmds(r.actions)).toEqual(["input.delete.word.forward"]); }); + it("3dw saves one undo snapshot around the repeated deletes", () => { + handleNormalKey(state, "3", ev("3"), mockPrompt); + handleNormalKey(state, "d", ev("d"), mockPrompt); + const r = handleNormalKey(state, "w", ev("w"), mockPrompt); + expect(saveUndoSnapshots(r.actions)).toHaveLength(1); + expect(cmds(r.actions)).toEqual([ + "input.delete.word.forward", + "input.delete.word.forward", + "input.delete.word.forward", + ]); + }); + it("d$ dispatches input.delete.to.line.end", () => { handleNormalKey(state, "d", ev("d"), mockPrompt); const r = handleNormalKey(state, "$", ev("4", { shift: true }), mockPrompt); @@ -505,6 +524,9 @@ describe("handleNormalKey — dG and cG", () => { const r = handleNormalKey(state, "G", ev("g", { shift: true }), midPrompt); expect(deleteRanges(r.actions)).toEqual([{ start: 12, end: 33 }]); expect(state.mode).toBe("normal"); + // Single snapshot source: the saveUndoSnapshot action, not a second + // push inside the deleteRange handler. + expect(saveUndoSnapshots(r.actions)).toHaveLength(1); }); it("cG deletes from cursor to buffer end, enters insert", () => { @@ -1265,4 +1287,75 @@ describe("undo snapshot — deleteRange + u", () => { await new Promise((r) => setTimeout(r, 20)); expect(dispatched).toContain("input.undo"); }); + + it("u after 3dw restores the full buffer via editBuffer.setText", async () => { + const original = "hello world second line third line"; + const { press, calls, dispatched, getCursor } = await setup(original, 0); + + press("3"); + press("d"); + press("w"); + + calls.length = 0; + press("u"); + + expect(calls).toContainEqual({ method: "setText", args: [original] }); + expect(getCursor()).toBe(0); + expect(dispatched).not.toContain("input.undo"); + }); + + it("u after 3dw then dd unwinds the snapshot stack one step per press", async () => { + const original = "hello world second line third line"; + const { press, calls, dispatched } = await setup(original, 0); + + // Two stacked undoable changes → two snapshots on the stack. + press("3"); + press("d"); + press("w"); + press("d"); + press("d"); + + // First u pops the dd snapshot, second pops the 3dw snapshot — each a + // local restore via setText, never the host's input.undo. + calls.length = 0; + dispatched.length = 0; + press("u"); + expect(calls.some((c) => c.method === "setText")).toBe(true); + expect(dispatched).not.toContain("input.undo"); + + calls.length = 0; + press("u"); + expect(calls.some((c) => c.method === "setText")).toBe(true); + expect(dispatched).not.toContain("input.undo"); + + // Stack is now empty — a third u falls through to host undo. + calls.length = 0; + press("u"); + expect(calls.every((c) => c.method !== "setText")).toBe(true); + await new Promise((r) => setTimeout(r, 20)); + expect(dispatched).toContain("input.undo"); + }); + + it("u after 3dw then an insert-mode edit falls back to host input.undo", async () => { + const { press, calls, dispatched } = await setup("hello world second line third line", 0); + + press("3"); + press("d"); + press("w"); + + // Enter insert and modify the buffer. The insert edit emits an + // insertText action, which clears the vim snapshot stack. + press("i"); + press("tab"); + press("escape"); + + calls.length = 0; + dispatched.length = 0; + press("u"); + + expect(calls.every((c) => c.method !== "setText")).toBe(true); + // input.undo is dispatched via setTimeout + await new Promise((r) => setTimeout(r, 20)); + expect(dispatched).toContain("input.undo"); + }); });