From 449ede1d576a72ed36be2e8f19de99be10c36bad Mon Sep 17 00:00:00 2001 From: miguel Date: Mon, 21 Oct 2019 14:55:22 -0400 Subject: [PATCH 1/5] Prevent search result from being deselected if it is the only result remove leading whitespace prevent single search term deselection in findPrevious --- addons/xterm-addon-search/src/SearchAddon.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 5893f79f..3c12e4f1 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -59,12 +59,12 @@ export class SearchAddon implements ITerminalAddon { let startCol = 0; let startRow = 0; - + let currentSelection = null; if (this._terminal.hasSelection()) { const incremental = searchOptions ? searchOptions.incremental : false; // Start from the selection end if there is a selection // For incremental search, use existing row - const currentSelection = this._terminal.getSelectionPosition()!; + currentSelection = this._terminal.getSelectionPosition()!; startRow = incremental ? currentSelection.startRow : currentSelection.endRow; startCol = incremental ? currentSelection.startColumn : currentSelection.endColumn; } @@ -97,6 +97,9 @@ export class SearchAddon implements ITerminalAddon { } } + // If there is only one result, return false. + if (!result && currentSelection) return false; + // Set selection and scroll if a result was found return this._selectResult(result); } @@ -123,8 +126,9 @@ export class SearchAddon implements ITerminalAddon { let startCol = this._terminal.cols; let result: ISearchResult | undefined = undefined; const incremental = searchOptions ? searchOptions.incremental : false; + let currentSelection = null; if (this._terminal.hasSelection()) { - const currentSelection = this._terminal.getSelectionPosition()!; + currentSelection = this._terminal.getSelectionPosition()!; // Start from selection start if there is a selection startRow = currentSelection.startRow; startCol = currentSelection.startColumn; @@ -161,6 +165,9 @@ export class SearchAddon implements ITerminalAddon { } } + // If there is only one result, return false. + if (!result && currentSelection) return false; + // Set selection and scroll if a result was found return this._selectResult(result); } From d02086e5c66bcdbf07998e4dcf2a6211dd7481d7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Mon, 21 Oct 2019 14:42:56 -0700 Subject: [PATCH 2/5] Add test to cover search multiple times --- addons/xterm-addon-search/src/SearchAddon.api.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.api.ts b/addons/xterm-addon-search/src/SearchAddon.api.ts index cb5a02b8..e3520116 100644 --- a/addons/xterm-addon-search/src/SearchAddon.api.ts +++ b/addons/xterm-addon-search/src/SearchAddon.api.ts @@ -15,7 +15,7 @@ const width = 800; const height = 600; describe('Search Tests', function (): void { - this.timeout(200000); + this.timeout(20000); before(async function (): Promise { browser = await puppeteer.launch({ @@ -98,6 +98,14 @@ describe('Search Tests', function (): void { await page.evaluate(`window.search.findNext('[A-Z]+', {regex: true, caseSensitive: true})`); assert.deepEqual(await page.evaluate(`window.term.getSelection()`), 'ABCD'); }); + + it('Search for single result twice should not unselect it', async () => { + await writeSync('abc def'); + assert.deepEqual(await page.evaluate(`window.search.findNext('abc')`), true); + assert.deepEqual(await page.evaluate(`window.term.getSelection()`), 'abc'); + assert.deepEqual(await page.evaluate(`window.search.findNext('abc')`), true); + assert.deepEqual(await page.evaluate(`window.term.getSelection()`), 'abc'); + }); }); async function openTerminal(options: ITerminalOptions = {}): Promise { From 495f97cf95034f36166e3b32978ade076752e096 Mon Sep 17 00:00:00 2001 From: Miguel Roncancio Date: Mon, 21 Oct 2019 23:39:01 -0400 Subject: [PATCH 3/5] change return value, import ISelectionPosition --- addons/xterm-addon-search/src/SearchAddon.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 3c12e4f1..5f85ebbf 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 } from 'xterm'; +import { Terminal, IDisposable, ITerminalAddon, ISelectionPosition } from 'xterm'; export interface ISearchOptions { regex?: boolean; @@ -59,7 +59,7 @@ export class SearchAddon implements ITerminalAddon { let startCol = 0; let startRow = 0; - let currentSelection = null; + let currentSelection: ISelectionPosition | undefined = undefined; if (this._terminal.hasSelection()) { const incremental = searchOptions ? searchOptions.incremental : false; // Start from the selection end if there is a selection @@ -98,7 +98,7 @@ export class SearchAddon implements ITerminalAddon { } // If there is only one result, return false. - if (!result && currentSelection) return false; + if (!result && currentSelection) return true; // Set selection and scroll if a result was found return this._selectResult(result); @@ -126,7 +126,7 @@ export class SearchAddon implements ITerminalAddon { let startCol = this._terminal.cols; let result: ISearchResult | undefined = undefined; const incremental = searchOptions ? searchOptions.incremental : false; - let currentSelection = null; + let currentSelection: ISelectionPosition | undefined = undefined; if (this._terminal.hasSelection()) { currentSelection = this._terminal.getSelectionPosition()!; // Start from selection start if there is a selection @@ -166,7 +166,7 @@ export class SearchAddon implements ITerminalAddon { } // If there is only one result, return false. - if (!result && currentSelection) return false; + if (!result && currentSelection) return true; // Set selection and scroll if a result was found return this._selectResult(result); From 5d547bffb0fe277ace5be2deb96e12fce2500aa8 Mon Sep 17 00:00:00 2001 From: Miguel Roncancio Date: Mon, 21 Oct 2019 23:39:01 -0400 Subject: [PATCH 4/5] change return value, import ISelectionPosition --- addons/xterm-addon-search/src/SearchAddon.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index 3c12e4f1..cfee3cb9 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 } from 'xterm'; +import { Terminal, IDisposable, ITerminalAddon, ISelectionPosition } from 'xterm'; export interface ISearchOptions { regex?: boolean; @@ -59,7 +59,7 @@ export class SearchAddon implements ITerminalAddon { let startCol = 0; let startRow = 0; - let currentSelection = null; + let currentSelection: ISelectionPosition | undefined = undefined; if (this._terminal.hasSelection()) { const incremental = searchOptions ? searchOptions.incremental : false; // Start from the selection end if there is a selection @@ -97,8 +97,8 @@ export class SearchAddon implements ITerminalAddon { } } - // If there is only one result, return false. - if (!result && currentSelection) return false; + // If there is only one result, return true. + if (!result && currentSelection) return true; // Set selection and scroll if a result was found return this._selectResult(result); @@ -126,7 +126,7 @@ export class SearchAddon implements ITerminalAddon { let startCol = this._terminal.cols; let result: ISearchResult | undefined = undefined; const incremental = searchOptions ? searchOptions.incremental : false; - let currentSelection = null; + let currentSelection: ISelectionPosition | undefined = undefined; if (this._terminal.hasSelection()) { currentSelection = this._terminal.getSelectionPosition()!; // Start from selection start if there is a selection @@ -165,8 +165,8 @@ export class SearchAddon implements ITerminalAddon { } } - // If there is only one result, return false. - if (!result && currentSelection) return false; + // If there is only one result, return true. + if (!result && currentSelection) return true; // Set selection and scroll if a result was found return this._selectResult(result); From 616d341e8ce6cd1f4b9c59157414882b9c825a5d Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Tue, 22 Oct 2019 07:17:37 -0700 Subject: [PATCH 5/5] Avoid initializing to undefined --- addons/xterm-addon-search/src/SearchAddon.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/addons/xterm-addon-search/src/SearchAddon.ts b/addons/xterm-addon-search/src/SearchAddon.ts index cfee3cb9..91fd3977 100644 --- a/addons/xterm-addon-search/src/SearchAddon.ts +++ b/addons/xterm-addon-search/src/SearchAddon.ts @@ -59,7 +59,7 @@ export class SearchAddon implements ITerminalAddon { let startCol = 0; let startRow = 0; - let currentSelection: ISelectionPosition | undefined = undefined; + let currentSelection: ISelectionPosition | undefined; if (this._terminal.hasSelection()) { const incremental = searchOptions ? searchOptions.incremental : false; // Start from the selection end if there is a selection @@ -124,9 +124,9 @@ export class SearchAddon implements ITerminalAddon { const isReverseSearch = true; let startRow = this._terminal.buffer.baseY + this._terminal.rows; let startCol = this._terminal.cols; - let result: ISearchResult | undefined = undefined; + let result: ISearchResult | undefined; const incremental = searchOptions ? searchOptions.incremental : false; - let currentSelection: ISelectionPosition | undefined = undefined; + let currentSelection: ISelectionPosition | undefined; if (this._terminal.hasSelection()) { currentSelection = this._terminal.getSelectionPosition()!; // Start from selection start if there is a selection