From b79091409a9e60f242abbe03906ae485da87dcde Mon Sep 17 00:00:00 2001 From: Robert Jin Date: Sun, 8 Dec 2019 16:05:17 +0000 Subject: [PATCH 1/4] #83252 Implement sorting for search results --- .../search/browser/search.contribution.ts | 18 ++++- .../contrib/search/browser/searchView.ts | 73 ++++++++++++++++--- .../contrib/search/common/searchModel.ts | 41 ++++++++++- .../search/test/browser/searchViewlet.test.ts | 14 ++++ .../services/search/common/search.ts | 12 +++ 5 files changed, 143 insertions(+), 15 deletions(-) diff --git a/src/vs/workbench/contrib/search/browser/search.contribution.ts b/src/vs/workbench/contrib/search/browser/search.contribution.ts index 9aef1001127..8a575f728b6 100644 --- a/src/vs/workbench/contrib/search/browser/search.contribution.ts +++ b/src/vs/workbench/contrib/search/browser/search.contribution.ts @@ -51,7 +51,7 @@ import { ISearchHistoryService, SearchHistoryService } from 'vs/workbench/contri import { FileMatchOrMatch, ISearchWorkbenchService, RenderableMatch, SearchWorkbenchService, FileMatch, Match, FolderMatch } from 'vs/workbench/contrib/search/common/searchModel'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { IPanelService } from 'vs/workbench/services/panel/common/panelService'; -import { ISearchConfiguration, ISearchConfigurationProperties, PANEL_ID, VIEWLET_ID, VIEW_ID, VIEW_CONTAINER } from 'vs/workbench/services/search/common/search'; +import { ISearchConfiguration, ISearchConfigurationProperties, PANEL_ID, VIEWLET_ID, VIEW_ID, VIEW_CONTAINER, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { ExplorerViewPaneContainer } from 'vs/workbench/contrib/files/browser/explorerViewlet'; @@ -826,7 +826,21 @@ configurationRegistry.registerConfiguration({ type: 'boolean', default: false, description: nls.localize('search.enableSearchEditorPreview', "Experimental: When enabled, allows opening workspace search results in an editor.") - } + }, + 'search.sortOrder': { + 'type': 'string', + 'enum': [SearchSortOrderConfiguration.DEFAULT, SearchSortOrderConfiguration.FILES_ONLY, SearchSortOrderConfiguration.TYPE, SearchSortOrderConfiguration.MODIFIED, SearchSortOrderConfiguration.COUNT_DESCENDING, SearchSortOrderConfiguration.COUNT_ASCENDING], + 'default': SearchSortOrderConfiguration.DEFAULT, + 'enumDescriptions': [ + nls.localize('searchSortOrder.default', 'Results are sorted by folder and file names, in alphabetical order.'), + nls.localize('searchSortOrder.filesOnly', 'Results are sorted by file names ignoring folder order, in alphabetical order.'), + nls.localize('searchSortOrder.type', 'Results are sorted by file extensions, in alphabetical order.'), + nls.localize('searchSortOrder.modified', 'Results are sorted by file last modified date, in descending order.'), + nls.localize('searchSortOrder.countDescending', 'Results are sorted by count per file, in descending order.'), + nls.localize('searchSortOrder.countAscending', 'Results are sorted by count per file, in ascending order.') + ], + 'description': nls.localize('search.sortOrder', "Controls sorting order of search results.") + }, } }); diff --git a/src/vs/workbench/contrib/search/browser/searchView.ts b/src/vs/workbench/contrib/search/browser/searchView.ts index f5198340005..ee76bacb8cb 100644 --- a/src/vs/workbench/contrib/search/browser/searchView.ts +++ b/src/vs/workbench/contrib/search/browser/searchView.ts @@ -194,6 +194,16 @@ export class SearchView extends ViewPane { this.enableSearchEditorPreview.set(this.searchConfig.enableSearchEditorPreview); } }); + this.configurationService.onDidChangeConfiguration(e => { + if (e.affectsConfiguration('search.sortOrder')) { + if (this.searchConfig.sortOrder !== 'modified') { + // If changing away from modified, remove all fileStats + // so that updated files are re-retrieved next time. + this.removeFileStats(); + } + this.refreshTree(); + } + }); this.viewModel = this._register(this.searchWorkbenchService.searchModel); this.queryBuilder = this.instantiationService.createInstance(QueryBuilder); @@ -495,13 +505,24 @@ export class SearchView extends ViewPane { const collapseResults = this.searchConfig.collapseResults; if (!event || event.added || event.removed) { // Refresh whole tree - this.tree.setChildren(null, this.createResultIterator(collapseResults)); + if (this.searchConfig.sortOrder === 'modified') { + // Ensure all matches have retrieved their file stat + this.retrieveFileStats() + .then(() => this.tree.setChildren(null, this.createResultIterator(collapseResults))); + } else { + this.tree.setChildren(null, this.createResultIterator(collapseResults)); + } } else { - // FileMatch modified, refresh those elements - event.elements.forEach(element => { - this.tree.setChildren(element, this.createIterator(element, collapseResults)); - this.tree.rerender(element); - }); + // If updated counts affect our search order, re-sort the view. + if (this.searchConfig.sortOrder === 'countAscending' || this.searchConfig.sortOrder === 'countDescending') { + this.tree.setChildren(null, this.createResultIterator(collapseResults)); + } else { + // FileMatch modified, refresh those elements + event.elements.forEach(element => { + this.tree.setChildren(element, this.createIterator(element, collapseResults)); + this.tree.rerender(element); + }); + } } } @@ -522,9 +543,10 @@ export class SearchView extends ViewPane { } private createFolderIterator(folderMatch: FolderMatch, collapseResults: ISearchConfigurationProperties['collapseResults']): Iterator> { + const sortOrder = this.searchConfig.sortOrder; const filesIt = Iterator.fromArray( folderMatch.matches() - .sort(searchMatchComparer)); + .sort((a, b) => searchMatchComparer(a, b, sortOrder))); return Iterator.map(filesIt, fileMatch => { const children = this.createFileIterator(fileMatch); @@ -1692,14 +1714,23 @@ export class SearchView extends ViewPane { } private onFilesChanged(e: FileChangesEvent): void { - if (!this.viewModel || !e.gotDeleted()) { + if (this.searchConfig.sortOrder !== 'modified' && (!this.viewModel || !e.gotDeleted())) { return; } const matches = this.viewModel.searchResult.matches(); + if (e.gotDeleted()) { + const deletedMatches = matches.filter(m => e.contains(m.resource, FileChangeType.DELETED)); - const changedMatches = matches.filter(m => e.contains(m.resource, FileChangeType.DELETED)); - this.viewModel.searchResult.remove(changedMatches); + this.viewModel.searchResult.remove(deletedMatches); + } else { + // Check if the changed file contained matches + const changedMatches = matches.filter(m => e.contains(m.resource)); + if (changedMatches.length && this.searchConfig.sortOrder === 'modified') { + // No matches need to be removed, but modified files need to have their file stat updated. + this.updateFileStats(changedMatches).then(() => this.refreshTree()); + } + } } getActions(): IAction[] { @@ -1771,6 +1802,28 @@ export class SearchView extends ViewPane { super.saveState(); } + private async retrieveFileStats(): Promise { + for (const fileMatch of this.searchResult.matches()) { + if (!fileMatch.fileStat) { + await fileMatch.resolveFileStat(this.fileService); + } + } + return Promise.resolve(undefined); + } + + private async updateFileStats(elements: FileMatch[]): Promise { + for (const fileMatch of elements) { + await fileMatch.resolveFileStat(this.fileService); + } + return Promise.resolve(undefined); + } + + private removeFileStats(): void { + for (const fileMatch of this.searchResult.matches()) { + fileMatch.fileStat = undefined; + } + } + dispose(): void { this.isDisposed = true; this.saveState(); diff --git a/src/vs/workbench/contrib/search/common/searchModel.ts b/src/vs/workbench/contrib/search/common/searchModel.ts index 99d8a29887d..1a2dd7d3aba 100644 --- a/src/vs/workbench/contrib/search/common/searchModel.ts +++ b/src/vs/workbench/contrib/search/common/searchModel.ts @@ -20,7 +20,7 @@ import { IModelService } from 'vs/editor/common/services/modelService'; import { createDecorator, IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IProgress, IProgressStep } from 'vs/platform/progress/common/progress'; import { ReplacePattern } from 'vs/workbench/services/search/common/replace'; -import { IFileMatch, IPatternInfo, ISearchComplete, ISearchProgressItem, ISearchConfigurationProperties, ISearchService, ITextQuery, ITextSearchPreviewOptions, ITextSearchMatch, ITextSearchStats, resultIsMatch, ISearchRange, OneLineRange, ITextSearchContext, ITextSearchResult } from 'vs/workbench/services/search/common/search'; +import { IFileMatch, IPatternInfo, ISearchComplete, ISearchProgressItem, ISearchConfigurationProperties, ISearchService, ITextQuery, ITextSearchPreviewOptions, ITextSearchMatch, ITextSearchStats, resultIsMatch, ISearchRange, OneLineRange, ITextSearchContext, ITextSearchResult, SearchSortOrder, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { overviewRulerFindMatchForeground, minimapFindMatch } from 'vs/platform/theme/common/colorRegistry'; import { themeColorFromId } from 'vs/platform/theme/common/themeService'; @@ -29,6 +29,8 @@ import { editorMatchesToTextSearchResults } from 'vs/workbench/services/search/c import { withNullAsUndefined } from 'vs/base/common/types'; import { memoize } from 'vs/base/common/decorators'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { compareFileNames, compareFileExtensions, comparePaths } from 'vs/base/common/comparers'; +import { IFileService, IFileStatWithMetadata } from 'vs/platform/files/common/files'; export class Match { @@ -188,6 +190,7 @@ export class FileMatch extends Disposable implements IFileMatch { readonly onDispose: Event = this._onDispose.event; private _resource: URI; + private _fileStat?: IFileStatWithMetadata; private _model: ITextModel | null = null; private _modelListener: IDisposable | null = null; private _matches: Map; @@ -406,6 +409,19 @@ export class FileMatch extends Disposable implements IFileMatch { } } + async resolveFileStat(fileService: IFileService): Promise { + this._fileStat = await fileService.resolve(this.resource, { resolveMetadata: true }); + return Promise.resolve(); + } + + public get fileStat(): IFileStatWithMetadata | undefined { + return this._fileStat; + } + + public set fileStat(stat: IFileStatWithMetadata | undefined) { + this._fileStat = stat; + } + dispose(): void { this.setSelectedMatch(null); this.unbindModel(); @@ -418,6 +434,7 @@ export interface IChangeEvent { elements: FileMatch[]; added?: boolean; removed?: boolean; + modified?: boolean; } export class FolderMatch extends Disposable { @@ -628,13 +645,31 @@ export class FolderMatchWithResource extends FolderMatch { * Compares instances of the same match type. Different match types should not be siblings * and their sort order is undefined. */ -export function searchMatchComparer(elementA: RenderableMatch, elementB: RenderableMatch): number { +export function searchMatchComparer(elementA: RenderableMatch, elementB: RenderableMatch, sortOrder: SearchSortOrder = 'default'): number { if (elementA instanceof FolderMatch && elementB instanceof FolderMatch) { return elementA.index() - elementB.index(); } if (elementA instanceof FileMatch && elementB instanceof FileMatch) { - return elementA.resource.fsPath.localeCompare(elementB.resource.fsPath) || elementA.name().localeCompare(elementB.name()); + switch (sortOrder) { + case SearchSortOrderConfiguration.COUNT_DESCENDING: + return elementB.count() - elementA.count(); + case SearchSortOrderConfiguration.COUNT_ASCENDING: + return elementA.count() - elementB.count(); + case SearchSortOrderConfiguration.TYPE: + return compareFileExtensions(elementA.name(), elementB.name()); + case SearchSortOrderConfiguration.FILES_ONLY: + return compareFileNames(elementA.name(), elementB.name()); + case SearchSortOrderConfiguration.MODIFIED: + const fileStatA = elementA.fileStat; + const fileStatB = elementB.fileStat; + if (fileStatA && fileStatB) { + return fileStatB.mtime - fileStatA.mtime; + } + // Fall through otherwise + default: + return comparePaths(elementA.resource.fsPath, elementB.resource.fsPath) || compareFileNames(elementA.name(), elementB.name()); + } } if (elementA instanceof Match && elementB instanceof Match) { diff --git a/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts b/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts index 447fffccfbe..805cd8373f8 100644 --- a/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts +++ b/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts @@ -80,6 +80,20 @@ suite('Search - Viewlet', () => { assert(searchMatchComparer(lineMatch2, lineMatch3) === 0); }); + test('Advanced Comparer', () => { + let fileMatch1 = aFileMatch('C:\\with\\path\\foo10'); + let fileMatch2 = aFileMatch('C:\\with\\path2\\foo1'); + let fileMatch3 = aFileMatch('C:\\with\\path2\\bar.a'); + let fileMatch4 = aFileMatch('C:\\with\\path2\\bar.b'); + + // By default, path < path2 + assert(searchMatchComparer(fileMatch1, fileMatch2) < 0); + // By filenames, foo10 > foo1 + assert(searchMatchComparer(fileMatch1, fileMatch2, 'filesOnly') > 0); + // By type, bar.a < bar.b + assert(searchMatchComparer(fileMatch3, fileMatch4, 'type') < 0); + }); + function aFileMatch(path: string, searchResult?: SearchResult, ...lineMatches: ITextSearchMatch[]): FileMatch { let rawMatch: IFileMatch = { resource: uri.file('C:\\' + path), diff --git a/src/vs/workbench/services/search/common/search.ts b/src/vs/workbench/services/search/common/search.ts index d02e1e53ece..c1fa0e04bde 100644 --- a/src/vs/workbench/services/search/common/search.ts +++ b/src/vs/workbench/services/search/common/search.ts @@ -311,6 +311,17 @@ export class OneLineRange extends SearchRange { } } +export const SearchSortOrderConfiguration = { + DEFAULT: 'default', + FILES_ONLY: 'filesOnly', + TYPE: 'type', + MODIFIED: 'modified', + COUNT_DESCENDING: 'countDescending', + COUNT_ASCENDING: 'countAscending' +}; + +export type SearchSortOrder = 'default' | 'filesOnly' | 'type' | 'modified' | 'countDescending' | 'countAscending'; + export interface ISearchConfigurationProperties { exclude: glob.IExpression; useRipgrep: boolean; @@ -333,6 +344,7 @@ export interface ISearchConfigurationProperties { searchOnTypeDebouncePeriod: number; enableSearchEditorPreview: boolean; searchEditorPreviewForceAbsolutePaths: boolean; + sortOrder: SearchSortOrder; } export interface ISearchConfiguration extends IFilesConfiguration { From 49b99c9dbed6c538d584ddf0efc85186ea0bd0df Mon Sep 17 00:00:00 2001 From: Robert Jin Date: Mon, 9 Dec 2019 00:35:43 +0000 Subject: [PATCH 2/4] Improve searchviewlet tests --- .../search/test/browser/searchViewlet.test.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts b/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts index 805cd8373f8..d37ca51b9fb 100644 --- a/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts +++ b/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts @@ -14,6 +14,7 @@ import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace import { TestWorkspace } from 'vs/platform/workspace/test/common/testWorkspace'; import { FileMatch, Match, searchMatchComparer, SearchResult } from 'vs/workbench/contrib/search/common/searchModel'; import { TestContextService } from 'vs/workbench/test/workbenchTestServices'; +import { isWindows } from 'vs/base/common/platform'; suite('Search - Viewlet', () => { let instantiation: TestInstantiationService; @@ -63,9 +64,9 @@ suite('Search - Viewlet', () => { }); test('Comparer', () => { - let fileMatch1 = aFileMatch('C:\\foo'); - let fileMatch2 = aFileMatch('C:\\with\\path'); - let fileMatch3 = aFileMatch('C:\\with\\path\\foo'); + let fileMatch1 = aFileMatch(isWindows ? 'C:\\foo' : '/c/foo'); + let fileMatch2 = aFileMatch(isWindows ? 'C:\\with\\path' : '/c/with/path'); + let fileMatch3 = aFileMatch(isWindows ? 'C:\\with\\path\\foo' : '/c/with/path/foo'); let lineMatch1 = new Match(fileMatch1, ['bar'], new OneLineRange(0, 1, 1), new OneLineRange(0, 1, 1)); let lineMatch2 = new Match(fileMatch1, ['bar'], new OneLineRange(0, 1, 1), new OneLineRange(2, 1, 1)); let lineMatch3 = new Match(fileMatch1, ['bar'], new OneLineRange(0, 1, 1), new OneLineRange(2, 1, 1)); @@ -81,10 +82,10 @@ suite('Search - Viewlet', () => { }); test('Advanced Comparer', () => { - let fileMatch1 = aFileMatch('C:\\with\\path\\foo10'); - let fileMatch2 = aFileMatch('C:\\with\\path2\\foo1'); - let fileMatch3 = aFileMatch('C:\\with\\path2\\bar.a'); - let fileMatch4 = aFileMatch('C:\\with\\path2\\bar.b'); + let fileMatch1 = aFileMatch(isWindows ? 'C:\\with\\path\\foo10' : '/c/with/path/foo10'); + let fileMatch2 = aFileMatch(isWindows ? 'C:\\with\\path2\\foo1' : '/c/with/path2/foo1'); + let fileMatch3 = aFileMatch(isWindows ? 'C:\\with\\path2\\bar.a' : '/c/with/path2/bar.a'); + let fileMatch4 = aFileMatch(isWindows ? 'C:\\with\\path2\\bar.b' : '/c/with/path2/bar.b'); // By default, path < path2 assert(searchMatchComparer(fileMatch1, fileMatch2) < 0); @@ -96,7 +97,7 @@ suite('Search - Viewlet', () => { function aFileMatch(path: string, searchResult?: SearchResult, ...lineMatches: ITextSearchMatch[]): FileMatch { let rawMatch: IFileMatch = { - resource: uri.file('C:\\' + path), + resource: uri.file(path), results: lineMatches }; return instantiation.createInstance(FileMatch, null, null, null, searchResult, rawMatch); From 6c8e4a2d2b091d94ed4961ebe413eacc6d04dc29 Mon Sep 17 00:00:00 2001 From: rzj17 Date: Fri, 13 Dec 2019 15:23:42 +0000 Subject: [PATCH 3/4] Move sortOrder to a single enum --- .../search/browser/search.contribution.ts | 4 +-- .../contrib/search/browser/searchView.ts | 27 ++++++++----------- .../contrib/search/common/searchModel.ts | 16 +++++------ .../search/test/browser/searchViewlet.test.ts | 6 ++--- .../services/search/common/search.ts | 20 +++++++------- 5 files changed, 32 insertions(+), 41 deletions(-) diff --git a/src/vs/workbench/contrib/search/browser/search.contribution.ts b/src/vs/workbench/contrib/search/browser/search.contribution.ts index 8a575f728b6..1f62e26c981 100644 --- a/src/vs/workbench/contrib/search/browser/search.contribution.ts +++ b/src/vs/workbench/contrib/search/browser/search.contribution.ts @@ -829,8 +829,8 @@ configurationRegistry.registerConfiguration({ }, 'search.sortOrder': { 'type': 'string', - 'enum': [SearchSortOrderConfiguration.DEFAULT, SearchSortOrderConfiguration.FILES_ONLY, SearchSortOrderConfiguration.TYPE, SearchSortOrderConfiguration.MODIFIED, SearchSortOrderConfiguration.COUNT_DESCENDING, SearchSortOrderConfiguration.COUNT_ASCENDING], - 'default': SearchSortOrderConfiguration.DEFAULT, + 'enum': [SearchSortOrderConfiguration.default, SearchSortOrderConfiguration.fileNames, SearchSortOrderConfiguration.type, SearchSortOrderConfiguration.modified, SearchSortOrderConfiguration.countDescending, SearchSortOrderConfiguration.countAscending], + 'default': SearchSortOrderConfiguration.default, 'enumDescriptions': [ nls.localize('searchSortOrder.default', 'Results are sorted by folder and file names, in alphabetical order.'), nls.localize('searchSortOrder.filesOnly', 'Results are sorted by file names ignoring folder order, in alphabetical order.'), diff --git a/src/vs/workbench/contrib/search/browser/searchView.ts b/src/vs/workbench/contrib/search/browser/searchView.ts index ee76bacb8cb..dc9c622f323 100644 --- a/src/vs/workbench/contrib/search/browser/searchView.ts +++ b/src/vs/workbench/contrib/search/browser/searchView.ts @@ -34,7 +34,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { TreeResourceNavigator2, WorkbenchObjectTree, getSelectionKeyboardEvent } from 'vs/platform/list/browser/listService'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { IProgressService, IProgressStep, IProgress } from 'vs/platform/progress/common/progress'; -import { IPatternInfo, ISearchComplete, ISearchConfiguration, ISearchConfigurationProperties, ITextQuery, VIEW_ID, VIEWLET_ID } from 'vs/workbench/services/search/common/search'; +import { IPatternInfo, ISearchComplete, ISearchConfiguration, ISearchConfigurationProperties, ITextQuery, VIEW_ID, VIEWLET_ID, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; import { ISearchHistoryService, ISearchHistoryValues } from 'vs/workbench/contrib/search/common/searchHistoryService'; import { diffInserted, diffInsertedOutline, diffRemoved, diffRemovedOutline, editorFindMatchHighlight, editorFindMatchHighlightBorder, listActiveSelectionForeground, foreground } from 'vs/platform/theme/common/colorRegistry'; import { ICssStyleCollector, ITheme, IThemeService, registerThemingParticipant } from 'vs/platform/theme/common/themeService'; @@ -196,7 +196,7 @@ export class SearchView extends ViewPane { }); this.configurationService.onDidChangeConfiguration(e => { if (e.affectsConfiguration('search.sortOrder')) { - if (this.searchConfig.sortOrder !== 'modified') { + if (this.searchConfig.sortOrder === SearchSortOrderConfiguration.modified) { // If changing away from modified, remove all fileStats // so that updated files are re-retrieved next time. this.removeFileStats(); @@ -505,7 +505,7 @@ export class SearchView extends ViewPane { const collapseResults = this.searchConfig.collapseResults; if (!event || event.added || event.removed) { // Refresh whole tree - if (this.searchConfig.sortOrder === 'modified') { + if (this.searchConfig.sortOrder === SearchSortOrderConfiguration.modified) { // Ensure all matches have retrieved their file stat this.retrieveFileStats() .then(() => this.tree.setChildren(null, this.createResultIterator(collapseResults))); @@ -514,7 +514,8 @@ export class SearchView extends ViewPane { } } else { // If updated counts affect our search order, re-sort the view. - if (this.searchConfig.sortOrder === 'countAscending' || this.searchConfig.sortOrder === 'countDescending') { + if (this.searchConfig.sortOrder === SearchSortOrderConfiguration.countAscending || + this.searchConfig.sortOrder === SearchSortOrderConfiguration.countDescending) { this.tree.setChildren(null, this.createResultIterator(collapseResults)); } else { // FileMatch modified, refresh those elements @@ -1714,7 +1715,7 @@ export class SearchView extends ViewPane { } private onFilesChanged(e: FileChangesEvent): void { - if (this.searchConfig.sortOrder !== 'modified' && (!this.viewModel || !e.gotDeleted())) { + if (!this.viewModel || (this.searchConfig.sortOrder !== SearchSortOrderConfiguration.modified && !e.gotDeleted())) { return; } @@ -1726,7 +1727,7 @@ export class SearchView extends ViewPane { } else { // Check if the changed file contained matches const changedMatches = matches.filter(m => e.contains(m.resource)); - if (changedMatches.length && this.searchConfig.sortOrder === 'modified') { + if (changedMatches.length && this.searchConfig.sortOrder === SearchSortOrderConfiguration.modified) { // No matches need to be removed, but modified files need to have their file stat updated. this.updateFileStats(changedMatches).then(() => this.refreshTree()); } @@ -1803,19 +1804,13 @@ export class SearchView extends ViewPane { } private async retrieveFileStats(): Promise { - for (const fileMatch of this.searchResult.matches()) { - if (!fileMatch.fileStat) { - await fileMatch.resolveFileStat(this.fileService); - } - } - return Promise.resolve(undefined); + const files = this.searchResult.matches().filter(f => !f.fileStat).map(f => f.resolveFileStat(this.fileService)); + await Promise.all(files); } private async updateFileStats(elements: FileMatch[]): Promise { - for (const fileMatch of elements) { - await fileMatch.resolveFileStat(this.fileService); - } - return Promise.resolve(undefined); + const files = this.searchResult.matches().map(f => f.resolveFileStat(this.fileService)); + await Promise.all(files); } private removeFileStats(): void { diff --git a/src/vs/workbench/contrib/search/common/searchModel.ts b/src/vs/workbench/contrib/search/common/searchModel.ts index 1a2dd7d3aba..73d0518940a 100644 --- a/src/vs/workbench/contrib/search/common/searchModel.ts +++ b/src/vs/workbench/contrib/search/common/searchModel.ts @@ -20,7 +20,7 @@ import { IModelService } from 'vs/editor/common/services/modelService'; import { createDecorator, IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IProgress, IProgressStep } from 'vs/platform/progress/common/progress'; import { ReplacePattern } from 'vs/workbench/services/search/common/replace'; -import { IFileMatch, IPatternInfo, ISearchComplete, ISearchProgressItem, ISearchConfigurationProperties, ISearchService, ITextQuery, ITextSearchPreviewOptions, ITextSearchMatch, ITextSearchStats, resultIsMatch, ISearchRange, OneLineRange, ITextSearchContext, ITextSearchResult, SearchSortOrder, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; +import { IFileMatch, IPatternInfo, ISearchComplete, ISearchProgressItem, ISearchConfigurationProperties, ISearchService, ITextQuery, ITextSearchPreviewOptions, ITextSearchMatch, ITextSearchStats, resultIsMatch, ISearchRange, OneLineRange, ITextSearchContext, ITextSearchResult, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { overviewRulerFindMatchForeground, minimapFindMatch } from 'vs/platform/theme/common/colorRegistry'; import { themeColorFromId } from 'vs/platform/theme/common/themeService'; @@ -411,7 +411,6 @@ export class FileMatch extends Disposable implements IFileMatch { async resolveFileStat(fileService: IFileService): Promise { this._fileStat = await fileService.resolve(this.resource, { resolveMetadata: true }); - return Promise.resolve(); } public get fileStat(): IFileStatWithMetadata | undefined { @@ -434,7 +433,6 @@ export interface IChangeEvent { elements: FileMatch[]; added?: boolean; removed?: boolean; - modified?: boolean; } export class FolderMatch extends Disposable { @@ -645,22 +643,22 @@ export class FolderMatchWithResource extends FolderMatch { * Compares instances of the same match type. Different match types should not be siblings * and their sort order is undefined. */ -export function searchMatchComparer(elementA: RenderableMatch, elementB: RenderableMatch, sortOrder: SearchSortOrder = 'default'): number { +export function searchMatchComparer(elementA: RenderableMatch, elementB: RenderableMatch, sortOrder: SearchSortOrderConfiguration = SearchSortOrderConfiguration.default): number { if (elementA instanceof FolderMatch && elementB instanceof FolderMatch) { return elementA.index() - elementB.index(); } if (elementA instanceof FileMatch && elementB instanceof FileMatch) { switch (sortOrder) { - case SearchSortOrderConfiguration.COUNT_DESCENDING: + case SearchSortOrderConfiguration.countDescending: return elementB.count() - elementA.count(); - case SearchSortOrderConfiguration.COUNT_ASCENDING: + case SearchSortOrderConfiguration.countAscending: return elementA.count() - elementB.count(); - case SearchSortOrderConfiguration.TYPE: + case SearchSortOrderConfiguration.type: return compareFileExtensions(elementA.name(), elementB.name()); - case SearchSortOrderConfiguration.FILES_ONLY: + case SearchSortOrderConfiguration.fileNames: return compareFileNames(elementA.name(), elementB.name()); - case SearchSortOrderConfiguration.MODIFIED: + case SearchSortOrderConfiguration.modified: const fileStatA = elementA.fileStat; const fileStatB = elementB.fileStat; if (fileStatA && fileStatB) { diff --git a/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts b/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts index d37ca51b9fb..570b8408b03 100644 --- a/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts +++ b/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts @@ -9,7 +9,7 @@ import { ModelServiceImpl } from 'vs/editor/common/services/modelServiceImpl'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; -import { IFileMatch, ITextSearchMatch, OneLineRange, QueryType } from 'vs/workbench/services/search/common/search'; +import { IFileMatch, ITextSearchMatch, OneLineRange, QueryType, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { TestWorkspace } from 'vs/platform/workspace/test/common/testWorkspace'; import { FileMatch, Match, searchMatchComparer, SearchResult } from 'vs/workbench/contrib/search/common/searchModel'; @@ -90,9 +90,9 @@ suite('Search - Viewlet', () => { // By default, path < path2 assert(searchMatchComparer(fileMatch1, fileMatch2) < 0); // By filenames, foo10 > foo1 - assert(searchMatchComparer(fileMatch1, fileMatch2, 'filesOnly') > 0); + assert(searchMatchComparer(fileMatch1, fileMatch2, SearchSortOrderConfiguration.fileNames) > 0); // By type, bar.a < bar.b - assert(searchMatchComparer(fileMatch3, fileMatch4, 'type') < 0); + assert(searchMatchComparer(fileMatch3, fileMatch4, SearchSortOrderConfiguration.type) < 0); }); function aFileMatch(path: string, searchResult?: SearchResult, ...lineMatches: ITextSearchMatch[]): FileMatch { diff --git a/src/vs/workbench/services/search/common/search.ts b/src/vs/workbench/services/search/common/search.ts index c1fa0e04bde..19da495ac20 100644 --- a/src/vs/workbench/services/search/common/search.ts +++ b/src/vs/workbench/services/search/common/search.ts @@ -311,16 +311,14 @@ export class OneLineRange extends SearchRange { } } -export const SearchSortOrderConfiguration = { - DEFAULT: 'default', - FILES_ONLY: 'filesOnly', - TYPE: 'type', - MODIFIED: 'modified', - COUNT_DESCENDING: 'countDescending', - COUNT_ASCENDING: 'countAscending' -}; - -export type SearchSortOrder = 'default' | 'filesOnly' | 'type' | 'modified' | 'countDescending' | 'countAscending'; +export const enum SearchSortOrderConfiguration { + default, + fileNames, + type, + modified, + countDescending, + countAscending +} export interface ISearchConfigurationProperties { exclude: glob.IExpression; @@ -344,7 +342,7 @@ export interface ISearchConfigurationProperties { searchOnTypeDebouncePeriod: number; enableSearchEditorPreview: boolean; searchEditorPreviewForceAbsolutePaths: boolean; - sortOrder: SearchSortOrder; + sortOrder: SearchSortOrderConfiguration; } export interface ISearchConfiguration extends IFilesConfiguration { From 479d4e6e41796e31aa7d361c26b90297713defaa Mon Sep 17 00:00:00 2001 From: Robert Jin Date: Fri, 13 Dec 2019 23:26:26 +0000 Subject: [PATCH 4/4] Add string descriptions to SearchSortOrder enum --- .../search/browser/search.contribution.ts | 6 +++--- .../contrib/search/browser/searchView.ts | 16 ++++++++-------- .../contrib/search/common/searchModel.ts | 14 +++++++------- .../search/test/browser/searchViewlet.test.ts | 6 +++--- .../workbench/services/search/common/search.ts | 16 ++++++++-------- 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/vs/workbench/contrib/search/browser/search.contribution.ts b/src/vs/workbench/contrib/search/browser/search.contribution.ts index 1f62e26c981..7ec30f5e733 100644 --- a/src/vs/workbench/contrib/search/browser/search.contribution.ts +++ b/src/vs/workbench/contrib/search/browser/search.contribution.ts @@ -51,7 +51,7 @@ import { ISearchHistoryService, SearchHistoryService } from 'vs/workbench/contri import { FileMatchOrMatch, ISearchWorkbenchService, RenderableMatch, SearchWorkbenchService, FileMatch, Match, FolderMatch } from 'vs/workbench/contrib/search/common/searchModel'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { IPanelService } from 'vs/workbench/services/panel/common/panelService'; -import { ISearchConfiguration, ISearchConfigurationProperties, PANEL_ID, VIEWLET_ID, VIEW_ID, VIEW_CONTAINER, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; +import { ISearchConfiguration, ISearchConfigurationProperties, PANEL_ID, VIEWLET_ID, VIEW_ID, VIEW_CONTAINER, SearchSortOrder } from 'vs/workbench/services/search/common/search'; import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { ExplorerViewPaneContainer } from 'vs/workbench/contrib/files/browser/explorerViewlet'; @@ -829,8 +829,8 @@ configurationRegistry.registerConfiguration({ }, 'search.sortOrder': { 'type': 'string', - 'enum': [SearchSortOrderConfiguration.default, SearchSortOrderConfiguration.fileNames, SearchSortOrderConfiguration.type, SearchSortOrderConfiguration.modified, SearchSortOrderConfiguration.countDescending, SearchSortOrderConfiguration.countAscending], - 'default': SearchSortOrderConfiguration.default, + 'enum': [SearchSortOrder.Default, SearchSortOrder.FileNames, SearchSortOrder.Type, SearchSortOrder.Modified, SearchSortOrder.CountDescending, SearchSortOrder.CountAscending], + 'default': SearchSortOrder.Default, 'enumDescriptions': [ nls.localize('searchSortOrder.default', 'Results are sorted by folder and file names, in alphabetical order.'), nls.localize('searchSortOrder.filesOnly', 'Results are sorted by file names ignoring folder order, in alphabetical order.'), diff --git a/src/vs/workbench/contrib/search/browser/searchView.ts b/src/vs/workbench/contrib/search/browser/searchView.ts index dc9c622f323..931d8e286e4 100644 --- a/src/vs/workbench/contrib/search/browser/searchView.ts +++ b/src/vs/workbench/contrib/search/browser/searchView.ts @@ -34,7 +34,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { TreeResourceNavigator2, WorkbenchObjectTree, getSelectionKeyboardEvent } from 'vs/platform/list/browser/listService'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { IProgressService, IProgressStep, IProgress } from 'vs/platform/progress/common/progress'; -import { IPatternInfo, ISearchComplete, ISearchConfiguration, ISearchConfigurationProperties, ITextQuery, VIEW_ID, VIEWLET_ID, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; +import { IPatternInfo, ISearchComplete, ISearchConfiguration, ISearchConfigurationProperties, ITextQuery, VIEW_ID, VIEWLET_ID, SearchSortOrder } from 'vs/workbench/services/search/common/search'; import { ISearchHistoryService, ISearchHistoryValues } from 'vs/workbench/contrib/search/common/searchHistoryService'; import { diffInserted, diffInsertedOutline, diffRemoved, diffRemovedOutline, editorFindMatchHighlight, editorFindMatchHighlightBorder, listActiveSelectionForeground, foreground } from 'vs/platform/theme/common/colorRegistry'; import { ICssStyleCollector, ITheme, IThemeService, registerThemingParticipant } from 'vs/platform/theme/common/themeService'; @@ -196,7 +196,7 @@ export class SearchView extends ViewPane { }); this.configurationService.onDidChangeConfiguration(e => { if (e.affectsConfiguration('search.sortOrder')) { - if (this.searchConfig.sortOrder === SearchSortOrderConfiguration.modified) { + if (this.searchConfig.sortOrder === SearchSortOrder.Modified) { // If changing away from modified, remove all fileStats // so that updated files are re-retrieved next time. this.removeFileStats(); @@ -505,7 +505,7 @@ export class SearchView extends ViewPane { const collapseResults = this.searchConfig.collapseResults; if (!event || event.added || event.removed) { // Refresh whole tree - if (this.searchConfig.sortOrder === SearchSortOrderConfiguration.modified) { + if (this.searchConfig.sortOrder === SearchSortOrder.Modified) { // Ensure all matches have retrieved their file stat this.retrieveFileStats() .then(() => this.tree.setChildren(null, this.createResultIterator(collapseResults))); @@ -514,8 +514,8 @@ export class SearchView extends ViewPane { } } else { // If updated counts affect our search order, re-sort the view. - if (this.searchConfig.sortOrder === SearchSortOrderConfiguration.countAscending || - this.searchConfig.sortOrder === SearchSortOrderConfiguration.countDescending) { + if (this.searchConfig.sortOrder === SearchSortOrder.CountAscending || + this.searchConfig.sortOrder === SearchSortOrder.CountDescending) { this.tree.setChildren(null, this.createResultIterator(collapseResults)); } else { // FileMatch modified, refresh those elements @@ -1715,7 +1715,7 @@ export class SearchView extends ViewPane { } private onFilesChanged(e: FileChangesEvent): void { - if (!this.viewModel || (this.searchConfig.sortOrder !== SearchSortOrderConfiguration.modified && !e.gotDeleted())) { + if (!this.viewModel || (this.searchConfig.sortOrder !== SearchSortOrder.Modified && !e.gotDeleted())) { return; } @@ -1727,7 +1727,7 @@ export class SearchView extends ViewPane { } else { // Check if the changed file contained matches const changedMatches = matches.filter(m => e.contains(m.resource)); - if (changedMatches.length && this.searchConfig.sortOrder === SearchSortOrderConfiguration.modified) { + if (changedMatches.length && this.searchConfig.sortOrder === SearchSortOrder.Modified) { // No matches need to be removed, but modified files need to have their file stat updated. this.updateFileStats(changedMatches).then(() => this.refreshTree()); } @@ -1809,7 +1809,7 @@ export class SearchView extends ViewPane { } private async updateFileStats(elements: FileMatch[]): Promise { - const files = this.searchResult.matches().map(f => f.resolveFileStat(this.fileService)); + const files = elements.map(f => f.resolveFileStat(this.fileService)); await Promise.all(files); } diff --git a/src/vs/workbench/contrib/search/common/searchModel.ts b/src/vs/workbench/contrib/search/common/searchModel.ts index 73d0518940a..714140379bd 100644 --- a/src/vs/workbench/contrib/search/common/searchModel.ts +++ b/src/vs/workbench/contrib/search/common/searchModel.ts @@ -20,7 +20,7 @@ import { IModelService } from 'vs/editor/common/services/modelService'; import { createDecorator, IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IProgress, IProgressStep } from 'vs/platform/progress/common/progress'; import { ReplacePattern } from 'vs/workbench/services/search/common/replace'; -import { IFileMatch, IPatternInfo, ISearchComplete, ISearchProgressItem, ISearchConfigurationProperties, ISearchService, ITextQuery, ITextSearchPreviewOptions, ITextSearchMatch, ITextSearchStats, resultIsMatch, ISearchRange, OneLineRange, ITextSearchContext, ITextSearchResult, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; +import { IFileMatch, IPatternInfo, ISearchComplete, ISearchProgressItem, ISearchConfigurationProperties, ISearchService, ITextQuery, ITextSearchPreviewOptions, ITextSearchMatch, ITextSearchStats, resultIsMatch, ISearchRange, OneLineRange, ITextSearchContext, ITextSearchResult, SearchSortOrder } from 'vs/workbench/services/search/common/search'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { overviewRulerFindMatchForeground, minimapFindMatch } from 'vs/platform/theme/common/colorRegistry'; import { themeColorFromId } from 'vs/platform/theme/common/themeService'; @@ -643,22 +643,22 @@ export class FolderMatchWithResource extends FolderMatch { * Compares instances of the same match type. Different match types should not be siblings * and their sort order is undefined. */ -export function searchMatchComparer(elementA: RenderableMatch, elementB: RenderableMatch, sortOrder: SearchSortOrderConfiguration = SearchSortOrderConfiguration.default): number { +export function searchMatchComparer(elementA: RenderableMatch, elementB: RenderableMatch, sortOrder: SearchSortOrder = SearchSortOrder.Default): number { if (elementA instanceof FolderMatch && elementB instanceof FolderMatch) { return elementA.index() - elementB.index(); } if (elementA instanceof FileMatch && elementB instanceof FileMatch) { switch (sortOrder) { - case SearchSortOrderConfiguration.countDescending: + case SearchSortOrder.CountDescending: return elementB.count() - elementA.count(); - case SearchSortOrderConfiguration.countAscending: + case SearchSortOrder.CountAscending: return elementA.count() - elementB.count(); - case SearchSortOrderConfiguration.type: + case SearchSortOrder.Type: return compareFileExtensions(elementA.name(), elementB.name()); - case SearchSortOrderConfiguration.fileNames: + case SearchSortOrder.FileNames: return compareFileNames(elementA.name(), elementB.name()); - case SearchSortOrderConfiguration.modified: + case SearchSortOrder.Modified: const fileStatA = elementA.fileStat; const fileStatB = elementB.fileStat; if (fileStatA && fileStatB) { diff --git a/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts b/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts index 570b8408b03..7c1ea08b6e3 100644 --- a/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts +++ b/src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts @@ -9,7 +9,7 @@ import { ModelServiceImpl } from 'vs/editor/common/services/modelServiceImpl'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; -import { IFileMatch, ITextSearchMatch, OneLineRange, QueryType, SearchSortOrderConfiguration } from 'vs/workbench/services/search/common/search'; +import { IFileMatch, ITextSearchMatch, OneLineRange, QueryType, SearchSortOrder } from 'vs/workbench/services/search/common/search'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { TestWorkspace } from 'vs/platform/workspace/test/common/testWorkspace'; import { FileMatch, Match, searchMatchComparer, SearchResult } from 'vs/workbench/contrib/search/common/searchModel'; @@ -90,9 +90,9 @@ suite('Search - Viewlet', () => { // By default, path < path2 assert(searchMatchComparer(fileMatch1, fileMatch2) < 0); // By filenames, foo10 > foo1 - assert(searchMatchComparer(fileMatch1, fileMatch2, SearchSortOrderConfiguration.fileNames) > 0); + assert(searchMatchComparer(fileMatch1, fileMatch2, SearchSortOrder.FileNames) > 0); // By type, bar.a < bar.b - assert(searchMatchComparer(fileMatch3, fileMatch4, SearchSortOrderConfiguration.type) < 0); + assert(searchMatchComparer(fileMatch3, fileMatch4, SearchSortOrder.Type) < 0); }); function aFileMatch(path: string, searchResult?: SearchResult, ...lineMatches: ITextSearchMatch[]): FileMatch { diff --git a/src/vs/workbench/services/search/common/search.ts b/src/vs/workbench/services/search/common/search.ts index 19da495ac20..4ef600d4629 100644 --- a/src/vs/workbench/services/search/common/search.ts +++ b/src/vs/workbench/services/search/common/search.ts @@ -311,13 +311,13 @@ export class OneLineRange extends SearchRange { } } -export const enum SearchSortOrderConfiguration { - default, - fileNames, - type, - modified, - countDescending, - countAscending +export const enum SearchSortOrder { + Default = 'default', + FileNames = 'fileNames', + Type = 'type', + Modified = 'modified', + CountDescending = 'countDescending', + CountAscending = 'countAscending' } export interface ISearchConfigurationProperties { @@ -342,7 +342,7 @@ export interface ISearchConfigurationProperties { searchOnTypeDebouncePeriod: number; enableSearchEditorPreview: boolean; searchEditorPreviewForceAbsolutePaths: boolean; - sortOrder: SearchSortOrderConfiguration; + sortOrder: SearchSortOrder; } export interface ISearchConfiguration extends IFilesConfiguration {