Skip to content

Fix outline filtering always selecting last match#50594

Merged
SomeoneToIgnore merged 8 commits intozed-industries:mainfrom
OmChillure:fix-outline-filtering-always-select-last-match
Mar 16, 2026
Merged

Fix outline filtering always selecting last match#50594
SomeoneToIgnore merged 8 commits intozed-industries:mainfrom
OmChillure:fix-outline-filtering-always-select-last-match

Conversation

@OmChillure
Copy link
Copy Markdown
Contributor

@OmChillure OmChillure commented Mar 3, 2026

Fixes #29774

When filtering the outline panel, matches with equal fuzzy scores previously defaulted to the last item due to iterator max_by_key semantics. This caused the bottommost match (e.g. C::f) to always be pre-selected regardless of cursor position.

Changes:

  • Select the match nearest to the cursor when scores are tied, using a multi-criteria comparison: score -> cursor containment depth -> proximity to cursor -> earlier index
  • Move outline search off the UI thread (smol::block_on -> async cx.spawn_in) to avoid blocking during filtering
  • Wrap Outline<Anchor> in Arc for cheap cloning into the async task
  • Add match_update_count to discard results from stale queries

Tests :
Adds a regression test: test_outline_filtered_selection_prefers_cursor_proximity_over_last_tie which passes

Video :
Screencast from 2026-03-03 17-01-32.webm

Release Notes:
Fixed the outline filtering always select last match

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Mar 3, 2026
@MrSubidubi MrSubidubi changed the title fix outline filtering always select last match Fix outline filtering always selecting last match Mar 3, 2026
@zed-industries-bot
Copy link
Copy Markdown
Contributor

zed-industries-bot commented Mar 3, 2026 •
edited
Loading

Warnings
⚠️

This PR is missing release notes.

Please add a "Release Notes" section that describes the change:

Release Notes:

- Added/Fixed/Improved ...

If your change is not user-facing, you can use "N/A" for the entry:

Release Notes:

- N/A

Generated by 🚫 dangerJS against a9f16bd

@OmChillure
Copy link
Copy Markdown
Contributor Author

OmChillure commented Mar 6, 2026

@SomeoneToIgnore sir would love your feedback here, do I require any changes or smthg?

Thank you

SomeoneToIgnore
SomeoneToIgnore reviewed Mar 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

I need time to do a context switch and focus, that's mainly it — pinging does not help much with this, if not hurts.
My plans to do this next week, maybe on Monday even.

Without understanding the context much: match_update_count usage seems peculiar as it's not clear why this is needed; re-sorting all outline items each time seems odd too — imagine we deal with editor.rs's outlines, and you do that on sorting on the main thread.

SomeoneToIgnore
SomeoneToIgnore reviewed Mar 9, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

  • Thank you for the fixes to block_on and related.
    I think we'd better rework the code so that all outline item (after matching the fuzzy match or not) are run as cx.executor().background_spawn(async move {... }) task, or at least on the task on the main thread, but asynchronously still, not like what the empty query code path does — this way we'll improve the rendering-related code and stop risking to block it for too long.

  • I have been testing in https://github.com/rust-lang/rust-analyzer/blob/master/crates/ide/src/inlay_hints.rs by placing a caret to the beginning and the end of the file, and have noticed that the implementation selects something that's not on the edge for both cases (as I'd expected based on the code changes)

cursor_weight.mov

Is it something expected or should we have more tests and fixes to the comparison code?

@OmChillure
Copy link
Copy Markdown
Contributor Author

OmChillure commented Mar 10, 2026

Thanks @SomeoneToIgnore for the feeback

Here are some changes which I did :

  • Unified both empty-query and filtered-query paths into a single shared comparator: [score -> cursor containment depth -> proximity to cursor -> earlier index]
  • Made the empty-query path async via cx.spawn_in instead of blocking the render thread
  • Removed redundant match_update_count -> the picker already drops stale tasks automatically
  • Removed dead last_query field and unnecessary query.clone()

@OmChillure
Copy link
Copy Markdown
Contributor Author

OmChillure commented Mar 12, 2026 •
edited
Loading

Hey @SomeoneToIgnore sorry it took some time , here are the comparisons :

1] Vs Code : same selects the first one

Screencast.from.2026-03-12.18-37-11.webm

2] RustRover by Jetbrains :

Screencast.from.2026-03-12.19-09-39.webm

3] Sublime :

Screencast.from.2026-03-12.18-42-14.webm

4] Helix :

Screencast.from.2026-03-12.18-59-10.webm

@OmChillure
Copy link
Copy Markdown
Contributor Author

OmChillure commented Mar 14, 2026 •
edited
Loading

Hello @SomeoneToIgnore , did some changes

Changes:

  • Moved outline search to async via cx.spawn_in - was blocking the UI thread with smol::block_on
  • Outline stored as Arc<Outline> for cheap cloning into the async task
  • Fixed filtered selection bug: max_by_key(score) returns last on ties → replaced with max_by that tiebreaks by earlier index
  • Empty query selection now only considers symbols containing the cursor, picks deepest, falls back to index 0
  • Removed dead fields last_query and match_update_count
  • Added two tests covering empty query and filtered query selection behavior

Note : I have made 1 change beyond the original discussion, based on manual testing across Helix, VS Code, and Zed with varying caret positions and symbol types (functions, structs, nested items).

  • the change is : use of max_by(score).then(earlier_index) rather than a simple return 0. A return 0 would incorrectly select context parent items (score 0) that outline.search() inserts before matched children for hierarchy display - not the actual match. The max_by approach correctly picks the highest-scored real match, with earlier document position winning on ties.

@OmChillure
Copy link
Copy Markdown
Contributor Author

OmChillure commented Mar 14, 2026 •
edited
Loading

Attaching some visuals example that i tested in Helix , Vscode & fixed in Zed respectively.

1] Search inlay with cursor at top
inlay.webm

2] Search inlay with cursor at random position
inlay.webm

3] Search function hash from cursor at random position
hash.webm

4] Search label (random) from random cursor positions
label.webm

SomeoneToIgnore
SomeoneToIgnore approved these changes Mar 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

Thank you for pushing it forward!

Seems that now we do not select the latest element that aggressively, yet something is a bit odd to me in how we do it that I fail to formulate — this is still quite an increment so let's see how more people find this.

@SomeoneToIgnore SomeoneToIgnore enabled auto-merge (squash) March 16, 2026 12:51
@SomeoneToIgnore SomeoneToIgnore merged commit 491ff01 into zed-industries:main Mar 16, 2026
29 checks passed
@zelenenka zelenenka added the guild Pull requests by someone in Zed Guild. NOTE: the label application is automated via github actions label Mar 16, 2026
@SomeoneToIgnore SomeoneToIgnore mentioned this pull request Mar 19, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SomeoneToIgnore SomeoneToIgnore SomeoneToIgnore approved these changes

Assignees

@SomeoneToIgnore SomeoneToIgnore

Labels

cla-signed The user has signed the Contributor License Agreement guild Pull requests by someone in Zed Guild. NOTE: the label application is automated via github actions

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Outline filtering always select the last match at first

4 participants

@OmChillure @zed-industries-bot @SomeoneToIgnore @zelenenka