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.

Sign up for GitHub

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

support ignored globs & limit in file search & plugin-ext #4428

Merged
merged 2 commits into from
Feb 28, 2019
Merged

support ignored globs & limit in file search & plugin-ext #4428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d954aba
support ignored globs & limit in file search & plugin-ext
Feb 26, 2019
2f4c9d9
update 'exclude' of file search in plugin api
Feb 26, 2019
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 packages/file-search/src/common/file-search-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export namespace FileSearchService {
fuzzyMatch?: boolean
limit?: number
useGitIgnore?: boolean
/** when `undefined`, no excludes will apply, when empty array, default excludes will apply */
defaultIgnorePatterns?: string[]
}
}
28 changes: 23 additions & 5 deletions packages/file-search/src/node/file-search-service-impl.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,23 +81,41 @@ describe('search-service', function () {

describe('search with glob', () => {
it('should support file searches with globs', async () => {
const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1')).toString();
const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1/sub2')).toString();

const matches = await service.find('**/*oo.*', { rootUris: [rootUri] });
expect(matches).to.be.not.undefined;
expect(matches.length).to.eq(3);
expect(matches.length).to.eq(1);
});

it('should support file searches with globs without the prefixed or trailing star (*)', async () => {
const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1')).toString();
const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1/sub2')).toString();

const trailingMatches = await service.find('*oo', { rootUris: [rootUri] });
expect(trailingMatches).to.be.not.undefined;
expect(trailingMatches.length).to.eq(3);
expect(trailingMatches.length).to.eq(1);

const prefixedMatches = await service.find('oo*', { rootUris: [rootUri] });
expect(prefixedMatches).to.be.not.undefined;
expect(prefixedMatches.length).to.eq(3);
expect(prefixedMatches.length).to.eq(1);
});
});

describe('search with ignored patterns', () => {
it('should ignore strings passed through the search options', async () => {
const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1/sub2')).toString();

const matches = await service.find('**/*oo.*', { rootUris: [rootUri], defaultIgnorePatterns: ['foo'] });
expect(matches).to.be.not.undefined;
expect(matches.length).to.eq(0);
});

it('should ignore globs passed through the search options', async () => {
const rootUri = FileUri.create(path.resolve(__dirname, '../../test-resources/subdir1/sub2')).toString();

const matches = await service.find('**/*oo.*', { rootUris: [rootUri], defaultIgnorePatterns: ['*fo*'] });
Copy link
Member

Choose a reason for hiding this comment

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

I tried changing the ignore pattern in the test, also was using rootUris: ['../../test-resources'], and there are some cases that I don't understand:

When I user defaultIgnorePatterns: ['f'] the search for **/*oo.* works.
Same with 'x' and '.' and 't'. Anything that is partially in one of the files work.
'z' doesn't ignore anything, so we get results back which fail the test, so that's good I guess.

Am I just confused?

Copy link
Contributor Author

@elaihau elaihau Feb 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

you are absolutely right. the client can define one or more string or glob in defaultIgnorePatterns. when you put 'z' in defaultIgnorePatterns, no files are matched and the function should return you foo.txt.

when you were using rootUris: ['../../test-resources'], please note that riggrep does not behave consistently with the patterns defined in .gitignore: BurntSushi/ripgrep#829. Jacques and I found even more scenarios where it failed to apply the rules defined in ignore files.
That's why I changed tests to find files from a folder where .gitignore does not exist :)

expect(matches).to.be.not.undefined;
expect(matches.length).to.eq(0);
});
});
});
41 changes: 30 additions & 11 deletions packages/file-search/src/node/file-search-service-impl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,16 @@ export class FileSearchServiceImpl implements FileSearchService {
@inject(RawProcessFactory) protected readonly rawProcessFactory: RawProcessFactory) { }

async find(searchPattern: string, options: FileSearchService.Options, cancellationToken?: CancellationToken): Promise<string[]> {
if (options.defaultIgnorePatterns && options.defaultIgnorePatterns.length === 0) { // default excludes
options.defaultIgnorePatterns.push('.git');
}
const opts = {
fuzzyMatch: true,
limit: Number.MAX_SAFE_INTEGER,
useGitIgnore: true,
defaultIgnorePatterns: [
'^.git$'
],
...options
};
const args: string[] = [
'--files',
'--sort-files',
];
if (!options.useGitIgnore) {
args.push('-uu');
}
args.push(...opts.rootUris.map(r => FileUri.fsPath(r)));
const args: string[] = this.getSearchArgs(opts);

const resultDeferred = new Deferred<string[]>();
try {
Expand Down Expand Up @@ -151,6 +144,32 @@ export class FileSearchServiceImpl implements FileSearchService {
return resultDeferred.promise;
}

private getSearchArgs(options: FileSearchService.Options): string[] {
const args: string[] = [
'--files'
];
if (!options.useGitIgnore) {
args.push('-uu');
}
if (options && options.defaultIgnorePatterns) {
options.defaultIgnorePatterns.filter(p => p !== '')
.forEach(ignore => {
if (!ignore.endsWith('*')) {
ignore = `${ignore}*`;
}
if (!ignore.startsWith('*')) {
ignore = `!*${ignore}`;
} else {
ignore = `!${ignore}`;
}
args.push('--glob');
args.push(ignore);
});
}
args.push(...options.rootUris.map(r => FileUri.fsPath(r)));
return args;
}

private setupCancellation(onCancel: () => void, cancellationToken?: CancellationToken) {
if (cancellationToken) {
if (cancellationToken.isCancellationRequested) {
Expand Down
15 changes: 14 additions & 1 deletion packages/plugin-ext/src/main/browser/workspace-main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,20 @@ export class WorkspaceMainImpl implements WorkspaceMain {

async $startFileSearch(includePattern: string, excludePatternOrDisregardExcludes?: string | false,
maxResults?: number, token?: theia.CancellationToken): Promise<UriComponents[]> {
const uriStrs = await this.fileSearchService.find(includePattern, { rootUris: this.roots.map(r => r.uri) });
const opts: FileSearchService.Options = { rootUris: this.roots.map(r => r.uri) };
if (typeof excludePatternOrDisregardExcludes === 'string') {
if (excludePatternOrDisregardExcludes === '') { // default excludes
opts.defaultIgnorePatterns = [];
} else {
opts.defaultIgnorePatterns = [excludePatternOrDisregardExcludes];
}
} else {
opts.defaultIgnorePatterns = undefined; // no excludes
}
if (typeof maxResults === 'number') {
opts.limit = maxResults;
}
const uriStrs = await this.fileSearchService.find(includePattern, opts);
return uriStrs.map(uriStr => Uri.parse(uriStr));
}

Expand Down
6 changes: 3 additions & 3 deletions packages/plugin-ext/src/plugin/plugin-context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ export function createAPIFactory(
};

const workspace: typeof theia.workspace = {
get rootPath(): string | undefined {
get rootPath(): string | undefined {
return workspaceExt.rootPath;
},
get workspaceFolders(): theia.WorkspaceFolder[] | undefined {
Expand Down Expand Up @@ -417,8 +417,8 @@ export function createAPIFactory(
ignoreDeleteEvents?: boolean): theia.FileSystemWatcher {
return workspaceExt.createFileSystemWatcher(globPattern, ignoreCreateEvents, ignoreChangeEvents, ignoreDeleteEvents);
},
findFiles(include: theia.GlobPattern, exclude?: theia.GlobPattern | undefined, maxResults?: number, token?: CancellationToken): PromiseLike<Uri[]> {
return workspaceExt.findFiles(include, undefined, maxResults, token);
findFiles(include: theia.GlobPattern, exclude?: theia.GlobPattern | null, maxResults?: number, token?: CancellationToken): PromiseLike<Uri[]> {
return workspaceExt.findFiles(include, exclude, maxResults, token);
},
applyEdit(edit: theia.WorkspaceEdit): PromiseLike<boolean> {
return editors.applyWorkspaceEdit(edit);
Expand Down
6 changes: 3 additions & 3 deletions packages/plugin-ext/src/plugin/workspace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class WorkspaceExtImpl implements WorkspaceExt {
});
}

findFiles(include: theia.GlobPattern, exclude?: theia.GlobPattern | undefined, maxResults?: number,
findFiles(include: theia.GlobPattern, exclude?: theia.GlobPattern | null, maxResults?: number,
token: CancellationToken = CancellationToken.None): PromiseLike<URI[]> {
let includePattern: string;
if (include) {
Expand All @@ -127,15 +127,15 @@ export class WorkspaceExtImpl implements WorkspaceExt {

let excludePatternOrDisregardExcludes: string | false;
if (exclude === undefined) {
excludePatternOrDisregardExcludes = false;
excludePatternOrDisregardExcludes = ''; // default excludes
} else if (exclude) {
if (typeof exclude === 'string') {
excludePatternOrDisregardExcludes = exclude;
} else {
excludePatternOrDisregardExcludes = exclude.pattern;
}
} else {
excludePatternOrDisregardExcludes = false;
excludePatternOrDisregardExcludes = false; // no excludes
}

if (token && token.isCancellationRequested) {
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin/src/theia.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4028,7 +4028,7 @@ declare module '@theia/plugin' {
* @return A thenable that resolves to an array of resource identifiers. Will return no results if no
* [workspace folders](#workspace.workspaceFolders) are opened.
*/
export function findFiles(include: GlobPattern, exclude?: GlobPattern | undefined, maxResults?: number, token?: CancellationToken): PromiseLike<Uri[]>;
export function findFiles(include: GlobPattern, exclude?: GlobPattern | null, maxResults?: number, token?: CancellationToken): PromiseLike<Uri[]>;

/**
* Make changes to one or many resources or create, delete, and rename resources as defined by the given
Expand Down