From 193d6895709b404d5fef0bd2fc44481421584a07 Mon Sep 17 00:00:00 2001 From: rafaelbdb Date: Mon, 27 Oct 2025 00:29:15 -0300 Subject: [PATCH 1/2] perf: optimize movement commands with count to execute instantly Previously, movement commands with counts (e.g., 20j, 50k) would loop through each individual movement, causing visible lag and poor performance. This commit optimizes the four basic movement commands (j, k, h, l) by overriding execActionWithCount to calculate the target position directly instead of looping: - MoveDown (j): Calculates target line as current + count - MoveUp (k): Calculates target line as current - count - MoveLeft (h): Calculates target character with surrogate pair handling - MoveRight (l): Calculates target character with surrogate pair handling All special cases are preserved: - Interactive window history navigation - Foldfix mode compatibility - Operator context (d20j, y10k work correctly) - Visual mode selections - Column preservation for vertical movements - Boundary checks (don't exceed document limits) Additionally fixed build issues: - Updated test/index.ts to use new glob API (async/await) - Added skipLibCheck to tsconfig.json for node_modules compatibility - Added ignoreDeprecations: "6.0" to silence baseUrl warning Fixes #8557 --- src/actions/motion.ts | 236 +++++++++++++++++++++++++++++++---- test/index.ts | 22 ++-- test/motionWithCount.test.ts | 216 ++++++++++++++++++++++++++++++++ tsconfig.json | 4 +- 4 files changed, 445 insertions(+), 33 deletions(-) create mode 100644 test/motionWithCount.test.ts diff --git a/src/actions/motion.ts b/src/actions/motion.ts index 07fc3c83c1b..953859016b8 100755 --- a/src/actions/motion.ts +++ b/src/actions/motion.ts @@ -1,34 +1,34 @@ import * as vscode from 'vscode'; -import { ChangeOperator, DeleteOperator, YankOperator } from './operator'; -import { CursorMoveByUnit, CursorMovePosition, TextEditor } from './../textEditor'; -import { isVisualMode, Mode } from './../mode/mode'; -import { PairMatcher } from './../common/matching/matcher'; -import { QuoteMatcher } from './../common/matching/quoteMatcher'; -import { RegisterAction } from './base'; -import { RegisterMode } from './../register/register'; -import { TagMatcher } from './../common/matching/tagMatcher'; -import { VimState } from './../state/vimState'; -import { configuration } from './../configuration/configuration'; -import { shouldWrapKey } from './wrapping'; +import { Position } from 'vscode'; +import { sorted } from '../common/motion/position'; +import { Notation } from '../configuration/notation'; import { VimError } from '../error'; -import { BaseMovement, SelectionType, IMovement, isIMovement, failedMovement } from './baseMotion'; +import { ModeDataFor } from '../mode/modeData'; import { globalState } from '../state/globalState'; -import { reportSearch } from '../util/statusBarTextUtils'; -import { SneakForward, SneakBackward } from './plugins/sneak'; -import { Notation } from '../configuration/notation'; import { StatusBar } from '../statusBar'; -import { clamp, isHighSurrogate, isLowSurrogate } from '../util/util'; import { getCurrentParagraphBeginning, getCurrentParagraphEnd } from '../textobject/paragraph'; -import { PythonDocument } from './languages/python/motion'; -import { Position } from 'vscode'; -import { sorted } from '../common/motion/position'; import { WordType } from '../textobject/word'; -import { CommandInsertAtCursor } from './commands/actions'; +import { reportSearch } from '../util/statusBarTextUtils'; +import { clamp, isHighSurrogate, isLowSurrogate } from '../util/util'; import { SearchDirection } from '../vimscript/pattern'; +import { PairMatcher } from './../common/matching/matcher'; +import { QuoteMatcher } from './../common/matching/quoteMatcher'; +import { TagMatcher } from './../common/matching/tagMatcher'; +import { configuration } from './../configuration/configuration'; +import { isVisualMode, Mode } from './../mode/mode'; +import { RegisterMode } from './../register/register'; +import { VimState } from './../state/vimState'; +import { CursorMoveByUnit, CursorMovePosition, TextEditor } from './../textEditor'; +import { RegisterAction } from './base'; +import { BaseMovement, failedMovement, IMovement, isIMovement, SelectionType } from './baseMotion'; +import { CommandInsertAtCursor } from './commands/actions'; +import { PythonDocument } from './languages/python/motion'; +import { ChangeOperator, DeleteOperator, YankOperator } from './operator'; +import { SneakBackward, SneakForward } from './plugins/sneak'; import { SmartQuoteMatcher, WhichQuotes } from './plugins/targets/smartQuotesMatcher'; import { useSmartQuotes } from './plugins/targets/targetsConfig'; -import { ModeDataFor } from '../mode/modeData'; +import { shouldWrapKey } from './wrapping'; /** * A movement is something like 'h', 'k', 'w', 'b', 'gg', etc. @@ -316,6 +316,45 @@ class MoveDown extends BaseMovement { vimState.currentRegisterMode = RegisterMode.LineWise; return position.getDown(); } + + public override async execActionWithCount( + position: Position, + vimState: VimState, + count: number, + ): Promise { + // Optimize: jump directly to target line instead of looping + count = clamp(count, 1, 99999); + + // Handle special cases with single iteration + if ( + vimState.currentMode === Mode.Insert && + this.keysPressed[0] === '' && + vimState.editor.document.uri.scheme === 'vscode-interactive-input' && + position.line === vimState.document.lineCount - 1 && + vimState.editor.selection.isEmpty + ) { + await vscode.commands.executeCommand('interactive.history.next'); + return vimState.editor.selection.active; + } + + if (configuration.foldfix && vimState.currentMode !== Mode.VisualBlock) { + return new MoveDownFoldFix().execActionWithCount(position, vimState, count); + } + + // For operators, set register mode and calculate target position directly + if (vimState.recordedState.operator) { + vimState.currentRegisterMode = RegisterMode.LineWise; + return position.getDown(count); + } + + // Calculate target line directly + const targetLine = Math.min(position.line + count, vimState.document.lineCount - 1); + if (targetLine === position.line) { + return position; + } + + return new Position(targetLine, vimState.desiredColumn); + } } @RegisterAction @@ -354,6 +393,45 @@ class MoveUp extends BaseMovement { vimState.currentRegisterMode = RegisterMode.LineWise; return position.getUp(); } + + public override async execActionWithCount( + position: Position, + vimState: VimState, + count: number, + ): Promise { + // Optimize: jump directly to target line instead of looping + count = clamp(count, 1, 99999); + + // Handle special cases with single iteration + if ( + vimState.currentMode === Mode.Insert && + this.keysPressed[0] === '' && + vimState.editor.document.uri.scheme === 'vscode-interactive-input' && + position.line === 0 && + vimState.editor.selection.isEmpty + ) { + await vscode.commands.executeCommand('interactive.history.previous'); + return vimState.editor.selection.active; + } + + if (configuration.foldfix && vimState.currentMode !== Mode.VisualBlock) { + return new MoveUpFoldFix().execActionWithCount(position, vimState, count); + } + + // For operators, set register mode and calculate target position directly + if (vimState.recordedState.operator) { + vimState.currentRegisterMode = RegisterMode.LineWise; + return position.getUp(count); + } + + // Calculate target line directly + const targetLine = Math.max(position.line - count, 0); + if (targetLine === position.line) { + return position; + } + + return new Position(targetLine, vimState.desiredColumn); + } } @RegisterAction @@ -725,6 +803,65 @@ class MoveLeft extends BaseMovement { ) : getLeftWhile(position); } + + public override async execActionWithCount( + position: Position, + vimState: VimState, + count: number, + ): Promise { + // Optimize: calculate target position directly instead of looping + count = clamp(count, 1, 99999); + + const shouldWrap = shouldWrapKey(vimState.currentMode, this.keysPressed[0]); + const includeEol = [Mode.Insert, Mode.Replace].includes(vimState.currentMode); + + if (shouldWrap) { + // For wrapping mode, we need to iterate since it can cross line boundaries + // But we can optimize by doing it more efficiently + let currentPos = position; + for (let i = 0; i < count; i++) { + currentPos = currentPos.getLeftThroughLineBreaks(includeEol); + if (currentPos.isEqual(position) && i === 0) { + // Can't move left (at document start) + break; + } + } + return currentPos; + } else { + // For non-wrapping mode, we can calculate directly + const line = vimState.document.lineAt(position.line).text; + let targetChar = position.character; + + // Move left count times, respecting surrogate pairs + for (let i = 0; i < count; i++) { + if (targetChar === 0) { + break; + } + + // Check for surrogate pair at current position + if ( + isLowSurrogate(line.charCodeAt(targetChar)) && + targetChar > 0 && + isHighSurrogate(line.charCodeAt(targetChar - 1)) + ) { + targetChar--; + } + + targetChar--; + + // Check for surrogate pair at new position + if ( + targetChar > 0 && + isLowSurrogate(line.charCodeAt(targetChar)) && + isHighSurrogate(line.charCodeAt(targetChar - 1)) + ) { + targetChar--; + } + } + + return new Position(position.line, Math.max(0, targetChar)); + } + } } @RegisterAction @@ -753,6 +890,63 @@ class MoveRight extends BaseMovement { ) : getRightWhile(position); } + + public override async execActionWithCount( + position: Position, + vimState: VimState, + count: number, + ): Promise { + // Optimize: calculate target position directly instead of looping + count = clamp(count, 1, 99999); + + const shouldWrap = shouldWrapKey(vimState.currentMode, this.keysPressed[0]); + const includeEol = [Mode.Insert, Mode.Replace].includes(vimState.currentMode); + + if (shouldWrap) { + // For wrapping mode, we need to iterate since it can cross line boundaries + // But we can optimize by doing it more efficiently + let currentPos = position; + for (let i = 0; i < count; i++) { + const prevPos = currentPos; + currentPos = currentPos.getRightThroughLineBreaks(includeEol); + if (currentPos.isEqual(prevPos)) { + // Can't move right (at document end) + break; + } + } + return currentPos; + } else { + // For non-wrapping mode, we can calculate directly + const line = vimState.document.lineAt(position.line).text; + let targetChar = position.character; + const lineLength = line.length; + + // Move right count times, respecting surrogate pairs + for (let i = 0; i < count; i++) { + if (targetChar >= lineLength) { + break; + } + + const newTargetChar = targetChar + 1; + if (newTargetChar >= lineLength) { + targetChar = newTargetChar; + break; + } + + // Check for surrogate pair at new position + if ( + isLowSurrogate(line.charCodeAt(newTargetChar)) && + isHighSurrogate(line.charCodeAt(targetChar)) + ) { + targetChar = newTargetChar + 1; + } else { + targetChar = newTargetChar; + } + } + + return new Position(position.line, Math.min(lineLength, targetChar)); + } + } } @RegisterAction diff --git a/test/index.ts b/test/index.ts index b622fdd1c1b..a3df95f623a 100644 --- a/test/index.ts +++ b/test/index.ts @@ -9,7 +9,7 @@ // host can call to run the tests. The test runner is expected to use console.log // to report the results back to the caller. When the tests are finished, return // a possible error to the callback or null if none. -import glob from 'glob'; +import { glob } from 'glob'; import Mocha from 'mocha'; import * as path from 'path'; @@ -19,7 +19,7 @@ import { Configuration } from './testConfiguration'; Globals.isTesting = true; Globals.mockConfiguration = new Configuration(); -export function run(): Promise { +export async function run(): Promise { const mochaGrep = new RegExp(process.env.MOCHA_GREP || ''); // Create the mocha test @@ -32,17 +32,15 @@ export function run(): Promise { const testsRoot = path.resolve(__dirname, '.'); - return new Promise((c, e) => { - glob('**/**.test.js', { cwd: testsRoot }, (err, files) => { - if (err) { - return e(err); - } + try { + const files = await glob('**/**.test.js', { cwd: testsRoot }); - // Add files to the test suite - files.forEach((f) => mocha.addFile(path.resolve(testsRoot, f))); + // Add files to the test suite + files.forEach((f: string) => mocha.addFile(path.resolve(testsRoot, f))); + // Run the mocha test + return new Promise((c, e) => { try { - // Run the mocha test mocha.run((failures) => { if (failures > 0) { e(new Error(`${failures} tests failed.`)); @@ -55,5 +53,7 @@ export function run(): Promise { e(error as Error); } }); - }); + } catch (err) { + throw err; + } } diff --git a/test/motionWithCount.test.ts b/test/motionWithCount.test.ts new file mode 100644 index 00000000000..22d1bc2d22d --- /dev/null +++ b/test/motionWithCount.test.ts @@ -0,0 +1,216 @@ +import { newTest } from './testSimplifier'; +import { setupWorkspace } from './testUtils'; + +suite('Motion with count optimization', () => { + suiteSetup(setupWorkspace); + + suite('j motion with count', () => { + newTest({ + title: 'Can handle j with count 1', + start: ['|line 1', 'line 2', 'line 3', 'line 4', 'line 5'], + keysPressed: '1j', + end: ['line 1', '|line 2', 'line 3', 'line 4', 'line 5'], + }); + + newTest({ + title: 'Can handle j with count 3', + start: ['|line 1', 'line 2', 'line 3', 'line 4', 'line 5'], + keysPressed: '3j', + end: ['line 1', 'line 2', 'line 3', '|line 4', 'line 5'], + }); + + newTest({ + title: 'Can handle j with large count (20)', + start: [ + '|line 1', + 'line 2', + 'line 3', + 'line 4', + 'line 5', + 'line 6', + 'line 7', + 'line 8', + 'line 9', + 'line 10', + 'line 11', + 'line 12', + 'line 13', + 'line 14', + 'line 15', + 'line 16', + 'line 17', + 'line 18', + 'line 19', + 'line 20', + 'line 21', + ], + keysPressed: '20j', + end: [ + 'line 1', + 'line 2', + 'line 3', + 'line 4', + 'line 5', + 'line 6', + 'line 7', + 'line 8', + 'line 9', + 'line 10', + 'line 11', + 'line 12', + 'line 13', + 'line 14', + 'line 15', + 'line 16', + 'line 17', + 'line 18', + 'line 19', + 'line 20', + '|line 21', + ], + }); + + newTest({ + title: 'Can handle j with count exceeding line count', + start: ['|line 1', 'line 2', 'line 3'], + keysPressed: '10j', + end: ['line 1', 'line 2', '|line 3'], + }); + + newTest({ + title: 'Can handle j with count and maintains column', + start: ['|abc', '123', 'xyz'], + keysPressed: '2j', + end: ['abc', '123', '|xyz'], + }); + }); + + suite('k motion with count', () => { + newTest({ + title: 'Can handle k with count 1', + start: ['line 1', 'line 2', 'line 3', 'line 4', '|line 5'], + keysPressed: '1k', + end: ['line 1', 'line 2', 'line 3', '|line 4', 'line 5'], + }); + + newTest({ + title: 'Can handle k with count 3', + start: ['line 1', 'line 2', 'line 3', 'line 4', '|line 5'], + keysPressed: '3k', + end: ['line 1', '|line 2', 'line 3', 'line 4', 'line 5'], + }); + + newTest({ + title: 'Can handle k with large count (20)', + start: [ + 'line 1', + 'line 2', + 'line 3', + 'line 4', + 'line 5', + 'line 6', + 'line 7', + 'line 8', + 'line 9', + 'line 10', + 'line 11', + 'line 12', + 'line 13', + 'line 14', + 'line 15', + 'line 16', + 'line 17', + 'line 18', + 'line 19', + 'line 20', + '|line 21', + ], + keysPressed: '20k', + end: [ + '|line 1', + 'line 2', + 'line 3', + 'line 4', + 'line 5', + 'line 6', + 'line 7', + 'line 8', + 'line 9', + 'line 10', + 'line 11', + 'line 12', + 'line 13', + 'line 14', + 'line 15', + 'line 16', + 'line 17', + 'line 18', + 'line 19', + 'line 20', + 'line 21', + ], + }); + + newTest({ + title: 'Can handle k with count exceeding line count', + start: ['line 1', 'line 2', '|line 3'], + keysPressed: '10k', + end: ['|line 1', 'line 2', 'line 3'], + }); + }); + + suite('h motion with count', () => { + newTest({ + title: 'Can handle h with count 3', + start: ['abc|def'], + keysPressed: '3h', + end: ['|abcdef'], + }); + + newTest({ + title: 'Can handle h with count exceeding line begin', + start: ['ab|cdef'], + keysPressed: '5h', + end: ['|abcdef'], + }); + }); + + suite('l motion with count', () => { + newTest({ + title: 'Can handle l with count 3', + start: ['|abcdef'], + keysPressed: '3l', + end: ['abc|def'], + }); + + newTest({ + title: 'Can handle l with count exceeding line end', + start: ['|abcdef'], + keysPressed: '10l', + end: ['abcde|f'], + }); + }); + + suite('j/k with operators', () => { + newTest({ + title: 'Can handle dj with count', + start: ['|line 1', 'line 2', 'line 3', 'line 4', 'line 5'], + keysPressed: 'd3j', + end: ['|line 5'], + }); + + newTest({ + title: 'Can handle dk with count', + start: ['line 1', 'line 2', 'line 3', '|line 4', 'line 5'], + keysPressed: 'd2k', + end: ['line 1', '|line 5'], + }); + + newTest({ + title: 'Can handle yj with count', + start: ['|line 1', 'line 2', 'line 3', 'line 4', 'line 5'], + keysPressed: 'y2jp', + end: ['line 1', 'line 2', 'line 3', 'line 1', 'line 2', '|line 3', 'line 4', 'line 5'], + }); + }); +}); diff --git a/tsconfig.json b/tsconfig.json index 4668390028b..cb65120e8aa 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -12,13 +12,15 @@ "strict": true, "useUnknownInCatchVariables": false, "experimentalDecorators": true, + "ignoreDeprecations": "6.0", "baseUrl": ".", "paths": { "platform/*": ["src/platform/node/*"] }, "resolveJsonModule": true, "forceConsistentCasingInFileNames": true, - "esModuleInterop": true + "esModuleInterop": true, + "skipLibCheck": true // "isolatedModules": true, }, "exclude": ["node_modules", "!node_modules/@types"] From 737d8f6f3c47b160d3dff66f1c89d6578f816669 Mon Sep 17 00:00:00 2001 From: rafaelbdb Date: Sat, 8 Nov 2025 08:15:21 -0300 Subject: [PATCH 2/2] refactor: address PR review feedback - Remove unnecessary outer try-catch in test/index.ts - Simplify MoveDown.execAction by delegating to execActionWithCount - Keep ignoreDeprecations in tsconfig.json (required for baseUrl deprecation warning)" --- src/actions/motion.ts | 22 +--------------------- test/index.ts | 44 ++++++++++++++++++++----------------------- 2 files changed, 21 insertions(+), 45 deletions(-) diff --git a/src/actions/motion.ts b/src/actions/motion.ts index 953859016b8..0e3181c2f45 100755 --- a/src/actions/motion.ts +++ b/src/actions/motion.ts @@ -286,27 +286,7 @@ class MoveDown extends BaseMovement { override preservesDesiredColumn = true; public override async execAction(position: Position, vimState: VimState): Promise { - if ( - vimState.currentMode === Mode.Insert && - this.keysPressed[0] === '' && - vimState.editor.document.uri.scheme === 'vscode-interactive-input' && - position.line === vimState.document.lineCount - 1 && - vimState.editor.selection.isEmpty - ) { - // navigate history in interactive window - await vscode.commands.executeCommand('interactive.history.next'); - return vimState.editor.selection.active; - } - - if (configuration.foldfix && vimState.currentMode !== Mode.VisualBlock) { - return new MoveDownFoldFix().execAction(position, vimState); - } - - if (position.line < vimState.document.lineCount - 1) { - return position.with({ character: vimState.desiredColumn }).getDown(); - } else { - return position; - } + return this.execActionWithCount(position, vimState, 1) as Promise; } public override async execActionForOperator( diff --git a/test/index.ts b/test/index.ts index a3df95f623a..4d017aae0bd 100644 --- a/test/index.ts +++ b/test/index.ts @@ -32,28 +32,24 @@ export async function run(): Promise { const testsRoot = path.resolve(__dirname, '.'); - try { - const files = await glob('**/**.test.js', { cwd: testsRoot }); - - // Add files to the test suite - files.forEach((f: string) => mocha.addFile(path.resolve(testsRoot, f))); - - // Run the mocha test - return new Promise((c, e) => { - try { - mocha.run((failures) => { - if (failures > 0) { - e(new Error(`${failures} tests failed.`)); - } else { - c(); - } - }); - } catch (error) { - console.error(error); - e(error as Error); - } - }); - } catch (err) { - throw err; - } + const files = await glob('**/**.test.js', { cwd: testsRoot }); + + // Add files to the test suite + files.forEach((f: string) => mocha.addFile(path.resolve(testsRoot, f))); + + // Run the mocha test + return new Promise((c, e) => { + try { + mocha.run((failures) => { + if (failures > 0) { + e(new Error(`${failures} tests failed.`)); + } else { + c(); + } + }); + } catch (error) { + console.error(error); + e(error as Error); + } + }); }