Skip to content

Commit

Permalink
Resolve multiple paths for include/exclude pattern
Browse files Browse the repository at this point in the history 
* Addresses review comments

Signed-off-by: Alvaro Sanchez-Leon <[email protected]>
  • Loading branch information
@alvsan09
alvsan09 committed Apr 15, 2021
1 parent 25aa385 commit 3132c25
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 73 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Change Log

## V1.13.0
- [search-in-workspace] Improve include/exclude search in workspace [#9307](https://github.com/eclipse-theia/theia/pull/9307)

<a name="breaking_changes_1.13.0">[Breaking Changes:](#breaking_changes_1.13.0)</a>

- [search-in-workspace] `RipgrepSearchInWorkspaceServer.getArgs` added new parameter `rootPaths: string[]` [#9307](https://github.com/eclipse-theia/theia/pull/9307)

## v1.12.0 - 3/25/2020

[1.12.0 Milestone](https://github.com/eclipse-theia/theia/milestone/17)
Expand Down
46 changes: 23 additions & 23 deletions packages/search-in-workspace/src/node/ripgrep-search-in-workspace-server.slow-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -931,35 +931,35 @@ describe('ripgrep-search-in-workspace-server', function (): void {
});
});

describe('ripgrep-search-in-workspace-server-matchPatternToPathResult', function (): void {
describe('#resolvePatternToPathMap', function (): void {
this.timeout(10000);
it('Resolve pattern to path for filename', function (): void {
it('should resolve pattern to path for filename', function (): void {
const pattern = 'carrots';
matchPatternToPathResult(pattern, path.join(rootDirA, pattern));
checkResolvedPathForPattern(pattern, path.join(rootDirA, pattern));
});

it('Resolve pattern to path for relative filename', function (): void {
it('should resolve pattern to path for relative filename', function (): void {
const filename = 'carrots';
const pattern = `./${filename}`;
matchPatternToPathResult(pattern, path.join(rootDirA, filename));
checkResolvedPathForPattern(pattern, path.join(rootDirA, filename));
});

it('Resolve relative pattern with sub-folders glob', function (): void {
it('should resolve relative pattern with sub-folders glob', function (): void {
const filename = 'carrots';
const pattern = `./${filename}/**`;
matchPatternToPathResult(pattern, path.join(rootDirA, filename));
checkResolvedPathForPattern(pattern, path.join(rootDirA, filename));
});

it('Resolve absolute path pattern', function (): void {
it('should resolve absolute path pattern', function (): void {
const pattern = `${rootDirA}/carrots`;
matchPatternToPathResult(pattern, pattern);
checkResolvedPathForPattern(pattern, pattern);
});
});

describe('ripgrep-search-in-workspace-server-filePatternToGlobs', function (): void {
describe('#patternToGlobCLIArguments', function (): void {
this.timeout(10000);

it('Resolve path to glob - filename', function (): void {
it('should resolve path to glob - filename', function (): void {
[true, false].forEach(excludeFlag => {
const excludePrefix = excludeFlag ? '!' : '';
const filename = 'carrots';
Expand All @@ -968,12 +968,12 @@ describe('ripgrep-search-in-workspace-server-filePatternToGlobs', function (): v
`--glob=${excludePrefix}**/${filename}/*`
];

const actual = ripgrepServer['filePatternToGlobs'](filename, rootDirA, excludeFlag);
const actual = ripgrepServer['patternToGlobCLIArguments'](filename, excludeFlag);
expect(expected).to.have.deep.members(actual);
});
});

it('Resolve path to glob - glob prefixed folder', function (): void {
it('should resolve path to glob - glob prefixed folder', function (): void {
[true, false].forEach(excludeFlag => {
const excludePrefix = excludeFlag ? '!' : '';
const filename = 'carrots';
Expand All @@ -983,12 +983,12 @@ describe('ripgrep-search-in-workspace-server-filePatternToGlobs', function (): v
`--glob=${excludePrefix}**/${filename}/*`
];

const actual = ripgrepServer['filePatternToGlobs'](inputPath, rootDirA, excludeFlag);
const actual = ripgrepServer['patternToGlobCLIArguments'](inputPath, excludeFlag);
expect(expected).to.have.deep.members(actual);
});
});

it('Resolve path to glob - path segment', function (): void {
it('should resolve path to glob - path segment', function (): void {
[true, false].forEach(excludeFlag => {
const excludePrefix = excludeFlag ? '!' : '';
const filename = 'carrots';
Expand All @@ -998,12 +998,12 @@ describe('ripgrep-search-in-workspace-server-filePatternToGlobs', function (): v
`--glob=${excludePrefix}**/${filename}/*`
];

const actual = ripgrepServer['filePatternToGlobs'](inputPath, rootDirA, excludeFlag);
const actual = ripgrepServer['patternToGlobCLIArguments'](inputPath, excludeFlag);
expect(expected).to.have.deep.members(actual);
});
});

it('Resolve path to glob - already a glob', function (): void {
it('should resolve path to glob - already a glob', function (): void {
[true, false].forEach(excludeFlag => {
const excludePrefix = excludeFlag ? '!' : '';
const filename = 'carrots';
Expand All @@ -1012,12 +1012,12 @@ describe('ripgrep-search-in-workspace-server-filePatternToGlobs', function (): v
`--glob=${excludePrefix}**/${filename}/**/*`,
];

const actual = ripgrepServer['filePatternToGlobs'](inputPath, rootDirA, excludeFlag);
const actual = ripgrepServer['patternToGlobCLIArguments'](inputPath, excludeFlag);
expect(expected).to.have.deep.members(actual);
});
});

it('Resolve path to glob - path segment glob suffixed', function (): void {
it('should resolve path to glob - path segment glob suffixed', function (): void {
[true, false].forEach(excludeFlag => {
const excludePrefix = excludeFlag ? '!' : '';
const filename = 'carrots';
Expand All @@ -1026,13 +1026,13 @@ describe('ripgrep-search-in-workspace-server-filePatternToGlobs', function (): v
`--glob=${excludePrefix}**/${filename}/**/*`,
];

const actual = ripgrepServer['filePatternToGlobs'](inputPath, rootDirA, excludeFlag);
const actual = ripgrepServer['patternToGlobCLIArguments'](inputPath, excludeFlag);
expect(expected).to.have.deep.members(actual);
});
});
});

function matchPatternToPathResult(pattern: string, expectedPath: string): void {
const resultMap: Map<string, string> = ripgrepServer['resolvePatternToPathMap']([pattern], [rootDirA]);
expect(resultMap.get(pattern)).equal(expectedPath);
function checkResolvedPathForPattern(pattern: string, expectedPath: string): void {
const resultMap: Map<string, string[]> = ripgrepServer['resolvePatternToPathsMap']([pattern], [rootDirA]);
expect(resultMap.get(pattern)![0]).equal(expectedPath);
}
80 changes: 30 additions & 50 deletions packages/search-in-workspace/src/node/ripgrep-search-in-workspace-server.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,10 @@ function bytesOrTextToString(obj: IRgBytesOrText): string {
}

/**
* Attempts to build an validate an absolute path from a given pattern an a root folder
* Attempts to build an validate an absolute path from a given pattern and a root folder
* If a first pass is not successful a second pass will consider finding it by removing
* a glob suffix '/**', if a valid path is successfully built the suffix can be appended
* back to the result string i.e. depending on the keepSuffix flag.
*
* @param root
* @param pattern
* @param keepSuffix
*/
function findPathInSuffixedPattern(root: string, pattern: string, keepSuffix: boolean): string | undefined {
let suffix = undefined;
Expand All @@ -56,7 +52,7 @@ function findPathInSuffixedPattern(root: string, pattern: string, keepSuffix: bo
return searchPath;
}

// Attempt to find a path in provided pattern but without a glob suffix
// Attempt to find a path in provided pattern but without a glob suffix.
let patternBase = undefined;
({ patternBase, suffix } = stripPatternGlobSuffix(pattern));
if (suffix) {
Expand All @@ -83,7 +79,6 @@ function findPathInPattern(root: string, pattern: string): string | undefined {
* Returns the original pattern if not successful.
*
* Returns the pattern with out the glob stars prefix '/**' or without '\\**'
* @param pattern
*/
function stripPatternGlobSuffix(pattern: string): { patternBase: string, suffix: string | undefined } {
let patternBase: string = pattern;
Expand Down Expand Up @@ -160,12 +155,10 @@ export class RipgrepSearchInWorkspaceServer implements SearchInWorkspaceServer {
const args = new Set<string>();

const appendGlobArgs = (rawPatterns: string[], exclude: boolean) => {
rootPaths.forEach(root => {
for (const rawPattern of rawPatterns) {
if (rawPattern !== '') {
const globArguments = this.filePatternToGlobs(rawPattern, root, exclude);
globArguments.forEach(arg => args.add(arg));
}
rawPatterns.forEach(rawPattern => {
if (rawPattern !== '') {
const globArguments = this.patternToGlobCLIArguments(rawPattern, exclude);
globArguments.forEach(arg => args.add(arg));
}
});
};
Expand Down Expand Up @@ -202,15 +195,10 @@ export class RipgrepSearchInWorkspaceServer implements SearchInWorkspaceServer {
}

/**
* Transforms a file pattern to a glob pattern (unless it's already a glob).
* It then creates corresponding 'ripgrep' CLI parameters,
*
* @param rawPattern
* @param rootFolder
* @param exclude
* Transforms a given file pattern to 'ripgrep' glob CLI arguments.
*/
protected filePatternToGlobs(pattern: string, rootFolder: string, exclude: boolean): string[] {
const prefixPattern = (inputPattern: string): string => {
protected patternToGlobCLIArguments(pattern: string, exclude: boolean): string[] {
const toCLIGlobArgument = (inputPattern: string): string => {
const globCommandArgument = '--glob=';
const excludeChar = exclude ? '!' : '';
const subDirGlobPattern = '**/';
Expand All @@ -222,16 +210,16 @@ export class RipgrepSearchInWorkspaceServer implements SearchInWorkspaceServer {
return `${globCommandArgument}${excludeChar}${updatedPattern}`;
};

const prefixedPattern = prefixPattern(pattern);
const globArgument = toCLIGlobArgument(pattern);

const globs = [prefixedPattern];
if (!prefixedPattern.endsWith('*')) {
// Add a generic glob entry to include files inside a given directory.
const suffix = prefixedPattern.endsWith('/') ? '*' : '/*';
globs.push(`${prefixedPattern}${suffix}`);
const globArgumentsArray = [globArgument];
if (!globArgument.endsWith('*')) {
// Add a generic glob CLI argument entry to include files inside a given directory.
const suffix = globArgument.endsWith('/') ? '*' : '/*';
globArgumentsArray.push(`${globArgument}${suffix}`);
}

return globs;
return globArgumentsArray;
};

/**
Expand All @@ -245,10 +233,6 @@ export class RipgrepSearchInWorkspaceServer implements SearchInWorkspaceServer {
* the search directories to the ones provided as includes.
* Relative paths are allowed, the application will attempt to translate them to valid absolute paths
* based on the applicable search directories.
*
* @param what
* @param rootUris
* @param opts
*/
search(what: string, rootUris: string[], opts?: SearchInWorkspaceOptions): Promise<number> {
// Start the rg process. Use --vimgrep to get one result per
Expand All @@ -271,7 +255,7 @@ export class RipgrepSearchInWorkspaceServer implements SearchInWorkspaceServer {
}
}

const args = [...rgArgs, what].concat(searchPaths);
const args = [...rgArgs, what, ...searchPaths];
const processOptions: RawProcessOptions = {
command: this.rgPath,
args
Expand Down Expand Up @@ -418,19 +402,18 @@ export class RipgrepSearchInWorkspaceServer implements SearchInWorkspaceServer {
/**
* Transform exclude option patterns from relative paths to absolute paths as long
* as the resulting paths get validated.
*
* @param opts
*/
protected adjustExcludePaths(rootPaths: string[], opts: SearchInWorkspaceOptions | undefined): void {
if (!opts || !opts.exclude) {
return;
}

const patternToPathMap: Map<string, string> = this.resolvePatternToPathMap(opts.exclude, rootPaths, true);
const patternToPathsMap: Map<string, string[]> = this.resolvePatternToPathsMap(opts.exclude, rootPaths, true);
const updatedExcludes: string[] = [];
opts.exclude.forEach(pattern => {
const adjustedPattern = patternToPathMap.get(pattern) ? patternToPathMap.get(pattern) as string : pattern;
updatedExcludes.push(adjustedPattern);
const maybePath = patternToPathsMap.get(pattern);
const adjustedPatterns = maybePath ? maybePath : [pattern];
updatedExcludes.push(...adjustedPatterns);
});
opts.exclude = updatedExcludes;
}
Expand All @@ -446,41 +429,38 @@ export class RipgrepSearchInWorkspaceServer implements SearchInWorkspaceServer {
*
* Any pattern that resulted in a valid search path will be removed from the 'include' list as it is
* provided as an equivalent search path instead.
*
* @param rootPaths - Workspace root paths
* @param opts
*/
protected resolveSearchPathsFromIncludes(rootPaths: string[], opts: SearchInWorkspaceOptions | undefined): string[] {
if (!opts || !opts.include) {
return rootPaths;
}

const includesAsPaths = this.resolvePatternToPathMap(opts.include, rootPaths);
const includesAsPaths = this.resolvePatternToPathsMap(opts.include, rootPaths);
const patternPaths = Array.from(includesAsPaths.keys());

// Exclude include file patters that were successfully translated to search paths
opts.include = opts.include.filter(item => !patternPaths.includes(item));
return includesAsPaths.size > 0 ? Array.from(includesAsPaths.values()) : rootPaths;

return includesAsPaths.size > 0 ? [].concat.apply([], Array.from(includesAsPaths.values())) : rootPaths;
}

/**
* Attempts to resolve valid file paths from a given list of patterns.
* The given (e.g. workspace) root paths are used to try resolving relative path patterns to an absolute path.
* The resulting map will include all patterns associated to its equivalent file path.
* The resulting map will include all patterns associated to its equivalent file paths.
* The given patterns that are not successfully mapped to a file system path are not included.
*
* @param rootPaths
* @param patterns
*/
protected resolvePatternToPathMap(patterns: string[], rootPaths: string[], keepSuffix: boolean = false): Map<string, string> {
const patternToPathMap = new Map<string, string>();
protected resolvePatternToPathsMap(patterns: string[], rootPaths: string[], keepSuffix: boolean = false): Map<string, string[]> {
const patternToPathMap = new Map<string, string[]>();

patterns.forEach(pattern => {
rootPaths.forEach(root => {
const foundPath = findPathInSuffixedPattern(root, pattern, keepSuffix);

if (foundPath) {
patternToPathMap.set(pattern, foundPath);
let pathArray = patternToPathMap.get(pattern);
pathArray = pathArray ? [...pathArray, foundPath] : [foundPath];
patternToPathMap.set(pattern, pathArray);
}
});
});
Expand Down

0 comments on commit 3132c25

Please sign in to comment.