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

This commit is contained in:
Ali Anari
2026-03-10 23:22:12 -07:00
committed by GitHub
parent e54c450bc1
commit f16f13fba8
3 changed files with 148 additions and 21 deletions
@@ -572,6 +572,21 @@ describe('vim-buffer-actions', () => {
const result = handleVimAction(state, action); const result = handleVimAction(state, action);
expect(result).toHaveOnlyValidCharacters(); expect(result).toHaveOnlyValidCharacters();
expect(result.lines[0]).toBe('hel'); 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); expect(result.cursorCol).toBe(3);
}); });
@@ -626,7 +641,7 @@ describe('vim-buffer-actions', () => {
const result = handleVimAction(state, action); const result = handleVimAction(state, action);
expect(result).toHaveOnlyValidCharacters(); expect(result).toHaveOnlyValidCharacters();
expect(result.lines[0]).toBe('hello '); 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', () => { 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).toHaveOnlyValidCharacters();
expect(result.lines[0]).toBe('foo '); 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', () => { describe('vim_delete_word_backward', () => {
@@ -751,7 +815,7 @@ describe('vim-buffer-actions', () => {
const result = handleVimAction(state, action); const result = handleVimAction(state, action);
expect(result).toHaveOnlyValidCharacters(); expect(result).toHaveOnlyValidCharacters();
expect(result.lines[0]).toBe('hello'); expect(result.lines[0]).toBe('hello');
expect(result.cursorCol).toBe(5); expect(result.cursorCol).toBe(4);
}); });
it('should do nothing at end of line', () => { it('should do nothing at end of line', () => {
@@ -781,7 +845,7 @@ describe('vim-buffer-actions', () => {
expect(result).toHaveOnlyValidCharacters(); expect(result).toHaveOnlyValidCharacters();
// 2D at position 5 on "line one" should delete "one" + entire "line two" // 2D at position 5 on "line one" should delete "one" + entire "line two"
expect(result.lines).toEqual(['line ', 'line three']); expect(result.lines).toEqual(['line ', 'line three']);
expect(result.cursorCol).toBe(5); expect(result.cursorCol).toBe(4);
}); });
it('should handle count exceeding available lines', () => { it('should handle count exceeding available lines', () => {
@@ -2094,6 +2158,18 @@ describe('vim-buffer-actions', () => {
}); });
expect(result.undoStack.length).toBeGreaterThan(0); 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)', () => { describe('vim_delete_to_char_backward (dF/dT)', () => {
@@ -2138,5 +2214,17 @@ describe('vim-buffer-actions', () => {
}); });
expect(result.undoStack.length).toBeGreaterThan(0); 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);
});
}); });
}); });
@@ -109,6 +109,20 @@ function findCharInLine(
return found; 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( export function handleVimAction(
state: TextBufferState, state: TextBufferState,
action: VimAction, action: VimAction,
@@ -143,7 +157,7 @@ export function handleVimAction(
if (endRow !== cursorRow || endCol !== cursorCol) { if (endRow !== cursorRow || endCol !== cursorCol) {
const nextState = detachExpandedPaste(pushUndo(state)); const nextState = detachExpandedPaste(pushUndo(state));
return replaceRangeInternal( const newState = replaceRangeInternal(
nextState, nextState,
cursorRow, cursorRow,
cursorCol, cursorCol,
@@ -151,6 +165,9 @@ export function handleVimAction(
endCol, endCol,
'', '',
); );
return action.type === 'vim_delete_word_forward'
? clampNormalCursor(newState)
: newState;
} }
return state; return state;
} }
@@ -185,7 +202,7 @@ export function handleVimAction(
if (endRow !== cursorRow || endCol !== cursorCol) { if (endRow !== cursorRow || endCol !== cursorCol) {
const nextState = pushUndo(state); const nextState = pushUndo(state);
return replaceRangeInternal( const newState = replaceRangeInternal(
nextState, nextState,
cursorRow, cursorRow,
cursorCol, cursorCol,
@@ -193,6 +210,9 @@ export function handleVimAction(
endCol, endCol,
'', '',
); );
return action.type === 'vim_delete_big_word_forward'
? clampNormalCursor(newState)
: newState;
} }
return state; return state;
} }
@@ -298,7 +318,7 @@ export function handleVimAction(
if (endRow !== cursorRow || endCol !== cursorCol) { if (endRow !== cursorRow || endCol !== cursorCol) {
const nextState = detachExpandedPaste(pushUndo(state)); const nextState = detachExpandedPaste(pushUndo(state));
return replaceRangeInternal( const newState = replaceRangeInternal(
nextState, nextState,
cursorRow, cursorRow,
cursorCol, cursorCol,
@@ -306,6 +326,9 @@ export function handleVimAction(
endCol, endCol,
'', '',
); );
return action.type === 'vim_delete_word_end'
? clampNormalCursor(newState)
: newState;
} }
return state; return state;
} }
@@ -351,7 +374,7 @@ export function handleVimAction(
if (endRow !== cursorRow || endCol !== cursorCol) { if (endRow !== cursorRow || endCol !== cursorCol) {
const nextState = pushUndo(state); const nextState = pushUndo(state);
return replaceRangeInternal( const newState = replaceRangeInternal(
nextState, nextState,
cursorRow, cursorRow,
cursorCol, cursorCol,
@@ -359,6 +382,9 @@ export function handleVimAction(
endCol, endCol,
'', '',
); );
return action.type === 'vim_delete_big_word_end'
? clampNormalCursor(newState)
: newState;
} }
return state; return state;
} }
@@ -432,12 +458,13 @@ export function handleVimAction(
const { count } = action.payload; const { count } = action.payload;
const currentLine = lines[cursorRow] || ''; const currentLine = lines[cursorRow] || '';
const totalLines = lines.length; const totalLines = lines.length;
const isDelete = action.type === 'vim_delete_to_end_of_line';
if (count === 1) { if (count === 1) {
// Single line: delete from cursor to end of current line // Single line: delete from cursor to end of current line
if (cursorCol < cpLen(currentLine)) { if (cursorCol < cpLen(currentLine)) {
const nextState = detachExpandedPaste(pushUndo(state)); const nextState = detachExpandedPaste(pushUndo(state));
return replaceRangeInternal( const newState = replaceRangeInternal(
nextState, nextState,
cursorRow, cursorRow,
cursorCol, cursorCol,
@@ -445,6 +472,7 @@ export function handleVimAction(
cpLen(currentLine), cpLen(currentLine),
'', '',
); );
return isDelete ? clampNormalCursor(newState) : newState;
} }
return state; return state;
} else { } else {
@@ -457,7 +485,7 @@ export function handleVimAction(
// No additional lines to delete, just delete to EOL // No additional lines to delete, just delete to EOL
if (cursorCol < cpLen(currentLine)) { if (cursorCol < cpLen(currentLine)) {
const nextState = detachExpandedPaste(pushUndo(state)); const nextState = detachExpandedPaste(pushUndo(state));
return replaceRangeInternal( const newState = replaceRangeInternal(
nextState, nextState,
cursorRow, cursorRow,
cursorCol, cursorCol,
@@ -465,6 +493,7 @@ export function handleVimAction(
cpLen(currentLine), cpLen(currentLine),
'', '',
); );
return isDelete ? clampNormalCursor(newState) : newState;
} }
return state; return state;
} }
@@ -472,7 +501,7 @@ export function handleVimAction(
// Delete from cursor position to end of endRow (including newlines) // Delete from cursor position to end of endRow (including newlines)
const nextState = detachExpandedPaste(pushUndo(state)); const nextState = detachExpandedPaste(pushUndo(state));
const endLine = lines[endRow] || ''; const endLine = lines[endRow] || '';
return replaceRangeInternal( const newState = replaceRangeInternal(
nextState, nextState,
cursorRow, cursorRow,
cursorCol, cursorCol,
@@ -480,6 +509,7 @@ export function handleVimAction(
cpLen(endLine), cpLen(endLine),
'', '',
); );
return isDelete ? clampNormalCursor(newState) : newState;
} }
} }
@@ -1035,7 +1065,7 @@ export function handleVimAction(
if (cursorCol < lineLength) { if (cursorCol < lineLength) {
const deleteCount = Math.min(count, lineLength - cursorCol); const deleteCount = Math.min(count, lineLength - cursorCol);
const nextState = detachExpandedPaste(pushUndo(state)); const nextState = detachExpandedPaste(pushUndo(state));
return replaceRangeInternal( const newState = replaceRangeInternal(
nextState, nextState,
cursorRow, cursorRow,
cursorCol, cursorCol,
@@ -1043,6 +1073,7 @@ export function handleVimAction(
cursorCol + deleteCount, cursorCol + deleteCount,
'', '',
); );
return clampNormalCursor(newState);
} }
return state; return state;
} }
@@ -1298,13 +1329,15 @@ export function handleVimAction(
if (found === -1) return state; if (found === -1) return state;
const endCol = till ? found : found + 1; const endCol = till ? found : found + 1;
const nextState = detachExpandedPaste(pushUndo(state)); const nextState = detachExpandedPaste(pushUndo(state));
return replaceRangeInternal( return clampNormalCursor(
nextState, replaceRangeInternal(
cursorRow, nextState,
cursorCol, cursorRow,
cursorRow, cursorCol,
endCol, cursorRow,
'', endCol,
'',
),
); );
} }
@@ -1331,7 +1364,11 @@ export function handleVimAction(
endCol, endCol,
'', '',
); );
return { ...resultState, cursorCol: startCol, preferredCol: null }; return clampNormalCursor({
...resultState,
cursorCol: startCol,
preferredCol: null,
});
} }
case 'vim_find_char_forward': { case 'vim_find_char_forward': {
+4 -2
View File
@@ -1044,9 +1044,10 @@ describe('useVim hook', () => {
}); });
// Should delete "world" (no trailing space at end), leaving "hello " // 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.lines).toEqual(['hello ']);
expect(result.cursorRow).toBe(0); expect(result.cursorRow).toBe(0);
expect(result.cursorCol).toBe(6); expect(result.cursorCol).toBe(5);
}); });
it('should delete multiple words with count', () => { it('should delete multiple words with count', () => {
@@ -1726,7 +1727,8 @@ describe('useVim hook', () => {
count: 1, count: 1,
expectedLines: ['hello '], expectedLines: ['hello '],
expectedCursorRow: 0, expectedCursorRow: 0,
expectedCursorCol: 6, // Cursor clamps to last valid index in NORMAL mode (col 5 = trailing space)
expectedCursorCol: 5,
}, },
{ {
command: 'D', command: 'D',