From 72832fb889729566eb150830d5b1da684963b098 Mon Sep 17 00:00:00 2001 From: Sijie Wang <3463757+sijieamoy@users.noreply.github.com> Date: Mon, 11 Aug 2025 11:58:32 -0700 Subject: Fix line end bugs in Vim mode (#5328) Co-authored-by: Jacob Richman --- .../components/shared/vim-buffer-actions.test.ts | 325 ++++++++++++++++++++- 1 file changed, 324 insertions(+), 1 deletion(-) (limited to 'packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts') 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 8f7f72ab..a07d8df1 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 @@ -140,6 +140,25 @@ describe('vim-buffer-actions', () => { expect(result.cursorRow).toBe(1); expect(result.cursorCol).toBe(0); }); + + it('should skip over combining marks to avoid cursor disappearing', () => { + // Test case for combining character cursor disappearing bug + // "café test" where é is represented as e + combining acute accent + const state = createTestState(['cafe\u0301 test'], 0, 2); // Start at 'f' + const action = { + type: 'vim_move_right' as const, + payload: { count: 1 }, + }; + + const result = handleVimAction(state, action); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(3); // Should be on 'e' of 'café' + + // Move right again - should skip combining mark and land on space + const result2 = handleVimAction(result, action); + expect(result2).toHaveOnlyValidCharacters(); + expect(result2.cursorCol).toBe(5); // Should be on space after 'café' + }); }); describe('vim_move_up', () => { @@ -169,7 +188,7 @@ describe('vim-buffer-actions', () => { const result = handleVimAction(state, action); expect(result).toHaveOnlyValidCharacters(); expect(result.cursorRow).toBe(0); - expect(result.cursorCol).toBe(5); // End of 'short' + expect(result.cursorCol).toBe(4); // Last character 't' of 'short', not past it }); }); @@ -236,6 +255,20 @@ describe('vim-buffer-actions', () => { expect(result).toHaveOnlyValidCharacters(); expect(result.cursorCol).toBe(5); // Start of ',' }); + + it('should move across empty lines when starting from within a word', () => { + // Testing the exact scenario: cursor on 'w' of 'hello world', w should move to next line + const state = createTestState(['hello world', ''], 0, 6); // At 'w' of 'world' + const action = { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }; + + const result = handleVimAction(state, action); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorRow).toBe(1); // Should move to empty line + expect(result.cursorCol).toBe(0); // Beginning of empty line + }); }); describe('vim_move_word_backward', () => { @@ -288,6 +321,85 @@ describe('vim-buffer-actions', () => { expect(result).toHaveOnlyValidCharacters(); expect(result.cursorCol).toBe(10); // End of 'world' }); + + it('should move across empty lines when at word end', () => { + const state = createTestState(['hello world', '', 'test'], 0, 10); // At 'd' of 'world' + const action = { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }; + + const result = handleVimAction(state, action); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorRow).toBe(2); // Should move to line with 'test' + expect(result.cursorCol).toBe(3); // Should be at 't' (end of 'test') + }); + + it('should handle consecutive word-end movements across empty lines', () => { + // Testing the exact scenario: cursor on 'w' of world, press 'e' twice + const state = createTestState(['hello world', ''], 0, 6); // At 'w' of 'world' + + // First 'e' should move to 'd' of 'world' + let result = handleVimAction(state, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorRow).toBe(0); + expect(result.cursorCol).toBe(10); // At 'd' of 'world' + + // Second 'e' should move to the empty line (end of file in this case) + result = handleVimAction(result, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorRow).toBe(1); // Should move to empty line + expect(result.cursorCol).toBe(0); // Empty line has col 0 + }); + + it('should handle combining characters - advance from end of base character', () => { + // Test case for combining character word end bug + // "café test" where é is represented as e + combining acute accent + const state = createTestState(['cafe\u0301 test'], 0, 0); // Start at 'c' + + // First 'e' command should move to the 'e' (position 3) + let result = handleVimAction(state, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(3); // At 'e' of café + + // Second 'e' command should advance to end of "test" (position 9), not stay stuck + result = handleVimAction(result, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(9); // At 't' of "test" + }); + + it('should handle precomposed characters with diacritics', () => { + // Test case with precomposed é for comparison + const state = createTestState(['café test'], 0, 0); // Start at 'c' + + // First 'e' command should move to the 'é' (position 3) + let result = handleVimAction(state, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(3); // At 'é' of café + + // Second 'e' command should advance to end of "test" (position 8) + result = handleVimAction(result, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(8); // At 't' of "test" + }); }); describe('Position commands', () => { @@ -793,4 +905,215 @@ describe('vim-buffer-actions', () => { expect(result.undoStack).toHaveLength(2); // Original plus new snapshot }); }); + + describe('UTF-32 character handling in word/line operations', () => { + describe('Right-to-left text handling', () => { + it('should handle Arabic text in word movements', () => { + const state = createTestState(['hello مرحبا world'], 0, 0); + + // Move to end of 'hello' + let result = handleVimAction(state, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(4); // End of 'hello' + + // Move to end of Arabic word + result = handleVimAction(result, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(10); // End of Arabic word 'مرحبا' + }); + }); + + describe('Chinese character handling', () => { + it('should handle Chinese characters in word movements', () => { + const state = createTestState(['hello 你好 world'], 0, 0); + + // Move to end of 'hello' + let result = handleVimAction(state, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(4); // End of 'hello' + + // Move forward to start of 'world' + result = handleVimAction(result, { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(6); // Start of '你好' + }); + }); + + describe('Mixed script handling', () => { + it('should handle mixed Latin and non-Latin scripts with word end commands', () => { + const state = createTestState(['test中文test'], 0, 0); + + let result = handleVimAction(state, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(3); // End of 'test' + + // Second word end command should move to end of '中文' + result = handleVimAction(result, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(5); // End of '中文' + }); + + it('should handle mixed Latin and non-Latin scripts with word forward commands', () => { + const state = createTestState(['test中文test'], 0, 0); + + let result = handleVimAction(state, { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(4); // Start of '中' + + // Second word forward command should move to start of final 'test' + result = handleVimAction(result, { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(6); // Start of final 'test' + }); + + it('should handle mixed Latin and non-Latin scripts with word backward commands', () => { + const state = createTestState(['test中文test'], 0, 9); // Start at end of final 'test' + + let result = handleVimAction(state, { + type: 'vim_move_word_backward' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(6); // Start of final 'test' + + // Second word backward command should move to start of '中文' + result = handleVimAction(result, { + type: 'vim_move_word_backward' as const, + payload: { count: 1 }, + }); + expect(result).toHaveOnlyValidCharacters(); + expect(result.cursorCol).toBe(4); // Start of '中' + }); + + it('should handle Unicode block characters consistently with w and e commands', () => { + const state = createTestState(['██ █████ ██'], 0, 0); + + // Test w command progression + let wResult = handleVimAction(state, { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }); + expect(wResult).toHaveOnlyValidCharacters(); + expect(wResult.cursorCol).toBe(3); // Start of second block sequence + + wResult = handleVimAction(wResult, { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }); + expect(wResult).toHaveOnlyValidCharacters(); + expect(wResult.cursorCol).toBe(9); // Start of third block sequence + + // Test e command progression from beginning + let eResult = handleVimAction(state, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(eResult).toHaveOnlyValidCharacters(); + expect(eResult.cursorCol).toBe(1); // End of first block sequence + + eResult = handleVimAction(eResult, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(eResult).toHaveOnlyValidCharacters(); + expect(eResult.cursorCol).toBe(7); // End of second block sequence + + eResult = handleVimAction(eResult, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(eResult).toHaveOnlyValidCharacters(); + expect(eResult.cursorCol).toBe(10); // End of third block sequence + }); + + it('should handle strings starting with Chinese characters', () => { + const state = createTestState(['中文test英文word'], 0, 0); + + // Test 'w' command - when at start of non-Latin word, w moves to next word + let wResult = handleVimAction(state, { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }); + expect(wResult).toHaveOnlyValidCharacters(); + expect(wResult.cursorCol).toBe(2); // Start of 'test' + + wResult = handleVimAction(wResult, { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }); + expect(wResult.cursorCol).toBe(6); // Start of '英文' + + // Test 'e' command + let eResult = handleVimAction(state, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(eResult).toHaveOnlyValidCharacters(); + expect(eResult.cursorCol).toBe(1); // End of 中文 + + eResult = handleVimAction(eResult, { + type: 'vim_move_word_end' as const, + payload: { count: 1 }, + }); + expect(eResult.cursorCol).toBe(5); // End of test + }); + + it('should handle strings starting with Arabic characters', () => { + const state = createTestState(['مرحباhelloسلام'], 0, 0); + + // Test 'w' command - when at start of non-Latin word, w moves to next word + let wResult = handleVimAction(state, { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }); + expect(wResult).toHaveOnlyValidCharacters(); + expect(wResult.cursorCol).toBe(5); // Start of 'hello' + + wResult = handleVimAction(wResult, { + type: 'vim_move_word_forward' as const, + payload: { count: 1 }, + }); + expect(wResult.cursorCol).toBe(10); // Start of 'سلام' + + // Test 'b' command from end + const bState = createTestState(['مرحباhelloسلام'], 0, 13); + let bResult = handleVimAction(bState, { + type: 'vim_move_word_backward' as const, + payload: { count: 1 }, + }); + expect(bResult).toHaveOnlyValidCharacters(); + expect(bResult.cursorCol).toBe(10); // Start of سلام + + bResult = handleVimAction(bResult, { + type: 'vim_move_word_backward' as const, + payload: { count: 1 }, + }); + expect(bResult.cursorCol).toBe(5); // Start of hello + }); + }); + }); }); -- cgit v1.2.3