editor: Consider mixed hover link kinds when navigating to multibuffer (#35828)

Previously when handling multiple hover links we filtered non-location
links out which may end up with a single location entry only, resulting
in us opening a multi buffer for a single location. This changes the
logic to do the filtering first, then deciding on whether to open a
single buffer or multi buffer.

Closes https://github.com/zed-industries/zed/issues/6730

Release Notes:

- N/A

crates/editor/src/editor.rs

@@ -118,7 +118,7 @@ use hover_links::{HoverLink, HoveredLinkState, InlayHighlight, find_file};
 use hover_popover::{HoverState, hide_hover};
 use indent_guides::ActiveIndentGuidesState;
 use inlay_hint_cache::{InlayHintCache, InlaySplice, InvalidationStrategy};
-use itertools::Itertools;
+use itertools::{Either, Itertools};
 use language::{
     AutoindentMode, BlockCommentConfig, BracketMatch, BracketPair, Buffer, BufferRow,
     BufferSnapshot, Capability, CharClassifier, CharKind, CodeLabel, CursorShape, DiagnosticEntry,
@@ -15666,12 +15666,9 @@ impl Editor {
         };
         let head = self.selections.newest::<usize>(cx).head();
         let buffer = self.buffer.read(cx);
-        let (buffer, head) = if let Some(text_anchor) = buffer.text_anchor_for_position(head, cx) {
+        let Some((buffer, head)) = buffer.text_anchor_for_position(head, cx) else {
-            text_anchor
-        } else {
             return Task::ready(Ok(Navigated::No));
         };
-
         let Some(definitions) = provider.definitions(&buffer, head, kind, cx) else {
             return Task::ready(Ok(Navigated::No));
         };
@@ -15776,62 +15773,109 @@ impl Editor {
     pub(crate) fn navigate_to_hover_links(
         &mut self,
         kind: Option<GotoDefinitionKind>,
-        mut definitions: Vec<HoverLink>,
+        definitions: Vec<HoverLink>,
         split: bool,
         window: &mut Window,
         cx: &mut Context<Editor>,
     ) -> Task<Result<Navigated>> {
-        // If there is one definition, just open it directly
+        // Separate out url and file links, we can only handle one of them at most or an arbitrary number of locations
-        if definitions.len() == 1 {
+        let mut first_url_or_file = None;
-            let definition = definitions.pop().unwrap();
+        let definitions: Vec<_> = definitions
-
+            .into_iter()
-            enum TargetTaskResult {
+            .filter_map(|def| match def {
-                Location(Option<Location>),
+                HoverLink::Text(link) => Some(Task::ready(anyhow::Ok(Some(link.target)))),
-                AlreadyNavigated,
-            }
-
-            let target_task = match definition {
-                HoverLink::Text(link) => {
-                    Task::ready(anyhow::Ok(TargetTaskResult::Location(Some(link.target))))
-                }
                 HoverLink::InlayHint(lsp_location, server_id) => {
                     let computation =
                         self.compute_target_location(lsp_location, server_id, window, cx);
-                    cx.background_spawn(async move {
+                    Some(cx.background_spawn(computation))
-                        let location = computation.await?;
-                        Ok(TargetTaskResult::Location(location))
-                    })
                 }
                 HoverLink::Url(url) => {
-                    cx.open_url(&url);
+                    first_url_or_file = Some(Either::Left(url));
-                    Task::ready(Ok(TargetTaskResult::AlreadyNavigated))
+                    None
                 }
                 HoverLink::File(path) => {
-                    if let Some(workspace) = self.workspace() {
+                    first_url_or_file = Some(Either::Right(path));
-                        cx.spawn_in(window, async move |_, cx| {
+                    None
-                            workspace
-                                .update_in(cx, |workspace, window, cx| {
-                                    workspace.open_resolved_path(path, window, cx)
-                                })?
-                                .await
-                                .map(|_| TargetTaskResult::AlreadyNavigated)
-                        })
-                    } else {
-                        Task::ready(Ok(TargetTaskResult::Location(None)))
-                    }
                 }
-            };
+            })
-            cx.spawn_in(window, async move |editor, cx| {
+            .collect();
-                let target = match target_task.await.context("target resolution task")? {
+
-                    TargetTaskResult::AlreadyNavigated => return Ok(Navigated::Yes),
+        let workspace = self.workspace();
-                    TargetTaskResult::Location(None) => return Ok(Navigated::No),
+
-                    TargetTaskResult::Location(Some(target)) => target,
+        cx.spawn_in(window, async move |editor, acx| {
+            let mut locations: Vec<Location> = future::join_all(definitions)
+                .await
+                .into_iter()
+                .filter_map(|location| location.transpose())
+                .collect::<Result<_>>()
+                .context("location tasks")?;
+
+            if locations.len() > 1 {
+                let Some(workspace) = workspace else {
+                    return Ok(Navigated::No);
                 };
 
-                editor.update_in(cx, |editor, window, cx| {
+                let tab_kind = match kind {
-                    let Some(workspace) = editor.workspace() else {
+                    Some(GotoDefinitionKind::Implementation) => "Implementations",
-                        return Navigated::No;
+                    _ => "Definitions",
-                    };
+                };
+                let title = editor
+                    .update_in(acx, |_, _, cx| {
+                        let origin = locations.first().unwrap();
+                        let buffer = origin.buffer.read(cx);
+                        format!(
+                            "{} for {}",
+                            tab_kind,
+                            buffer
+                                .text_for_range(origin.range.clone())
+                                .collect::<String>()
+                        )
+                    })
+                    .context("buffer title")?;
+
+                let opened = workspace
+                    .update_in(acx, |workspace, window, cx| {
+                        Self::open_locations_in_multibuffer(
+                            workspace,
+                            locations,
+                            title,
+                            split,
+                            MultibufferSelectionMode::First,
+                            window,
+                            cx,
+                        )
+                    })
+                    .is_ok();
+
+                anyhow::Ok(Navigated::from_bool(opened))
+            } else if locations.is_empty() {
+                // If there is one definition, just open it directly
+                match first_url_or_file {
+                    Some(Either::Left(url)) => {
+                        acx.update(|_, cx| cx.open_url(&url))?;
+                        Ok(Navigated::Yes)
+                    }
+                    Some(Either::Right(path)) => {
+                        let Some(workspace) = workspace else {
+                            return Ok(Navigated::No);
+                        };
+
+                        workspace
+                            .update_in(acx, |workspace, window, cx| {
+                                workspace.open_resolved_path(path, window, cx)
+                            })?
+                            .await?;
+                        Ok(Navigated::Yes)
+                    }
+                    None => Ok(Navigated::No),
+                }
+            } else {
+                let Some(workspace) = workspace else {
+                    return Ok(Navigated::No);
+                };
+
+                let target = locations.pop().unwrap();
+                editor.update_in(acx, |editor, window, cx| {
                     let pane = workspace.read(cx).active_pane().clone();
 
                     let range = target.range.to_point(target.buffer.read(cx));
@@ -15872,81 +15916,8 @@ impl Editor {
                     }
                     Navigated::Yes
                 })
-            })
+            }
-        } else if !definitions.is_empty() {
+        })
-            cx.spawn_in(window, async move |editor, cx| {
-                let (title, location_tasks, workspace) = editor
-                    .update_in(cx, |editor, window, cx| {
-                        let tab_kind = match kind {
-                            Some(GotoDefinitionKind::Implementation) => "Implementations",
-                            _ => "Definitions",
-                        };
-                        let title = definitions
-                            .iter()
-                            .find_map(|definition| match definition {
-                                HoverLink::Text(link) => link.origin.as_ref().map(|origin| {
-                                    let buffer = origin.buffer.read(cx);
-                                    format!(
-                                        "{} for {}",
-                                        tab_kind,
-                                        buffer
-                                            .text_for_range(origin.range.clone())
-                                            .collect::<String>()
-                                    )
-                                }),
-                                HoverLink::InlayHint(_, _) => None,
-                                HoverLink::Url(_) => None,
-                                HoverLink::File(_) => None,
-                            })
-                            .unwrap_or(tab_kind.to_string());
-                        let location_tasks = definitions
-                            .into_iter()
-                            .map(|definition| match definition {
-                                HoverLink::Text(link) => Task::ready(Ok(Some(link.target))),
-                                HoverLink::InlayHint(lsp_location, server_id) => editor
-                                    .compute_target_location(lsp_location, server_id, window, cx),
-                                HoverLink::Url(_) => Task::ready(Ok(None)),
-                                HoverLink::File(_) => Task::ready(Ok(None)),
-                            })
-                            .collect::<Vec<_>>();
-                        (title, location_tasks, editor.workspace().clone())
-                    })
-                    .context("location tasks preparation")?;
-
-                let locations: Vec<Location> = future::join_all(location_tasks)
-                    .await
-                    .into_iter()
-                    .filter_map(|location| location.transpose())
-                    .collect::<Result<_>>()
-                    .context("location tasks")?;
-
-                if locations.is_empty() {
-                    return Ok(Navigated::No);
-                }
-
-                let Some(workspace) = workspace else {
-                    return Ok(Navigated::No);
-                };
-
-                let opened = workspace
-                    .update_in(cx, |workspace, window, cx| {
-                        Self::open_locations_in_multibuffer(
-                            workspace,
-                            locations,
-                            title,
-                            split,
-                            MultibufferSelectionMode::First,
-                            window,
-                            cx,
-                        )
-                    })
-                    .ok();
-
-                anyhow::Ok(Navigated::from_bool(opened.is_some()))
-            })
-        } else {
-            Task::ready(Ok(Navigated::No))
-        }
     }
 
     fn compute_target_location(