From 91ef7f24a60d6ac59c7a425fe7e72a5dc5624ea6 Mon Sep 17 00:00:00 2001 From: Noj Vek Date: Mon, 10 Dec 2018 15:54:35 -0800 Subject: [PATCH 1/3] Fixes #1660: Search as you type --- demo/client.ts | 38 ++++++++++++++---------------- src/addons/search/Interfaces.ts | 2 ++ src/addons/search/SearchHelper.ts | 39 +++++++++++++++++++------------ 3 files changed, 44 insertions(+), 35 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index d5196d37..7fd2a894 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -14,6 +14,7 @@ import * as fullscreen from '../lib/addons/fullscreen/fullscreen'; import * as search from '../lib/addons/search/search'; import * as webLinks from '../lib/addons/webLinks/webLinks'; import * as winptyCompat from '../lib/addons/winptyCompat/winptyCompat'; +import { ISearchOptions } from '../lib/addons/search/Interfaces'; // Pulling in the module's types relies on the above, it's looks a // little weird here as we're importing "this" module @@ -50,6 +51,14 @@ function setPadding(): void { term.fit(); } +function getSearchOptions(): ISearchOptions { + return { + regex: (document.getElementById('regex') as HTMLInputElement).checked, + wholeWord: (document.getElementById('whole-word') as HTMLInputElement).checked, + caseSensitive: (document.getElementById('case-sensitive') as HTMLInputElement).checked, + }; +} + createTerminal(); const disposeRecreateButtonHandler = () => { @@ -97,27 +106,16 @@ function createTerminal(): void { addDomListener(paddingElement, 'change', setPadding); - addDomListener(actionElements.findNext, 'keypress', (e) => { - if (e.key === 'Enter') { - e.preventDefault(); - const searchOptions = { - regex: (document.getElementById('regex') as HTMLInputElement).checked, - wholeWord: (document.getElementById('whole-word') as HTMLInputElement).checked, - caseSensitive: (document.getElementById('case-sensitive') as HTMLInputElement).checked - }; - term.findNext(actionElements.findNext.value, searchOptions); - } + addDomListener(actionElements.findNext, 'keyup', (e) => { + const searchOptions = getSearchOptions(); + searchOptions.incremental = e.key !== `Enter`; + term.findNext(actionElements.findNext.value, searchOptions); }); - addDomListener(actionElements.findPrevious, 'keypress', (e) => { - if (e.key === 'Enter') { - e.preventDefault(); - const searchOptions = { - regex: (document.getElementById('regex') as HTMLInputElement).checked, - wholeWord: (document.getElementById('whole-word') as HTMLInputElement).checked, - caseSensitive: (document.getElementById('case-sensitive') as HTMLInputElement).checked - }; - term.findPrevious(actionElements.findPrevious.value, searchOptions); - } + + addDomListener(actionElements.findPrevious, 'keyup', (e) => { + const searchOptions = getSearchOptions(); + searchOptions.incremental = e.key !== `Enter`; + term.findPrevious(actionElements.findPrevious.value, searchOptions); }); // fit is called within a setTimeout, cols and rows need this. diff --git a/src/addons/search/Interfaces.ts b/src/addons/search/Interfaces.ts index af06c5d1..96788120 100644 --- a/src/addons/search/Interfaces.ts +++ b/src/addons/search/Interfaces.ts @@ -25,6 +25,8 @@ export interface ISearchOptions { regex?: boolean; wholeWord?: boolean; caseSensitive?: boolean; + /** Assume caller implements 'search as you type' where findNext gets called when search input changes */ + incremental?: boolean; } export interface ISearchResult { diff --git a/src/addons/search/SearchHelper.ts b/src/addons/search/SearchHelper.ts index 7919932a..d5ec0b48 100644 --- a/src/addons/search/SearchHelper.ts +++ b/src/addons/search/SearchHelper.ts @@ -24,29 +24,34 @@ export class SearchHelper implements ISearchHelper { * @return Whether a result was found. */ public findNext(term: string, searchOptions?: ISearchOptions): boolean { + const selectionManager = this._terminal._core.selectionManager; + const {incremental} = searchOptions; + let result: ISearchResult; + if (!term || term.length === 0) { + selectionManager.clearSelection(); return false; } - let result: ISearchResult; - let startRow = this._terminal._core.buffer.ydisp; - if (this._terminal._core.selectionManager.selectionEnd) { + + if (selectionManager.selectionEnd) { // Start from the selection end if there is a selection + // For incremental search, use existing row if (this._terminal.getSelection().length !== 0) { - startRow = this._terminal._core.selectionManager.selectionEnd[1]; + startRow = incremental ? selectionManager.selectionStart[1] : selectionManager.selectionEnd[1]; } } - // Search from ydisp + 1 to end - for (let y = startRow + 1; y < this._terminal._core.buffer.ybase + this._terminal.rows; y++) { + // Search from startRow to end + for (let y = incremental ? startRow: startRow + 1; y < this._terminal._core.buffer.ybase + this._terminal.rows; y++) { result = this._findInLine(term, y, searchOptions); if (result) { break; } } - // Search from the top to the current ydisp + // Search from the top to the startRow if (!result) { for (let y = 0; y < startRow; y++) { result = this._findInLine(term, y, searchOptions); @@ -68,29 +73,33 @@ export class SearchHelper implements ISearchHelper { * @return Whether a result was found. */ public findPrevious(term: string, searchOptions?: ISearchOptions): boolean { + const selectionManager = this._terminal._core.selectionManager; + const {incremental} = searchOptions; + let result: ISearchResult; + if (!term || term.length === 0) { + selectionManager.clearSelection(); return false; } - let result: ISearchResult; - let startRow = this._terminal._core.buffer.ydisp; - if (this._terminal._core.selectionManager.selectionStart) { - // Start from the selection end if there is a selection + + if (selectionManager.selectionStart) { + // Start from the selection start if there is a selection if (this._terminal.getSelection().length !== 0) { - startRow = this._terminal._core.selectionManager.selectionStart[1]; + startRow = selectionManager.selectionStart[1]; } } - // Search from ydisp + 1 to end - for (let y = startRow - 1; y >= 0; y--) { + // Search from startRow to top + for (let y = incremental ? startRow : startRow - 1; y >= 0; y--) { result = this._findInLine(term, y, searchOptions); if (result) { break; } } - // Search from the top to the current ydisp + // Search from the bottom to startRow if (!result) { for (let y = this._terminal._core.buffer.ybase + this._terminal.rows - 1; y > startRow; y--) { result = this._findInLine(term, y, searchOptions); From 9005de1c42cc4cc3684aa87e7222efee4209b5d5 Mon Sep 17 00:00:00 2001 From: Noj Vek Date: Mon, 10 Dec 2018 17:13:28 -0800 Subject: [PATCH 2/3] adding a linesCache ttl and cursor move invalidation --- demo/client.ts | 2 +- src/addons/search/SearchHelper.ts | 55 ++++++++++++++++++++++++++----- src/addons/search/tsconfig.json | 1 + 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/demo/client.ts b/demo/client.ts index 700fa38d..d99ff66d 100644 --- a/demo/client.ts +++ b/demo/client.ts @@ -55,7 +55,7 @@ function getSearchOptions(): ISearchOptions { return { regex: (document.getElementById('regex') as HTMLInputElement).checked, wholeWord: (document.getElementById('whole-word') as HTMLInputElement).checked, - caseSensitive: (document.getElementById('case-sensitive') as HTMLInputElement).checked, + caseSensitive: (document.getElementById('case-sensitive') as HTMLInputElement).checked }; } diff --git a/src/addons/search/SearchHelper.ts b/src/addons/search/SearchHelper.ts index d5ec0b48..cfb29c8b 100644 --- a/src/addons/search/SearchHelper.ts +++ b/src/addons/search/SearchHelper.ts @@ -4,16 +4,24 @@ */ import { ISearchHelper, ISearchAddonTerminal, ISearchOptions, ISearchResult } from './Interfaces'; -const nonWordCharacters = ' ~!@#$%^&*()+`-=[]{}|\;:"\',./<>?'; + +const NON_WORD_CHARACTERS = ' ~!@#$%^&*()+`-=[]{}|\;:"\',./<>?'; +const LINES_CACHE_TIME_TO_LIVE = 15 * 1000; // 15 secs /** * A class that knows how to search the terminal and how to display the results. */ export class SearchHelper implements ISearchHelper { + /** + * translateBufferLineToStringWithWrap is a fairly expensive call. + * We memoize the calls into an array that has a time based ttl. + * _linesCache is also invalidated when the terminal cursor moves. + */ + private _linesCache: string[] = null; + private _linesCacheTimeoutId = 0; + constructor(private _terminal: ISearchAddonTerminal) { - // TODO: Search for multiple instances on 1 line - // TODO: Don't use the actual selection, instead use a "find selection" so multiple instances can be highlighted - // TODO: Highlight other instances in the viewport + this._destroyLinesCache = this._destroyLinesCache.bind(this); } /** @@ -43,8 +51,10 @@ export class SearchHelper implements ISearchHelper { } } + this._initLinesCache(); + // Search from startRow to end - for (let y = incremental ? startRow: startRow + 1; y < this._terminal._core.buffer.ybase + this._terminal.rows; y++) { + for (let y = incremental ? startRow : startRow + 1; y < this._terminal._core.buffer.ybase + this._terminal.rows; y++) { result = this._findInLine(term, y, searchOptions); if (result) { break; @@ -91,6 +101,8 @@ export class SearchHelper implements ISearchHelper { } } + this._initLinesCache(); + // Search from startRow to top for (let y = incremental ? startRow : startRow - 1; y >= 0; y--) { result = this._findInLine(term, y, searchOptions); @@ -113,6 +125,28 @@ export class SearchHelper implements ISearchHelper { return this._selectResult(result); } + /** + * Sets up a line cache with a ttl + */ + private _initLinesCache(): void { + if (!this._linesCache) { + this._linesCache = new Array(this._terminal._core.buffer.length); + this._terminal.on('cursormove', this._destroyLinesCache); + } + + window.clearTimeout(this._linesCacheTimeoutId); + this._linesCacheTimeoutId = window.setTimeout(() => this._destroyLinesCache(), LINES_CACHE_TIME_TO_LIVE); + } + + private _destroyLinesCache(): void { + this._linesCache = null; + this._terminal.off('cursormove', this._destroyLinesCache); + if (this._linesCacheTimeoutId) { + window.clearTimeout(this._linesCacheTimeoutId); + this._linesCacheTimeoutId = 0; + } + } + /** * A found substring is a whole word if it doesn't have an alphanumeric character directly adjacent to it. * @param searchIndex starting indext of the potential whole word substring @@ -120,8 +154,8 @@ export class SearchHelper implements ISearchHelper { * @param term the substring that starts at searchIndex */ private _isWholeWord(searchIndex: number, line: string, term: string): boolean { - return (((searchIndex === 0) || (nonWordCharacters.indexOf(line[searchIndex - 1]) !== -1)) && - (((searchIndex + term.length) === line.length) || (nonWordCharacters.indexOf(line[searchIndex + term.length]) !== -1))); + return (((searchIndex === 0) || (NON_WORD_CHARACTERS.indexOf(line[searchIndex - 1]) !== -1)) && + (((searchIndex + term.length) === line.length) || (NON_WORD_CHARACTERS.indexOf(line[searchIndex + term.length]) !== -1))); } /** @@ -139,7 +173,12 @@ export class SearchHelper implements ISearchHelper { return; } - const stringLine = this.translateBufferLineToStringWithWrap(y, true); + let stringLine = this._linesCache[y]; + if (stringLine === void 0) { + stringLine = this.translateBufferLineToStringWithWrap(y, true); + this._linesCache[y] = stringLine; + } + const searchStringLine = searchOptions.caseSensitive ? stringLine : stringLine.toLowerCase(); const searchTerm = searchOptions.caseSensitive ? term : term.toLowerCase(); let searchIndex = -1; diff --git a/src/addons/search/tsconfig.json b/src/addons/search/tsconfig.json index c34a0bc5..e7a1ff3d 100644 --- a/src/addons/search/tsconfig.json +++ b/src/addons/search/tsconfig.json @@ -3,6 +3,7 @@ "module": "commonjs", "target": "es5", "lib": [ + "dom", "es5" ], "rootDir": ".", From a0d583b075e454ef9f2f2fe3d4c08ed4c47f664c Mon Sep 17 00:00:00 2001 From: Noj Vek Date: Mon, 10 Dec 2018 17:23:28 -0800 Subject: [PATCH 3/3] handle non-cached scenario in tests land --- src/addons/search/SearchHelper.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/addons/search/SearchHelper.ts b/src/addons/search/SearchHelper.ts index cfb29c8b..8e6e4e12 100644 --- a/src/addons/search/SearchHelper.ts +++ b/src/addons/search/SearchHelper.ts @@ -173,10 +173,12 @@ export class SearchHelper implements ISearchHelper { return; } - let stringLine = this._linesCache[y]; + let stringLine = this._linesCache ? this._linesCache[y] : void 0; if (stringLine === void 0) { stringLine = this.translateBufferLineToStringWithWrap(y, true); - this._linesCache[y] = stringLine; + if (this._linesCache) { + this._linesCache[y] = stringLine; + } } const searchStringLine = searchOptions.caseSensitive ? stringLine : stringLine.toLowerCase();