diff --git a/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts b/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts index 98f2cafcf0..2bf5cfed08 100644 --- a/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts +++ b/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts @@ -572,6 +572,21 @@ describe('vim-buffer-actions', () => { const result = handleVimAction(state, action); expect(result).toHaveOnlyValidCharacters(); expect(result.lines[0]).toBe('hel'); + // Cursor clamps to last char of the shortened line (vim NORMAL mode + // cursor cannot rest past the final character). + expect(result.cursorCol).toBe(2); + }); + + it('should clamp cursor when deleting the last character on a line', () => { + const state = createTestState(['hello'], 0, 4); + const action = { + type: 'vim_delete_char' as const, + payload: { count: 1 }, + }; + + const result = handleVimAction(state, action); + expect(result).toHaveOnlyValidCharacters(); + expect(result.lines[0]).toBe('hell'); expect(result.cursorCol).toBe(3); }); @@ -626,7 +641,7 @@ describe('vim-buffer-actions', () => { const result = handleVimAction(state, action); expect(result).toHaveOnlyValidCharacters(); expect(result.lines[0]).toBe('hello '); - expect(result.cursorCol).toBe(6); + expect(result.cursorCol).toBe(5); }); it('should delete only the word characters if it is the last word followed by whitespace', () => { @@ -666,6 +681,55 @@ describe('vim-buffer-actions', () => { expect(result).toHaveOnlyValidCharacters(); expect(result.lines[0]).toBe('foo '); }); + + it('should clamp cursor when dW removes the last word leaving only a trailing space', () => { + // cursor on 'w' in 'hello world'; dW deletes 'world' → 'hello ' + const state = createTestState(['hello world'], 0, 6); + const result = handleVimAction(state, { + type: 'vim_delete_big_word_forward' as const, + payload: { count: 1 }, + }); + expect(result.lines[0]).toBe('hello '); + // col 6 is past the new line end (len 6, max valid = 5) + expect(result.cursorCol).toBe(5); + }); + }); + + describe('vim_delete_word_end', () => { + it('should clamp cursor when de removes the last word on a line', () => { + // cursor on 'w' in 'hello world'; de deletes through 'd' → 'hello ' + const state = createTestState(['hello world'], 0, 6); + const result = handleVimAction(state, { + type: 'vim_delete_word_end' as const, + payload: { count: 1 }, + }); + expect(result.lines[0]).toBe('hello '); + expect(result.cursorCol).toBe(5); + }); + }); + + describe('vim_delete_big_word_end', () => { + it('should delete from cursor to end of WORD (skipping punctuation)', () => { + // cursor on 'b' in 'foo bar.baz qux'; dE treats 'bar.baz' as one WORD + const state = createTestState(['foo bar.baz qux'], 0, 4); + const result = handleVimAction(state, { + type: 'vim_delete_big_word_end' as const, + payload: { count: 1 }, + }); + expect(result.lines[0]).toBe('foo qux'); + expect(result.cursorCol).toBe(4); + }); + + it('should clamp cursor when dE removes the last WORD on a line', () => { + // cursor on 'w' in 'hello world'; dE deletes through 'd' → 'hello ' + const state = createTestState(['hello world'], 0, 6); + const result = handleVimAction(state, { + type: 'vim_delete_big_word_end' as const, + payload: { count: 1 }, + }); + expect(result.lines[0]).toBe('hello '); + expect(result.cursorCol).toBe(5); + }); }); describe('vim_delete_word_backward', () => { @@ -751,7 +815,7 @@ describe('vim-buffer-actions', () => { const result = handleVimAction(state, action); expect(result).toHaveOnlyValidCharacters(); expect(result.lines[0]).toBe('hello'); - expect(result.cursorCol).toBe(5); + expect(result.cursorCol).toBe(4); }); it('should do nothing at end of line', () => { @@ -781,7 +845,7 @@ describe('vim-buffer-actions', () => { expect(result).toHaveOnlyValidCharacters(); // 2D at position 5 on "line one" should delete "one" + entire "line two" expect(result.lines).toEqual(['line ', 'line three']); - expect(result.cursorCol).toBe(5); + expect(result.cursorCol).toBe(4); }); it('should handle count exceeding available lines', () => { @@ -2094,6 +2158,18 @@ describe('vim-buffer-actions', () => { }); expect(result.undoStack.length).toBeGreaterThan(0); }); + + it('df: clamps cursor when deleting through the last char on the line', () => { + // cursor at 1 in 'hello'; dfo finds 'o' at col 4 and deletes [1,4] → 'h' + const state = createTestState(['hello'], 0, 1); + const result = handleVimAction(state, { + type: 'vim_delete_to_char_forward' as const, + payload: { char: 'o', count: 1, till: false }, + }); + expect(result.lines[0]).toBe('h'); + // cursor was at col 1, new line has only col 0 valid + expect(result.cursorCol).toBe(0); + }); }); describe('vim_delete_to_char_backward (dF/dT)', () => { @@ -2138,5 +2214,17 @@ describe('vim-buffer-actions', () => { }); expect(result.undoStack.length).toBeGreaterThan(0); }); + + it('dF: clamps cursor when deletion removes chars up to end of line', () => { + // 'hello', cursor on last char 'o' (col 4), dFe finds 'e' at col 1 + // deletes [1, 5) → 'h'; without clamp cursor would be at col 1 (past end) + const state = createTestState(['hello'], 0, 4); + const result = handleVimAction(state, { + type: 'vim_delete_to_char_backward' as const, + payload: { char: 'e', count: 1, till: false }, + }); + expect(result.lines[0]).toBe('h'); + expect(result.cursorCol).toBe(0); + }); }); }); diff --git a/packages/cli/src/ui/components/shared/vim-buffer-actions.ts b/packages/cli/src/ui/components/shared/vim-buffer-actions.ts index 28cd20f054..c4a65d0d67 100644 --- a/packages/cli/src/ui/components/shared/vim-buffer-actions.ts +++ b/packages/cli/src/ui/components/shared/vim-buffer-actions.ts @@ -109,6 +109,20 @@ function findCharInLine( return found; } +/** + * In NORMAL mode the cursor can never rest past the last character of a line. + * Call this after any delete action that stays in NORMAL mode to enforce that + * invariant. Change actions must NOT use this — they immediately enter INSERT + * mode where the cursor is allowed to sit at the end of the line. + */ +function clampNormalCursor(state: TextBufferState): TextBufferState { + const line = state.lines[state.cursorRow] || ''; + const len = cpLen(line); + const maxCol = Math.max(0, len - 1); + if (state.cursorCol <= maxCol) return state; + return { ...state, cursorCol: maxCol }; +} + export function handleVimAction( state: TextBufferState, action: VimAction, @@ -143,7 +157,7 @@ export function handleVimAction( if (endRow !== cursorRow || endCol !== cursorCol) { const nextState = detachExpandedPaste(pushUndo(state)); - return replaceRangeInternal( + const newState = replaceRangeInternal( nextState, cursorRow, cursorCol, @@ -151,6 +165,9 @@ export function handleVimAction( endCol, '', ); + return action.type === 'vim_delete_word_forward' + ? clampNormalCursor(newState) + : newState; } return state; } @@ -185,7 +202,7 @@ export function handleVimAction( if (endRow !== cursorRow || endCol !== cursorCol) { const nextState = pushUndo(state); - return replaceRangeInternal( + const newState = replaceRangeInternal( nextState, cursorRow, cursorCol, @@ -193,6 +210,9 @@ export function handleVimAction( endCol, '', ); + return action.type === 'vim_delete_big_word_forward' + ? clampNormalCursor(newState) + : newState; } return state; } @@ -298,7 +318,7 @@ export function handleVimAction( if (endRow !== cursorRow || endCol !== cursorCol) { const nextState = detachExpandedPaste(pushUndo(state)); - return replaceRangeInternal( + const newState = replaceRangeInternal( nextState, cursorRow, cursorCol, @@ -306,6 +326,9 @@ export function handleVimAction( endCol, '', ); + return action.type === 'vim_delete_word_end' + ? clampNormalCursor(newState) + : newState; } return state; } @@ -351,7 +374,7 @@ export function handleVimAction( if (endRow !== cursorRow || endCol !== cursorCol) { const nextState = pushUndo(state); - return replaceRangeInternal( + const newState = replaceRangeInternal( nextState, cursorRow, cursorCol, @@ -359,6 +382,9 @@ export function handleVimAction( endCol, '', ); + return action.type === 'vim_delete_big_word_end' + ? clampNormalCursor(newState) + : newState; } return state; } @@ -432,12 +458,13 @@ export function handleVimAction( const { count } = action.payload; const currentLine = lines[cursorRow] || ''; const totalLines = lines.length; + const isDelete = action.type === 'vim_delete_to_end_of_line'; if (count === 1) { // Single line: delete from cursor to end of current line if (cursorCol < cpLen(currentLine)) { const nextState = detachExpandedPaste(pushUndo(state)); - return replaceRangeInternal( + const newState = replaceRangeInternal( nextState, cursorRow, cursorCol, @@ -445,6 +472,7 @@ export function handleVimAction( cpLen(currentLine), '', ); + return isDelete ? clampNormalCursor(newState) : newState; } return state; } else { @@ -457,7 +485,7 @@ export function handleVimAction( // No additional lines to delete, just delete to EOL if (cursorCol < cpLen(currentLine)) { const nextState = detachExpandedPaste(pushUndo(state)); - return replaceRangeInternal( + const newState = replaceRangeInternal( nextState, cursorRow, cursorCol, @@ -465,6 +493,7 @@ export function handleVimAction( cpLen(currentLine), '', ); + return isDelete ? clampNormalCursor(newState) : newState; } return state; } @@ -472,7 +501,7 @@ export function handleVimAction( // Delete from cursor position to end of endRow (including newlines) const nextState = detachExpandedPaste(pushUndo(state)); const endLine = lines[endRow] || ''; - return replaceRangeInternal( + const newState = replaceRangeInternal( nextState, cursorRow, cursorCol, @@ -480,6 +509,7 @@ export function handleVimAction( cpLen(endLine), '', ); + return isDelete ? clampNormalCursor(newState) : newState; } } @@ -1035,7 +1065,7 @@ export function handleVimAction( if (cursorCol < lineLength) { const deleteCount = Math.min(count, lineLength - cursorCol); const nextState = detachExpandedPaste(pushUndo(state)); - return replaceRangeInternal( + const newState = replaceRangeInternal( nextState, cursorRow, cursorCol, @@ -1043,6 +1073,7 @@ export function handleVimAction( cursorCol + deleteCount, '', ); + return clampNormalCursor(newState); } return state; } @@ -1298,13 +1329,15 @@ export function handleVimAction( if (found === -1) return state; const endCol = till ? found : found + 1; const nextState = detachExpandedPaste(pushUndo(state)); - return replaceRangeInternal( - nextState, - cursorRow, - cursorCol, - cursorRow, - endCol, - '', + return clampNormalCursor( + replaceRangeInternal( + nextState, + cursorRow, + cursorCol, + cursorRow, + endCol, + '', + ), ); } @@ -1331,7 +1364,11 @@ export function handleVimAction( endCol, '', ); - return { ...resultState, cursorCol: startCol, preferredCol: null }; + return clampNormalCursor({ + ...resultState, + cursorCol: startCol, + preferredCol: null, + }); } case 'vim_find_char_forward': { diff --git a/packages/cli/src/ui/hooks/vim.test.tsx b/packages/cli/src/ui/hooks/vim.test.tsx index a8d854c821..8842c83162 100644 --- a/packages/cli/src/ui/hooks/vim.test.tsx +++ b/packages/cli/src/ui/hooks/vim.test.tsx @@ -1044,9 +1044,10 @@ describe('useVim hook', () => { }); // Should delete "world" (no trailing space at end), leaving "hello " + // Cursor clamps to last valid index in NORMAL mode (col 5 = trailing space) expect(result.lines).toEqual(['hello ']); expect(result.cursorRow).toBe(0); - expect(result.cursorCol).toBe(6); + expect(result.cursorCol).toBe(5); }); it('should delete multiple words with count', () => { @@ -1726,7 +1727,8 @@ describe('useVim hook', () => { count: 1, expectedLines: ['hello '], expectedCursorRow: 0, - expectedCursorCol: 6, + // Cursor clamps to last valid index in NORMAL mode (col 5 = trailing space) + expectedCursorCol: 5, }, { command: 'D',