fix(ui): clamp cursor to last char after all NORMAL mode deletes

[aanari]

Follow-up to #21932, flagged by @jacob314 in review.

In NORMAL mode the cursor can't rest past the last character. replaceRangeInternal leaves the cursor at the pre-delete column after any delete, which can be one past the new line end. Added a clampNormalCursor() helper that applies max(0, lineLen - 1) and hooked it up across all NORMAL-mode deletes that stay in NORMAL mode.

Affected commands: x, dw, dW, de, dE, D, df/dt (and their backward variants)

Change commands (cw, C, cf, etc.) are intentionally excluded — they immediately enter INSERT mode where off-the-end cursor is valid.

• updated existing tests that had stale expected cursor positions
• added tests for dW, dE, df cursor clamp behavior

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a cursor positioning bug in Vim's NORMAL mode where the cursor could end up past the last character of a line after a deletion. The changes introduce a clamping mechanism to correctly position the cursor and include updated and new test cases to validate this behavior, ensuring a more accurate emulation of Vim's cursor handling.

Highlights

• Cursor Clamping Logic: Implemented a new cursor clamping mechanism after character deletion in Vim's NORMAL mode to ensure the cursor does not rest past the last character of the line.
• Test Case Update: Corrected the expected cursor column in an existing test case to reflect the proper Vim NORMAL mode behavior after deletion.
• New Test Case: Added a dedicated test case to explicitly verify that the cursor clamps correctly when the last character on a line is deleted.

Changelog

• packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts
  • Corrected the expected cursor column in an existing test case from 3 to 2.
  • Added a new test case to verify cursor clamping when deleting the last character on a line.
• packages/cli/src/ui/components/shared/vim-buffer-actions.ts
  • Modified the vim_delete_char action to apply cursor clamping.
  • Introduced logic to calculate the new line length and adjust cursorCol using Math.max(0, Math.min(newState.cursorCol, newLineLength - 1)).

Activity

• This pull request is a follow-up to feat(ui): add missing vim mode motions (X, ~, r, f/F/t/T, df/dt and friends) #21932, addressing feedback from @jacob314 regarding cursor behavior.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly fixes an issue where the cursor could be positioned past the end of a line after deleting a character with x in Vim's NORMAL mode. The change introduces logic to clamp the cursor to the last valid character position. While the fix is correct for the vim_delete_char case, I've identified that the same underlying bug affects other delete/change commands like D and C. My review includes a suggestion to apply this fix more broadly to ensure consistent behavior.

[jacob314]

Great work on this! This correctly addresses the cursor out-of-bounds issue for NORMAL mode shrinking deletions.

I ran the test suite locally and noticed two small things that need to be addressed before we can merge:

1. Failing Tests
The implementation is correct, but three existing tests in packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts are still asserting the old, out-of-bounds cursorCol values (e.g., expecting 6 instead of 5, and 5 instead of 4). These need to be updated to expect the newly clamped positions, specifically in:

• vim_delete_word_forward > should delete to end if no more words
• vim_delete_to_end_of_line > should delete from cursor to end of line
• vim_delete_to_end_of_line > should delete to end of line plus additional lines with count > 1

2. Missing clamping for dF/dT (vim_delete_to_char_backward)
While forward deletions (df, dt) were covered, backward character deletions can also leave the newly placed cursor out of bounds if it deletes backward through the last word on a line.

You can fix this by wrapping the return state of vim_delete_to_char_backward in clampNormalCursor just like the forward equivalents:

// In packages/cli/src/ui/components/shared/vim-buffer-actions.ts (around line 1367)
-      return { ...resultState, cursorCol: startCol, preferredCol: null };
+      return clampNormalCursor({ ...resultState, cursorCol: startCol, preferredCol: null });

It would also be good to add a test case for dF in vim-buffer-actions.test.ts to ensure the clamping works as expected.

Once the tests are updated and the backward delete clamping is added, this is good to go!

[aanari]

Thanks for catching those! Fixed in the latest commit:

• Updated the 3 stale test expectations (dw no-more-words, D basic, 2D)
• Added clampNormalCursor to vim_delete_to_char_backward — you're right, dT especially can land startCol one past the end of the shortened line (e.g. abc + cursor on 'c' + dTa → a with startCol = 1)
• Added a dF clamp test

Good to go on your end?

[jacob314]