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

[search-in-workspace] ensure a stable search-in-workspace result order #5669

Merged
merged 1 commit into from
Jul 10, 2019

Conversation

vince-fugnitto
Copy link
Member

Fixes #4113

Currently, the search-in-workspace result order is not consistent/stable and can lead to different ordering between search results. The fix was to ensure that the tree is properly rendered in order in the onDone callback. The first step is to sort by the root folders and then update it's children by sorting them as well.

Signed-off-by: Vincent Fugnitto [email protected]

@vince-fugnitto vince-fugnitto added bug bugs found in the application search in workspace issues related to the search-in-workspace labels Jul 8, 2019
@vince-fugnitto vince-fugnitto requested review from fangnx and elaihau July 8, 2019 18:41
@vince-fugnitto vince-fugnitto self-assigned this Jul 8, 2019
Copy link

@fangnx fangnx left a comment

Choose a reason for hiding this comment

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

Nice improvement! Works well when I tested it.

@elaihau
Copy link
Contributor

elaihau commented Jul 9, 2019

Tested in a large multi root workspace in my local env (vscode + theia + a few other folders). Performance wise it is still good.

Fixes #4113

Currently, the `search-in-workspace` result order is not consistent/stable
and can lead to different ordering between search results. The fix was to
ensure that the tree is properly rendered in order in the `onDone` callback.
The first step is to sort by the `root folders` and then update it's children
by sorting them as well.

Signed-off-by: Vincent Fugnitto <[email protected]>
@elaihau
Copy link
Contributor

elaihau commented Jul 10, 2019

Tested again in the setting where I found problems yesterday. Now everything looks good.

@vince-fugnitto
Copy link
Member Author

Thanks @elaihau @fangnx !

@vince-fugnitto
Copy link
Member Author

vince-fugnitto commented Jul 10, 2019

I kept testing and it looks like if we reach the limit and re-attempt the same search the results are not stable, (example: search for a)

Screen Shot 2019-07-10 at 8 27 43 AM

I believe this is the #4113 (comment).
Looks like the results we receive from the backend are not always consistent when we reach our limit.

I'm not sure the PR should address the results from ripgrep or simply ensure that whatever result we do receive should be properly ordered.

@vince-fugnitto vince-fugnitto merged commit 021cfc5 into master Jul 10, 2019
@vince-fugnitto vince-fugnitto deleted the GH-4113 branch July 10, 2019 18:34
@elaihau
Copy link
Contributor

elaihau commented Jul 10, 2019

Looks like the results we receive from the backend are not always consistent when we reach our limit.

yes. once it reaches the limit, the backend stops searching.

I'm not sure the PR should address the results from ripgrep or simply ensure that whatever result we do receive should be properly ordered.

i believe that you have in this PR is a good enough solution for the frontend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application search in workspace issues related to the search-in-workspace
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[search in workspace] Unstable ordering of search results
3 participants