From 10dc662f49863d57742c6ef0ebfa81c8b6803a35 Mon Sep 17 00:00:00 2001 From: ori Date: Sat, 30 May 2026 23:43:12 +0300 Subject: [PATCH] fix: read cursor position from editor widget instead of lineTracker lineTracker drifted on clicks, arrow keys, and word motions, causing yy to yank the wrong line. Replaced with visualCursor.logicalRow read through PromptAccess.getCursorLine(). --- AGENTS.md | 11 +++++----- BACKLOG.md | 16 +++++++------- CHANGELOG.md | 6 +++++- README.md | 1 - src/index.ts | 1 + src/vim.ts | 18 ++++------------ test/vim.test.ts | 56 +++++++++++++++++++++++++++++------------------- 7 files changed, 58 insertions(+), 51 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a5f169a..70b2766 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,7 @@ vimcode is a TUI plugin for [OpenCode](https://opencode.ai). Before working on i ### Editor widget API -`api.renderer.currentFocusedEditor` (same object as `currentFocusedRenderable`) exposes the full underlying Textarea widget. This is not part of the documented plugin API but is stable and available at runtime. The current codebase only uses `plainText`, `insertText()`, and `editorView` — most of the surface below is untapped. +`api.renderer.currentFocusedEditor` (same object as `currentFocusedRenderable`) exposes the underlying Textarea widget. Not part of the documented plugin API, but stable and available at runtime. The codebase currently uses `plainText`, `cursorOffset`, `visualCursor`, `cursorStyle`, `insertText()`, and `editorView`. The rest of the surface below is available but unused. **Top-level properties (read/write):** - `cursorOffset: number` — absolute cursor position, readable and writable @@ -53,12 +53,12 @@ This API surface makes text objects (`ciw`, `di"`), direct cursor manipulation, ``` src/ - index.ts (134 lines) Plugin entry: intercept registration, action application - vim.ts (469 lines) Pure vim engine: state, handlers, command tables, types + index.ts (136 lines) Plugin entry: intercept registration, action application + vim.ts (459 lines) Pure vim engine: state, handlers, command tables, types 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 (671 lines) Characterization tests for all key handling branches + vim.test.ts (676 lines) Characterization tests for all key handling branches ``` **Data flow:** @@ -109,7 +109,6 @@ To add a new motion that works with operators: ### Known limitations - **`g` fires immediately as `input.buffer.home`** — should wait for a second `g` (needs sequence state). Single `g` = go to top, which is wrong for vim. -- **`lineTracker` drifts** — only j/k/G/g/o update it. Clicks, arrow keys, word motions don't. `yy` can yank the wrong line. Solvable now via `cursorOffset` + `visualCursor` — the tracker can be replaced with direct cursor reads. - **`setTimeout` dispatch** — commands are deferred to avoid re-entrancy. Multi-command sequences (like `O` = home + newline + up) rely on ordered setTimeout execution, which works in practice but isn't guaranteed by spec. Many of these can now be replaced with direct widget manipulation (e.g., setting `cursorOffset`, calling `insertText`). ## Development @@ -124,6 +123,8 @@ The `dev-tui.json` config is picked up only by `just dev`. Running `opencode` no ## Git Workflow +**Never commit, push, or create PRs unless explicitly asked.** Present the changes and wait for the human to decide when to commit. + All changes go through pull requests. Direct pushes to `main` are blocked. CI (`just check`) must pass before merge. PRs are squash-merged — the PR title becomes the commit on `main`. Branch naming: `type/description` — e.g. `feat/replace-char`, `fix/escape-handling`. Types match commit prefixes (`feat`, `fix`, `refactor`, `chore`, `test`, `docs`). diff --git a/BACKLOG.md b/BACKLOG.md index 8ad5bfe..fcf73c9 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -1,10 +1,10 @@ # Backlog -Prioritized list of improvements for vimcode. Items within each category are ordered by priority. +Ordered by priority within each category. ## Stability / Bug fixes -1. **Replace `lineTracker` with direct cursor reads.** `lineTracker` drifts whenever the cursor moves by means other than j/k/G/g/o (clicks, arrow keys, word motions). This causes `yy` to yank the wrong line. `visualCursor.logicalRow` gives the real line — read it directly. +1. ~~**Replace `lineTracker` with direct cursor reads.**~~ Done. 2. **Fix `gg` requiring two keypresses.** Single `g` fires `input.buffer.home` immediately. Real vim waits for a second `g`. Add pending-key state for `g` with a timeout or second-key check, similar to how `r` already works with `pendingChar`. @@ -16,9 +16,9 @@ Prioritized list of improvements for vimcode. Items within each category are ord 1. ~~**Set cursor style via `cursorStyle` property instead of DECSCUSR escapes.**~~ Done. -2. **Replace `dispatchCommand`-based motions with direct cursor manipulation.** Motions like h/l/j/k/w/b can use `moveCursorLeft/Right/Up/Down()` or write `cursorOffset` directly instead of dispatching `input.move.*` commands through setTimeout. Reduces latency and eliminates re-entrancy concerns. +2. **Replace `dispatchCommand`-based motions with direct cursor manipulation.** Motions like h/l/j/k/w/b can call `moveCursorLeft/Right/Up/Down()` or write `cursorOffset` directly instead of dispatching `input.move.*` commands through setTimeout. -3. **Replace selection commands with `setSelection`/`setSelectionInclusive`.** Visual mode currently dispatches `input.select.*` commands. The widget exposes `setSelection(start, end)` and `setSelectionInclusive(start, end)` — use these for immediate, accurate selections. +3. **Replace selection commands with `setSelection`/`setSelectionInclusive`.** Visual mode currently dispatches `input.select.*` commands. The widget has `setSelection(start, end)` and `setSelectionInclusive(start, end)`, which would be immediate and accurate. 4. **Replace `yankSelection` setTimeout with synchronous read.** The current `yankSelection` action defers to let select commands finish. With direct `setSelection` + `getSelectedText()`, the yank can happen synchronously. @@ -26,7 +26,7 @@ Prioritized list of improvements for vimcode. Items within each category are ord ## New features -1. **Text objects (`ciw`, `diw`, `yiw`, `ci"`, `di"`, `da(`, etc.).** Now feasible with cursor position access. Read `plainText` + `cursorOffset`, compute the object range in pure logic, apply the edit via `setSelection` + `deleteSelectedText` or direct text manipulation. Start with word and quote objects, then add bracket/paren. +1. **Text objects (`ciw`, `diw`, `yiw`, `ci"`, `di"`, `da(`, etc.).** Feasible now that we have cursor position access. Read `plainText` + `cursorOffset`, compute the object range in pure logic, apply the edit via `setSelection` + `deleteSelectedText` or direct text manipulation. Start with word and quote objects, then add bracket/paren. 2. **Visual-line mode (`V`).** The widget's `getLineInfo()` and `setSelection()` make line-wise selection straightforward. Extend the existing visual mode with a `visual-line` variant. @@ -38,13 +38,13 @@ Prioritized list of improvements for vimcode. Items within each category are ord 6. **Custom keymaps.** User-configurable key remapping per mode via `tui.json` options. Common requests: `jk`/`kj` to exit insert mode, `Y` mapped to `y$`. Needs multi-key sequence support with a configurable timeout. -7. **Pending key display.** Show partial key sequences (like `d` waiting for a motion, or the count accumulator) somewhere visible. Currently these are invisible — the user doesn't know vimcode is waiting for more input. +7. **Pending key display.** Show partial key sequences (like `d` waiting for a motion, or the count accumulator) somewhere visible. Right now these are invisible, so the user doesn't know vimcode is waiting for more input. -8. **Yank flash.** Brief highlight on yanked text using `selectionBg`/`selectionFg` with a short timer (200-300ms). Gives visual confirmation like Neovim's `vim.highlight.on_yank()`. +8. **Yank flash.** Brief highlight on yanked text using `selectionBg`/`selectionFg` with a short timer (200-300ms), like Neovim's `vim.highlight.on_yank()`. 9. **Completion-aware j/k.** When the cursor follows `@` or `/` (autocomplete triggers), normal-mode j/k should navigate the completion popup rather than move the cursor. -10. **Persistent mode indicator.** Replace the fading toast with a persistent visual. Blocked by the no-external-imports limitation for slot-based UI, but could potentially use `api.renderer.keyInput.processParsedKey()` or find another approach. +10. **Persistent mode indicator.** Replace the fading toast with something persistent. Blocked by the no-external-imports limitation for slot-based UI. Might be possible via `api.renderer.keyInput.processParsedKey()` or another workaround. ## Polish diff --git a/CHANGELOG.md b/CHANGELOG.md index a2035aa..852bdd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,11 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Version ### Changed -- Cursor shape now uses the editor widget's `cursorStyle` property instead of writing DECSCUSR escape sequences to stdout. Works in terminals that don't support DECSCUSR (e.g. macOS Terminal.app). +- Cursor shape uses the editor widget's `cursorStyle` property instead of writing DECSCUSR escape sequences to stdout. Works in terminals without DECSCUSR support (e.g. macOS Terminal.app). + +### Fixed + +- `yy` reads the cursor position directly from the editor widget instead of a line counter. The old counter drifted on clicks, arrow keys, and word motions, so `yy` would yank the wrong line. ## [0.9.0] — 2026-05-29 diff --git a/README.md b/README.md index ae02b97..8d3d2bf 100644 --- a/README.md +++ b/README.md @@ -159,7 +159,6 @@ All normal-mode motions work for extending the selection: `h` `j` `k` `l` `w` `b - `gg` - single `g` goes to buffer start immediately, doesn't wait for a second keypress - `dG`, `cG` - delete/change to buffer end not yet implemented (`yG` works) - `e` behaves the same as `w` - the host doesn't expose a separate "end of word" command -- `yy` accuracy - line position is tracked with a counter that drifts on clicks and arrow keys - No persistent mode indicator - the toast fades after about a second. Cursor shape is the persistent signal, but a status bar indicator would need the host's SolidJS runtime, which external plugins can't access. Configurable key bindings are next once the core vim coverage stabilizes. diff --git a/src/index.ts b/src/index.ts index 1ace795..0fa70f8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -13,6 +13,7 @@ const plugin: TuiPluginModule = { const prompt = { getLine: (n: number) => getInputText().split("\n")[n] ?? "", getLineCount: () => getInputText().split("\n").length, + getCursorLine: () => api.renderer?.currentFocusedEditor?.visualCursor?.logicalRow ?? 0, }; // api.prompt doesn't exist on the TUI plugin API. The actual text lives diff --git a/src/vim.ts b/src/vim.ts index 2d05ee4..3122712 100644 --- a/src/vim.ts +++ b/src/vim.ts @@ -20,7 +20,6 @@ export type VimState = { pendingOp: Operator; pendingChar: "r" | null; count: number; - lineTracker: number; yankRegister: string; }; @@ -36,6 +35,7 @@ export type KeyEvent = { export type PromptAccess = { getLine: (n: number) => string; getLineCount: () => number; + getCursorLine: () => number; }; export const MOTIONS: Record = { @@ -81,7 +81,7 @@ const PASS: HandlerResult = { consume: false, actions: [] }; const _CONSUME: HandlerResult = { consume: true, actions: [] }; export function createVimState(): VimState { - return { mode: "insert", pendingOp: null, pendingChar: null, count: 0, lineTracker: 0, yankRegister: "" }; + return { mode: "insert", pendingOp: null, pendingChar: null, count: 0, yankRegister: "" }; } export function translateKey(ev: KeyEvent): string { @@ -99,7 +99,6 @@ export function translateKey(ev: KeyEvent): string { export function handleInsertKey(state: VimState, _key: string, ev: KeyEvent): HandlerResult { if (ev.name === "escape") { state.mode = "normal"; - state.lineTracker = 0; return { consume: true, actions: [{ type: "mode", mode: "normal" }] }; } if (ev.name === "return" && ev.ctrl) { @@ -217,8 +216,9 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom if (state.pendingOp === key) { const n = consumeCount(state); if (key === "y") { + const cursorLine = prompt.getCursorLine(); const lines: string[] = []; - for (let i = 0; i < n; i++) lines.push(prompt.getLine(state.lineTracker + i)); + for (let i = 0; i < n; i++) lines.push(prompt.getLine(cursorLine + i)); const text = `${lines.join("\n")}\n`; state.yankRegister = text; actions.push({ type: "yank", text }); @@ -295,14 +295,12 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom return { consume: true, actions }; } pushN(actions, MOTIONS[key], n); - updateLineTracker(state, key, n, prompt); return { consume: true, actions }; } // gg (buffer home) if (key === "g") { actions.push({ type: "cmd", cmd: "input.buffer.home" }); - state.lineTracker = 0; resetPending(state); return { consume: true, actions }; } @@ -351,7 +349,6 @@ export function handleNormalKey(state: VimState, key: string, ev: KeyEvent, prom if (key === "o") { actions.push({ type: "cmd", cmd: "input.line.end" }); actions.push({ type: "cmd", cmd: "input.newline" }); - state.lineTracker++; enterInsert(state, actions); return { consume: true, actions }; } @@ -457,13 +454,6 @@ function pushN(actions: Action[], cmd: string, n: number) { for (let i = 0; i < n; i++) actions.push({ type: "cmd", cmd }); } -function updateLineTracker(state: VimState, key: string, n: number, prompt: PromptAccess) { - if (key === "j") state.lineTracker += n; - else if (key === "k") state.lineTracker = Math.max(0, state.lineTracker - n); - else if (key === "G") state.lineTracker = prompt.getLineCount() - 1; - else if (key === "g") state.lineTracker = 0; -} - function isInputEmpty(prompt: PromptAccess): boolean { return prompt.getLineCount() === 1 && prompt.getLine(0) === ""; } diff --git a/test/vim.test.ts b/test/vim.test.ts index fad22af..0d49a5e 100644 --- a/test/vim.test.ts +++ b/test/vim.test.ts @@ -25,11 +25,13 @@ const ev = (name: string, opts?: { shift?: boolean; ctrl?: boolean; meta?: boole const mockPrompt: PromptAccess = { getLine: (n) => ["hello world", "second line", "third line"][n] ?? "", getLineCount: () => 3, + getCursorLine: () => 0, }; const emptyPrompt: PromptAccess = { getLine: () => "", getLineCount: () => 1, + getCursorLine: () => 0, }; let state: VimState; @@ -406,12 +408,10 @@ describe("handleNormalKey — insert entries", () => { expect(state.mode).toBe("insert"); }); - it("o dispatches input.line.end + input.newline, enters insert, lineTracker increments", () => { - const tracker = state.lineTracker; + it("o dispatches input.line.end + input.newline, enters insert", () => { const r = handleNormalKey(state, "o", ev("o"), mockPrompt); expect(cmds(r.actions)).toEqual(["input.line.end", "input.newline"]); expect(state.mode).toBe("insert"); - expect(state.lineTracker).toBe(tracker + 1); }); it("O dispatches input.line.home + input.newline + input.move.up, enters insert", () => { @@ -421,30 +421,42 @@ describe("handleNormalKey — insert entries", () => { }); }); -// ── handleNormalKey — line tracker ────────────────────────── +// ── handleNormalKey — yy uses cursor position ───────────── -describe("handleNormalKey — line tracker", () => { - it("j increments lineTracker", () => { - const before = state.lineTracker; - handleNormalKey(state, "j", ev("j"), mockPrompt); - expect(state.lineTracker).toBe(before + 1); - }); - - it("k clamps lineTracker at 0", () => { - state.lineTracker = 0; - handleNormalKey(state, "k", ev("k"), mockPrompt); - expect(state.lineTracker).toBe(0); +describe("handleNormalKey — yy uses cursor position", () => { + it("yy yanks the line at getCursorLine, not a tracked counter", () => { + const prompt: PromptAccess = { + getLine: (n) => ["first", "second", "third"][n] ?? "", + getLineCount: () => 3, + getCursorLine: () => 1, + }; + handleNormalKey(state, "y", ev("y"), prompt); + const r = handleNormalKey(state, "y", ev("y"), prompt); + expect(state.yankRegister).toBe("second\n"); + expect(r.actions.some((a) => a.type === "yank" && a.text === "second\n")).toBe(true); }); - it("G sets lineTracker to last line", () => { - handleNormalKey(state, "G", ev("g", { shift: true }), mockPrompt); - expect(state.lineTracker).toBe(2); // getLineCount() - 1 + it("2yy from cursor line 1 yanks lines 1 and 2", () => { + const prompt: PromptAccess = { + getLine: (n) => ["first", "second", "third"][n] ?? "", + getLineCount: () => 3, + getCursorLine: () => 1, + }; + handleNormalKey(state, "2", ev("2"), prompt); + handleNormalKey(state, "y", ev("y"), prompt); + const r = handleNormalKey(state, "y", ev("y"), prompt); + expect(state.yankRegister).toBe("second\nthird\n"); }); - it("g resets lineTracker to 0", () => { - state.lineTracker = 2; - handleNormalKey(state, "g", ev("g"), mockPrompt); - expect(state.lineTracker).toBe(0); + it("yy on last line yanks that line", () => { + const prompt: PromptAccess = { + getLine: (n) => ["first", "second", "third"][n] ?? "", + getLineCount: () => 3, + getCursorLine: () => 2, + }; + handleNormalKey(state, "y", ev("y"), prompt); + const r = handleNormalKey(state, "y", ev("y"), prompt); + expect(state.yankRegister).toBe("third\n"); }); });