maxmustermann/vscode
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fixes and 💄 around maxResults for search providers

Browse source
This commit is contained in:
Rob Lourens 2018-05-26 12:19:40 -07:00
parent 284759e356
commit 4eaf58348b
4 changed files with 141 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
extensions/search-rg/src/ripgrepFileSearch.ts
View file

@ -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<string>, token: vscode.CancellationToken): Thenable<void> {
@ -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 {

12
extensions/search-rg/src/ripgrepTextSearch.ts
View file

@ -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');

13
src/vs/workbench/api/node/extHostSearch.ts
View file

@ -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))

153
src/vs/workbench/test/electron-browser/api/extHostSearch.test.ts
View file

@ -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<URI[]> {
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 (<UriComponents[]>mockMainThreadSearch.results).map(r => URI.revive(r));
return {
results: (<UriComponents[]>mockMainThreadSearch.results).map(r => URI.revive(r)),
stats
};
}
async function runTextSearch(pattern: IPatternInfo, query: IRawSearchQuery, cancel = false): TPromise<IFileMatch[]> {
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 (<IRawFileMatch2[]>mockMainThreadSearch.results).map(r => ({
const results = (<IRawFileMatch2[]>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<string>, token: vscode.CancellationToken): Thenable<void> {
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<vscode.TextSearchResult>, token: vscode.CancellationToken): Thenable<void> {
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);
});
});