vi mode: make . replay the insert command instead of injecting buffer text - #2955
Merged
Conversation
…ursors
Dot-repeat of the cursor-repositioning insert commands corrupted the
buffer: `o hello Esc` then `.` on another line injected the original
line's content plus the typed text mid-word, and `a`/`A` re-injected the
original line's leading char / whole content. The cause was the recording
model: entering insert mode snapshotted the cursor position from the
plugin's state snapshot, but `executeAction("move_line_end")` and friends
are queued and applied on the editor thread, so the snapshot still held
the PRE-reposition position. The Escape-time capture then read everything
from that stale offset to the insert end — the intervening buffer text —
and `.` re-inserted it verbatim.
Record the command instead of a byte span, per Vim (`:help .`): each of
`i`/`a`/`I`/`A`/`o`/`O` stores which command entered insert mode, the
repositioning ones flush the queued actions first so the captured span is
exactly the typed keystrokes, and `.` re-executes the command's motion at
the current cursor before re-inserting that text.
Two related defects fall out of the same change:
- `O` was broken before `.` entered the picture: `insert_newline` +
`move_up` left the cursor one line above the new empty line, so the
typed text corrupted the line above and no change was recorded. It now
inserts a line terminator at the line-start offset and places the cursor
there, both computed from the same pre-command cursor.
- An insert that followed an unrelated `x` was never recorded, because the
capture only overwrote `lastChange` when it was absent or already an
insert. Recording the entering command makes the insert always win.
Fixes #2443
Refs #2447
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
The dot-repeat corruption reached users because nothing drove `.` after `o`/`O`/`a`/`A` — the existing coverage repeated operators (`cw`, `dW`, `x`), which take a different path through `vi_repeat`. Add e2e reproducers that press the real keys: `o`/`a`/`A` followed by `.` on another line, `O`'s cursor landing on the line it opened (and repeating), and an `i`-insert after an `x` being what `.` replays. Two controls (`i`+`.`, `x`+`.`) pin the paths that already behaved, so a future recording change can't silently trade one for the other. The reproducers observe only rendered output — the text rows with the gutter and scrollbar chrome stripped, plus the status line's mode and `Ln N, Col N` readouts — rather than reading the buffer or cursor out of the editor. They also have to *fail*, not stall. Waiting for the expected text alone means a regression never asserts: the buffer settles on the corrupted content and the test runs until the external timeout kills it, reporting a hang instead of naming what went wrong. `assert_renders` therefore waits for either the expected rows or a screen that has stopped changing, then asserts — both conditions observed, neither a deadline. Reverting the plugin fix turns each of the five reproducers into a ~1.6s failure quoting the exact corruption from the issue (`charalpha`/`hellolie`, `aYbravo`, `bravalphaXo`, `bheyravo`, and the `x` replayed in place of the insert). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y
sinelaw
force-pushed
the
claude/fix-vi-dot-repeat-insert
branch
from
August 10, 2026 19:28
10c9780 to
7b6a670
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2443
Refs #2447
Motivation
Dot-repeat of the cursor-repositioning insert commands silently corrupted the buffer.
o hello Escthen.on another line injected the original line's content plus a newline plus the typed text mid-word;a Y Escthen.re-injected the original line's leading character;A X Escthen.injected the entire original line's content.iandxrepeated correctly, which is what localised the defect to the commands that move the cursor before inserting.Two related defects came out of the same code path:
Owas broken before.was involved at all: the empty line opened in the right place, but the cursor landed one line above it, so the typed text corrupted the preceding line, and.afterwards reported "No change to repeat".xwas never recorded:x, then lateri+text+Esc, then.repeated the oldx.The fix (all inside
crates/fresh-editor/plugins/vi_mode.ts)The recording model was "remember the byte span from the cursor at insert-mode entry to the cursor at Escape". That span was wrong because
executeAction("move_line_end")/move_right/insert_newlineare queued and applied on the editor thread, whilegetCursorPosition()reads the plugin's state snapshot — so the recorded start was the pre-reposition cursor, and the captured "inserted text" was really the intervening buffer text, which.then re-inserted verbatim.Per Vim (
:help .), the change is now recorded as the command plus the literally typed text, and replay re-executes the command's semantics at the new cursor:i/a/I/A/o/Oeach record which command entered insert mode (pendingInsertCommand).await editor.flush()before switching to insert mode, soinsertStartPosis the post-reposition cursor and the Escape-time capture spans exactly the typed keystrokes.vi_repeat'sinsertbranch re-executes the motion (move_right/move_line_start/move_line_end/move_line_end+insert_newline/ open-line-above) and then inserts the recorded text, leaving the cursor where Escape would have.Ono longer usesinsert_newline+move_up. It inserts the buffer's own line terminator at the line-start byte offset and sets the cursor there — both derived from the same pre-command cursor, so the new line and the cursor cannot disagree. The same helper is reused by..x-then-icase: the capture no longer refuses to overwrite an unrelated previous change.No Rust code changed; the plugin is embedded in the binary, so a rebuild is required to pick it up.
Tests
Added to
crates/fresh-editor/tests/e2e/vi_mode_bugs.rs. They drive real keystrokes and observe only rendered output: the text rows (line-number gutter and scrollbar chrome stripped) and the status line's mode indicator andLn N, Col Nreadout. Nothing reads the buffer, cursor, or editor mode out of the model. Each test gets its ownTempDirproject root and internal clipboard.The assertion helper (
assert_renders) waits for either the expected rows or a screen that has stopped changing, then asserts. Both arms observe state; neither is a wall-clock deadline. The second arm is what makes these useful: waiting only for the expected text means a regression never asserts at all — it runs until the external timeout kills it and reports a hang instead of naming the corruption. That was measured, not assumed: an earlier revision of the helper turned the reverted-plugin runs into stalls, and with the settle arm the same runs fail in ~1.6s quoting the exact wrong text.With the fix
Rebased onto
origin/master(clean; master had not touchedplugins/vi_mode.ts,tests/e2e/vi_mode_bugs.rs, or the e2e harness). On that rebased tree, the seven tests were run by name from a freshly builte2e_testsbinary: 7 passed; 0 failed.Without the fix
plugins/vi_mode.tsreverted to master's copy, the binary rebuilt, each test run individually. Because the shared build directory is used by other work and served stale artifacts more than once, every binary used here was verified to actually contain (or, for the reverted build, to not contain) this change before being run...._dot_repeat_open_belowo hello Esc,j j,.["alpha","hello","bravo","charalpha","hellolie","delta"]vs expected[…,"charlie","hello","delta"]..._dot_repeat_append_after_cursora Y Esc,j 0,.["aYlpha","aYbravo"]vs expected["aYlpha","bYravo"]..._dot_repeat_append_line_endA X Esc,j,.["alphaX","bravalphaXo"]vs expected["alphaX","bravoX"]..._open_above_cursor_placement_and_repeatj j O hey Esc,.["alpha","bheyravo","","charlie"]vs expected["alpha","bravo","hey","charlie"]..._insert_after_delete_char_is_recordedx,i Q Esc,j 0,.["Qbc","yz"](thexreplayed) vs expected["Qbc","Qxyz"]..._control_dot_repeat_insert_beforei AB Esc,j,...._control_dot_repeat_delete_charx,.,.Manual validation
Walked every case from the issue's reproduction comment in tmux (isolated
HOME, private tmux socket) against a freshly built binary, onalpha/bravo/charlie/delta/echo. The plugin file on this branch is byte-identical to the copy that binary was built from.o hello Esc,j j,.alpha/hello/bravo/charlie/hello/delta/echo✅a Y Esc,j 0 .aYlpha/bYravo✅A X Esc,j .alphaX/bravoX✅j j O hey Escalpha/bravo/hey/charlie— cursor on the opened line ✅.alpha/bravo/hey/hey/charlie✅x,i Q Esc,j 0 .Qlpha/Qbravo✅ (the insert repeats, not thex)i AB Esc,j .ABalpha/bABravo✅x,.,.alpha→lpha→ha✅Other checks
cargo fmt -p fresh-editor— no changes.plugins/check-types.sh vi_mode.ts— fourActionSpec.argserrors, all pre-existing: the same four are reported when the checker is run against master's copy of the file. This change adds none.Not run
Stated explicitly so the reviewer does not assume otherwise:
cargo clippyandcargo check(including--features gui) were not run on this branch, and no suite broader than the seven tests named above was run after the rebase. CI covers those.🤖 Generated with Claude Code
https://claude.ai/code/session_01D6SkGXXTcJsytTjF1TBf8Y