Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from
Jun 25, 2018
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a23bfb
Fix #51597 - move searching with absolute path queries out of search …
roblourens Jun 24, 2018
1df0be9
#51597 - PR feedback
roblourens Jun 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 33 additions & 34 deletions src/vs/workbench/parts/search/browser/openFileHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
123 changes: 48 additions & 75 deletions src/vs/workbench/services/search/node/fileSearch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

Expand Down Expand Up @@ -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);
Expand Down
19 changes: 7 additions & 12 deletions src/vs/workbench/services/search/node/rawSearchService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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>;
Expand Down