From f3654c59e02d573a415dbfd812008f59c9918748 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 17 Sep 2018 17:12:22 -0700 Subject: [PATCH] Fix #55790 - allow extensions to return a hitLimit flag --- extensions/search-rg/src/extension.ts | 8 ++- extensions/search-rg/src/ripgrepTextSearch.ts | 25 ++------ src/vs/vscode.proposed.d.ts | 20 +++++- .../electron-browser/mainThreadWorkspace.ts | 7 ++- src/vs/workbench/api/node/extHost.protocol.ts | 2 +- src/vs/workbench/api/node/extHostSearch.ts | 63 +++++++++---------- src/vs/workbench/api/node/extHostWorkspace.ts | 5 +- .../parts/search/browser/searchView.ts | 2 +- 8 files changed, 66 insertions(+), 66 deletions(-) diff --git a/extensions/search-rg/src/extension.ts b/extensions/search-rg/src/extension.ts index 9c7252667962c..60c2e40815060 100644 --- a/extensions/search-rg/src/extension.ts +++ b/extensions/search-rg/src/extension.ts @@ -26,7 +26,7 @@ class RipgrepSearchProvider implements vscode.FileIndexProvider, vscode.TextSear process.once('exit', () => this.dispose()); } - provideTextSearchResults(query: vscode.TextSearchQuery, options: vscode.TextSearchOptions, progress: vscode.Progress, token: vscode.CancellationToken): Thenable { + provideTextSearchResults(query: vscode.TextSearchQuery, options: vscode.TextSearchOptions, progress: vscode.Progress, token: vscode.CancellationToken): Thenable { const engine = new RipgrepTextSearchEngine(this.outputChannel); return this.withEngine(engine, () => engine.provideTextSearchResults(query, options, progress, token)); } @@ -43,10 +43,12 @@ class RipgrepSearchProvider implements vscode.FileIndexProvider, vscode.TextSear .then(() => results); } - private withEngine(engine: SearchEngine, fn: () => Thenable): Thenable { + private withEngine(engine: SearchEngine, fn: () => Thenable): Thenable { this.inProgress.add(engine); - return fn().then(() => { + return fn().then(result => { this.inProgress.delete(engine); + + return result; }); } diff --git a/extensions/search-rg/src/ripgrepTextSearch.ts b/extensions/search-rg/src/ripgrepTextSearch.ts index 6853cec9deb40..e28ba942abfc5 100644 --- a/extensions/search-rg/src/ripgrepTextSearch.ts +++ b/extensions/search-rg/src/ripgrepTextSearch.ts @@ -16,9 +16,6 @@ import { anchorGlob, createTextSearchResult } from './utils'; // If vscode-ripgrep is in an .asar file, then the binary is unpacked. const rgDiskPath = rgPath.replace(/\bnode_modules\.asar\b/, 'node_modules.asar.unpacked'); -// TODO@roblou move to SearchService -const MAX_TEXT_RESULTS = 10000; - export class RipgrepTextSearchEngine { private isDone = false; private rgProc: cp.ChildProcess; @@ -39,7 +36,7 @@ export class RipgrepTextSearchEngine { } } - provideTextSearchResults(query: vscode.TextSearchQuery, options: vscode.TextSearchOptions, progress: vscode.Progress, token: vscode.CancellationToken): Thenable { + provideTextSearchResults(query: vscode.TextSearchQuery, options: vscode.TextSearchOptions, progress: vscode.Progress, token: vscode.CancellationToken): Thenable { this.outputChannel.appendLine(`provideTextSearchResults ${query.pattern}, ${JSON.stringify({ ...options, ...{ @@ -67,13 +64,15 @@ export class RipgrepTextSearchEngine { }); let gotResult = false; - this.ripgrepParser = new RipgrepParser(MAX_TEXT_RESULTS, cwd, options.previewOptions); + this.ripgrepParser = new RipgrepParser(options.maxResults, cwd, options.previewOptions); this.ripgrepParser.on('result', (match: vscode.TextSearchResult) => { gotResult = true; progress.report(match); }); + let limitHit = false; this.ripgrepParser.on('hitLimit', () => { + limitHit = true; this.cancel(); }); @@ -96,7 +95,7 @@ export class RipgrepTextSearchEngine { this.outputChannel.appendLine(gotResult ? 'Got result from parser' : 'No result from parser'); this.outputChannel.appendLine(''); if (this.isDone) { - resolve(); + resolve({ limitHit }); } else { // Trigger last result this.ripgrepParser.flush(); @@ -105,7 +104,7 @@ export class RipgrepTextSearchEngine { if (stderr && !gotData && (displayMsg = rgErrorMsgForDisplay(stderr))) { reject(new Error(displayMsg)); } else { - resolve(); + resolve({ limitHit }); } } }); @@ -219,16 +218,6 @@ export class RipgrepParser extends EventEmitter { lineText = stripUTF8BOM(lineText); } - // if (!this.currentFile) { - // // When searching a single file and no folderQueries, rg does not print the file line, so create it here - // const singleFile = this.extraSearchFiles[0]; - // if (!singleFile) { - // throw new Error('Got match line for unknown file'); - // } - - // this.currentFile = this.getFileUri(singleFile); - // } - let lastMatchEndPos = 0; let matchTextStartPos = -1; @@ -373,8 +362,6 @@ function getRgArgs(query: vscode.TextSearchQuery, options: vscode.TextSearchOpti return args; } -// TODO@roblou organize away - interface RegExpOptions { matchCase?: boolean; wholeWord?: boolean; diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index bddbe9f45dc90..297b38e234731 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -144,6 +144,20 @@ declare module 'vscode' { encoding?: string; } + /** + * Information collected when text search is complete. + */ + export interface TextSearchComplete { + /** + * Whether the search hit the limit on the maximum number of search results. + * `maxResults` on [`TextSearchOptions`](#TextSearchOptions) specifies the max number of results. + * - If exactly that number of matches exist, this should be false. + * - If `maxResults` matches are returned and more exist, this should be true. + * - If search hits an internal limit which is less than `maxResults`, this should be true. + */ + limitHit?: boolean; + } + /** * The parameters of a query for file search. */ @@ -258,7 +272,7 @@ declare module 'vscode' { * @param progress A progress callback that must be invoked for all results. * @param token A cancellation token. */ - provideTextSearchResults(query: TextSearchQuery, options: TextSearchOptions, progress: Progress, token: CancellationToken): Thenable; + provideTextSearchResults(query: TextSearchQuery, options: TextSearchOptions, progress: Progress, token: CancellationToken): Thenable; } /** @@ -354,7 +368,7 @@ declare module 'vscode' { * @param token A token that can be used to signal cancellation to the underlying search engine. * @return A thenable that resolves when the search is complete. */ - export function findTextInFiles(query: TextSearchQuery, callback: (result: TextSearchResult) => void, token?: CancellationToken): Thenable; + export function findTextInFiles(query: TextSearchQuery, callback: (result: TextSearchResult) => void, token?: CancellationToken): Thenable; /** * Search text in files across all [workspace folders](#workspace.workspaceFolders) in the workspace. @@ -364,7 +378,7 @@ declare module 'vscode' { * @param token A token that can be used to signal cancellation to the underlying search engine. * @return A thenable that resolves when the search is complete. */ - export function findTextInFiles(query: TextSearchQuery, options: FindTextInFilesOptions, callback: (result: TextSearchResult) => void, token?: CancellationToken): Thenable; + export function findTextInFiles(query: TextSearchQuery, options: FindTextInFilesOptions, callback: (result: TextSearchResult) => void, token?: CancellationToken): Thenable; } //#endregion diff --git a/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts b/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts index 29a81a2b32d9c..84d463aa91fcb 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts @@ -25,6 +25,7 @@ import { ITextFileService } from 'vs/workbench/services/textfile/common/textfile import { IWorkspaceEditingService } from 'vs/workbench/services/workspace/common/workspaceEditing'; import { ExtHostContext, ExtHostWorkspaceShape, IExtHostContext, MainContext, MainThreadWorkspaceShape } from '../node/extHost.protocol'; import { CancellationTokenSource, CancellationToken } from 'vs/base/common/cancellation'; +import { TextSearchComplete } from 'vscode'; @extHostNamedCustomer(MainContext.MainThreadWorkspace) export class MainThreadWorkspace implements MainThreadWorkspaceShape { @@ -168,7 +169,7 @@ export class MainThreadWorkspace implements MainThreadWorkspaceShape { }); } - $startTextSearch(pattern: IPatternInfo, options: IQueryOptions, requestId: number, token: CancellationToken): Thenable { + $startTextSearch(pattern: IPatternInfo, options: IQueryOptions, requestId: number, token: CancellationToken): Thenable { const workspace = this._contextService.getWorkspace(); const folders = workspace.folders.map(folder => folder.uri); @@ -182,8 +183,8 @@ export class MainThreadWorkspace implements MainThreadWorkspaceShape { }; const search = this._searchService.search(query, token, onProgress).then( - () => { - return null; + result => { + return { limitHit: result.limitHit }; }, err => { if (!isPromiseCanceledError(err)) { diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 6b04f28b0dba3..079fb3a64d81a 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -471,7 +471,7 @@ export interface ExtHostUrlsShape { export interface MainThreadWorkspaceShape extends IDisposable { $startFileSearch(includePattern: string, includeFolder: string, excludePatternOrDisregardExcludes: string | false, maxResults: number, token: CancellationToken): Thenable; - $startTextSearch(query: IPatternInfo, options: IQueryOptions, requestId: number, token: CancellationToken): Thenable; + $startTextSearch(query: IPatternInfo, options: IQueryOptions, requestId: number, token: CancellationToken): Thenable; $checkExists(includes: string[], token: CancellationToken): Thenable; $saveAll(includeUntitled?: boolean): Thenable; $updateWorkspaceFolders(extensionName: string, index: number, deleteCount: number, workspaceFoldersToAdd: { uri: UriComponents, name?: string }[]): Thenable; diff --git a/src/vs/workbench/api/node/extHostSearch.ts b/src/vs/workbench/api/node/extHostSearch.ts index d84497b032b2d..1d63049f374b2 100644 --- a/src/vs/workbench/api/node/extHostSearch.ts +++ b/src/vs/workbench/api/node/extHostSearch.ts @@ -315,11 +315,13 @@ class TextSearchEngine { // For each root folder TPromise.join(folderQueries.map((fq, i) => { return this.searchInFolder(fq, r => onResult(r, i), tokenSource.token); - })).then(() => { + })).then(results => { tokenSource.dispose(); this.collector.flush(); + + const someFolderHitLImit = results.some(result => result && result.limitHit); resolve({ - limitHit: this.isLimitHit, + limitHit: this.isLimitHit || someFolderHitLImit, stats: { type: 'textSearchProvider' } @@ -335,40 +337,33 @@ class TextSearchEngine { }); } - private searchInFolder(folderQuery: IFolderQuery, onResult: (result: vscode.TextSearchResult) => void, token: CancellationToken): TPromise { - return new TPromise((resolve, reject) => { - - const queryTester = new QueryGlobTester(this.config, folderQuery); - const testingPs = []; - const progress = { - report: (result: vscode.TextSearchResult) => { - const hasSibling = folderQuery.folder.scheme === 'file' && glob.hasSiblingPromiseFn(() => { - return this.readdir(path.dirname(result.uri.fsPath)); - }); + private searchInFolder(folderQuery: IFolderQuery, onResult: (result: vscode.TextSearchResult) => void, token: CancellationToken): TPromise { + const queryTester = new QueryGlobTester(this.config, folderQuery); + const testingPs = []; + const progress = { + report: (result: vscode.TextSearchResult) => { + const hasSibling = folderQuery.folder.scheme === 'file' && glob.hasSiblingPromiseFn(() => { + return this.readdir(path.dirname(result.uri.fsPath)); + }); - const relativePath = path.relative(folderQuery.folder.fsPath, result.uri.fsPath); - testingPs.push( - queryTester.includedInQuery(relativePath, path.basename(relativePath), hasSibling) - .then(included => { - if (included) { - onResult(result); - } - })); - } - }; + const relativePath = path.relative(folderQuery.folder.fsPath, result.uri.fsPath); + testingPs.push( + queryTester.includedInQuery(relativePath, path.basename(relativePath), hasSibling) + .then(included => { + if (included) { + onResult(result); + } + })); + } + }; - const searchOptions = this.getSearchOptionsForFolder(folderQuery); - new TPromise(resolve => process.nextTick(resolve)) - .then(() => { - return this.provider.provideTextSearchResults(patternInfoToQuery(this.pattern), searchOptions, progress, token); - }) - .then(() => { - return TPromise.join(testingPs); - }) - .then( - () => resolve(null), - reject); - }); + const searchOptions = this.getSearchOptionsForFolder(folderQuery); + return new TPromise(resolve => process.nextTick(resolve)) + .then(() => this.provider.provideTextSearchResults(patternInfoToQuery(this.pattern), searchOptions, progress, token)) + .then(result => { + return TPromise.join(testingPs) + .then(() => result); + }); } private readdir(dirname: string): TPromise { diff --git a/src/vs/workbench/api/node/extHostWorkspace.ts b/src/vs/workbench/api/node/extHostWorkspace.ts index 26f0f0bf0bb3d..0aca78832455f 100644 --- a/src/vs/workbench/api/node/extHostWorkspace.ts +++ b/src/vs/workbench/api/node/extHostWorkspace.ts @@ -379,7 +379,7 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape { .then(data => Array.isArray(data) ? data.map(URI.revive) : []); } - findTextInFiles(query: vscode.TextSearchQuery, options: vscode.FindTextInFilesOptions, callback: (result: vscode.TextSearchResult) => void, extensionId: string, token: vscode.CancellationToken = CancellationToken.None) { + findTextInFiles(query: vscode.TextSearchQuery, options: vscode.FindTextInFilesOptions, callback: (result: vscode.TextSearchResult) => void, extensionId: string, token: vscode.CancellationToken = CancellationToken.None): Thenable { this._logService.trace(`extHostWorkspace#findTextInFiles: textSearch, extension: ${extensionId}, entryPoint: findTextInFiles`); if (options.previewOptions && options.previewOptions.totalChars <= options.previewOptions.leadingChars) { @@ -432,8 +432,9 @@ export class ExtHostWorkspace implements ExtHostWorkspaceShape { } return this._proxy.$startTextSearch(query, queryOptions, requestId, token).then( - () => { + result => { delete this._activeSearchCallbacks[requestId]; + return result; }, err => { delete this._activeSearchCallbacks[requestId]; diff --git a/src/vs/workbench/parts/search/browser/searchView.ts b/src/vs/workbench/parts/search/browser/searchView.ts index 33fe487483679..010348ef4ad98 100644 --- a/src/vs/workbench/parts/search/browser/searchView.ts +++ b/src/vs/workbench/parts/search/browser/searchView.ts @@ -65,7 +65,7 @@ const $ = dom.$; export class SearchView extends Viewlet implements IViewlet, IPanel { - private static readonly MAX_TEXT_RESULTS = 10000; + private static readonly MAX_TEXT_RESULTS = 1000; private static readonly SHOW_REPLACE_STORAGE_KEY = 'vs.search.show.replace'; private static readonly WIDE_CLASS_NAME = 'wide';