From cc5136aeb3ee478b7220efe4ab2f371ea7466e1d Mon Sep 17 00:00:00 2001 From: Jean Pierre Date: Fri, 12 May 2023 16:08:11 -0500 Subject: [PATCH] Address feedback --- addons/xterm-addon-search/src/SearchAddon.ts | 46 ++++++++++++------- .../test/SearchAddon.api.ts | 34 ++++++++++++++ .../typings/xterm-addon-search.d.ts | 17 ++++++- 3 files changed, 79 insertions(+), 18 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 9dd1ffc8..59afe581 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -3,7 +3,7 @@ * @license MIT */ -import { Terminal, IDisposable, ITerminalAddon, IBufferRange, IDecoration } from 'xterm'; +import { Terminal, IDisposable, ITerminalAddon, IDecoration } from 'xterm'; import { EventEmitter } from 'common/EventEmitter'; import { Disposable, toDisposable, disposeArray } from 'common/Lifecycle'; @@ -30,6 +30,10 @@ export interface ISearchPosition { startRow: number; } +export interface ISearchAddonOptions { + highlightLimit: number; +} + export interface ISearchResult { term: string; col: number; @@ -48,15 +52,22 @@ type LineCacheEntry = [ lineOffsets: number[] ]; +interface IHighlight extends IDisposable { + decoration: IDecoration; + match: ISearchResult; +} + const NON_WORD_CHARACTERS = ' ~!@#$%^&*()+`-=[]{}|\\;:"\',./<>?'; const LINES_CACHE_TIME_TO_LIVE = 15 * 1000; // 15 secs +const DEFAULT_HIGHLIGHT_LIMIT = 1000; export class SearchAddon extends Disposable implements ITerminalAddon { private _terminal: Terminal | undefined; private _cachedSearchTerm: string | undefined; private _highlightedLines: Set = new Set(); - private _highlightDecorations: (IDecoration & { match: ISearchResult })[] = []; - private _selectedDecoration: IDecoration & { match: ISearchResult } | undefined; + private _highlightDecorations: IHighlight[] = []; + private _selectedDecoration: IHighlight | undefined; + private _highlightLimit: number; private _onDataDisposable: IDisposable | undefined; private _onResizeDisposable: IDisposable | undefined; private _lastSearchOptions: ISearchOptions | undefined; @@ -74,7 +85,11 @@ export class SearchAddon extends Disposable implements ITerminalAddon { private readonly _onDidChangeResults = this.register(new EventEmitter<{ resultIndex: number, resultCount: number }>()); public readonly onDidChangeResults = this._onDidChangeResults.event; - public readonly MATCHES_LIMIT = 1000; + constructor(options?: Partial) { + super(); + + this._highlightLimit = options?.highlightLimit ?? DEFAULT_HIGHLIGHT_LIMIT; + } public activate(terminal: Terminal): void { this._terminal = terminal; @@ -93,14 +108,15 @@ export class SearchAddon extends Disposable implements ITerminalAddon { } if (this._cachedSearchTerm && this._lastSearchOptions?.decorations) { this._highlightTimeout = setTimeout(() => { - this.findPrevious(this._cachedSearchTerm!, { ...this._lastSearchOptions, incremental: true, noScroll: true }); - this._fireResults(this._lastSearchOptions); + const term = this._cachedSearchTerm; + this._cachedSearchTerm = undefined; + this.findPrevious(term!, { ...this._lastSearchOptions, incremental: true, noScroll: true }); }, 200); } } public clearDecorations(retainCachedSearchTerm?: boolean): void { - this._selectedDecoration?.dispose(); + this.clearActiveDecoration(); disposeArray(this._highlightDecorations); this._highlightDecorations = []; this._highlightedLines.clear(); @@ -156,7 +172,7 @@ export class SearchAddon extends Disposable implements ITerminalAddon { let prevResult: ISearchResult | undefined = undefined; let result = this._find(term, 0, 0, searchOptions); while (result && (prevResult?.row !== result.row || prevResult?.col !== result.col)) { - if (searchResultsWithHighlight.length >= this.MATCHES_LIMIT) { + if (searchResultsWithHighlight.length >= this._highlightLimit) { break; } prevResult = result; @@ -168,12 +184,11 @@ export class SearchAddon extends Disposable implements ITerminalAddon { searchOptions ); } - for (const result of searchResultsWithHighlight) { - const resultDecoration = this._createResultDecoration(result, searchOptions.decorations!); - if (resultDecoration) { - this._highlightedLines.add(resultDecoration.marker.line); - (resultDecoration as unknown as { match: ISearchResult }).match = result; - this._highlightDecorations.push(resultDecoration as (IDecoration & { match: ISearchResult })); + for (const match of searchResultsWithHighlight) { + const decoration = this._createResultDecoration(match, searchOptions.decorations!); + if (decoration) { + this._highlightedLines.add(decoration.marker.line); + this._highlightDecorations.push({ decoration, match, dispose() { decoration.dispose(); } }); } } } @@ -654,8 +669,7 @@ export class SearchAddon extends Disposable implements ITerminalAddon { disposables.push(marker); disposables.push(decoration.onRender((e) => this._applyStyles(e, options.activeMatchBorder, true))); disposables.push(decoration.onDispose(() => disposeArray(disposables))); - (decoration as unknown as { match: ISearchResult }).match = result; - this._selectedDecoration = decoration as (IDecoration & { match: ISearchResult }); + this._selectedDecoration = { decoration, match: result, dispose() { decoration.dispose(); } }; } } } diff --git a/addons/xterm-addon-search/test/SearchAddon.api.ts b/addons/xterm-addon-search/test/SearchAddon.api.ts index df942735..0c20084a 100644 --- a/addons/xterm-addon-search/test/SearchAddon.api.ts +++ b/addons/xterm-addon-search/test/SearchAddon.api.ts @@ -242,6 +242,23 @@ describe('Search Tests', function (): void { { resultCount: 1000, resultIndex: 1 } ]); }); + it('should fire when writing to terminal', async () => { + await page.evaluate(` + window.calls = []; + window.search.onDidChangeResults(e => window.calls.push(e)); + `); + await writeSync(page, 'abc bc c\\n\\r'.repeat(2)); + assert.strictEqual(await page.evaluate(`window.search.findNext('abc', { decorations: { activeMatchColorOverviewRuler: '#ff0000' } })`), true); + assert.deepStrictEqual(await page.evaluate('window.calls'), [ + { resultCount: 2, resultIndex: 0 } + ]); + await writeSync(page, 'abc bc c\\n\\r'); + await timeout(300); + assert.deepStrictEqual(await page.evaluate('window.calls'), [ + { resultCount: 2, resultIndex: 0 }, + { resultCount: 3, resultIndex: 0 } + ]); + }); }); describe('findPrevious', () => { it('should not fire unless the decorations option is set', async () => { @@ -359,6 +376,23 @@ describe('Search Tests', function (): void { { resultCount: 1000, resultIndex: -1 } ]); }); + it('should fire when writing to terminal', async () => { + await page.evaluate(` + window.calls = []; + window.search.onDidChangeResults(e => window.calls.push(e)); + `); + await writeSync(page, 'abc bc c\\n\\r'.repeat(2)); + assert.strictEqual(await page.evaluate(`window.search.findPrevious('abc', { decorations: { activeMatchColorOverviewRuler: '#ff0000' } })`), true); + assert.deepStrictEqual(await page.evaluate('window.calls'), [ + { resultCount: 2, resultIndex: 1 } + ]); + await writeSync(page, 'abc bc c\\n\\r'); + await timeout(300); + assert.deepStrictEqual(await page.evaluate('window.calls'), [ + { resultCount: 2, resultIndex: 1 }, + { resultCount: 3, resultIndex: 1 } + ]); + }); }); }); diff --git a/addons/xterm-addon-search/typings/xterm-addon-search.d.ts b/addons/xterm-addon-search/typings/xterm-addon-search.d.ts index 05cb8ebb..3a3b7c89 100644 --- a/addons/xterm-addon-search/typings/xterm-addon-search.d.ts +++ b/addons/xterm-addon-search/typings/xterm-addon-search.d.ts @@ -75,14 +75,27 @@ declare module 'xterm-addon-search' { activeMatchColorOverviewRuler: string; } + /** + * Options for the search addon. + */ + export interface ISearchAddonOptions { + /** + * Max number of matches highlighted when decorations are enabled. + * Defaults to 1000 highlighted matches + */ + highlightLimit: number + } + /** * An xterm.js addon that provides search functionality. */ export class SearchAddon implements ITerminalAddon { + /** - * Max number of matches when decorations are enabled + * Creates a new search addon. + * @param options Options for the search addon. */ - public readonly MATCHES_LIMIT: number; + constructor(options?: Partial); /** * Activates the addon