Search include/exclude patterns are not applied to open files - fix #…

…29658

src/vs/base/common/glob.ts

@@ -5,6 +5,7 @@
 'use strict';
 
 import arrays = require('vs/base/common/arrays');
+import objects = require('vs/base/common/objects');
 import strings = require('vs/base/common/strings');
 import paths = require('vs/base/common/paths');
 import { BoundedMap } from 'vs/base/common/map';
@@ -19,6 +20,10 @@ export function getEmptyExpression(): IExpression {
  return Object.create(null);
 }
 
+export function mergeExpressions(...expressions: IExpression[]): IExpression {
+ return objects.assign(getEmptyExpression(), ...expressions.filter(expr => !!expr));
+}
+
 export interface SiblingClause {
  when: string;
 }

src/vs/base/test/node/glob.test.ts

@@ -865,4 +865,23 @@ suite('Glob', () => {
  function nativeSep(slashPath: string): string {
  return slashPath.replace(/\//g, path.sep);
  }
+
+ test('mergeExpressions', () => {
+ // Empty => empty
+ assert.deepEqual(glob.mergeExpressions(), glob.getEmptyExpression());
+
+ // Doesn't modify given expressions
+ const expr1 = { 'a': true };
+ glob.mergeExpressions(expr1, { 'b': true });
+ assert.deepEqual(expr1, { 'a': true });
+
+ // Merges correctly
+ assert.deepEqual(glob.mergeExpressions({ 'a': true }, { 'b': true }), { 'a': true, 'b': true });
+
+ // Ignores null/undefined portions
+ assert.deepEqual(glob.mergeExpressions(undefined, { 'a': true }, null, { 'b': true }), { 'a': true, 'b': true });
+
+ // Later expressions take precedence
+ assert.deepEqual(glob.mergeExpressions({ 'a': true, 'b': false, 'c': true }, { 'a': false, 'b': true }), { 'a': false, 'b': true, 'c': true });
+ });
 });

src/vs/platform/search/common/search.ts

@@ -6,7 +6,6 @@
 
 import { PPromise, TPromise } from 'vs/base/common/winjs.base';
 import uri from 'vs/base/common/uri';
-import * as paths from 'vs/base/common/paths';
 import * as objects from 'vs/base/common/objects';
 import { IExpression } from 'vs/base/common/glob';
 import { IFilesConfiguration } from 'vs/platform/files/common/files';
@@ -175,25 +174,3 @@ export function getExcludes(configuration: ISearchConfiguration): IExpression {
 
  return allExcludes;
 }
-
-export function getMergedExcludes(query: ISearchQuery, absolutePaths?: boolean): IExpression {
- const globalExcludePattern: IExpression = query.excludePattern || {};
-
- return query.folderQueries
- .map(folderQuery => {
- const mergedFolderExclude = objects.assign({}, globalExcludePattern, folderQuery.excludePattern || {});
- return absolutePaths ?
- makeExcludesAbsolute(mergedFolderExclude, folderQuery.folder) :
- mergedFolderExclude;
- });
-}
-
-function makeExcludesAbsolute(excludePattern: IExpression, rootFolder: uri) {
- return Object.keys(excludePattern)
- .reduce((absolutePattern: IExpression, key: string) => {
- const value = excludePattern[key];
- key = paths.join(rootFolder.fsPath, key);
- absolutePattern[key] = value;
- return absolutePattern;
- }, {});
-}

src/vs/workbench/parts/search/common/queryBuilder.ts

@@ -60,11 +60,35 @@ export class QueryBuilder {
  return folderConfig.search.useRipgrep;
  });
 
+ // Filter extraFileResources against global include/exclude patterns - they are already expected to not belong to a workspace
+ let extraFileResources = options.extraFileResources && options.extraFileResources.filter(extraFile => {
+ if (excludePattern && glob.match(excludePattern, extraFile.fsPath)) {
+ return false;
+ }
+
+ if (includePattern && !glob.match(includePattern, extraFile.fsPath)) {
+ return false;
+ }
+
+ // If searchPaths are being used, the extra file must be in a subfolder and match the pattern, if present
+ if (searchPaths) {
+ return searchPaths.every(searchPath => {
+ if (paths.isEqualOrParent(extraFile.fsPath, searchPath.searchPath.fsPath)) {
+ return !searchPath.pattern || glob.match(searchPath.pattern, extraFile.fsPath);
+ } else {
+ return false;
+ }
+ });
+ }
+
+ return true;
+ });
+ extraFileResources = extraFileResources && extraFileResources.length ? extraFileResources : undefined;
 
  return {
  type,
  folderQueries,
- extraFileResources: options.extraFileResources,
+ extraFileResources,
  filePattern: options.filePattern,
  excludePattern,
  includePattern,

src/vs/workbench/parts/search/test/common/queryBuilder.test.ts

@@ -240,6 +240,62 @@ suite('QueryBuilder', () => {
  });
  });
 
