Go to reference when there's only one

[wasd96040501]

close #4796 Similar to implementation in navigate_to_definitions Because find all references in rust-analyzer will return locations containing location we passed in, so we need remove it. And I'm not sure is it the correct way to do this(line 7299~7307)

[cla-bot[bot]]

We require contributors to sign our Contributor License Agreement, and we don't have @wasd96040501 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'.

[wasd96040501]

@cla-bot check

[cla-bot[bot]]

The cla-bot has been summoned, and re-checked this pull request!

[wasd96040501]

@SomeoneToIgnore Need help! It seems that "selects the entire Test with the caret" creates a new buffer. What can I do to figure out where this buffer from? So that I can compare where this buffer's from with target to fix this.

[SomeoneToIgnore]

I've added this on top of your PR

diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs
index 6ca7bb6d2..719e1d69d 100644
--- a/crates/editor/src/editor.rs
+++ b/crates/editor/src/editor.rs
@@ -7311,18 +7311,21 @@ impl Editor {
             }
 
             // If there is one reference, just open it directly
-            if locations.len() == 1 {
+            if dbg!(locations.len()) == 1 {
                 let target = locations.pop().unwrap();
 
                 return editor.update(&mut cx, |editor, cx| {
                     let range = target.range.to_offset(target.buffer.read(cx));
                     let range = editor.range_for_match(&range);
 
-                    if Some(&target.buffer) == editor.buffer().read(cx).as_singleton().as_ref() {
+                    if Some(&target.buffer)
+                        == dbg!(editor.buffer().read(cx).as_singleton().as_ref())
+                    {
                         editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                             s.select_ranges([range]);
                         });
                     } else {
+                        dbg!("???????");
                         cx.window_context().defer(move |cx| {
                             let target_editor: View<Self> =
                                 workspace.update(cx, |workspace, cx| {

and did the same set of steps described in the review comment above, receiving this output:

[crates/editor/src/editor.rs:7314] locations.len() = 1
[crates/editor/src/editor.rs:7322] editor.buffer().read(cx).as_singleton().as_ref() = Some(
    Model { entity_id: EntityId(116v1), entity_type: "language::buffer::Buffer" },
)
[crates/editor/src/editor.rs:7314] locations.len() = 2

that last [crates/editor/src/editor.rs:7314] locations.len() = 2 comes right before the things break and open the new multibuffer. This seems to be right, looking at the surrounding code: we never hit the if locations.len() == 1 condition added in this PR and go down to

workspace.update(&mut cx, |workspace, cx| {
    let title = locations
        .first()
        .as_ref()
        .map(|location| {
            let buffer = location.buffer.read(cx);
            format!(
                "References to `{}`",
                buffer
                    .text_for_range(location.range.clone())
                    .collect::<String>()
            )
        })
        .unwrap();
    Self::open_locations_in_multibuffer(
        workspace, locations, replica_id, title, false, cx,
    );
})?;

code which does open the multibuffer.

I have not looked where does that 2 come from, but this is the culprit worth investigating

---

Again, I would strongly advocate for writing a test for that, there's an editor_tests.rs file, consider test_clone as a donor for your new one: https://github.com/zed-industries/zed/blob/939fa0d1ef7a37e51b63730251690bf326e2121a/crates/editor/src/editor_tests.rs#L472

It sets up a buffer with some contents and carets (ˇ symbol, there's multiple of them there, but we need only one), calls for actions:

editor.change_selections(None, cx, |s| s.select_ranges(selection_ranges.clone()));

and does some assertions. We need to call our action and assert that the caret moves back and forth in that test.

Having a test to rerun speeds up things a lot and proves that the feature works as intended.

[normal-user-2]

Hey! Any update on this?

[wasd96040501]

Hey! Any update on this?

Hah, will work on this these days.

[wasd96040501]

@SomeoneToIgnore Have a look! Bug is fixed when caret is at the end of the word. And I added a unit test for find_all_references also.

[hamza72x]

CI failed, btw

https://github.com/zed-industries/zed/actions/runs/8093595834/job/22116562441

[SomeoneToIgnore]

Yes, old branch was not running some clippy checks, fixing.

[SomeoneToIgnore]

https://github.com/zed-industries/zed/pull/8590

[normal-user-2]

Is this reverted? If not: Then it's not working.

https://github.com/zed-industries/zed/assets/123471178/a87654d8-afac-4d1b-9ca8-07f1b6da5617

[normal-user-2]

Zed: v0.125.2 (Zed Preview)
OS: macOS 14.3.1
Memory: 16 GiB
Architecture: aarch64

[SomeoneToIgnore]

It's not reverted, but looks like there are more corner cases to consider. Do you have a project with the example we can reproduce this issue with?

[SomeoneToIgnore]

Hmm, I actually do not see it in Preview despite working for me in Nightly: could be that it's not released yet, let's recheck tomorrow, after a new Preview is released.

[maxbrunsfeld]

Sorry for the delayed response, but I think we need to revert this change. It's too inconsistent to sometimes open a multi-buffer and sometimes jump to a single location.

Also, omitting the current reference from the list, in the case where you open a multi-buffer is very misleading.