From 47367f18936918a71ec6897fe17607784ce8e2c0 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Wed, 20 Jul 2016 22:48:47 -0700 Subject: [PATCH 1/2] Fix #438 - Limit the number of matches, and try to only recalculate when the searchString changes, or the document changes --- src/actions/actions.ts | 10 ++--- src/mode/modeHandler.ts | 82 +++++++++++++++++++++++------------------ src/textEditor.ts | 4 ++ 3 files changed, 56 insertions(+), 40 deletions(-) diff --git a/src/actions/actions.ts b/src/actions/actions.ts index 6b4b417303b..b0e720a9d9d 100644 --- a/src/actions/actions.ts +++ b/src/actions/actions.ts @@ -1,4 +1,4 @@ -import { VimSpecialCommands, VimState, SearchState } from './../mode/modeHandler'; +import { VimSpecialCommands, VimState, SearchState, SearchDirection } from './../mode/modeHandler'; import { ModeName } from './../mode/mode'; import { TextEditor } from './../textEditor'; import { Register, RegisterMode } from './../register/register'; @@ -490,7 +490,7 @@ class CommandStar extends BaseCommand { public async exec(position: Position, vimState: VimState): Promise { const currentWord = CommandStar.GetWordAtPosition(position); - vimState.searchState = new SearchState(+1, vimState.cursorPosition, currentWord); + vimState.searchState = new SearchState(SearchDirection.Forward, vimState.cursorPosition, currentWord); do { vimState.cursorPosition = vimState.searchState.getNextSearchMatchPosition(vimState.cursorPosition).pos; @@ -517,7 +517,7 @@ class CommandHash extends BaseCommand { public async exec(position: Position, vimState: VimState): Promise { const currentWord = CommandStar.GetWordAtPosition(position); - vimState.searchState = new SearchState(-1, vimState.cursorPosition, currentWord); + vimState.searchState = new SearchState(SearchDirection.Backward, vimState.cursorPosition, currentWord); do { // use getWordLeft() on position to start at the beginning of the word. @@ -606,7 +606,7 @@ export class CommandSearchForwards extends BaseCommand { isMotion = true; public async exec(position: Position, vimState: VimState): Promise { - vimState.searchState = new SearchState(+1, vimState.cursorPosition); + vimState.searchState = new SearchState(SearchDirection.Forward, vimState.cursorPosition); vimState.currentMode = ModeName.SearchInProgressMode; return vimState; @@ -620,7 +620,7 @@ export class CommandSearchBackwards extends BaseCommand { isMotion = true; public async exec(position: Position, vimState: VimState): Promise { - vimState.searchState = new SearchState(-1, vimState.cursorPosition); + vimState.searchState = new SearchState(SearchDirection.Backward, vimState.cursorPosition); vimState.currentMode = ModeName.SearchInProgressMode; return vimState; diff --git a/src/mode/modeHandler.ts b/src/mode/modeHandler.ts index 4f126ef1a54..c6c000dbc37 100644 --- a/src/mode/modeHandler.ts +++ b/src/mode/modeHandler.ts @@ -128,50 +128,63 @@ export class VimSettings { useCtrlKeys = false; } +export enum SearchDirection { + Forward = 1, + Backward = -1 +} + export class SearchState { + private static readonly MAX_SEARCH_RANGES = 1000; + /** * Every range in the document that matches the search string. */ - public matchRanges: vscode.Range[] = []; + private _matchRanges: vscode.Range[] = []; + public get matchRanges(): vscode.Range[] { + return this._matchRanges; + } + + private _searchCursorStartPosition: Position; + public get searchCursorStartPosition(): Position { + return this._searchCursorStartPosition; + } + + private _matchesDocVersion: number; + private _searchDirection: SearchDirection = SearchDirection.Forward; private _searchString = ""; public get searchString(): string { return this._searchString; } - public searchCursorStartPosition: Position; - - /** - * 1 === forward - * -1 === backward - */ - public searchDirection = 1; - - public set searchString(search: string){ - this._searchString = search; - - this._recalculateSearchRanges(); + if (this._searchString !== search) { + this._searchString = search; + this._recalculateSearchRanges(/*forceRecalc=*/true); + } } - private _recalculateSearchRanges(): void { + private _recalculateSearchRanges(forceRecalc?: boolean): void { const search = this.searchString; if (search === "") { return; } - // Calculate and store all matching ranges - this.matchRanges = []; + if (this._matchesDocVersion !== TextEditor.getDocumentVersion() || forceRecalc) { + // Calculate and store all matching ranges + this._matchesDocVersion = TextEditor.getDocumentVersion(); + this._matchRanges = []; - for (let lineIdx = 0; lineIdx < TextEditor.getLineCount(); lineIdx++) { - const line = TextEditor.getLineAt(new Position(lineIdx, 0)).text; + for (let lineIdx = 0; lineIdx < TextEditor.getLineCount() && this._matchRanges.length < SearchState.MAX_SEARCH_RANGES; lineIdx++) { + const line = TextEditor.getLineAt(new Position(lineIdx, 0)).text; - let i = line.indexOf(search); + let i = line.indexOf(search); - for (; i !== -1; i = line.indexOf(search, i + search.length)) { - this.matchRanges.push(new vscode.Range( - new Position(lineIdx, i), - new Position(lineIdx, i + search.length) - )); + for (; i !== -1 && this._matchRanges.length < SearchState.MAX_SEARCH_RANGES; i = line.indexOf(search, i + search.length)) { + this._matchRanges.push(new vscode.Range( + new Position(lineIdx, i), + new Position(lineIdx, i + search.length) + )); + } } } } @@ -184,15 +197,15 @@ export class SearchState { public getNextSearchMatchPosition(startPosition: Position, direction = 1): { pos: Position, match: boolean} { this._recalculateSearchRanges(); - if (this.matchRanges.length === 0) { + if (this._matchRanges.length === 0) { // TODO(bell) return { pos: startPosition, match: false }; } - const effectiveDirection = direction * this.searchDirection; + const effectiveDirection = direction * this._searchDirection; - if (effectiveDirection === 1) { - for (let matchRange of this.matchRanges) { + if (effectiveDirection === SearchDirection.Forward) { + for (let matchRange of this._matchRanges) { if (matchRange.start.compareTo(startPosition) > 0) { return { pos: Position.FromVSCodePosition(matchRange.start), match: true }; } @@ -200,9 +213,9 @@ export class SearchState { // Wrap around // TODO(bell) - return { pos: Position.FromVSCodePosition(this.matchRanges[0].start), match: true }; + return { pos: Position.FromVSCodePosition(this._matchRanges[0].start), match: true }; } else { - for (let matchRange of this.matchRanges.slice(0).reverse()) { + for (let matchRange of this._matchRanges.slice(0).reverse()) { if (matchRange.start.compareTo(startPosition) < 0) { return { pos: Position.FromVSCodePosition(matchRange.start), match: true }; } @@ -210,15 +223,15 @@ export class SearchState { // TODO(bell) return { - pos: Position.FromVSCodePosition(this.matchRanges[this.matchRanges.length - 1].start), + pos: Position.FromVSCodePosition(this._matchRanges[this._matchRanges.length - 1].start), match: true }; } } - constructor(direction: number, startPosition: Position, searchString = "") { - this.searchDirection = direction; - this.searchCursorStartPosition = startPosition; + constructor(direction: SearchDirection, startPosition: Position, searchString = "") { + this._searchDirection = direction; + this._searchCursorStartPosition = startPosition; this.searchString = searchString; } } @@ -576,7 +589,6 @@ export class ModeHandler implements vscode.Disposable { } // Update view - await this.updateView(vimState); return vimState; diff --git a/src/textEditor.ts b/src/textEditor.ts index 5b72898b6aa..220d21a755d 100644 --- a/src/textEditor.ts +++ b/src/textEditor.ts @@ -42,6 +42,10 @@ export class TextEditor { }); } + static getDocumentVersion(): number { + return vscode.window.activeTextEditor.document.version; + } + /** * Removes all text in the entire document. */ From e4de85ed8c9fdc2faff7bf98b9742c1b42158e31 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 26 Jul 2016 13:17:13 -0700 Subject: [PATCH 2/2] Change to take an options object, and move MAX_SEARCH_RANGES check (PR feedback) --- src/mode/modeHandler.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/mode/modeHandler.ts b/src/mode/modeHandler.ts index c6c000dbc37..b573a23d430 100644 --- a/src/mode/modeHandler.ts +++ b/src/mode/modeHandler.ts @@ -160,11 +160,11 @@ export class SearchState { public set searchString(search: string){ if (this._searchString !== search) { this._searchString = search; - this._recalculateSearchRanges(/*forceRecalc=*/true); + this._recalculateSearchRanges({ forceRecalc: true }); } } - private _recalculateSearchRanges(forceRecalc?: boolean): void { + private _recalculateSearchRanges({ forceRecalc }: { forceRecalc?: boolean } = {}): void { const search = this.searchString; if (search === "") { return; } @@ -174,12 +174,17 @@ export class SearchState { this._matchesDocVersion = TextEditor.getDocumentVersion(); this._matchRanges = []; - for (let lineIdx = 0; lineIdx < TextEditor.getLineCount() && this._matchRanges.length < SearchState.MAX_SEARCH_RANGES; lineIdx++) { + outer: + for (let lineIdx = 0; lineIdx < TextEditor.getLineCount(); lineIdx++) { const line = TextEditor.getLineAt(new Position(lineIdx, 0)).text; let i = line.indexOf(search); - for (; i !== -1 && this._matchRanges.length < SearchState.MAX_SEARCH_RANGES; i = line.indexOf(search, i + search.length)) { + for (; i !== -1; i = line.indexOf(search, i + search.length)) { + if (this._matchRanges.length >= SearchState.MAX_SEARCH_RANGES) { + break outer; + } + this._matchRanges.push(new vscode.Range( new Position(lineIdx, i), new Position(lineIdx, i + search.length)