+ test('extraFileResources', () => {
+ assertEqualQueries(
+ queryBuilder.text(
+ PATTERN_INFO,
+ [ROOT_1_URI],
+ { extraFileResources: [getUri('/foo/bar.js')] }
+ ),
+ <ISearchQuery>{
+ contentPattern: PATTERN_INFO,
+ folderQueries: [{
+ folder: ROOT_1_URI
+ }],
+ extraFileResources: [getUri('/foo/bar.js')],
+ type: QueryType.Text,
+ useRipgrep: true
+ });
+
+ assertEqualQueries(
+ queryBuilder.text(
+ PATTERN_INFO,
+ [ROOT_1_URI],
+ {
+ extraFileResources: [getUri('/foo/bar.js')],
+ excludePattern: '**/*.js'
+ }
+ ),
+ <ISearchQuery>{
+ contentPattern: PATTERN_INFO,
+ folderQueries: [{
+ folder: ROOT_1_URI
+ }],
+ excludePattern: patternsToIExpression(globalGlob('**/*.js')),
+ type: QueryType.Text,
+ useRipgrep: true
+ });
+
+ assertEqualQueries(
+ queryBuilder.text(
+ PATTERN_INFO,
+ [ROOT_1_URI],
+ {
+ extraFileResources: [getUri('/foo/bar.js')],
+ includePattern: '**/*.txt'
+ }
+ ),
+ <ISearchQuery>{
+ contentPattern: PATTERN_INFO,
+ folderQueries: [{
+ folder: ROOT_1_URI
+ }],
+ includePattern: patternsToIExpression(globalGlob('**/*.txt')),
+ type: QueryType.Text,
+ useRipgrep: true
+ });
+ });
+
  suite('parseSearchPaths', () => {
  test('simple includes', () => {
  function testSimpleIncludes(includePattern: string, expectedPatterns: string[]): void {
@@ -506,12 +562,19 @@ function assertEqualQueries(actual: ISearchQuery, expected: ISearchQuery): void
  };
  };
 
+ // Avoid comparing URI objects, not a good idea
  if (expected.folderQueries) {
  assert.deepEqual(actual.folderQueries.map(folderQueryToCompareObject), expected.folderQueries.map(folderQueryToCompareObject));
  delete actual.folderQueries;
  delete expected.folderQueries;
  }
 
+ if (expected.extraFileResources) {
+ assert.deepEqual(actual.extraFileResources.map(extraFile => extraFile.fsPath), expected.extraFileResources.map(extraFile => extraFile.fsPath));
+ delete expected.extraFileResources;
+ delete actual.extraFileResources;
+ }
+
  actual.includePattern = normalizeExpression(actual.includePattern);
  actual.excludePattern = normalizeExpression(actual.excludePattern);
  cleanUndefinedQueryValues(actual);

src/vs/workbench/services/search/node/searchService.ts

@@ -6,13 +6,12 @@
 
 import { PPromise, TPromise } from 'vs/base/common/winjs.base';
 import uri from 'vs/base/common/uri';
-import glob = require('vs/base/common/glob');
 import objects = require('vs/base/common/objects');
 import scorer = require('vs/base/common/scorer');
 import strings = require('vs/base/common/strings');
 import { getNextTickChannel } from 'vs/base/parts/ipc/common/ipc';
 import { Client } from 'vs/base/parts/ipc/node/ipc.cp';
-import { IProgress, LineMatch, FileMatch, ISearchComplete, ISearchProgressItem, QueryType, IFileMatch, ISearchQuery, ISearchConfiguration, ISearchService, getMergedExcludes } from 'vs/platform/search/common/search';
+import { IProgress, LineMatch, FileMatch, ISearchComplete, ISearchProgressItem, QueryType, IFileMatch, ISearchQuery, ISearchConfiguration, ISearchService } from 'vs/platform/search/common/search';
 import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
@@ -153,42 +152,22 @@ export class SearchService implements ISearchService {
  }
 
  private matches(resource: uri, query: ISearchQuery): boolean {
- const {
- filePattern,
- includePattern,
- } = query;
-
  // file pattern
- if (filePattern) {
+ if (query.filePattern) {
  if (resource.scheme !== 'file') {
  return false; // if we match on file pattern, we have to ignore non file resources
  }
 
- if (!scorer.matches(resource.fsPath, strings.stripWildcards(filePattern).toLowerCase())) {
+ if (!scorer.matches(resource.fsPath, strings.stripWildcards(query.filePattern).toLowerCase())) {
  return false;
  }
  }
 
  // includes
- if (includePattern) {
+ if (query.includePattern) {
  if (resource.scheme !== 'file') {
  return false; // if we match on file patterns, we have to ignore non file resources
  }
-
- if (!glob.match(includePattern, resource.fsPath)) {
- return false;
- }
- }
-
- // excludes
-
- const allFolderExcludes = getMergedExcludes(query, /*absolutePaths=*/true);
- if (resource.scheme !== 'file') {
- return true; // e.g. untitled files can never be excluded with file patterns
- }
-
- if (glob.match(allFolderExcludes, resource.fsPath)) {
- return false;
  }
 
  return true;