diff --git a/extensions/search-rg/src/ripgrepFileSearch.ts b/extensions/search-rg/src/ripgrepFileSearch.ts index c97893fb532..eca337d9691 100644 --- a/extensions/search-rg/src/ripgrepFileSearch.ts +++ b/extensions/search-rg/src/ripgrepFileSearch.ts @@ -4,14 +4,13 @@ *--------------------------------------------------------------------------------------------*/ import * as cp from 'child_process'; -import * as path from 'path'; import { Readable } from 'stream'; import { NodeStringDecoder, StringDecoder } from 'string_decoder'; import * as vscode from 'vscode'; import { rgPath } from 'vscode-ripgrep'; import { normalizeNFC, normalizeNFD } from './normalization'; -import { rgErrorMsgForDisplay } from './ripgrepTextSearch'; import { anchorGlob } from './ripgrepHelpers'; +import { rgErrorMsgForDisplay } from './ripgrepTextSearch'; const isMac = process.platform === 'darwin'; @@ -19,12 +18,16 @@ const isMac = process.platform === 'darwin'; const rgDiskPath = rgPath.replace(/\bnode_modules\.asar\b/, 'node_modules.asar.unpacked'); export class RipgrepFileSearchEngine { - private isDone = false; private rgProc: cp.ChildProcess; private killRgProcFn: (code?: number) => void; constructor(private outputChannel: vscode.OutputChannel) { this.killRgProcFn = () => this.rgProc && this.rgProc.kill(); + process.once('exit', this.killRgProcFn); + } + + private dispose() { + process.removeListener('exit', this.killRgProcFn); } provideFileSearchResults(options: vscode.SearchOptions, progress: vscode.Progress, token: vscode.CancellationToken): Thenable { @@ -38,7 +41,7 @@ export class RipgrepFileSearchEngine { return new Promise((resolve, reject) => { let isDone = false; const cancel = () => { - this.isDone = true; + isDone = true; this.rgProc.kill(); }; token.onCancellationRequested(cancel); @@ -53,7 +56,7 @@ export class RipgrepFileSearchEngine { this.outputChannel.appendLine(`rg ${escapedArgs}\n - cwd: ${cwd}\n`); this.rgProc = cp.spawn(rgDiskPath, rgArgs, { cwd }); - process.once('exit', this.killRgProcFn); + this.rgProc.on('error', e => { console.log(e); reject(e); @@ -90,7 +93,6 @@ export class RipgrepFileSearchEngine { }); if (last) { - process.removeListener('exit', this.killRgProcFn); if (isDone) { resolve(); } else { @@ -104,7 +106,12 @@ export class RipgrepFileSearchEngine { } } }); - }); + }).then( + () => this.dispose(), + err => { + this.dispose(); + return Promise.reject(err); + }); } private collectStdout(cmd: cp.ChildProcess, cb: (err: Error, stdout?: string, last?: boolean) => void): void { diff --git a/extensions/search-rg/src/ripgrepTextSearch.ts b/extensions/search-rg/src/ripgrepTextSearch.ts index 3a03509e047..9e87b5aa1cd 100644 --- a/extensions/search-rg/src/ripgrepTextSearch.ts +++ b/extensions/search-rg/src/ripgrepTextSearch.ts @@ -5,17 +5,15 @@ 'use strict'; -import * as vscode from 'vscode'; - -import { EventEmitter } from 'events'; -import * as path from 'path'; -import { StringDecoder, NodeStringDecoder } from 'string_decoder'; - import * as cp from 'child_process'; +import { EventEmitter } from 'events'; +import { NodeStringDecoder, StringDecoder } from 'string_decoder'; +import * as vscode from 'vscode'; import { rgPath } from 'vscode-ripgrep'; -import { start } from 'repl'; import { anchorGlob } from './ripgrepHelpers'; + + // 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'); diff --git a/src/vs/workbench/api/node/extHostSearch.ts b/src/vs/workbench/api/node/extHostSearch.ts index f2666010f65..0f8c774dd73 100644 --- a/src/vs/workbench/api/node/extHostSearch.ts +++ b/src/vs/workbench/api/node/extHostSearch.ts @@ -401,13 +401,15 @@ class TextSearchEngine { return; } - this.resultCount++; - this.collector.add(match, folderIdx); - if (this.resultCount >= this.config.maxResults) { this.isLimitHit = true; this.cancel(); } + + if (!this.isLimitHit) { + this.resultCount++; + this.collector.add(match, folderIdx); + } }; // For each root folder @@ -492,7 +494,8 @@ class TextSearchEngine { includes, useIgnoreFiles: !this.config.disregardIgnoreFiles, followSymlinks: !this.config.ignoreSymlinks, - encoding: this.config.fileEncoding + encoding: this.config.fileEncoding, + maxFileSize: this.config.maxFileSize }; } } @@ -589,7 +592,7 @@ class FileSearchEngine { PPromise.join(folderQueries.map(fq => { return this.searchInFolder(fq).then(null, null, onResult); })).then(() => { - resolve({ isLimitHit: false }); + resolve({ isLimitHit: this.isLimitHit }); }, (errs: Error[]) => { const errMsg = errs .map(err => toErrorMessage(err)) diff --git a/src/vs/workbench/test/electron-browser/api/extHostSearch.test.ts b/src/vs/workbench/test/electron-browser/api/extHostSearch.test.ts index b831fbd49f6..b1b4ee95411 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostSearch.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostSearch.test.ts @@ -6,18 +6,18 @@ import * as assert from 'assert'; import * as path from 'path'; -import * as extfs from 'vs/base/node/extfs'; +import { isPromiseCanceledError } from 'vs/base/common/errors'; +import { dispose } from 'vs/base/common/lifecycle'; +import { joinPath } from 'vs/base/common/resources'; import URI, { UriComponents } from 'vs/base/common/uri'; import { TPromise } from 'vs/base/common/winjs.base'; -import { IRawFileMatch2, IRawSearchQuery, QueryType, ISearchQuery, IPatternInfo, IFileMatch } from 'vs/platform/search/common/search'; +import * as extfs from 'vs/base/node/extfs'; +import { IFileMatch, IPatternInfo, IRawFileMatch2, IRawSearchQuery, ISearchCompleteStats, ISearchQuery, QueryType } from 'vs/platform/search/common/search'; import { MainContext, MainThreadSearchShape } from 'vs/workbench/api/node/extHost.protocol'; import { ExtHostSearch } from 'vs/workbench/api/node/extHostSearch'; +import { Range } from 'vs/workbench/api/node/extHostTypes'; import { TestRPCProtocol } from 'vs/workbench/test/electron-browser/api/testRPCProtocol'; import * as vscode from 'vscode'; -import { dispose } from 'vs/base/common/lifecycle'; -import { isPromiseCanceledError } from 'vs/base/common/errors'; -import { Range } from 'vs/workbench/api/node/extHostTypes'; -import { joinPath } from 'vs/base/common/resources'; let rpcProtocol: TestRPCProtocol; let extHostSearch: ExtHostSearch; @@ -59,7 +59,8 @@ suite('ExtHostSearch', () => { await rpcProtocol.sync(); } - async function runFileSearch(query: IRawSearchQuery, cancel = false): TPromise { + async function runFileSearch(query: IRawSearchQuery, cancel = false): TPromise<{ results: URI[]; stats: ISearchCompleteStats }> { + let stats: ISearchCompleteStats; try { const p = extHostSearch.$provideFileSearchResults(mockMainThreadSearch.lastHandle, 0, query); if (cancel) { @@ -67,7 +68,7 @@ suite('ExtHostSearch', () => { p.cancel(); } - await p; + stats = await p; } catch (err) { if (!isPromiseCanceledError(err)) { await rpcProtocol.sync(); @@ -76,10 +77,14 @@ suite('ExtHostSearch', () => { } await rpcProtocol.sync(); - return (mockMainThreadSearch.results).map(r => URI.revive(r)); + return { + results: (mockMainThreadSearch.results).map(r => URI.revive(r)), + stats + }; } - async function runTextSearch(pattern: IPatternInfo, query: IRawSearchQuery, cancel = false): TPromise { + async function runTextSearch(pattern: IPatternInfo, query: IRawSearchQuery, cancel = false): TPromise<{ results: IFileMatch[], stats: ISearchCompleteStats }> { + let stats: ISearchCompleteStats; try { const p = extHostSearch.$provideTextSearchResults(mockMainThreadSearch.lastHandle, 0, pattern, query); if (cancel) { @@ -87,7 +92,7 @@ suite('ExtHostSearch', () => { p.cancel(); } - await p; + stats = await p; } catch (err) { if (!isPromiseCanceledError(err)) { await rpcProtocol.sync(); @@ -96,12 +101,14 @@ suite('ExtHostSearch', () => { } await rpcProtocol.sync(); - return (mockMainThreadSearch.results).map(r => ({ + const results = (mockMainThreadSearch.results).map(r => ({ ...r, ...{ resource: URI.revive(r.resource) } })); + + return { results, stats }; } setup(() => { @@ -153,7 +160,8 @@ suite('ExtHostSearch', () => { } }); - const results = await runFileSearch(getSimpleQuery()); + const { results, stats } = await runFileSearch(getSimpleQuery()); + assert(!stats.limitHit); assert(!results.length); }); @@ -171,18 +179,12 @@ suite('ExtHostSearch', () => { } }); - const results = await runFileSearch(getSimpleQuery()); + const { results, stats } = await runFileSearch(getSimpleQuery()); + assert(!stats.limitHit); assert.equal(results.length, 3); compareURIs(results, reportedResults); }); - // Sibling clauses - // Extra files - // Max result count - // Absolute/relative logic - // Includes/excludes passed to provider correctly - // Provider misbehaves - test('Search canceled', async () => { let cancelRequested = false; await registerTestSearchProvider({ @@ -198,7 +200,7 @@ suite('ExtHostSearch', () => { } }); - const results = await runFileSearch(getSimpleQuery(), true); + const { results } = await runFileSearch(getSimpleQuery(), true); assert(cancelRequested); assert(!results.length); }); @@ -376,7 +378,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runFileSearch(query); + const { results } = await runFileSearch(query); compareURIs( results, [ @@ -436,7 +438,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runFileSearch(query); + const { results } = await runFileSearch(query); compareURIs( results, [ @@ -479,7 +481,8 @@ suite('ExtHostSearch', () => { ] }; - const results = await runFileSearch(query); + const { results, stats } = await runFileSearch(query); + assert(stats.limitHit, 'Expected to return limitHit'); assert.equal(results.length, 1); compareURIs(results, reportedResults.slice(0, 1)); assert(wasCanceled, 'Expected to be canceled when hitting limit'); @@ -515,12 +518,49 @@ suite('ExtHostSearch', () => { ] }; - const results = await runFileSearch(query); + const { results, stats } = await runFileSearch(query); + assert(stats.limitHit, 'Expected to return limitHit'); assert.equal(results.length, 2); compareURIs(results, reportedResults.slice(0, 2)); assert(wasCanceled, 'Expected to be canceled when hitting limit'); }); + test('provider returns maxResults exactly', async () => { + const reportedResults = [ + joinPath(rootFolderA, 'file1.ts'), + joinPath(rootFolderA, 'file2.ts'), + ]; + + let wasCanceled = false; + await registerTestSearchProvider({ + provideFileSearchResults(options: vscode.FileSearchOptions, progress: vscode.Progress, token: vscode.CancellationToken): Thenable { + reportedResults.forEach(r => progress.report(path.basename(r.fsPath))); + token.onCancellationRequested(() => wasCanceled = true); + + return TPromise.wrap(null); + } + }); + + const query: ISearchQuery = { + type: QueryType.File, + + filePattern: '', + maxResults: 2, + + folderQueries: [ + { + folder: rootFolderA + } + ] + }; + + const { results, stats } = await runFileSearch(query); + assert(!stats.limitHit, 'Expected not to return limitHit'); + assert.equal(results.length, 2); + compareURIs(results, reportedResults); + assert(!wasCanceled, 'Expected not to be canceled when just reaching limit'); + }); + test('multiroot max results', async () => { let cancels = 0; await registerTestSearchProvider({ @@ -557,7 +597,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runFileSearch(query); + const { results } = await runFileSearch(query); assert.equal(results.length, 2); // Don't care which 2 we got assert.equal(cancels, 2, 'Expected all invocations to be canceled when hitting limit'); }); @@ -588,7 +628,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runFileSearch(query); + const { results } = await runFileSearch(query); assert.equal(results.length, 1); compareURIs(results, reportedResults.slice(2)); }); @@ -618,7 +658,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runFileSearch(query); + const { results } = await runFileSearch(query); compareURIs(results, reportedResults); }); @@ -639,7 +679,7 @@ suite('ExtHostSearch', () => { // }); // const queriedFilePath = queriedFile.fsPath; - // const results = await runFileSearch(getSimpleQuery(queriedFilePath)); + // const { results } = await runFileSearch(getSimpleQuery(queriedFilePath)); // assert.equal(results.length, 1); // compareURIs(results, [queriedFile]); // }); @@ -722,7 +762,8 @@ suite('ExtHostSearch', () => { } }); - const results = await runTextSearch(getPattern('foo'), getSimpleQuery()); + const { results, stats } = await runTextSearch(getPattern('foo'), getSimpleQuery()); + assert(!stats.limitHit); assert(!results.length); }); @@ -739,7 +780,8 @@ suite('ExtHostSearch', () => { } }); - const results = await runTextSearch(getPattern('foo'), getSimpleQuery()); + const { results, stats } = await runTextSearch(getPattern('foo'), getSimpleQuery()); + assert(!stats.limitHit); assertResults(results, providedResults); }); @@ -903,7 +945,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runTextSearch(getPattern('foo'), query); + const { results } = await runTextSearch(getPattern('foo'), query); assertResults(results, providedResults.slice(1)); }); @@ -975,7 +1017,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runTextSearch(getPattern('foo'), query); + const { results } = await runTextSearch(getPattern('foo'), query); assertResults(results, [ makeTextResult('folder/fileA.scss'), makeTextResult('folder/file2.css'), @@ -1009,7 +1051,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runTextSearch(getPattern('foo'), query); + const { results } = await runTextSearch(getPattern('foo'), query); assertResults(results, providedResults.slice(1)); }); @@ -1038,7 +1080,8 @@ suite('ExtHostSearch', () => { ] }; - const results = await runTextSearch(getPattern('foo'), query); + const { results, stats } = await runTextSearch(getPattern('foo'), query); + assert(stats.limitHit, 'Expected to return limitHit'); assertResults(results, providedResults.slice(0, 1)); assert(wasCanceled, 'Expected to be canceled'); }); @@ -1069,11 +1112,43 @@ suite('ExtHostSearch', () => { ] }; - const results = await runTextSearch(getPattern('foo'), query); + const { results, stats } = await runTextSearch(getPattern('foo'), query); + assert(stats.limitHit, 'Expected to return limitHit'); assertResults(results, providedResults.slice(0, 2)); assert(wasCanceled, 'Expected to be canceled'); }); + test('provider returns maxResults exactly', async () => { + const providedResults: vscode.TextSearchResult[] = [ + makeTextResult('file1.ts'), + makeTextResult('file2.ts') + ]; + + let wasCanceled = false; + await registerTestSearchProvider({ + provideTextSearchResults(query: vscode.TextSearchQuery, options: vscode.TextSearchOptions, progress: vscode.Progress, token: vscode.CancellationToken): Thenable { + token.onCancellationRequested(() => wasCanceled = true); + providedResults.forEach(r => progress.report(r)); + return TPromise.wrap(null); + } + }); + + const query: ISearchQuery = { + type: QueryType.Text, + + maxResults: 2, + + folderQueries: [ + { folder: rootFolderA } + ] + }; + + const { results, stats } = await runTextSearch(getPattern('foo'), query); + assert(!stats.limitHit, 'Expected not to return limitHit'); + assertResults(results, providedResults); + assert(!wasCanceled, 'Expected not to be canceled'); + }); + test('multiroot max results', async () => { let cancels = 0; await registerTestSearchProvider({ @@ -1101,7 +1176,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runTextSearch(getPattern('foo'), query); + const { results } = await runTextSearch(getPattern('foo'), query); assert.equal(results.length, 2); assert.equal(cancels, 2); }); @@ -1128,7 +1203,7 @@ suite('ExtHostSearch', () => { ] }; - const results = await runTextSearch(getPattern('foo'), query); + const { results } = await runTextSearch(getPattern('foo'), query); assertResults(results, providedResults, fancySchemeFolderA); }); });