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 file searches with globs #4406

Merged
merged 1 commit into from
Feb 26, 2019
Merged

Conversation

elaihau
Copy link
Contributor

@elaihau elaihau commented Feb 22, 2019

  • vscode.workspace.findFiles() requires the search service to support finding files with globs, while the theia service only supports string match at the moment. Since the Theia goal is making the plugin system compatible with vscode extensions, the glob support should be added to theia file search service.

Signed-off-by: elaihau [email protected]

@elaihau
Copy link
Contributor Author

elaihau commented Feb 22, 2019

This is in the vscode.workspace api:

findFiles(include: GlobPattern, exclude?: GlobPattern | null, maxResults?: number, token?: CancellationToken): Thenable<Uri[]>

where an example of GlobPattern is something like "**/*work*": https://code.visualstudio.com/api/references/vscode-api#workspace

With the current theia code, if "**/*work*" is passed to file-search-service, an empty array will most likely be returned, as our service only support string match at the moment.

@elaihau elaihau force-pushed the search-glob branch 2 times, most recently from 5692e70 to c0ac8e2 Compare February 22, 2019 18:46
@kittaakos
Copy link
Contributor

kittaakos commented Feb 25, 2019

Nice addition, but I am a bit puzzled. Perhaps I misunderstood it. Can you please explain the following search results? Why do I see them?

Here, I have expected to see all the package.jsons from the packages folder:
screen shot 2019-02-25 at 09 17 07

I do not know why I see anything from the .git/logs/refs/remotes/origin folder:
screen shot 2019-02-25 at 09 16 38

Is it possible to highlight the matches somehow:
screen shot 2019-02-25 at 09 26 56

@elaihau
Copy link
Contributor Author

elaihau commented Feb 25, 2019

@kittaakos Thank you for the review !

Here, I have expected to see all the package.jsons from the packages folder:

I tested on both gitpod and my local env, and looks like it worked properly when i key-in **/packages/**/package.json:
peek 2019-02-25 06-59

@kittaakos
Copy link
Contributor

tested on both gitpod

Crazy. Thanks for checking! Something is odd either in my Chrome or with GItpod. I had an odd behavior when I was reviewing #4392 too. Can someone else verify this PR?

@elaihau
Copy link
Contributor Author

elaihau commented Feb 25, 2019

I do not know why I see anything from the .git/logs/refs/remotes/origin folder:

This one is interesting. I got what you saw when tried to search two stars: **. But with **/examples/ i don't get the result showed in your screenshot:

peek 2019-02-25 07-32

@elaihau
Copy link
Contributor Author

elaihau commented Feb 25, 2019

Is it possible to highlight the matches somehow

it would be a nice feature to add, but I am not sure how to implement it. can i create a github issue so that i can resolve it separately, after this one gets merged ?

@kittaakos
Copy link
Contributor

can i create a github issue so that i can resolve it separately,

Sure.

@elaihau
Copy link
Contributor Author

elaihau commented Feb 25, 2019

@lmcbout I made the change that you and Marc suggested. Now the users don't have to key-in the trailing star (*).

- vscode.workspace.findFiles() requires the search service to support finding files with globs, while the theia service only supports string match at the moment. Since the Theia goal is making the plugin system compatible with vscode extensions, the glob support should be added to theia file search service.

Signed-off-by: elaihau <[email protected]>
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 overall, although there is something that I don't really understand, which is that in VS Code, the * character seems to be ignored in the file-quick-open widget.

Still in VS Code, say there is a file like ...vscode/src/vs/platform/list/browser/listService.ts
I can type the following in order to access it, and it seems to make no difference:

platform//browser
platform/*/browser
platform/**/browser
pl*atform/***************/brow*****ser***

Back to Theia, IIUC this change was originally to fix the API for plugins, but why does it have an impact on the file search from the quick open menu?

@elaihau
Copy link
Contributor Author

elaihau commented Feb 25, 2019

the * character seems to be ignored in the file-quick-open widget.

You are right. vscode api supports globs, while its frontend ignores the stars (*).

Back to Theia, IIUC this change was originally to fix the API for plugins, but why does it have an impact on the file search from the quick open menu?

theia front end sends whatever the user enters to the backend, and therefore you can use the frontend to test functional changes i made to the backend.

regarding the impact on the file search experience, we had a small group discussion in the office with Jacques, Marc, & Jean. here is what I personally think:

the user can still do whatever they used to do to search the files. The string match and fuzzy match are still performed. what this change offers the user is using globs, which helps the more advanced users to more precisely locate the files they need.

entering stars (*) into the vscode file search is equivalent to using spaces, e.g., j*son is treated as searching with j and son. while theia-this-patch handles this use case, theia-master-branch does not support it. From this perspective, this patch is an enhancement and brings theia a little closer to vscode.

the case that vscode supports and theia-this-patch does not, is searching with spaces, e.g., searching with j son in vscode returns json files, while in theia the user gets nothing.

@marechal-p

@elaihau elaihau merged commit 46fffd6 into eclipse-theia:master Feb 26, 2019
@elaihau elaihau deleted the search-glob branch February 26, 2019 16:36
@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.

3 participants