Preserve selected entry in file_finder

[kshokhin]

Closes #11737

If the query has not changed and entry is still in the matches list keep it selected

Release Notes:

• Fixes #11737

[kshokhin]

Hi @SomeoneToIgnore !

Here's the raw sketch of the solution we've discussed in the related issue

This fixes the problem I've faced during my work

However, if new entries appear above the selected one, it results into some kind of "jumping" anyway(please, see the attached video), which can be quite annoying. Does it look like an issue to you and is it worth to put effort into fixing it if so?

https://github.com/zed-industries/zed/assets/29820589/0cd22d95-9e80-4796-92fd-a4f690fb4aeb

Apart from that, I'm thinking of moving "selected_*" stuff inside the Matches, probably that would allow to keep track of the selected entry during the sorting and some other simplifications, like do not reassign the selected_path if it hasn't changed

[kshokhin]

I have a simple script that touches new files either in the root directory or in the sub-folder(the simplest way I've found to play with entries' scores) every second. I'll try branch switching and folder copying as well.

every selection we can remember the scroll position

No idea how do it at the moment, to be honest :) But I'll have a look what can be done in that regards.

[SomeoneToIgnore]

If that helps, the scrolling part could be approached via UniformListScrollHandle that has to be somewhere in the picker too:

https://github.com/zed-industries/zed/blob/d50d1611b91481008fbfb9d3b15886a3aa0956bb/crates/language_tools/src/syntax_tree_view.rs#L200

and that can get/set the item index scrolled to/to scroll to.

[kshokhin]

Hey, @SomeoneToIgnore !

I've tackled the scrolling part for some time, but still no good solution, yet. I've ended up trying to update the UniformList prepaint logic to keep the relative position of the selected element stable. E.g. if we have 2nd element of the list selected, then a bunch of entries added before it, so it becomes 10th, then render entries starting with the 9th element, so that the selected element still rendered as the 2nd.

Currently I'm thinking of storing the relative index of the selected entry in file_finder, and update UniformListScrollHandle::scroll_to_item method to store the requested relative position as well, then use it in the prepaint logic to calculate the correct scroll offset.

I've also tried adding and removing folder with git during the file search, and the jumping becomes even more obvious. The usecase seems to be rather exaggerated, though, behaviour when copying large folder to the project should be pretty similar

https://github.com/user-attachments/assets/964ddd74-9dd0-48af-93fa-14d2c5c73caf

[SomeoneToIgnore]

Let me know when I should go and check this all, presumably this PR is no longer a draft then?

[kshokhin]

I agree, we could get this part merged first if you find it sufficient, and then work on the scrolling. Marked as ready for review.

I have a version that works like this with the same git switches in the background:

https://github.com/user-attachments/assets/b9000533-dc02-42b0-86f0-74e8ceb16e6a

Which to me looks even better.

I've had to modify the UniformList, though, and I think it's worth to be a separate PR to review the whole approach and decide whether it's necessary at all.

[SomeoneToIgnore]

Sorry, I still have to make another review pass on top and find time for this — I expect to do this early next week.

[SomeoneToIgnore]

Sorry, I am away for this week also, and expect to look at the PR early next week. Please ping anybody else if you want the review faster.

[kshokhin]

Hey, @SomeoneToIgnore !

Hope you're doing well

Any chance that you've had an opportunity to take a look at the PR?

[kshokhin]

I think we are overthinking it, given how close are we to merge this

Maybe, but during this overthinking, it seems like I've finally(after two months, yikes!) understood how this Matches::push_new_matches thing works, and made some refactoring according to this understanding. I've left a bunch of comments, hope they'll be useful.

To sum up:

1. History score is gone. We've used it iff there's no query, so just handling this case explicitly.
2. Building separate collections for new history matches and new search matches, then merge them with the retained old matches.
3. Duplicates in history and search matches are eliminated using the HashMap of history matches. Duplicate in new and old search matches eliminated during the merge.
4. Matches::cmp_matches results are now aligned with the ordering of ProjectPanelOrdMatch
5. No need for custom PartialEq and weird comparisons anymore, though not sure that the diff is now smaller, seems quite the opposite :)
6. Do not do anything on update_matches, if query is empty and it was empty on the last update, since nothing could've changed.

It's definitely worth to review the updates before merging.