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

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

Merged
merged 2 commits into from
Feb 28, 2019

Conversation

elaihau
Copy link
Contributor

@elaihau elaihau commented Feb 26, 2019

  • in current Theia, passing "ignored globs" or "limit" to plugins-api from plugins does not work, as the data is dropped before reaching the file search service. This change ensures the data goes through and handled by the service.

Signed-off-by: elaihau liang.huang@ericsson.com

  • vscode.workspace.findFiles() supports passing either a GlobPattern, undefined, or null to represent the pattern to ignore, while in theia, passing null is not allowed. This change aligned the theia plugin api with that of vscode.

Signed-off-by: elaihau liang.huang@ericsson.com

Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

LGTM :)

elaihau added 2 commits February 26, 2019 15:19
- in current Theia, passing "ignored globs" or "limit" to plugins-api from plugins does not work, as the data is dropped before reaching the file search service. This change ensures the data goes through and handled by the service.

Signed-off-by: elaihau <liang.huang@ericsson.com>
- vscode.workspace.findFiles() supports passing either a GlobPattern, undefined, or null to represent the pattern to ignore, while in theia, passing null is not allowed. This change aligned the theia pplugin api with that of vscode.

Signed-off-by: elaihau <liang.huang@ericsson.com>
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

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 :)

@vparfonov
Copy link
Contributor

vparfonov commented Feb 28, 2019

@elaihau Thanks, it should fix #4323, you did my work :). I do pretty the same, but your code looks better

@elaihau
Copy link
Contributor Author

elaihau commented Feb 28, 2019

@elaihau Thanks, it should fix #4323, you did my work :). I do pretty the same, but your code looks better

Thank you. I did this to support using some vscode extensions that perform file searches.

@evidolob @benoitf could you please take a look at this change when you get the chance ?

@elaihau elaihau merged commit 7e49f87 into eclipse-theia:master Feb 28, 2019
@elaihau elaihau deleted the search_ignore branch February 28, 2019 13:11
@elaihau
Copy link
Contributor Author

elaihau commented Mar 1, 2019

related to #4216

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants