fix(vi): enforce cursor invariant in normal mode#1069
Open
sim590 wants to merge 7 commits into
Open
Conversation
In Vi normal mode the cursor sits *on* a character and must not go past the last grapheme. Add a clamp_cursor() method to Editor that enforces this invariant whenever the edit mode is Vi normal. Call clamp_cursor() after every EditCommand in run_edit_command(), and in set_buffer(), move_to_end() and move_to_line_end() so that history loading and end-of-line motions always respect the bound. Also adjust is_cursor_at_buffer_end() to treat "cursor on the last grapheme" as equivalent to "at buffer end" in Vi normal mode, so that prefix history search and inline hints continue to work. Fixes nushell#694 (pt 2), nushell#788 (pt 4)
In Vi, insert mode places the cursor between characters while normal mode places it on a character. Pressing Escape to leave insert mode must move the cursor one position left to match standard Vi behavior. When Escape is pressed from normal mode the cursor is not moved. Fixes nushell#694 (pt 1), nushell#788 (pt 1)
ReedlineEvent::Esc is handled outside of run_edit_commands(), so the editor's edit_mode was never updated on mode transitions triggered by Escape. Sync the editor's mode and clamp the cursor at that point so that the cursor invariant is enforced immediately.
Add EditCommand::ClampCursorToNormalMode, implemented in Editor as a call to clamp_cursor(). The Vi normal mode emits this command after every editing operation (delete, yank, paste, undo, replace) via command.rs and to_reedline_with_motion. Add clamp_cursor() guards to move_right, move_word_right* and move_big_word_right* in Editor so that pure motion commands also respect the Vi normal mode cursor invariant. LineBuffer remains fully mode-agnostic. The Vi mode owns its own cursor invariant at the dispatch boundary, per the design discussed in nushell#1066.
All Vi normal mode editing commands now emit ClampCursorToNormalMode after their edit command. Update the expected ReedlineEvent values in test_reedline_move, test_reedline_memory_move and test_reedline_move_in_visual_mode accordingly.
Add tests verifying that Vi normal mode commands emit ClampCursorToNormalMode when they should (x, p, P, u, dd, D, ~, r, dw, db, d$, yw) and do not emit it when they transition to insert mode (i, a, cc, cw, s).
False positive from the multibyte cursor clamping test that uses the string "café" and asserts against "caf".len().
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
1 participant
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.
Summary
In Vi normal mode, the cursor sits on a character and must not go past the last grapheme. This was not enforced, causing the cursor to land one position past the end of the line after various operations (movements, deletions, paste, undo, etc.).
This PR implements the approach discussed in #1066 (Approach C1): the Vi mode owns its cursor invariant and emits
EditCommand::ClampCursorToNormalModeafter each editing command that stays in normal mode.LineBufferremains fully mode-agnostic.Changes
EditCommand::ClampCursorToNormalMode, implemented inEditor::clamp_cursor()ClampCursorToNormalModeafter every Vi normal mode editing command incommand.rs(to_reedlineandto_reedline_with_motion)clamp_cursor()guards on rightward movement methods inEditor(move_right,move_to_end,move_to_line_end,move_word_right*,move_big_word_right*)set_bufferso that history load, completion accept, and paste also respect the invariantEditoron Escape event so the cursor invariant is enforced immediatelyis_cursor_at_buffer_end()to treat "cursor on last grapheme" as "at buffer end" in Vi normal mode, preserving prefix history search and inline hintsTest plan
x,p,P,u,dd,D,~,r,dw,db,d$,yw) emitClampCursorToNormalModei,a,cc,cw,s) do not emit itCloses #694 (cursor past end in normal mode)
Relates to #1066