fix(vim): vim support that feels (more) complete#18755
fix(vim): vim support that feels (more) complete#18755scidomino merged 12 commits intogoogle-gemini:mainfrom
Conversation
- Add Ctrl+[ as an alternative key binding for ESCAPE command, matching standard Vim behavior where Ctrl+[ acts as ESC - Allow Ctrl+U (kill line left) and Ctrl+K (kill line right) to pass through to InputPrompt in Vim insert mode, enabling standard line editing shortcuts Fixes google-gemini#14689, google-gemini#17380
Summary of ChangesHello @ppgranger, I'm Gemini Code Assist1! 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 enhances the Vim mode usability by introducing two key features. It adds support for Ctrl+[ as an alternative to the Escape key, a common Vim practice, and allows Ctrl+U and Ctrl+K for line editing in insert mode to pass through to the input prompt. These changes aim to provide a more complete and familiar Vim experience for users. Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
- gg with count: 5gg now correctly goes to line 5 instead of line 1 - $ with count: 2$ now moves down 1 line then to end of line - . (repeat) with count: 3. now repeats last command 3 times instead of using the original count
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two quality-of-life improvements for vim mode. It adds Ctrl+[ as a standard alternative for the Escape key and enables Ctrl+U and Ctrl+K for line editing within insert mode. The changes are well-implemented and align with standard vim and shell behaviors. I have reviewed the code and found no issues.
gemini-cli
bot
added
priority/p2
|
/review |
- Add 'u' handler in normal mode to undo changes (supports count prefix) - D command now respects count prefix (2D deletes to end of 2 lines) - C command now respects count prefix (2C changes to end of 2 lines)
The ^ command now correctly positions the cursor at the last valid character instead of past the end of line when the line is empty or contains only whitespace.
ppgranger
changed the title
- Add dh/dj/dk/dl commands to delete with directional motions - Add d$ command to delete from cursor to end of line - Add d0 command to delete from start of line to cursor - Support arrow keys with delete operator (d+arrow) - Add delete movement commands to executeCommand for repeat (.) support - Create vimDeleteToStartOfLine method in text buffer - Fix cj/ck/dj/dk to correctly delete current line + count lines (VIM behavior: 2dj deletes current line + 2 lines below = 3 lines) Fixes google-gemini#15988
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly improves the Vim mode by adding many missing keybindings and commands, and fixing several count-related bugs. The changes for gg, $, ., and directional movements like dj/dk are great additions and align well with standard Vim behavior.
I've identified a critical issue where the count prefix for the D, C, and d$ commands is not correctly implemented. While the PR description mentions this is fixed, the underlying logic doesn't handle multi-line operations for these commands. I've left a detailed comment with suggestions on how to address this.
Add missing VIM operator-motion commands: - d^/c^: delete/change to first non-whitespace character - c0: change to start of line - dgg/cgg: delete/change from first line (or line N) to current line - dG/cG: delete/change from current line to last line (or line N) Fix D/C/d$/c$ to properly handle count prefix (e.g., 2D deletes to EOL plus next line). Add comprehensive tests for all new commands including edge cases for whitespace-only lines and bidirectional deletion.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the Vim mode by adding many missing keybindings and commands, and fixing several bugs. The security review found no vulnerabilities. One issue was identified with the implementation of the ^ command's behavior on whitespace-only lines. Otherwise, the new features like d$, d0, dgg, u with counts, and Ctrl+[ for escape are great additions.
Align vim_move_to_first_nonwhitespace with standard Vim behavior: on a whitespace-only or empty line, ^ should move the cursor to column 0, not to the last character of the line.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is a fantastic pull request that significantly enhances the Vim mode with many much-needed features and fixes. The implementation of operators like d and c with various motions, along with correct count handling for commands like gg, D, C, and . is a huge improvement for Vim users. The code is well-structured, separating the Vim state management in the useVim hook from the text manipulation logic in vim-buffer-actions. I've identified a critical issue regarding Unicode character handling for cj and ck commands which could lead to data corruption, but otherwise, the changes are solid and the added tests are comprehensive. Great work!
Rewrite cj and ck movement handlers to use direct array splice instead of getLineRangeOffsets/getPositionFromOffsets. The previous implementation used string.length (UTF-16 code units) for offset calculations, which caused data corruption when lines contained Unicode characters like emojis. The new implementation: - Directly splices lines from the array, avoiding UTF-16/code point mismatch - Properly handles edge cases (delete all lines, first/last line) - Adds comprehensive tests including Unicode characters Tests added: - Unicode characters in cj (down) and ck (up) - cj on first line of 2 lines (delete all) - cj on last line (delete only current) - ck on first line (delete only current) - 2cj and 2ck from middle line
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly enhances the Vim mode functionality, making it more complete and familiar for Vim users. It introduces numerous bug fixes and features, including correct handling of count prefixes for commands like gg, $, ., D, C, and implements missing commands such as u, d$, d0, and d+motions. The implementation is robust, well-structured, and thoroughly tested. No specific security vulnerabilities or issues of high or critical severity were identified.
|
@jacob314 Ready for review :) |
scidomino
left a comment
scidomino
left a comment
There was a problem hiding this comment.
Solid work. I'm worried we are going to run into hotkey conflicts but I can't find any.
|
We have some test breakages. Please fix. |
|
Ah, you modified the keybindings but didn't run the update script to regen keyboard-shortcuts.md. Please fix. Otherwise I will fix it for you eventually if I find the time. |
5b48846
2 participants
Summary
Improve Vim mode by adding missing keybindings, fixing count prefix handling, and implementing delete operator with motions.
Details
Keybinding additions
Count prefix fixes
Missing commands
Cursor position fixes
Behavior fixes
Related Issues
Fixes #14689
Fixes #15988
Fixes #17380
Related to #16846
Pre-Merge Checklist