Merge pull request #52768 from Microsoft/roblou/quickOpenAbsPaths

Fix #51597 - move searching with absolute path queries out of search service and into openFileHandler

src/vs/workbench/parts/search/browser/openFileHandler.ts

@@ -26,7 +26,7 @@ import { EditorInput, IWorkbenchEditorConfiguration } from 'vs/workbench/common/
 import { IResourceInput } from 'vs/platform/editor/common/editor';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
-import { IQueryOptions, ISearchService, ISearchStats, ISearchQuery } from 'vs/platform/search/common/search';
+import { IQueryOptions, ISearchService, ISearchStats, ISearchQuery, ISearchComplete } from 'vs/platform/search/common/search';
 import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
 import { IRange } from 'vs/editor/common/core/range';
@@ -152,29 +152,44 @@ export class OpenFileHandler extends QuickOpenHandler {
 	}
 
 	private doFindResults(query: IPreparedQuery, cacheKey?: string, maxSortedResults?: number): TPromise<FileQuickOpenModel> {
-		return this.doResolveQueryOptions(query, cacheKey, maxSortedResults).then(queryOptions => {
-			let iconClass: string;
-			if (this.options && this.options.forceUseIcons && !this.themeService.getFileIconTheme()) {
-				iconClass = 'file'; // only use a generic file icon if we are forced to use an icon and have no icon theme set otherwise
-			}
+		const queryOptions = this.doResolveQueryOptions(query, cacheKey, maxSortedResults);
+		let iconClass: string;
+		if (this.options && this.options.forceUseIcons && !this.themeService.getFileIconTheme()) {
+			iconClass = 'file'; // only use a generic file icon if we are forced to use an icon and have no icon theme set otherwise
+		}
 
-			return this.searchService.search(this.queryBuilder.file(this.contextService.getWorkspace().folders.map(folder => folder.uri), queryOptions)).then(complete => {
-				const results: QuickOpenEntry[] = [];
-				for (let i = 0; i < complete.results.length; i++) {
-					const fileMatch = complete.results[i];
+		return this.getAbsolutePathResult(query).then(result => {
+			// If the original search value is an existing file on disk, return it immediately and bypass the search service
+			if (result) {
+				return TPromise.wrap(<ISearchComplete>{ results: [{ resource: result }] });
+			} else {
+				return this.searchService.search(this.queryBuilder.file(this.contextService.getWorkspace().folders.map(folder => folder.uri), queryOptions));
+			}
+		}).then(complete => {
+			const results: QuickOpenEntry[] = [];
+			for (let i = 0; i < complete.results.length; i++) {
+				const fileMatch = complete.results[i];
 
-					const label = paths.basename(fileMatch.resource.fsPath);
-					const description = labels.getPathLabel(resources.dirname(fileMatch.resource), this.environmentService, this.contextService);
+				const label = paths.basename(fileMatch.resource.fsPath);
+				const description = labels.getPathLabel(resources.dirname(fileMatch.resource), this.environmentService, this.contextService);
 
-					results.push(this.instantiationService.createInstance(FileEntry, fileMatch.resource, label, description, iconClass));
-				}
+				results.push(this.instantiationService.createInstance(FileEntry, fileMatch.resource, label, description, iconClass));
+			}
 
-				return new FileQuickOpenModel(results, complete.stats);
-			});
+			return new FileQuickOpenModel(results, complete.stats);
 		});
 	}
 
-	private doResolveQueryOptions(query: IPreparedQuery, cacheKey?: string, maxSortedResults?: number): TPromise<IQueryOptions> {
+	private getAbsolutePathResult(query: IPreparedQuery): TPromise<URI> {
+		if (paths.isAbsolute(query.original)) {
+			const resource = URI.file(query.original);
+			return this.fileService.resolveFile(resource).then(stat => stat.isDirectory ? void 0 : resource, error => void 0);
+		} else {
+			return TPromise.as(null);
+		}
+	}
+
+	private doResolveQueryOptions(query: IPreparedQuery, cacheKey?: string, maxSortedResults?: number): IQueryOptions {
 		const queryOptions: IQueryOptions = {
 			extraFileResources: getOutOfWorkspaceEditorResources(this.editorService, this.contextService),
 			filePattern: query.value,
@@ -186,23 +201,7 @@ export class OpenFileHandler extends QuickOpenHandler {
 			queryOptions.sortByScore = true;
 		}
 
-		let queryIsAbsoluteFilePromise: TPromise<URI>;
-		if (paths.isAbsolute(query.original)) {
-			const resource = URI.file(query.original);
-			queryIsAbsoluteFilePromise = this.fileService.resolveFile(resource).then(stat => stat.isDirectory ? void 0 : resource, error => void 0);
-		} else {
-			queryIsAbsoluteFilePromise = TPromise.as(null);
-		}
-
-		return queryIsAbsoluteFilePromise.then(resource => {
-			if (resource) {
-				// if the original search value is an existing file on disk, add it to the
-				// extra file resources to consider (fixes https://github.com/Microsoft/vscode/issues/42726)
-				queryOptions.extraFileResources.push(resource);
-			}
-
-			return queryOptions;
-		});
+		return queryOptions;
 	}
 
 	public hasShortResponseTime(): boolean {

src/vs/workbench/services/search/node/fileSearch.ts

@@ -123,78 +123,61 @@ export class FileWalker {
 		this.fileWalkStartTime = Date.now();
 
 		// Support that the file pattern is a full path to a file that exists
-		this.checkFilePatternAbsoluteMatch((exists, size) => {
-			if (this.isCanceled) {
-				return done(null, this.isLimitHit);
-			}
-
-			// Report result from file pattern if matching
-			if (exists) {
-				this.resultCount++;
-				onResult({
-					relativePath: this.filePattern,
-					basename: path.basename(this.filePattern),
-					size
-				});
-
-				// Optimization: a match on an absolute path is a good result and we do not
-				// continue walking the entire root paths array for other matches because
-				// it is very unlikely that another file would match on the full absolute path
-				return done(null, this.isLimitHit);
-			}
+		if (this.isCanceled) {
+			return done(null, this.isLimitHit);
+		}
 
-			// For each extra file
-			if (extraFiles) {
-				extraFiles.forEach(extraFilePath => {
-					const basename = path.basename(extraFilePath);
-					if (this.globalExcludePattern && this.globalExcludePattern(extraFilePath, basename)) {
-						return; // excluded
-					}
+		// For each extra file
+		if (extraFiles) {
+			extraFiles.forEach(extraFilePath => {
+				const basename = path.basename(extraFilePath);
+				if (this.globalExcludePattern && this.globalExcludePattern(extraFilePath, basename)) {
+					return; // excluded
+				}
 
-					// File: Check for match on file pattern and include pattern
-					this.matchFile(onResult, { relativePath: extraFilePath /* no workspace relative path */, basename });
-				});
-			}
+				// File: Check for match on file pattern and include pattern
+				this.matchFile(onResult, { relativePath: extraFilePath /* no workspace relative path */, basename });
+			});
+		}
 
-			let traverse = this.nodeJSTraversal;
-			if (!this.maxFilesize) {
-				if (this.useRipgrep) {
-					this.traversal = Traversal.Ripgrep;
-					traverse = this.cmdTraversal;
-				} else if (platform.isMacintosh) {
-					this.traversal = Traversal.MacFind;
-					traverse = this.cmdTraversal;
-					// Disable 'dir' for now (#11181, #11179, #11183, #11182).
-				} /* else if (platform.isWindows) {
-					this.traversal = Traversal.WindowsDir;
-					traverse = this.windowsDirTraversal;
-				} */ else if (platform.isLinux) {
-					this.traversal = Traversal.LinuxFind;
-					traverse = this.cmdTraversal;
-				}
+		let traverse = this.nodeJSTraversal;
+		if (!this.maxFilesize) {
+			if (this.useRipgrep) {
+				this.traversal = Traversal.Ripgrep;
+				traverse = this.cmdTraversal;
+			} else if (platform.isMacintosh) {
+				this.traversal = Traversal.MacFind;
+				traverse = this.cmdTraversal;
+				// Disable 'dir' for now (#11181, #11179, #11183, #11182).
+			} /* else if (platform.isWindows) {
+				this.traversal = Traversal.WindowsDir;
+				traverse = this.windowsDirTraversal;
+			} */ else if (platform.isLinux) {
+				this.traversal = Traversal.LinuxFind;
+				traverse = this.cmdTraversal;
 			}
+		}
 
-			const isNodeTraversal = traverse === this.nodeJSTraversal;
-			if (!isNodeTraversal) {
-				this.cmdForkStartTime = Date.now();
-			}
+		const isNodeTraversal = traverse === this.nodeJSTraversal;
+		if (!isNodeTraversal) {
+			this.cmdForkStartTime = Date.now();
+		}
 
-			// For each root folder
-			flow.parallel<IFolderSearch, void>(folderQueries, (folderQuery: IFolderSearch, rootFolderDone: (err: Error, result: void) => void) => {
-				this.call(traverse, this, folderQuery, onResult, onMessage, (err?: Error) => {
-					if (err) {
-						const errorMessage = toErrorMessage(err);
-						console.error(errorMessage);
-						this.errors.push(errorMessage);
-						rootFolderDone(err, undefined);
-					} else {
-						rootFolderDone(undefined, undefined);
-					}
-				});
-			}, (errors, result) => {
-				const err = errors ? errors.filter(e => !!e)[0] : null;
-				done(err, this.isLimitHit);
+		// For each root folder
+		flow.parallel<IFolderSearch, void>(folderQueries, (folderQuery: IFolderSearch, rootFolderDone: (err: Error, result: void) => void) => {
+			this.call(traverse, this, folderQuery, onResult, onMessage, (err?: Error) => {
+				if (err) {
+					const errorMessage = toErrorMessage(err);
+					console.error(errorMessage);
+					this.errors.push(errorMessage);
+					rootFolderDone(err, undefined);
+				} else {
+					rootFolderDone(undefined, undefined);
+				}
 			});
+		}, (errors, result) => {
+			const err = errors ? errors.filter(e => !!e)[0] : null;
+			done(err, this.isLimitHit);
 		});
 	}
 
@@ -568,16 +551,6 @@ export class FileWalker {
 		};
 	}
 
-	private checkFilePatternAbsoluteMatch(clb: (exists: boolean, size?: number) => void): void {
-		if (!this.filePattern || !path.isAbsolute(this.filePattern)) {
-			return clb(false);
-		}
-
-		return fs.stat(this.filePattern, (error, stat) => {
-			return clb(!error && !stat.isDirectory(), stat && stat.size); // only existing files
-		});
-	}
-
 	private checkFilePatternRelativeMatch(basePath: string, clb: (matchPath: string, size?: number) => void): void {
 		if (!this.filePattern || path.isAbsolute(this.filePattern)) {
 			return clb(null);

src/vs/workbench/services/search/node/rawSearchService.ts

@@ -6,23 +6,22 @@
 'use strict';
 
 import * as fs from 'fs';
-import { isAbsolute, sep, join } from 'path';
-
 import * as gracefulFs from 'graceful-fs';
-gracefulFs.gracefulify(fs);
-
+import { join, sep } from 'path';
 import * as arrays from 'vs/base/common/arrays';
 import * as objects from 'vs/base/common/objects';
 import * as strings from 'vs/base/common/strings';
 import { PPromise, TPromise } from 'vs/base/common/winjs.base';
-import { FileWalker, Engine as FileSearchEngine } from 'vs/workbench/services/search/node/fileSearch';
+import { compareItemsByScore, IItemAccessor, prepareQuery, ScorerCache } from 'vs/base/parts/quickopen/common/quickOpenScorer';
 import { MAX_FILE_SIZE } from 'vs/platform/files/node/files';
+import { ICachedSearchStats, IProgress } from 'vs/platform/search/common/search';
+import { Engine as FileSearchEngine, FileWalker } from 'vs/workbench/services/search/node/fileSearch';
 import { RipgrepEngine } from 'vs/workbench/services/search/node/ripgrepTextSearch';
 import { Engine as TextSearchEngine } from 'vs/workbench/services/search/node/textSearch';
 import { TextSearchWorkerProvider } from 'vs/workbench/services/search/node/textSearchWorkerProvider';
-import { IRawSearchService, IRawSearch, IRawFileMatch, ISerializedFileMatch, ISerializedSearchProgressItem, ISerializedSearchComplete, ISearchEngine, IFileSearchProgressItem, ITelemetryEvent } from './search';
-import { ICachedSearchStats, IProgress } from 'vs/platform/search/common/search';
-import { compareItemsByScore, IItemAccessor, ScorerCache, prepareQuery } from 'vs/base/parts/quickopen/common/quickOpenScorer';
+import { IFileSearchProgressItem, IRawFileMatch, IRawSearch, IRawSearchService, ISearchEngine, ISerializedFileMatch, ISerializedSearchComplete, ISerializedSearchProgressItem, ITelemetryEvent } from './search';
+
+gracefulFs.gracefulify(fs);
 
 export class SearchService implements IRawSearchService {
 
@@ -271,10 +270,6 @@ export class SearchService implements IRawSearchService {
 	}
 
 	private getResultsFromCache(cache: Cache, searchValue: string): PPromise<[ISerializedSearchComplete, IRawFileMatch[], CacheStats], IProgress> {
-		if (isAbsolute(searchValue)) {
-			return null; // bypass cache if user looks up an absolute path where matching goes directly on disk
-		}
-
 		// Find cache entries by prefix of search value
 		const hasPathSep = searchValue.indexOf(sep) >= 0;
 		let cached: PPromise<[ISerializedSearchComplete, IRawFileMatch[]], IFileSearchProgressItem>;