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.

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

Search In Open Editors #107756

Merged
merged 11 commits into from
Jan 22, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/search/browser/searchView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1489,6 +1489,7 @@ export class SearchView extends ViewPane {

this.inputPatternExcludes.setValue('');
this.inputPatternIncludes.setValue('');
this.inputPatternIncludes.setOnlySearchInOpenEditors(false);

this.triggerQueryChange({ preserveFocus: false });
}));
Expand Down
64 changes: 41 additions & 23 deletions src/vs/workbench/contrib/search/common/queryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,20 +151,21 @@ export class QueryBuilder {
};
}

private handleIncludeExclude(pattern: string | string[] | undefined, expandPatterns: boolean | undefined): ISearchPathsInfo {
private handleIncludeExclude(pattern: string | string[] | undefined, expandPatterns: 'strict' | 'loose' | 'none'): ISearchPathsInfo {
if (!pattern) {
return {};
}

pattern = Array.isArray(pattern) ? pattern.map(normalizeSlashes) : normalizeSlashes(pattern);
return expandPatterns ?
this.parseSearchPaths(pattern) :
{ pattern: patternListToIExpression(...(Array.isArray(pattern) ? pattern : [pattern])) };
return expandPatterns === 'none' ?
{ pattern: patternListToIExpression(...(Array.isArray(pattern) ? pattern : [pattern])) } :
this.parseSearchPaths(pattern, expandPatterns === 'strict');
}

private commonQuery(folderResources: (IWorkspaceFolderData | URI)[] = [], options: ICommonQueryBuilderOptions = {}): ICommonQueryProps<uri> {
const includeSearchPathsInfo: ISearchPathsInfo = this.handleIncludeExclude(options.includePattern, options.expandPatterns);
const excludeSearchPathsInfo: ISearchPathsInfo = this.handleIncludeExclude(options.excludePattern, options.expandPatterns);
private commonQuery(folderResources: (IWorkspaceFolderData | URI)[] = [], options: ICommonQueryBuilderOptions = {}, strict?: boolean): ICommonQueryProps<uri> {
const patternExpansionMode = strict ? 'strict' : options.expandPatterns ? 'loose' : 'none';
const includeSearchPathsInfo: ISearchPathsInfo = this.handleIncludeExclude(options.includePattern, patternExpansionMode);
const excludeSearchPathsInfo: ISearchPathsInfo = this.handleIncludeExclude(options.excludePattern, patternExpansionMode);

// Build folderQueries from searchPaths, if given, otherwise folderResources
const includeFolderName = folderResources.length > 1;
Expand All @@ -185,18 +186,35 @@ export class QueryBuilder {
maxResults: options.maxResults
};

// When "onlyOpenEditors" is enabled, filter all opened editors by the existing include/exclude patterns,
// then rerun the query build setting the includes to those remaining editors
if (options.onlyOpenEditors) {
const openEditors = arrays.coalesce(arrays.flatten(this.editorGroupsService.groups.map(group => group.editors.map(editor => editor.resource))));
const openEditorsInQuery = openEditors.filter(editor => pathIncludedInQuery(queryProps, editor.fsPath));
queryProps.extraFileResources = openEditorsInQuery;
queryProps.folderQueries = [];
}
else {
// Filter extraFileResources against global include/exclude patterns - they are already expected to not belong to a workspace
const extraFileResources = options.extraFileResources && options.extraFileResources.filter(extraFile => pathIncludedInQuery(queryProps, extraFile.fsPath));
queryProps.extraFileResources = extraFileResources && extraFileResources.length ? extraFileResources : undefined;
const openEditorIncludes = openEditorsInQuery.map(editor => {
const workspace = this.workspaceContextService.getWorkspaceFolder(editor);
if (workspace) {
const relPath = path.relative(workspace?.uri.fsPath, editor.fsPath);
return includeFolderName ? `./${workspace.name}/${relPath}` : `${relPath}`;
}
else {
return editor.fsPath.replace(/^\//, '');
}
});
return this.commonQuery(folderResources, {
...options,
onlyOpenEditors: false,
includePattern: openEditorIncludes,
excludePattern: openEditorIncludes.length
? options.excludePattern
: '**/*' // when there are no included editors, explicitly exclude all other files
}, true);
}

// Filter extraFileResources against global include/exclude patterns - they are already expected to not belong to a workspace
const extraFileResources = options.extraFileResources && options.extraFileResources.filter(extraFile => pathIncludedInQuery(queryProps, extraFile.fsPath));
queryProps.extraFileResources = extraFileResources && extraFileResources.length ? extraFileResources : undefined;

return queryProps;
}

Expand Down Expand Up @@ -236,11 +254,11 @@ export class QueryBuilder {

/**
* Take the includePattern as seen in the search viewlet, and split into components that look like searchPaths, and
* glob patterns. Glob patterns are expanded from 'foo/bar' to '{foo/bar/**, **\/foo/bar}.
* glob patterns. When `strict` is false, patterns are expanded from 'foo/bar' to '{foo/bar/**, **\/foo/bar}.
*
* Public for test.
*/
parseSearchPaths(pattern: string | string[]): ISearchPathsInfo {
parseSearchPaths(pattern: string | string[], strict = false): ISearchPathsInfo {
const isSearchPath = (segment: string) => {
// A segment is a search path if it is an absolute path or starts with ./, ../, .\, or ..\
return path.isAbsolute(segment) || /^\.\.?([\/\\]|$)/.test(segment);
Expand All @@ -263,15 +281,15 @@ export class QueryBuilder {
.map(s => strings.rtrim(s, '/'))
.map(s => strings.rtrim(s, '\\'))
.map(p => {
if (p[0] === '.') {
if (!strict && p[0] === '.') {
p = '*' + p; // convert ".js" to "*.js"
}

return expandGlobalGlob(p);
return strict ? [p] : expandGlobalGlob(p);
});

const result: ISearchPathsInfo = {};
const searchPaths = this.expandSearchPathPatterns(groups.searchPaths || []);
const searchPaths = this.expandSearchPathPatterns(groups.searchPaths || [], strict);
if (searchPaths && searchPaths.length) {
result.searchPaths = searchPaths;
}
Expand All @@ -294,7 +312,7 @@ export class QueryBuilder {
/**
* Split search paths (./ or ../ or absolute paths in the includePatterns) into absolute paths and globs applied to those paths
*/
private expandSearchPathPatterns(searchPaths: string[]): ISearchPathPattern[] {
private expandSearchPathPatterns(searchPaths: string[], strict: boolean): ISearchPathPattern[] {
if (!searchPaths || !searchPaths.length) {
// No workspace => ignore search paths
return [];
Expand All @@ -314,7 +332,7 @@ export class QueryBuilder {

// Expanded search paths to multiple resolved patterns (with ** and without)
return arrays.flatten(
oneExpanded.map(oneExpandedResult => this.resolveOneSearchPathPattern(oneExpandedResult, globPortion)));
oneExpanded.map(oneExpandedResult => this.resolveOneSearchPathPattern(oneExpandedResult, globPortion, strict)));
}));

const searchPathPatternMap = new Map<string, ISearchPathPattern>();
Expand Down Expand Up @@ -400,7 +418,7 @@ export class QueryBuilder {
return [];
}

private resolveOneSearchPathPattern(oneExpandedResult: IOneSearchPathPattern, globPortion?: string): IOneSearchPathPattern[] {
private resolveOneSearchPathPattern(oneExpandedResult: IOneSearchPathPattern, globPortion: string | undefined, strict: boolean): IOneSearchPathPattern[] {
const pattern = oneExpandedResult.pattern && globPortion ?
`${oneExpandedResult.pattern}/${globPortion}` :
oneExpandedResult.pattern || globPortion;
Expand All @@ -411,7 +429,7 @@ export class QueryBuilder {
pattern
}];

if (pattern && !pattern.endsWith('**')) {
if (!strict && pattern && !pattern.endsWith('**')) {
results.push({
searchPath: oneExpandedResult.searchPath,
pattern: pattern + '/**'
Expand Down
32 changes: 18 additions & 14 deletions src/vs/workbench/services/search/common/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,21 +412,25 @@ export function pathIncludedInQuery(queryProps: ICommonQueryProps<URI>, fsPath:
return false;
}

if (queryProps.includePattern && !glob.match(queryProps.includePattern, fsPath)) {
return false;
}
if (queryProps.includePattern || queryProps.usingSearchPaths) {
if (queryProps.includePattern && glob.match(queryProps.includePattern, fsPath)) {
return true;
}

// If searchPaths are being used, the extra file must be in a subfolder and match the pattern, if present
if (queryProps.usingSearchPaths) {
return !!queryProps.folderQueries && queryProps.folderQueries.every(fq => {
const searchPath = fq.folder.fsPath;
if (extpath.isEqualOrParent(fsPath, searchPath)) {
const relPath = relative(searchPath, fsPath);
return !fq.includePattern || !!glob.match(fq.includePattern, relPath);
} else {
return false;
}
});
// If searchPaths are being used, the extra file must be in a subfolder and match the pattern, if present
if (queryProps.usingSearchPaths) {
return !!queryProps.folderQueries && queryProps.folderQueries.some(fq => {
const searchPath = fq.folder.fsPath;
if (extpath.isEqualOrParent(fsPath, searchPath)) {
const relPath = relative(searchPath, fsPath);
return !fq.includePattern || !!glob.match(fq.includePattern, relPath);
} else {
return false;
}
});
}

return false;
}

return true;
Expand Down
15 changes: 4 additions & 11 deletions src/vs/workbench/services/search/node/ripgrepTextSearchEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ export class RipgrepTextSearchEngine {

const cwd = options.folder.fsPath;

console.log({ query, options, rgArgs, cwd });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait I removed that already, I think you're looking at an old version

Copy link
Member

@roblourens roblourens Jan 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I think the gh notification just takes me to some random new commit which is not necessarily the latest one



const escapedArgs = rgArgs
.map(arg => arg.match(/^-/) ? arg : `'${arg}'`)
.join(' ');
Expand Down Expand Up @@ -385,13 +388,7 @@ function getRgArgs(query: TextSearchQuery, options: TextSearchOptions): string[]

if (otherIncludes && otherIncludes.length) {
const uniqueOthers = new Set<string>();
otherIncludes.forEach(other => {
if (!other.endsWith('/**')) {
other += '/**';
}

uniqueOthers.add(other);
});
otherIncludes.forEach(other => { uniqueOthers.add(other); });

args.push('-g', '!*');
uniqueOthers
Expand Down Expand Up @@ -508,10 +505,6 @@ function getRgArgs(query: TextSearchQuery, options: TextSearchOptions): string[]
*/
export function spreadGlobComponents(globArg: string): string[] {
const components = splitGlobAware(globArg, '/');
if (components[components.length - 1] !== '**') {
components.push('**');
}

return components.map((_, i) => components.slice(0, i + 1).join('/'));
}

Expand Down