Fix #32761 - don't use {} to expand search globs to match globally

This commit is contained in:
Rob Lourens 2017-08-26 17:36:33 -07:00
parent 41f0c5a526
commit 97fc799b6f
2 changed files with 20 additions and 32 deletions

View file

@ -100,14 +100,15 @@ export class QueryBuilder {
const groups = collections.groupBy(segments, const groups = collections.groupBy(segments,
segment => isSearchPath(segment) ? 'searchPaths' : 'exprSegments'); segment => isSearchPath(segment) ? 'searchPaths' : 'exprSegments');
const exprSegments = (groups.exprSegments || []) const expandedExprSegments = (groups.exprSegments || [])
.map(p => { .map(p => {
if (p[0] === '.') { if (p[0] === '.') {
p = '*' + p; // convert ".js" to "*.js" p = '*' + p; // convert ".js" to "*.js"
} }
return toGlobalGlob(p); return expandGlobalGlob(p);
}); });
const exprSegments = arrays.flatten(expandedExprSegments);
const result: ISearchPathsResult = {}; const result: ISearchPathsResult = {};
const searchPaths = this.expandSearchPathPatterns(groups.searchPaths); const searchPaths = this.expandSearchPathPatterns(groups.searchPaths);
@ -299,9 +300,11 @@ function splitGlobPattern(pattern: string): string[] {
} }
/** /**
* Avoid double ** pattern, see https://github.com/Microsoft/vscode/issues/32325 * Note - we used {} here previously but ripgrep can't handle nested {} patterns. See https://github.com/Microsoft/vscode/issues/32761
*/ */
function toGlobalGlob(pattern: string): string { function expandGlobalGlob(pattern: string): string[] {
const globalGlob = strings.format('{**/{0}/**,**/{0}}', pattern); // convert foo to {**/foo/**,**/foo} to cover files and folders return [
return globalGlob.replace(/\*\*([/\\]\*\*)+/g, '**'); `**/${pattern}/**`,
`**/${pattern}`
];
} }

View file

@ -7,6 +7,7 @@
import * as assert from 'assert'; import * as assert from 'assert';
import { IExpression } from 'vs/base/common/glob'; import { IExpression } from 'vs/base/common/glob';
import * as paths from 'vs/base/common/paths'; import * as paths from 'vs/base/common/paths';
import * as arrays from 'vs/base/common/arrays';
import uri from 'vs/base/common/uri'; import uri from 'vs/base/common/uri';
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
@ -216,7 +217,7 @@ suite('QueryBuilder', () => {
folder: ROOT_1_URI folder: ROOT_1_URI
}], }],
type: QueryType.Text, type: QueryType.Text,
excludePattern: patternsToIExpression(globalGlob('foo')), excludePattern: patternsToIExpression(...globalGlob('foo')),
useRipgrep: true useRipgrep: true
}); });
}); });
@ -302,7 +303,7 @@ suite('QueryBuilder', () => {
folderQueries: [{ folderQueries: [{
folder: ROOT_1_URI folder: ROOT_1_URI
}], }],
excludePattern: patternsToIExpression(globalGlob('*.js')), excludePattern: patternsToIExpression(...globalGlob('*.js')),
type: QueryType.Text, type: QueryType.Text,
useRipgrep: true useRipgrep: true
}); });
@ -321,7 +322,7 @@ suite('QueryBuilder', () => {
folderQueries: [{ folderQueries: [{
folder: ROOT_1_URI folder: ROOT_1_URI
}], }],
includePattern: patternsToIExpression(globalGlob('*.txt')), includePattern: patternsToIExpression(...globalGlob('*.txt')),
type: QueryType.Text, type: QueryType.Text,
useRipgrep: true useRipgrep: true
}); });
@ -333,7 +334,7 @@ suite('QueryBuilder', () => {
assert.deepEqual( assert.deepEqual(
queryBuilder.parseSearchPaths(includePattern), queryBuilder.parseSearchPaths(includePattern),
<ISearchPathsResult>{ <ISearchPathsResult>{
pattern: patternsToIExpression(...expectedPatterns.map(globalGlob)) pattern: patternsToIExpression(...arrays.flatten(expectedPatterns.map(globalGlob)))
}, },
includePattern); includePattern);
} }
@ -348,24 +349,6 @@ suite('QueryBuilder', () => {
].forEach(([includePattern, expectedPatterns]) => testSimpleIncludes(<string>includePattern, <string[]>expectedPatterns)); ].forEach(([includePattern, expectedPatterns]) => testSimpleIncludes(<string>includePattern, <string[]>expectedPatterns));
}); });
test('double-star patterns are normalized', () => {
function testLiteralIncludes(includePattern: string, expectedPattern: string): void {
assert.deepEqual(
queryBuilder.parseSearchPaths(includePattern),
<ISearchPathsResult>{
pattern: patternsToIExpression(expectedPattern)
},
includePattern);
}
[
['**/*.*', '{**/*.*/**,**/*.*}'],
['foo/**', '{**/foo/**,**/foo/**}'],
['**/**/foo', '{**/foo/**,**/foo}'],
['**/**', '{**,**}']
].forEach(([includePattern, expectedPattern]) => testLiteralIncludes(includePattern, expectedPattern));
});
function testIncludes(includePattern: string, expectedResult: ISearchPathsResult): void { function testIncludes(includePattern: string, expectedResult: ISearchPathsResult): void {
assertEqualSearchPathResults( assertEqualSearchPathResults(
queryBuilder.parseSearchPaths(includePattern), queryBuilder.parseSearchPaths(includePattern),
@ -389,7 +372,7 @@ suite('QueryBuilder', () => {
fixPath('/foo/bar') + ',' + 'a', fixPath('/foo/bar') + ',' + 'a',
<ISearchPathsResult>{ <ISearchPathsResult>{
searchPaths: [{ searchPath: getUri('/foo/bar') }], searchPaths: [{ searchPath: getUri('/foo/bar') }],
pattern: patternsToIExpression(globalGlob('a')) pattern: patternsToIExpression(...globalGlob('a'))
} }
], ],
[ [
@ -662,9 +645,11 @@ function cleanUndefinedQueryValues(q: any): void {
return q; return q;
} }
function globalGlob(str: string): string { function globalGlob(pattern: string): string[] {
const globalGlob = `{**/${str}/**,**/${str}}`; return [
return globalGlob.replace(/\*\*([/\\]\*\*)+/g, '**'); `**/${pattern}/**`,
`**/${pattern}`
];
} }
function patternsToIExpression(...patterns: string[]): IExpression { function patternsToIExpression(...patterns: string[]): IExpression {