From 702f9ba070ac2e3e7bec2479a8c495eb30133dfa Mon Sep 17 00:00:00 2001 From: Jackson Kearl Date: Tue, 5 May 2020 15:00:06 -0700 Subject: [PATCH 1/3] Revert "Distinguish a user removing a row from 'clear' in search" This reverts commit 4cf9c461bac8ae487d0a7d3e17e888b7cd121804. --- .../contrib/search/browser/searchActions.ts | 2 +- .../contrib/search/common/searchModel.ts | 37 +++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/vs/workbench/contrib/search/browser/searchActions.ts b/src/vs/workbench/contrib/search/browser/searchActions.ts index 034134ea9b42b..96c16f7b7e397 100644 --- a/src/vs/workbench/contrib/search/browser/searchActions.ts +++ b/src/vs/workbench/contrib/search/browser/searchActions.ts @@ -615,7 +615,7 @@ export class RemoveAction extends AbstractSearchAndReplaceAction { this.viewer.setFocus([nextFocusElement], getSelectionKeyboardEvent()); } - this.element.parent().remove(this.element, true); + this.element.parent().remove(this.element); this.viewer.domFocus(); return Promise.resolve(); diff --git a/src/vs/workbench/contrib/search/common/searchModel.ts b/src/vs/workbench/contrib/search/common/searchModel.ts index 65948afd51033..1b64c0b3159ed 100644 --- a/src/vs/workbench/contrib/search/common/searchModel.ts +++ b/src/vs/workbench/contrib/search/common/searchModel.ts @@ -192,8 +192,8 @@ export class FileMatch extends Disposable implements IFileMatch { return (selected ? FileMatch._CURRENT_FIND_MATCH : FileMatch._FIND_MATCH); } - private _onChange = this._register(new Emitter<{ didRemove?: boolean; forceUpdateModel?: boolean; userRemoved?: boolean }>()); - readonly onChange: Event<{ didRemove?: boolean; forceUpdateModel?: boolean; userRemoved?: boolean }> = this._onChange.event; + private _onChange = this._register(new Emitter<{ didRemove?: boolean; forceUpdateModel?: boolean }>()); + readonly onChange: Event<{ didRemove?: boolean; forceUpdateModel?: boolean }> = this._onChange.event; private _onDispose = this._register(new Emitter()); readonly onDispose: Event = this._onDispose.event; @@ -354,10 +354,10 @@ export class FileMatch extends Disposable implements IFileMatch { return values(this._matches); } - remove(match: Match, userRemoved?: boolean): void { + remove(match: Match): void { this.removeMatch(match); this._removedMatches.add(match.id()); - this._onChange.fire({ didRemove: true, userRemoved }); + this._onChange.fire({ didRemove: true }); } replace(toReplace: Match): Promise { @@ -447,7 +447,6 @@ export interface IChangeEvent { elements: FileMatch[]; added?: boolean; removed?: boolean; - userRemoved?: boolean; } export class FolderMatch extends Disposable { @@ -530,7 +529,7 @@ export class FolderMatch extends Disposable { const fileMatch = this.instantiationService.createInstance(FileMatch, this._query.contentPattern, this._query.previewOptions, this._query.maxResults, this, rawFileMatch); this.doAdd(fileMatch); added.push(fileMatch); - const disposable = fileMatch.onChange(({ didRemove, userRemoved }) => this.onFileChange(fileMatch, didRemove, userRemoved)); + const disposable = fileMatch.onChange(({ didRemove }) => this.onFileChange(fileMatch, didRemove)); fileMatch.onDispose(() => disposable.dispose()); } }); @@ -541,14 +540,14 @@ export class FolderMatch extends Disposable { } } - clear(userRemoved?: boolean): void { + clear(): void { const changed: FileMatch[] = this.matches(); this.disposeMatches(); - this._onChange.fire({ elements: changed, removed: true, userRemoved }); + this._onChange.fire({ elements: changed, removed: true }); } - remove(matches: FileMatch | FileMatch[], userRemoved?: boolean): void { - this.doRemove(matches, undefined, undefined, userRemoved); + remove(matches: FileMatch | FileMatch[]): void { + this.doRemove(matches); } replace(match: FileMatch): Promise { @@ -578,7 +577,7 @@ export class FolderMatch extends Disposable { return this.matches().reduce((prev, match) => prev + match.count(), 0); } - private onFileChange(fileMatch: FileMatch, removed = false, userRemoved = false): void { + private onFileChange(fileMatch: FileMatch, removed = false): void { let added = false; if (!this._fileMatches.has(fileMatch.resource)) { this.doAdd(fileMatch); @@ -590,7 +589,7 @@ export class FolderMatch extends Disposable { removed = true; } if (!this._replacingAll) { - this._onChange.fire({ elements: [fileMatch], added: added, removed: removed, userRemoved }); + this._onChange.fire({ elements: [fileMatch], added: added, removed: removed }); } } @@ -601,7 +600,7 @@ export class FolderMatch extends Disposable { } } - private doRemove(fileMatches: FileMatch | FileMatch[], dispose: boolean = true, trigger: boolean = true, userRemoved: boolean = false): void { + private doRemove(fileMatches: FileMatch | FileMatch[], dispose: boolean = true, trigger: boolean = true): void { if (!Array.isArray(fileMatches)) { fileMatches = [fileMatches]; } @@ -616,7 +615,7 @@ export class FolderMatch extends Disposable { } if (trigger) { - this._onChange.fire({ elements: fileMatches, removed: true, userRemoved }); + this._onChange.fire({ elements: fileMatches, removed: true }); } } @@ -718,7 +717,7 @@ export class SearchResult extends Disposable { this._register(this.modelService.onModelAdded(model => this.onModelAdded(model))); this._register(this.onChange(e => { - if (e.userRemoved) { + if (e.removed) { this._hasRemovedResults = true; } })); @@ -809,14 +808,14 @@ export class SearchResult extends Disposable { this._otherFilesMatch = null; } - remove(matches: FileMatch | FolderMatch | (FileMatch | FolderMatch)[], userRemoved?: boolean): void { + remove(matches: FileMatch | FolderMatch | (FileMatch | FolderMatch)[]): void { if (!Array.isArray(matches)) { matches = [matches]; } matches.forEach(m => { if (m instanceof FolderMatch) { - m.clear(userRemoved); + m.clear(); } }); @@ -828,11 +827,11 @@ export class SearchResult extends Disposable { return; } - this.getFolderMatch(matches[0].resource).remove(matches, userRemoved); + this.getFolderMatch(matches[0].resource).remove(matches); }); if (other.length) { - this.getFolderMatch(other[0].resource).remove(other, userRemoved); + this.getFolderMatch(other[0].resource).remove(other); } } From eefd6376b410ec08094b25c9551dfa8c9a3a95d4 Mon Sep 17 00:00:00 2001 From: Jackson Kearl Date: Tue, 5 May 2020 15:00:58 -0700 Subject: [PATCH 2/3] Revert "Fix searchResult tests" This reverts commit 7646b358e458599f3e26119337357344346a9c4a. --- .../contrib/search/test/common/searchResult.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/search/test/common/searchResult.test.ts b/src/vs/workbench/contrib/search/test/common/searchResult.test.ts index a82b823eb96c7..f46f158ac2931 100644 --- a/src/vs/workbench/contrib/search/test/common/searchResult.test.ts +++ b/src/vs/workbench/contrib/search/test/common/searchResult.test.ts @@ -236,7 +236,7 @@ suite('SearchResult', () => { testObject.remove(objectToRemove); assert.ok(target.calledOnce); - assert.deepEqual([{ elements: [objectToRemove], removed: true, userRemoved: false }], target.args[0]); + assert.deepEqual([{ elements: [objectToRemove], removed: true }], target.args[0]); }); test('remove array triggers change event', function () { @@ -253,7 +253,7 @@ suite('SearchResult', () => { testObject.remove(arrayToRemove); assert.ok(target.calledOnce); - assert.deepEqual([{ elements: arrayToRemove, removed: true, userRemoved: false }], target.args[0]); + assert.deepEqual([{ elements: arrayToRemove, removed: true }], target.args[0]); }); test('remove triggers change event', function () { @@ -268,7 +268,7 @@ suite('SearchResult', () => { testObject.remove(objectToRemove); assert.ok(target.calledOnce); - assert.deepEqual([{ elements: [objectToRemove], removed: true, userRemoved: false }], target.args[0]); + assert.deepEqual([{ elements: [objectToRemove], removed: true }], target.args[0]); }); test('Removing all line matches and adding back will add file back to result', function () { @@ -315,7 +315,7 @@ suite('SearchResult', () => { return voidPromise.then(() => { assert.ok(target.calledOnce); - assert.deepEqual([{ elements: [objectToRemove], removed: true, userRemoved: false }], target.args[0]); + assert.deepEqual([{ elements: [objectToRemove], removed: true }], target.args[0]); }); }); From 74113b0ab958a76fc88d6ff865bd63b4c456fb72 Mon Sep 17 00:00:00 2001 From: Jackson Kearl Date: Tue, 5 May 2020 15:04:56 -0700 Subject: [PATCH 3/3] Make search dirty on any removal, but ony if it does not leave the view empty. --- src/vs/workbench/contrib/search/browser/searchView.ts | 2 +- src/vs/workbench/contrib/search/common/searchModel.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/search/browser/searchView.ts b/src/vs/workbench/contrib/search/browser/searchView.ts index f80381fac4058..c3507839f121a 100644 --- a/src/vs/workbench/contrib/search/browser/searchView.ts +++ b/src/vs/workbench/contrib/search/browser/searchView.ts @@ -874,7 +874,7 @@ export class SearchView extends ViewPane { selectedText = strings.escapeRegExpCharacters(selectedText); } - if (allowSearchOnType && !this.viewModel.searchResult.hasRemovedResults) { + if (allowSearchOnType && !this.viewModel.searchResult.isDirty) { this.searchWidget.setValue(selectedText); } else { this.pauseSearching = true; diff --git a/src/vs/workbench/contrib/search/common/searchModel.ts b/src/vs/workbench/contrib/search/common/searchModel.ts index 1b64c0b3159ed..9f90d468cf93a 100644 --- a/src/vs/workbench/contrib/search/common/searchModel.ts +++ b/src/vs/workbench/contrib/search/common/searchModel.ts @@ -702,7 +702,7 @@ export class SearchResult extends Disposable { private _rangeHighlightDecorations: RangeHighlightDecorations; private disposePastResults: () => void = () => { }; - private _hasRemovedResults = false; + private _isDirty = false; constructor( private _searchModel: SearchModel, @@ -718,13 +718,13 @@ export class SearchResult extends Disposable { this._register(this.onChange(e => { if (e.removed) { - this._hasRemovedResults = true; + this._isDirty = !this.isEmpty(); } })); } - get hasRemovedResults(): boolean { - return this._hasRemovedResults; + get isDirty(): boolean { + return this._isDirty; } get query(): ITextQuery | null { @@ -737,7 +737,7 @@ export class SearchResult extends Disposable { new Promise(resolve => this.disposePastResults = resolve) .then(() => oldFolderMatches.forEach(match => match.clear())) .then(() => oldFolderMatches.forEach(match => match.dispose())) - .then(() => this._hasRemovedResults = false); + .then(() => this._isDirty = false); this._rangeHighlightDecorations.removeHighlightRange(); this._folderMatchesMap = TernarySearchTree.forUris();