Fix cmd+click find all references fallback not working in Vim mode
Exclude go-to-definition links returned by LSP that points to the current cursor position. This fixes #10392 . Related PR #9243 .
The previous implementation first performs go-to-definition, and if the selected text covers the clicked position, it figures out that it is already clicking on a definition, and should instead look for references.
However, the selected range does not necessarily cover the clicked position after clicking on a definition, as in VIM mode.
After the PR, if cmd+click on definitions, the definition links would be an empty list, so no go-to-definition is performed, and find-all-references is performed instead.
Release Notes:
- Fixed #10392 , now
cmd+clicking to find all references works in vim mode.
We require contributors to sign our Contributor License Agreement, and we don't have @Congyuwang on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.
@cla-bot check
The cla-bot has been summoned, and re-checked this pull request!
If I'm understanding this feature correctly, we had this functionality previously and reverted it due to both issues in the implementation and not being satisfied with the UX.
I think we'll need to evaluate this closely to make sure the behavior is exactly what we want.
Here is the other PR: #6924 And the PR that reverted it: #10094
Apologies if the two are unrelated, they just read as very similar to me.
Not really, this is quite different from #6924. #6924 changes how findAllReferences behaves, which this PR does not touch.
I think this PR only tries to achieve the same thing as #9243 (not reverted), but with a slightly different logic, so that it also works in VIM mode.
@Congyuwang Thanks for this, and sorry for the slow reply, it took a while to get familiar with the codepaths here.
Two changes needed to get this merged:
- Currently we filter links before showing them, however given that clicking on the definition does do something, it should be underlined. I think the best way to do this would be to remove the filter from
show_link_definitionand do it incmd_click_reveal_taskbefore passing the links in to navigate_to_hover_links. - As
exclude_link_to_positionis only used by the hover links code, it should not be defined on Project, but inhover_links.rs. Secondly, as you are taking aModel<Buffer>as the first argument, you don't need to pass the snapshot, instead passcxand then you can get one withlet snapshot = buffer.read(cx).snapshot(cx).
Let me know if that works out, or if there are further complexities I missed. Happy to pair on this if you'd like https://calendly.com/conradirwin/pairing
@ConradIrwin Great. I see what you mean. It seems to work after the changes above, and the definition underlines are back. Also, Project is not touched now.