fix(cli): make vim cc clear non-last and astral-character lines#27554
fix(cli): make vim cc clear non-last and astral-character lines#27554Pluviobyte wants to merge 1 commit into
cc clear non-last and astral-character lines#27554Conversation
`cc` (change-line) silently did nothing on any non-last line of a multi-line buffer, and on a single line containing an astral character (e.g. an emoji). The change-line handler derived the end of the range with two helpers that (a) counted a trailing newline after the last changed line and (b) measured columns in UTF-16 code units. The resulting end column landed one past the end of the line (or split a surrogate pair), so `replaceRangeInternal` treated the range as invalid and returned the state unchanged — while the UI still switched to INSERT mode. Compute the range directly in code points instead: from column 0 of the first line to the code-point length of the last changed line. This correctly clears single lines, non-last lines, multi-line counts (collapsing them into one empty line) and astral characters. The now-unused getLineRangeOffsets / getPositionFromOffsets helpers (only used here) are removed. Adds regression tests for the multi-line, emoji and multi-count cases.
Summary of ChangesHello, 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 addresses a bug in the Vim emulation layer where the Highlights
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 the 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 counterproductive. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the vim_change_line action in the text buffer to correctly handle astral characters (such as emojis) and non-last lines of multi-line buffers. It removes the offset-based helper functions getPositionFromOffsets and getLineRangeOffsets from text-buffer.ts and instead computes the target range directly using code point lengths (cpLen). Additionally, comprehensive unit tests have been added to verify these scenarios. There are no review comments, and I have no additional feedback to provide.
Summary
Vim
cc(change-line) silently does nothing on any non-last line of a multi-line buffer, and on a single line containing an astral character (e.g. an emoji). It clears the line correctly only when the cursor is on the buffer's last line and the line is ASCII/BMP-only.Details
vim_change_linederived the end of the range with two helpers (getLineRangeOffsets/getPositionFromOffsets) that:So the computed end column landed one position past the end of the line (e.g.
endCol = 12for"hello world"), or split a surrogate pair (endCol = 2for"😀").replaceRangeInternalrejects a range whoseendCol > cpLen(line)and returns the state unchanged — a silent no-op, while the UI still switched to INSERT mode.The fix computes the range directly in code points: from column 0 of the first line to
cpLen(lastChangedLine). This clears single lines, non-last lines, multi-line counts (collapsing them into a single empty line, matching Vim), and astral characters. The two offset helpers were only used here, so they are removed.Before / after on
['hello world', 'foo']with the cursor on line 0:cc→['hello world', 'foo'](unchanged)cc→['', 'foo']Related Issues
None — found while reviewing the text-buffer vim handlers. Happy to file a tracking issue if preferred.
How to Validate
cd packages/cli npm run typecheck npx vitest run src/ui/components/shared/vim-buffer-actions.test.ts src/ui/components/shared/text-buffer.test.tsNew tests in
vim-buffer-actions.test.tscoverccon a non-last line, on a single-line emoji buffer, and with a count (2cc). They fail on the previous implementation (the buffer is unchanged) and pass with this fix. Existing single-linecc,dd, and alltext-buffertests continue to pass (427 tests).Pre-Merge Checklist
cc)vitest+tsc --noEmit)Made with Cursor