return 0 instead of -1 for no result find count (#3874)

xtermjs · Jun 27, 2022 · e77a0ec · e77a0ec
1 parent 1bf2ccc
commit e77a0ec
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 19 deletions.
diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts
@@ -55,7 +55,7 @@ export class SearchAddon implements ITerminalAddon {
   private _cachedSearchTerm: string | undefined;
   private _selectedDecoration: IDecoration | undefined;
   private _resultDecorations: Map<number, IDecoration[]> | undefined;
-  private _searchResults:  Map<string, ISearchResult> | undefined;
+  private _searchResults: Map<string, ISearchResult> | undefined;
   private _onDataDisposable: IDisposable | undefined;
   private _onResizeDisposable: IDisposable | undefined;
   private _lastSearchOptions: ISearchOptions | undefined;
@@ -72,7 +72,7 @@ export class SearchAddon implements ITerminalAddon {
 
   private _resultIndex: number | undefined;
 
-  private readonly _onDidChangeResults = new EventEmitter<{resultIndex: number, resultCount: number} | undefined>();
+  private readonly _onDidChangeResults = new EventEmitter<{ resultIndex: number, resultCount: number } | undefined>();
   public readonly onDidChangeResults = this._onDidChangeResults.event;
 
   public activate(terminal: Terminal): void {
@@ -87,9 +87,9 @@ export class SearchAddon implements ITerminalAddon {
     }
     if (this._cachedSearchTerm && this._lastSearchOptions?.decorations) {
       this._highlightTimeout = setTimeout(() => {
-        this.findPrevious(this._cachedSearchTerm!,  { ...this._lastSearchOptions, incremental: true, noScroll: true });
-        this._resultIndex = this._searchResults ? this._searchResults.size -1 : -1;
-        this._onDidChangeResults.fire({ resultIndex: this._searchResults ? this._searchResults.size - 1 : -1, resultCount: this._searchResults ? this._searchResults.size : -1 });
+        this.findPrevious(this._cachedSearchTerm!, { ...this._lastSearchOptions, incremental: true, noScroll: true });
+        this._resultIndex = this._searchResults ? this._searchResults.size - 1 : -1;
+        this._onDidChangeResults.fire({ resultIndex: this._resultIndex, resultCount: this._searchResults?.size ?? -1 });
       }, 200);
     }
   }
@@ -324,10 +324,8 @@ export class SearchAddon implements ITerminalAddon {
 
   private _fireResults(term: string, found: boolean, searchOptions?: ISearchOptions): boolean {
     if (searchOptions?.decorations) {
-      if (found && this._resultIndex !== undefined && this._searchResults?.size) {
+      if (this._resultIndex !== undefined && this._searchResults?.size !== undefined) {
         this._onDidChangeResults.fire({ resultIndex: this._resultIndex, resultCount: this._searchResults.size });
-      } else if (this._resultIndex === -1) {
-        this._onDidChangeResults.fire({ resultIndex: -1, resultCount: -1 });
       } else {
         this._onDidChangeResults.fire(undefined);
       }
@@ -693,7 +691,7 @@ export class SearchAddon implements ITerminalAddon {
     }
 
     if (!noScroll) {
-    // If it is not in the viewport then we scroll else it just gets selected
+      // If it is not in the viewport then we scroll else it just gets selected
       if (result.row >= (terminal.buffer.active.viewportY + terminal.rows) || result.row < terminal.buffer.active.viewportY) {
         let scroll = result.row - terminal.buffer.active.viewportY;
         scroll -= Math.floor(terminal.rows / 2);

diff --git a/addons/xterm-addon-search/test/SearchAddon.api.ts b/addons/xterm-addon-search/test/SearchAddon.api.ts
@@ -16,8 +16,8 @@ let page: Page;
 const width = 800;
 const height = 600;
 
-describe('Search Tests', function(): void {
-  before(async function(): Promise<any> {
+describe('Search Tests', function (): void {
+  before(async function (): Promise<any> {
     browser = await launchBrowser();
     page = await (await browser.newContext()).newPage();
     await page.setViewportSize({ width, height });
@@ -155,15 +155,15 @@ describe('Search Tests', function(): void {
         assert.deepStrictEqual(await page.evaluate('window.calls'), [
           { resultCount: 1, resultIndex: 0 },
           { resultCount: 2, resultIndex: 0 },
-          { resultCount: -1, resultIndex: -1 }
+          { resultCount: 0, resultIndex: -1 }
         ]);
         assert.strictEqual(await page.evaluate(`window.search.findNext('c', { decorations: { activeMatchColorOverviewRuler: '#ff0000' } })`), true);
         assert.strictEqual(await page.evaluate(`window.search.findNext('c', { decorations: { activeMatchColorOverviewRuler: '#ff0000' } })`), true);
         assert.strictEqual(await page.evaluate(`window.search.findNext('c', { decorations: { activeMatchColorOverviewRuler: '#ff0000' } })`), true);
         assert.deepStrictEqual(await page.evaluate('window.calls'), [
           { resultCount: 1, resultIndex: 0 },
           { resultCount: 2, resultIndex: 0 },
-          { resultCount: -1, resultIndex: -1 },
+          { resultCount: 0, resultIndex: -1 },
           { resultCount: 3, resultIndex: 0 },
           { resultCount: 3, resultIndex: 1 },
           { resultCount: 3, resultIndex: 2 }
@@ -203,7 +203,7 @@ describe('Search Tests', function(): void {
           { resultCount: 2, resultIndex: 0 },
           { resultCount: 2, resultIndex: 0 },
           { resultCount: 2, resultIndex: 1 },
-          { resultCount: -1, resultIndex: -1 }
+          { resultCount: 0, resultIndex: -1 }
         ]);
       });
     });
@@ -239,15 +239,15 @@ describe('Search Tests', function(): void {
         assert.deepStrictEqual(await page.evaluate('window.calls'), [
           { resultCount: 1, resultIndex: 0 },
           { resultCount: 2, resultIndex: 1 },
-          { resultCount: -1, resultIndex: -1 }
+          { resultCount: 0, resultIndex: -1 }
         ]);
         assert.strictEqual(await page.evaluate(`window.search.findPrevious('c', { decorations: { activeMatchColorOverviewRuler: '#ff0000' } })`), true);
         assert.strictEqual(await page.evaluate(`window.search.findPrevious('c', { decorations: { activeMatchColorOverviewRuler: '#ff0000' } })`), true);
         assert.strictEqual(await page.evaluate(`window.search.findPrevious('c', { decorations: { activeMatchColorOverviewRuler: '#ff0000' } })`), true);
         assert.deepStrictEqual(await page.evaluate('window.calls'), [
           { resultCount: 1, resultIndex: 0 },
           { resultCount: 2, resultIndex: 1 },
-          { resultCount: -1, resultIndex: -1 },
+          { resultCount: 0, resultIndex: -1 },
           { resultCount: 3, resultIndex: 2 },
           { resultCount: 3, resultIndex: 1 },
           { resultCount: 3, resultIndex: 0 }
@@ -287,7 +287,7 @@ describe('Search Tests', function(): void {
           { resultCount: 2, resultIndex: 1 },
           { resultCount: 2, resultIndex: 1 },
           { resultCount: 2, resultIndex: 0 },
-          { resultCount: -1, resultIndex: -1 }
+          { resultCount: 0, 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
@@ -121,8 +121,8 @@ declare module 'xterm-addon-search' {
     /**
      * When decorations are enabled, fires when
      * the search results change.
-     * @returns -1 if there are no matches and
-     * @returns undefined when the threshold of 1k results
+     * @returns -1 for resultIndex for a resultCount of 0
+     * and @returns undefined when the threshold of 1k results
      * is exceeded and decorations are disposed of.
      */
     readonly onDidChangeResults: IEvent<{ resultIndex: number, resultCount: number } | undefined>;