Merge pull request #86563 from jzyrobert/83252-search-sort

#82352 Implement sorting for search results

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, 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';
@@ -827,7 +827,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': [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.'),
+				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.")
+		},
 	}
 });
 

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, 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';
@@ -197,6 +197,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 === SearchSortOrder.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);
@@ -503,13 +513,25 @@ 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 === SearchSortOrder.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 === SearchSortOrder.CountAscending ||
+				this.searchConfig.sortOrder === SearchSortOrder.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);
+				});
+			}
 		}
 	}
 
@@ -530,9 +552,10 @@ export class SearchView extends ViewPane {
 	}
 
 	private createFolderIterator(folderMatch: FolderMatch, collapseResults: ISearchConfigurationProperties['collapseResults']): Iterator<ITreeElement<RenderableMatch>> {
+		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);
@@ -1703,14 +1726,23 @@ export class SearchView extends ViewPane {
 	}
 
 	private onFilesChanged(e: FileChangesEvent): void {
-		if (!this.viewModel || !e.gotDeleted()) {
+		if (!this.viewModel || (this.searchConfig.sortOrder !== SearchSortOrder.Modified && !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 === SearchSortOrder.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[] {
@@ -1783,6 +1815,22 @@ export class SearchView extends ViewPane {
 		super.saveState();
 	}
 
+	private async retrieveFileStats(): Promise<void> {
+		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<void> {
+		const files = elements.map(f => f.resolveFileStat(this.fileService));
+		await Promise.all(files);
+	}
+
+	private removeFileStats(): void {
+		for (const fileMatch of this.searchResult.matches()) {
+			fileMatch.fileStat = undefined;
+		}
+	}
+
 	dispose(): void {
 		this.isDisposed = true;
 		this.saveState();

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 } 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, addContextToEditorMatches } from 'vs/
 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<void> = this._onDispose.event;
 
 	private _resource: URI;
+	private _fileStat?: IFileStatWithMetadata;
 	private _model: ITextModel | null = null;
 	private _modelListener: IDisposable | null = null;
 	private _matches: Map<string, Match>;
@@ -411,6 +414,18 @@ export class FileMatch extends Disposable implements IFileMatch {
 		}
 	}
 
+	async resolveFileStat(fileService: IFileService): Promise<void> {
+		this._fileStat = await fileService.resolve(this.resource, { resolveMetadata: true });
+	}
+
+	public get fileStat(): IFileStatWithMetadata | undefined {
+		return this._fileStat;
+	}
+
+	public set fileStat(stat: IFileStatWithMetadata | undefined) {
+		this._fileStat = stat;
+	}
+
 	dispose(): void {
 		this.setSelectedMatch(null);
 		this.unbindModel();
@@ -633,13 +648,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 = 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 SearchSortOrder.CountDescending:
+				return elementB.count() - elementA.count();
+			case SearchSortOrder.CountAscending:
+				return elementA.count() - elementB.count();
+			case SearchSortOrder.Type:
+				return compareFileExtensions(elementA.name(), elementB.name());
+			case SearchSortOrder.FileNames:
+				return compareFileNames(elementA.name(), elementB.name());
+			case SearchSortOrder.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) {

src/vs/workbench/contrib/search/test/browser/searchViewlet.test.ts

@@ -9,11 +9,12 @@ 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, 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';
 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));
@@ -80,9 +81,23 @@ suite('Search - Viewlet', () => {
 		assert(searchMatchComparer(lineMatch2, lineMatch3) === 0);
 	});
 
+	test('Advanced Comparer', () => {
+		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);
+		// By filenames, foo10 > foo1
+		assert(searchMatchComparer(fileMatch1, fileMatch2, SearchSortOrder.FileNames) > 0);
+		// By type, bar.a < bar.b
+		assert(searchMatchComparer(fileMatch3, fileMatch4, SearchSortOrder.Type) < 0);
+	});
+
 	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);

src/vs/workbench/services/search/common/search.ts

@@ -311,6 +311,15 @@ export class OneLineRange extends SearchRange {
 	}
 }
 
+export const enum SearchSortOrder {
+	Default = 'default',
+	FileNames = 'fileNames',
+	Type = 'type',
+	Modified = 'modified',
+	CountDescending = 'countDescending',
+	CountAscending = 'countAscending'
+}
+
 export interface ISearchConfigurationProperties {
 	exclude: glob.IExpression;
 	useRipgrep: boolean;
@@ -333,6 +342,7 @@ export interface ISearchConfigurationProperties {
 	searchOnTypeDebouncePeriod: number;
 	enableSearchEditorPreview: boolean;
 	searchEditorPreviewForceAbsolutePaths: boolean;
+	sortOrder: SearchSortOrder;
 }
 
 export interface ISearchConfiguration extends IFilesConfiguration {