Fix/issue 2224 via pr 5842#9998
Open
AlexanderFarkas wants to merge 36 commits into
Open
Conversation
- Implements 'keymodel' setting - Make backspace work on all visual modes - Implement '<S-BS>' and '<C-BS>' - Implement shifted special keys (arrow keys, Home/End keys, PageUp/PageDown) - Implement shifted special keys on commandLine modes - Implement 'modeBeforeEnteringVisualMode'. It allows to have the pseudo insert/replace visual modes that go back to insert/replace after escaping visual instead of going back to normal - Add rest of visual modes and replace mode to possible wrapping modes - Implement '<C-o>' in replaceMode
- This creates a new motion action MoveLeftMouse that runs when clicking - This allows to use left mouse click with operators and allows to remap the left mouse click
…ly on visual mode
- Fix <C-a>, <C-x> (Increment/Decrement number) in visual mode changing the number twice (it was getting the ranges duplicated) - fix <C-a>, <C-x> (Increment/Decrement number) in visual mode updating outside the range (in visual mode if you select '1' on the number '14' it should increase to '24' but it was going to '15')
- Implements all Select modes - Implement 'selectmode' setting - Implement SelectMode remaps
- make 'recordedState.count' be cleared after 'executeOperator' and after running a completeAction 'BaseCommand' - made the enter visual mode command 'v' be a complete action. (why was it not a complete action before???) - changed BaseCommand to have 'isCompleteAction' false by default so that we know that when setting it to true we need to either handled the count properly or have the 'runsOnceForEachCountPrefix' set to true
Adopts PR VSCodeVim#5842 ("Select mode implementation with `keymodel` and `selectmode` settings") to close VSCodeVim#2224. Merges 5 years of master drift into berknam's branch. Architecture preserved verbatim where possible (Mode.Select* family, pseudo-modes, currentModeIncludingPseudoModes, modeBeforeEnteringVisualMode, MoveShift* motions, isCompleteAction default flip); master's structure taken where APIs diverged (Cursor.atPosition, modeData/setModeData, getOperatorState, master's commandLine.ts CommandLineAction abstraction). Deferred to follow-up PRs: berknam's commandline-shifted-key extensions; actions/commands/{actions,insert,put}.ts wholesale (heavy structural divergence); in-Select :vmap → temporary Visual swap in remapper.ts; ArrowsInReplaceMode family; pseudo-mode statusBarText cases (master moved statusBarText into statusBar.ts). Mouse last-char fix (1e0984b) dropped — already in master as a7f71a1. Closes VSCodeVim#2224 (subject to verification checklist /Users/alexander/.claude/plans/cozy-kindling-pebble-checklist.md). Supersedes VSCodeVim#5842 (abandoned). Co-Authored-By: berknam <berknam@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This is a single squashed fixup applied on top of the merge commit (which
preserves berknam's 11 commits per J-Fields' 2021-02-18 review request).
Changes against berknam's keyboardSelections branch as merged:
1. package.json
- Dedupe ctrl+left, ctrl+right, ctrl+home, ctrl+end keybindings.
Master's narrower (`vim.mode != 'Insert'`) variants were duplicates of
berknam's broader entries; the duplicate command-name registrations made
`vscode.commands.registerCommand` throw, aborting the registration loop in
`extensionBase.ts:474` and leaving downstream keybindings
(`ctrl+shift+home`/`end`) unregistered. Kept berknam's broader when-clauses
since they're needed for keymodel handling in Insert mode.
- Address stevenguh's review feedback: simplify `vim.selectmode` regex to
`^$|^(mouse|key|cmd)(,(mouse|key|cmd))*$`; clarify
`vim.mouseSelectionGoesIntoVisualMode` and `vim.selectmode` descriptions
so they cross-reference each other.
2. src/actions/base.ts
- Revert `BaseCommand.isCompleteAction` default to `true` (master's behavior).
Berknam flipped to `false` and explicitly set `true` on every subclass that
needed it; we took master's actions.ts which only opts into `false` on two
commands, so the flip silently broke macro/dot-repeat/count clearing on
most BaseCommand subclasses.
3. src/mode/modeHandler.ts
- Remove duplicate count-clearing inside the `isCompleteAction` branch.
Master clears `recordedState.count` once at the end of `handleKey` (after
`ExitInsertMode` has had a chance to read it for the `[count]i`/`[count]a`
repeat-insert path); berknam's added clearing inside the action-type branch
was a duplicate that fired before `ExitInsertMode` could see the count.
- Revert `handleSelectionChange` `allowedModes` arrays to master's narrower
scope: `[Normal]` (+ Insert/Replace if no snippet) for the multi-cursor-
count-change branch, and `[Normal, Visual, VisualLine]` (+ Insert/Replace)
for the Command-kind branch. Berknam's widening to include Visual/Select
variants caused over-triggering of the cursor-overwrite + Visual transition
logic on selection-change events that shouldn't drive mode changes.
4. src/actions/motion.ts
- `MoveDownArrow`: drop Insert/Replace from override modes — they conflicted
with `ArrowsInInsertMode` (insert.ts:764) and `ArrowsInReplaceMode`
(replace.ts:348), master's purpose-built handlers for arrows in those
modes (which create undo points).
- `MoveLeft.keys`: re-add `<BS>`, `<C-BS>`, `<S-BS>`. Master had these in
`MoveLeft` for whichwrap-aware behavior via `shouldWrapKey`; berknam had
pulled them into `BackSpaceInNormalMode`/`Visual` which always crossed line
boundaries, ignoring whichwrap.
- `MoveRight.keys`: re-add `' '` (space) for the same reason
(`MoveRightWithSpace` always crossed line boundaries).
- Drop `BackSpaceInNormalMode`, `BackSpaceInVisualMode`, `MoveRightWithSpace`.
5. src/state/vimState.ts
- `setCurrentMode`: only set `modeBeforeEnteringVisualMode` when the source
mode is Insert or Replace. Berknam stored ANY non-visual source mode (e.g.
`SearchInProgressMode` during `\n` to submit `/`/`?`), which then incorrectly
forced the user back to that source mode on Visual-exit. The override path
is only meaningful for the `<C-o>v` and `<S-arrow>`-from-Insert flows.
6. src/statusBar.ts
- Switch `statusBarText()` from `vimState.modeData.mode` to
`vimState.currentModeIncludingPseudoModes`; add case arms for the 15
pseudo-modes (`InsertNormal`, `ReplaceNormal`,
`Insert/ReplaceVisual{,Block,Line}`, `Insert/ReplaceSelect{,Block,Line}`).
Pseudo-modes from VSCodeVim#5842 were synthesized for the remapper but never had
matching status-bar labels because master moved `statusBarText` from
`mode.ts` to `statusBar.ts` after berknam's last sync.
Test status: 3157 passing, 19 pending, 0 failing — full parity with master at
e4b1b47. tsc + lint clean.
Co-Authored-By: berknam <berknam@users.noreply.github.com>
Self-review found three issues unrelated to or beyond VSCodeVim#2224's scope: 1. Dead code: `RecordedState.waitingForRegisterKeyAfterCtrlR` field + 5 read sites in modeHandler.ts. Berknam intended this for the `<C-r>"` register-paste UX in Insert/Select mode, but the corresponding setter (in a `<C-r>` handler) was never ported. The field stays at its default `false` forever, making the 5 reads dead conditions. Removed the field and all 5 reads (capture, comparison, early-return, virtual-char display, and a doc comment). 2. Insert-mode behavior change: berknam widened the `ctrl+left`/`right`/ `home`/`end` keybinding when-clauses (master had `vim.mode != 'Insert'`) AND added Insert+Replace to `MoveCtrlRightArrow`/`MoveCtrlLeftArrow`/ `MoveCtrlHome`/`MoveCtrlEnd` modes. Net effect: in Insert mode, plain ctrl+arrow now ran Vim's word/line-end motion (Vim's `iskeyword` definition) instead of VSCode's native (user's `wordSeparators`). This is a user-visible regression for existing master users that isn't needed for VSCodeVim#2224 (which is about <S-arrow>, not <C-arrow>). Restored master's narrower when-clauses and removed Insert/Replace from the four MoveCtrl* modes; <C-S-arrow> handlers (MoveCtrlShift*) still keep Insert/Replace, since they're the keymodel-startsel path that DOES need Insert-mode handling. 3. Misleading TODO at motion.ts:471 claimed master "doesn't have" `ArrowsInReplaceMode` and warned about duplicate `@RegisterAction` registrations. Master DOES have it at `replace.ts:348`. Removed. Co-Authored-By: berknam <berknam@users.noreply.github.com>
…promotion Two bugs uncovered while writing tests for the merged-in VSCodeVim#5842 work: 1) src/actions/wrapping.ts — shouldWrapKey threw "called with unexpected key" for `<S-right>` and `<S-left>`. MoveShiftRightArrow / MoveShiftLeftArrow delegate to MoveRight / MoveLeft passing the original keysPressed, which then reaches shouldWrapKey with the shifted variant. Treat shifted arrows the same as unshifted for whichwrap purposes. 2) src/mode/modeHandler.ts — handleSelectionChange only promoted to Visual on Command-kind events. VSCode's `expandLineSelection`, `cursorRightSelect`, `cursorDownSelect` (and similar extension commands) emit Keyboard-kind events for the actual selection — these were silently dropped, leaving the user in Normal mode with a VSCode selection that Vim operators couldn't act on. Added a Keyboard-kind branch that promotes to Visual when the selection is non-empty and we're in Normal/Insert/Replace. Internal Vim navigation produces empty selections (anchor === active) and is unaffected. Closes the remaining half of VSCodeVim#2224 (Shift+Arrow alone was already handled by the MoveShift*Arrow keymodel layer). Co-Authored-By: berknam <noreply@github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…odeVim#2224 Closes the test-coverage gap inherited from VSCodeVim#5842, which shipped no functional tests for the new keymodel/selectmode/Select-mode behavior. New files (test/mode/): - keymodel.test.ts (19 tests): keymodel=startsel,stopsel; keymodel="" (terminal-Vim, no Visual entry); keymodel=stopsel (unshifted exits Visual). Covers <S-right>, <S-left>, <S-up>, <S-down>, <S-Home>, <S-End>, <C-S-right>, <C-S-left>, <C-S-Home>, <C-S-End> from Normal/Visual. - pseudoModes.test.ts (12 tests, 1 pending): Insert ↔ Visual round-trip via <S-arrow>, Replace ↔ Visual round-trip, <C-o> from Insert. Asserts status-bar literals "-- (insert) VISUAL --", "-- (replace) VISUAL --", "-- (insert) --". Pending: <C-o> from Replace (action not registered for Replace mode in current code). - selectmode.test.ts (7 tests, 2 pending): selectmode=key routes <S-arrow> to Mode.Select; default empty stays in Visual. Pending: gh / <C-g> entry commands not yet ported from VSCodeVim#5842's actions.ts. - issue2224.test.ts (4 tests): expandLineSelection, smartSelect.grow, cursorRightSelect, cursorDownSelect — all promote Normal → Visual via the fixed handleSelectionChange path. - selectionPromotion.test.ts (16 tests): regression-guards. Vim navigation (j, k, l, h, w, gg, G, <right>, <down>) stays in Normal. i+<right>/<down> stays in Insert. cursorRight / cursorDown (non-select) stays in Normal. v + l/j/w stays in Visual. Constrains the new Keyboard-kind promotion. - bugfixRegressions.test.ts (2 tests): recordedState.count clears between actions; gv re-enters Visual at last range. Total: 60 new tests, 57 passing, 3 pending. Full suite 3214 passing, 22 pending, 0 failing (baseline was 3157/19/0 — net +57 passing, +3 pending). Co-Authored-By: berknam <noreply@github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR VSCodeVim#5842 introduced the scaffolding for Vim's Select mode (Mode.Select / SelectLine / SelectBlock plus the InsertSelect* / ReplaceSelect* pseudo-modes, the `selectmode` config option with `mouse`/`key`/`cmd` tokens, the selectModeKeyBindings remap field, and 30-odd lines branching on configuration.selectmodeKey across the MoveShift* motion classes), but the *differentiating* behavior — typing in Select replaces selection, paste in Select replaces selection, gh/gH/g<C-h> entry commands, <C-g> swap — was never ported. So Mode.Select reachable today is functionally equivalent to Mode.Visual with a different status-bar string. Worse for users (status bar says SELECT, behavior is Visual) and a permanent foot-gun for maintenance. Removing it: - src/mode/mode.ts: drop Select / SelectLine / SelectBlock from the Mode enum and the 6 InsertSelect* / ReplaceSelect* pseudo-modes; drop isSelectMode helper; tighten isVisualMode to just Visual / VisualLine / VisualBlock. - src/actions/motion.ts: drop Select* entries from `modes` arrays in 21 motion classes; collapse the `if (selectmodeKey) setMode(Select) else setMode(Visual)` branches to always `setMode(Visual)` in the 10 MoveShift* classes; drop MoveLineBeginSelectMode and MoveLineEndSelectMode (Select- mode-only motions). - src/mode/modeHandler.ts: drop isSelectMode import; drop Select* cases from the selectionMode switch; drop the `selectmodeMouse`-gated branch in handleSelectionChange (mouse drag now always enters Visual when mouseSelectionGoesIntoVisualMode is on); drop Select* cases from the cursor-style switch. - src/state/vimState.ts: drop Select / SelectLine / SelectBlock from the pseudo-mode synthesis switch in currentModeIncludingPseudoModes. - src/statusBar.ts: drop the 9 SELECT / (insert) SELECT* / (replace) SELECT* status-bar string arms. - src/configuration/{configuration,iconfiguration}.ts: drop selectmode, selectmodeMouse, selectmodeKey, selectmodeCmd config fields and their derivation from the selectmode token list; drop selectModeKeyBindings, selectModeKeyBindingsNonRecursive, selectModeKeyBindingsMap. - src/configuration/remapper.ts: drop SelectModeRemapper class; drop Select* from AllVisualModeRemapper's mode list. - src/configuration/vimrc.ts: drop :smap / :sunmap / :snoremap / :smapclear ex-command keyword routes; drop selectModeKeyBindings from removeAllRemaps. - src/configuration/validators/remappingValidator.ts: drop selectMode binding-key entries from the validation list. - src/actions/commands/put.ts: drop Select* exclusion in the gv-tracking guard (no longer needed). - package.json: drop vim.selectmode and vim.selectModeKeyBindings* schemas; rewrite vim.keymodel and vim.allVisualModeKeyBindings descriptions to no longer reference Select mode; rewrite vim.mouseSelectionGoesIntoVisualMode description to drop the selectmode cross-reference. - test: delete test/mode/selectmode.test.ts; drop selectmode/selectmodeKey from per-test config blocks in keymodel.test.ts and pseudoModes.test.ts; drop selectModeKeyBindings* from testConfiguration.ts and testSimplifier.ts; drop the selectModeKeyBindings assertion from remappingValidator.test.ts. Subtle bug caught while refactoring: the post-action cursor "shift forward by one" logic (modeHandler.ts:899) and pre-action "shift back by one" logic (:772) used `[Mode.Visual, Mode.Select].includes(currentMode)` — i.e. only charwise Visual + Select got the +1 cursor shift. An earlier draft of this refactor replaced both with `isVisualMode(currentMode)`, which incorrectly applied the shift to VisualLine and VisualBlock too, breaking 26 tests (VisualBlock <C-a>, gv after VisualBlock yank, etc.). Fixed by using the narrower `currentMode === Mode.Visual` check, matching the original pre-VSCodeVim#5842 behavior for VisualLine and VisualBlock. Test impact: -5 passing (the 5 G1-G5 selectmode tests we deleted), -2 pending (the G6/G7 TODOs), 0 regressions. Suite goes from 3214/22/0 to 3209/20/0. Net code change: 458 deletions, 34 insertions across 19 files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The mouse-drag branch of handleSelectionChange (modeHandler.ts:407) only updated `cursorStartPosition` (which sets `cursor.start` only), leaving `cursor.stop` stale at its pre-drag position. Result: after a mouse drag selecting N characters, pressing `d`/`y`/`c` operated on a 1-char range (anchor only), not the full selection. Reproducer: open a buffer with `hello world`, mouse-drag to select `hello`, press `d`. Before fix: `ello world` (only `h` deleted). After fix: ` world` (full `hello` deleted). Replaced the `cursorStartPosition = selectionStart` assignment with `this.vimState.cursor = new Cursor(selectionStart, selection.active)` so both endpoints are set from the post-drag selection. This is a long-standing bug present in master too, just not previously covered by tests because the harness has no real mouse-event simulator. The new test/mode/mouseSelection.test.ts dispatches synthetic Mouse-kind TextEditorSelectionChangeEvents directly to handleSelectionChange (the same path VSCode itself triggers on mouse-up), which is sufficient to exercise the cursor-state plumbing. Tests added (4): - mouse drag from Normal enters Visual - d after mouse drag deletes the full selection - y after mouse drag yanks the full selection - mouse drag backwards (active before anchor) still operates on full range Full suite: 3213 passing, 20 pending, 0 failing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
No behavior change. Mechanical clean-up of the new tests and recently-edited src comments to drop references to the iterative process that produced them and to make each test self-explanatory to a reviewer with no prior context: - Test titles: drop A1-A13 / B1-B4 / C1-C2 / D1-D5 / E1-E3 / F1-F4 / H1-H4 / I1-I2 internal labels — each title now describes the behavior being tested. - File and inline comments: drop "PR VSCodeVim#5842 / merge", "berknam's", "the merge resolution"; drop pin-pointed source line numbers ("modeHandler.ts:407", ":320-345", etc.) that rot when surrounding code shifts. - mouseSelection.test.ts: replace `(await getAndUpdateModeHandler())!` null assertion with an explicit `assert.ok` check; factor the duplicated "type 'hello world', exit to col 0" setup into a `setupHelloWorld` helper; drop the lax fourth test ("backward drag asserts only that doc shrunk") since a 1-char delete (the bug being guarded) would also pass that assertion. - src/mode/modeHandler.ts: tighten the comment on the new Keyboard-kind promotion branch and the mouse-drag cursor-update fix to describe what the code does, not its history. - src/state/vimState.ts: rewrite the `returnToInsertAfterCommand` doc comment to explain the field's purpose rather than its origin. Full suite: 3212 passing, 20 pending, 0 failing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The existing VSCodeVim#2224 tests only checked the resulting Vim mode label, which is insufficient to catch the bug class where the mode flips to Visual but cursor.start / cursor.stop are stale (the same shape as the mouse-drag bug fixed earlier in this branch). Adding three tests that promote via a VSCode selection command, then run a Vim operator, and assert the document mutation: - d after expandLineSelection deletes the full line - d after cursorRightSelect deletes the selected char - y after editor.action.smartSelect.grow yanks the grown selection Adding two count-prefix tests for the shifted-arrow motions, exercising the execActionWithCount path that was otherwise untested: - 3<S-right> selects three chars - 2<S-down> extends Visual two lines down Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop dead state and make the entry-side narrowing of
modeBeforeEnteringVisualMode statically enforced rather than runtime-checked.
- src/state/vimState.ts:
- Remove the boolean alias `returnToInsertAfterCommand`. Berknam's design
routed every site through `modeToReturnToAfterNormalCommand` (typed
`Mode.Insert | Mode.Replace | undefined`); the alias was a back-compat
shim from the merge.
- Narrow `modeBeforeEnteringVisualMode` to `Mode.Insert | Mode.Replace |
undefined`. The entry side already only stored Insert/Replace, but the
field was typed `Mode | undefined` so the consumer needed runtime guards
(`!== Mode.Normal`, defensive `previous === undefined` arm). Narrowing
the type makes those branches statically dead — drop them.
- Force the entry-side narrowing through a TS-narrowable expression
(`target === Mode.Insert || target === Mode.Replace`) rather than
`Array.includes`, which doesn't narrow.
- Use explicit `!= null` instead of truthy `&&` checks on these fields
everywhere they're tested.
- Fix typo in `currentModeIncludingPseudoModes` doc comment.
- src/mode/modeHandler.ts: drop dead `let toDraw = false` (both write sites
were removed when berknam's `<LeftMouse>` action took over the click
paths). Same `!= null` cleanup at the two call sites here.
- src/actions/commands/insert.ts: migrate the two boolean call sites
(`returnToInsertAfterCommand && ...`, `= true`) to read/write
`modeToReturnToAfterNormalCommand` directly.
- src/actions/commands/put.ts: same migration; the local `getCursorPosition`
parameter name stays as `returnToInsertAfterCommand` since it describes
semantics, not the underlying field.
- src/configuration/iconfiguration.ts: declare the derived
`keymodelStartsSelection`/`keymodelStopsSelection` booleans on
`IConfiguration` so cross-module reads are fully type-checked.
`keymodel` itself stays as the user-facing string (matches Vim's option
name; vimrc compatibility).
3217 passing, 20 pending, 0 failing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…emoval The original PR introduced two parallel binding maps to mirror Vim's vmap (visual + select) vs xmap (visual only) distinction: - visualModeKeyBindings — visual modes - allVisualModeKeyBindings — visual + select modes When Select mode was dropped (commit 7f4f99d), the two remappers ended up applying to the exact same nine modes (Visual/VisualLine/VisualBlock plus the six Insert*/Replace* pseudo-Visual variants). The maps were then functionally indistinguishable: two parallel storage arrays consulted by remappers covering identical mode sets. Without Select mode, vmap and xmap are equivalent. Route both vimrc keywords (vmap/xmap, vnoremap/xnoremap, vunmap/xunmap, vmapclear/ xmapclear) to visualModeKeyBindings; remove the AllVisualModeRemapper, the four config fields and one map, the IConfiguration entries, the validator key strings, and the package.json settings. Drop the stale 'allVisualModeKeyBindings' reference from the post-remap redraw comment in modeHandler.ts. No migration concern: the option never shipped — it was added by this PR (superseding VSCodeVim#5842) and has no users in any settings.json. 3217 passing, 20 pending, 0 failing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Mouse> remap Three behaviors that the original PR's package.json + iconfiguration.ts implied support for, but for which no Insert-mode handler or normal-mode action class existed. Without these, the keys were captured by VSCodeVim and silently dropped — strictly worse than master, which let VSCode's defaults pass through. src/actions/commands/scroll.ts: - Add <PageUp>/<PageDown> as alternate keys on CommandMoveFullPageUp/ CommandMoveFullPageDown (mirroring <C-b>/<C-f>). - New MoveShiftPageUp/MoveShiftPageDown for <S-PageUp>/<S-PageDown>: enter Visual when keymodelStartsSelection is set, then delegate to the unshifted scroll-and-move implementation. Available in Insert/Replace too so the pseudo-Visual entry from those modes works. src/actions/commands/insert.ts: - Add <S-BS> to BackspaceInInsertMode keys (treat as plain <BS>, single char delete-left). - New CtrlBackspaceInInsertMode dispatching VSCode's `deleteWordLeft`, matching master's pre-PR behavior (Ctrl+Backspace = delete word back) but routed through VSCodeVim so user remaps and whichwrap apply. test/mode/keymodel.test.ts: - 6 new tests: <S-PageUp>/<S-PageDown> enter Visual under startsel; <S-PageUp>/<PageUp>/<PageDown> stay Normal under empty keymodel. test/mode/modeInsert.test.ts: - 2 new tests: <C-BS> deletes previous word; <S-BS> deletes one char. test/mode/mouseSelection.test.ts: - New `<LeftMouse> remap` suite (1 test): synthesized empty Mouse-kind selection-change with a normalModeKeyBindings remap fires the remap. 3225 passing, 20 pending, 0 failing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The four pseudo-modes (InsertVisualLine, InsertVisualBlock, ReplaceVisualLine,
ReplaceVisualBlock) had no test coverage even though they're reachable via
i<C-o>V / i<C-o><C-v> / R<C-o>V / R<C-o><C-v>. The status bar text refinement
they provide ('-- (insert) VISUAL LINE --' etc.) is the user-visible signal
that <Esc> will return to Insert/Replace, not Normal — worth pinning.
- Insert + <C-o>V status bar + Insert + <C-o>V<Esc> round-trip: tested.
- Insert + <C-o><C-v> status bar + round-trip: tested.
- Replace + <C-o>V / <C-o><C-v>: skipped, gated on R<C-o> being wired
(which is the existing skipped 'Replace + <C-o>w' gap above).
3229 passing, 22 pending, 0 failing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… op-pending status bar) - package.json: PageUp/PageDown/Shift+PageUp/Shift+PageDown bindings now honor !suggestWidgetVisible && !parameterHintsVisible. Without the guard the keys hijack the suggestion widget — autocomplete navigation was broken when these were added. - motion.ts: drop dead `MoveBeginningWordShiftBS`. `MoveLeft` keeps claiming `<S-BS>` so it stays a char-back motion (matches VSCode's default Shift+Backspace = deleteLeft and the Insert-mode <S-BS> = <BS> symmetry). `<C-BS>` no longer collides with `MoveLeft`, so the dedicated `MoveBeginningFullWordCtrlBS` finally fires — `<C-BS>` is now a word-back motion (B) in Normal/Visual, matching Insert-mode's deleteWordLeft. - vimState.ts / statusBar.ts: split `currentModeIncludingPseudoModes` into a non-OP-pending variant. Status bar uses the new getter so the bar keeps showing the underlying mode label while an operator is pending (matches Vim). Previously the bar blanked on `d` / `y` / `c`. - test/mode/pr9998Regressions.test.ts: red regression suite for the three fixes — PageUp \`when\` clauses, <C-BS> = B / <S-BS> = char-back semantics, and non-empty status bar during operator-pending. - test/mode/modeNormal.test.ts: extend the char-back iteration loop to cover \`<S-BS>\` alongside \`<BS>\` (whichwrap behavior). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- <C-Right>/<C-Left> now dispatch to MoveWordBegin/MoveBeginningWord (small word `w`/`b`), matching :help <C-Right> and the <C-S-Right> sibling. Drops the dead-shadow `<C-right>`/`<C-left>` aliases on MoveFullWordBegin and MoveBeginningWord (same key-collision pattern that originally hid <C-BS>). - Adds inverse-gated MoveShiftUpAsPage/MoveShiftDownAsPage so <S-Up>/<S-Down> fall back to <C-B>/<C-F> page motion when keymodel lacks startsel (matches :help <S-Up>); previously a silent no-op. - ExecuteOneNormalCommandInInsertMode now registered for Mode.Replace and captures the actual current mode as the return target. R<C-o>w returns to Replace; (replace) NORMAL/VISUAL/VISUAL LINE/VISUAL BLOCK pseudo-modes reachable end-to-end. Red tests in pr9998Regressions.test.ts (Bug VSCodeVim#5/6/7); un-skipped three R<C-o> cases in pseudoModes.test.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…stopsel - src/actions/commands/scroll.ts: extend CommandMoveFullPageUp/Down modes to include Insert and Replace. Without this, the package.json bindings for plain PageUp/PageDown captured the keys in Insert but no Vim action matched, leaving the keys as silent no-ops (master had no binding, so VSCode native handled them). - package.json: vim.keymodel default 'startsel' -> 'startsel,stopsel'. Bare 'startsel' enters Visual on <S-arrow> but never exits on <arrow> (no stopsel), stranding users in Visual until <Esc>. The new default matches gVim and avoids the trap. - test/mode/pr9998Regressions.test.ts: add red tests for both regressions (now green) plus a guard for the click-past-EOL cursor-clamp invariant that survives the LeftMouse rewrite via a downstream clamp in runAction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ests When the drag's selectionStart is after newPosition, master shifted selectionStart left by 1 to match Vim's inclusive Visual edges; the PR dropped that adjustment, so a right-to-left drag selected one extra character vs. the equivalent left-to-right drag. Re-add the getLeft(). The new test/mode/pr9998ReviewCoverage.test.ts pins twelve invariants raised in the PR review (mouse-drag symmetry, Insert→InsertVisual via drag, count chaining, <C-o> state cleanup, isPseudoMode coverage, vmap in pseudo-Visual, Keyboard-kind promotion vs VisualLine, and others). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
On macOS, option+arrow is the platform-native word-jump modifier; the PR
claimed only ctrl+(shift+)arrow which left mac users without working
word-select. Bind alt+arrow / alt+shift+arrow on macOS only (gated via
isMac in the `when` clause) and route them to the existing MoveCtrl*
handlers via an <A-...> alias.
Vim itself has no default for <A-arrow>, so this is a host-convention
alias rather than a vim-spec change. On Linux/Windows, alt+arrow is
VSCode's workbench navigateBack/navigateForward — claiming it would
silently break that for every vim user, so we leave it unbound.
The extension.vim_alt+... commands still exist on those platforms;
users who want the binding can add it themselves in keybindings.json.
- notation.ts: add `alt+|a-` -> `A-` rule so `alt+left` normalises to
`<A-left>` and `alt+shift+left` to `<A-S-left>`.
- motion.ts: extend MoveCtrl{Left,Right,ShiftLeft,ShiftRight}Arrow keys
to also accept the <A-...> form. No new handlers — semantics identical.
- package.json: bind alt+(shift+)left/right with `isMac && ...` so
Windows/Linux retain navigateBack/Forward.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit added four mac-only keybinding entries with `when: isMac && ...` and parallel `extension.vim_alt+...` commands. configuration.ts already has platform-aware key dispatch built in (reads `mac:` / `linux:` overrides on each entry), so the idiomatic shape is one entry per command with platform-specific chords. ctrl+left on macOS is "window jump" (workspace navigation), so we don't need to keep ctrl+arrow active on mac. Add `mac: alt+arrow` to the existing ctrl+arrow entries; configuration.ts on darwin then substitutes alt+arrow as the trigger and normalises the boundKey to `<A-left>` etc. The motion classes already accept the <A-...> form, so no further wiring is required. - package.json: drop the four `extension.vim_alt+...` entries; add `mac:` overrides to the four `extension.vim_ctrl+...` arrow entries. - pr9998ReviewCoverage.test.ts (suite L): rewrite the package.json assertion to verify the mac overrides instead of the dropped isMac-gated commands. Behavior tests for `<A-arrow>` / `<A-S-arrow>` are unchanged — they already drive `handleKeyEvent` directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
|
@J-Fields Hope the size of the PR doesn't scare you - it's mostly tests 😄 |
…LeftMouse coupling - `<C-BS>` in Normal/Visual now uses small-word (`b`) instead of WORD (`B`), matching `<C-Left>` and Insert's `deleteWordLeft`. Earlier review commit fixed `<C-Right>`/`<C-Left>` but missed `<C-BS>`. - Added test that distinguishes word from WORD via punctuation (`hello.world |foo` → `hello.|world foo`); the previous test case put the cursor mid-word so `b` and `B` produced identical results. - Comment in `BackspaceInReplaceMode` explains why `<C-BS>` aliases `<BS>` in Replace (one-char restore preserves Replace's per-char tracking invariant) instead of word-deleting like Insert. - Comment in `MoveLeftMouse` notes the implicit dependency on `setCurrentMode`'s leave-Visual override that rewrites Normal → Insert/Replace when leaving pseudo-Visual. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In the past-EOL branch of `handleSelectionChange`, the inner `if (currentMode !== Insert)` skipped the `<LeftMouse>` dispatch in Insert mode, then the click silently fell through into the empty "This prevents you from selecting EOL" sub-branch. Result: Insert-mode clicks past EOL were no-ops — the cursor never moved. Always dispatch `<LeftMouse>` now; `lastClickWasPastEol` (used by the follow-up drag-extends-inclusive logic) is still set only for non-Insert modes. Also dropped a dead `newPosition.withColumn(lineEnd-1)` local reassignment that was overwritten and immediately discarded by `return`; the actual cap happens later in `runActionInternal`'s post-action bounds-clamp (which runs for Normal/Visual but not Insert, because Insert allows the cursor at lineEnd). Pinned by `mouse click past EOL > click past EOL in Insert moves cursor to lineEnd` in mouseSelection.test.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Existing suite covered drag-from-Normal + d/y/c. Adds: - Cross-line drag (line 0 col 0 → line 1 col 3) and `d` deletes across. - Right-to-left drag selects same range as left-to-right (regression guard for the cursor.start-after-newPosition adjustment). - Drag past EOL on a single line; selection still spans correctly. - Drag from Insert / Replace enters pseudo-Visual; Esc returns to the source mode; operators (d) also return to source mode. - Click in Visual exits to Normal. - Click in pseudo-(insert)Visual / pseudo-(replace)Visual returns to Insert / Replace — pins the implicit `MoveLeftMouse` ↔ `setCurrentMode` leave-Visual override coupling. - Click past EOL: caps to last char in Normal; lands at lineEnd in Insert (post-fix from the previous commit). - `mouseSelectionGoesIntoVisualMode = false` opt-out: drag from Normal / Insert stays in current mode. - `gv` reselects after a mouse drag (lastVisualSelection populated). - Second drag while already in Visual updates without double-promoting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
handleSelectionChange's Visual-mode early-return at line ~286 swallowed
Keyboard/undefined-kind selection-change events (from `smartSelect.grow`,
`expandLineSelection`, `cursorRightSelect`, etc.) when triggered while
the user was already in Visual. vimState.cursor stayed pinned to the
post-first-grow range; the next render reverted VSCode's grown selection.
User-visible symptom from dogfooding the PR: first smartSelect.grow
entered Visual on `when` correctly; second grow lost the selection and
snapped the cursor back to the line indent.
The pre-existing Command-kind branch at line ~254 already handled this
for command-dispatched updates (`vscode.commands.executeCommand` →
Command-kind event). The new block mirrors that handling for
Keyboard/undefined kinds, matching the grouping in the comment at line
~282.
Tests: a new TypeScript-file test suite drives smartSelect.grow/shrink
on the exact line from the user's repro
(` if (keybinding.when.includes('listFocus')) {` with caret on the
dot after `when`) and asserts mode + selected-text + vimState.cursor at
each step. Test path goes through executeCommand → fires Command-kind,
so it pins the existing line ~254 behavior end-to-end. The
Keyboard/undefined fix is verified by manual smoke test in real VSCode.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The post-remap branch in handleKeyEvent (modeHandler.ts:656-660) called
updateView() synchronously after a visual-mode remap. For remaps with
`commands` (executing VSCode commands like editor.action.smartSelect.grow),
this raced with VSCode's deferred onDidChangeTextEditorSelection event:
1. remapper runs `executeCommand('editor.action.smartSelect.grow')`
2. command sets editor.selection to the grown range (synchronous)
3. VSCode queues the selection-change event (asynchronous)
4. executeCommand returns
5. modeHandler's updateView runs SYNCHRONOUSLY, reads the still-stale
vimState.cursor, writes editor.selection BACK to the pre-grow range
6. async handleSelectionChange fires later but the selection is already
reverted
User-visible symptom: vmap `=` -> editor.action.smartSelect.grow appeared
to be a no-op on the first press; the second press finally took effect
(because by then the deferred handleSelectionChange had updated cursor).
The post-remap updateView was originally added for Select-mode -> Visual
transitions (allVisualModeKeyBindings flow). The PR dropped Select mode
entirely (commit 7f4f99d), so the original purpose is gone. For remaining
remap shapes:
- `after: keys` -> each replayed key goes through runAction, which
redraws via runActionInternal's existing updateView calls.
- `commands` -> external command sets editor.selection; the deferred
handleSelectionChange updates vimState.cursor when the event fires.
Either way, the post-remap updateView is unnecessary and harmful.
Test: vmap `=` -> smartSelect.grow after `v` produces a selection wider
than 1 char (`smartSelectGrowShrink.test.ts`).
Full suite: 3321 passing, 0 failing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
3 participants
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.
What this PR does / why we need it:
Resurrects @berknam's #5842 (abandoned 2021), merged
onto current master and reconciled for ~5 years of
drift. All the behavioral work is berknam's; I just
merged it (using Claude Code) and trimmed scope
unrelated to the issue. Adds keymodel support,
pseudo-modes for / round-trips,
in Replace, and Keyboard-kind selection-change
promotion (the actual #2224 fix).
Which issue(s) this PR fixes
fixes #2224
Special notes for your reviewer:
merge commit 6712628; my fixups are on top.
were never ported, mode constants had no behavioral
hookup) and allVisualModeKeyBindings (became
indistinguishable from visualModeKeyBindings once
Select mode was gone).