diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index e10fde467a..fe401d4e9d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7610,36 +7610,52 @@ impl Editor { } } - pub fn go_to_definition(&mut self, _: &GoToDefinition, cx: &mut ViewContext) { - self.go_to_definition_of_kind(GotoDefinitionKind::Symbol, false, cx); + pub fn go_to_definition( + &mut self, + _: &GoToDefinition, + cx: &mut ViewContext, + ) -> Task> { + self.go_to_definition_of_kind(GotoDefinitionKind::Symbol, false, cx) } - pub fn go_to_implementation(&mut self, _: &GoToImplementation, cx: &mut ViewContext) { - self.go_to_definition_of_kind(GotoDefinitionKind::Implementation, false, cx); + pub fn go_to_implementation( + &mut self, + _: &GoToImplementation, + cx: &mut ViewContext, + ) -> Task> { + self.go_to_definition_of_kind(GotoDefinitionKind::Implementation, false, cx) } pub fn go_to_implementation_split( &mut self, _: &GoToImplementationSplit, cx: &mut ViewContext, - ) { - self.go_to_definition_of_kind(GotoDefinitionKind::Implementation, true, cx); + ) -> Task> { + self.go_to_definition_of_kind(GotoDefinitionKind::Implementation, true, cx) } - pub fn go_to_type_definition(&mut self, _: &GoToTypeDefinition, cx: &mut ViewContext) { - self.go_to_definition_of_kind(GotoDefinitionKind::Type, false, cx); + pub fn go_to_type_definition( + &mut self, + _: &GoToTypeDefinition, + cx: &mut ViewContext, + ) -> Task> { + self.go_to_definition_of_kind(GotoDefinitionKind::Type, false, cx) } - pub fn go_to_definition_split(&mut self, _: &GoToDefinitionSplit, cx: &mut ViewContext) { - self.go_to_definition_of_kind(GotoDefinitionKind::Symbol, true, cx); + pub fn go_to_definition_split( + &mut self, + _: &GoToDefinitionSplit, + cx: &mut ViewContext, + ) -> Task> { + self.go_to_definition_of_kind(GotoDefinitionKind::Symbol, true, cx) } pub fn go_to_type_definition_split( &mut self, _: &GoToTypeDefinitionSplit, cx: &mut ViewContext, - ) { - self.go_to_definition_of_kind(GotoDefinitionKind::Type, true, cx); + ) -> Task> { + self.go_to_definition_of_kind(GotoDefinitionKind::Type, true, cx) } fn go_to_definition_of_kind( @@ -7647,16 +7663,16 @@ impl Editor { kind: GotoDefinitionKind, split: bool, cx: &mut ViewContext, - ) { + ) -> Task> { let Some(workspace) = self.workspace() else { - return; + return Task::ready(Ok(false)); }; let buffer = self.buffer.read(cx); let head = self.selections.newest::(cx).head(); let (buffer, head) = if let Some(text_anchor) = buffer.text_anchor_for_position(head, cx) { text_anchor } else { - return; + return Task::ready(Ok(false)); }; let project = workspace.read(cx).project().clone(); @@ -7668,17 +7684,18 @@ impl Editor { cx.spawn(|editor, mut cx| async move { let definitions = definitions.await?; - editor.update(&mut cx, |editor, cx| { - editor.navigate_to_hover_links( - Some(kind), - definitions.into_iter().map(HoverLink::Text).collect(), - split, - cx, - ); - })?; - Ok::<(), anyhow::Error>(()) + let navigated = editor + .update(&mut cx, |editor, cx| { + editor.navigate_to_hover_links( + Some(kind), + definitions.into_iter().map(HoverLink::Text).collect(), + split, + cx, + ) + })? + .await?; + anyhow::Ok(navigated) }) - .detach_and_log_err(cx); } pub fn open_url(&mut self, _: &OpenUrl, cx: &mut ViewContext) { @@ -7707,7 +7724,7 @@ impl Editor { mut definitions: Vec, split: bool, cx: &mut ViewContext, - ) { + ) -> Task> { // If there is one definition, just open it directly if definitions.len() == 1 { let definition = definitions.pop().unwrap(); @@ -7726,7 +7743,7 @@ impl Editor { if let Some(target) = target { editor.update(&mut cx, |editor, cx| { let Some(workspace) = editor.workspace() else { - return; + return false; }; let pane = workspace.read(cx).active_pane().clone(); @@ -7763,12 +7780,12 @@ impl Editor { }); }); } + true }) } else { - Ok(()) + Ok(false) } }) - .detach_and_log_err(cx); } else if !definitions.is_empty() { let replica_id = self.replica_id(cx); cx.spawn(|editor, mut cx| async move { @@ -7817,9 +7834,9 @@ impl Editor { .context("location tasks")?; let Some(workspace) = workspace else { - return Ok(()); + return Ok(false); }; - workspace + let opened = workspace .update(&mut cx, |workspace, cx| { Self::open_locations_in_multibuffer( workspace, locations, replica_id, title, split, cx, @@ -7827,9 +7844,10 @@ impl Editor { }) .ok(); - anyhow::Ok(()) + anyhow::Ok(opened.is_some()) }) - .detach_and_log_err(cx); + } else { + Task::ready(Ok(false)) } } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 55c451dd22..2a4b16f382 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -258,12 +258,28 @@ impl EditorElement { register_action(view, cx, Editor::go_to_prev_diagnostic); register_action(view, cx, Editor::go_to_hunk); register_action(view, cx, Editor::go_to_prev_hunk); - register_action(view, cx, Editor::go_to_definition); - register_action(view, cx, Editor::go_to_definition_split); - register_action(view, cx, Editor::go_to_implementation); - register_action(view, cx, Editor::go_to_implementation_split); - register_action(view, cx, Editor::go_to_type_definition); - register_action(view, cx, Editor::go_to_type_definition_split); + register_action(view, cx, |editor, a, cx| { + editor.go_to_definition(a, cx).detach_and_log_err(cx); + }); + register_action(view, cx, |editor, a, cx| { + editor.go_to_definition_split(a, cx).detach_and_log_err(cx); + }); + register_action(view, cx, |editor, a, cx| { + editor.go_to_implementation(a, cx).detach_and_log_err(cx); + }); + register_action(view, cx, |editor, a, cx| { + editor + .go_to_implementation_split(a, cx) + .detach_and_log_err(cx); + }); + register_action(view, cx, |editor, a, cx| { + editor.go_to_type_definition(a, cx).detach_and_log_err(cx); + }); + register_action(view, cx, |editor, a, cx| { + editor + .go_to_type_definition_split(a, cx) + .detach_and_log_err(cx); + }); register_action(view, cx, Editor::open_url); register_action(view, cx, Editor::fold); register_action(view, cx, Editor::fold_at); diff --git a/crates/editor/src/hover_links.rs b/crates/editor/src/hover_links.rs index 67295df73b..9d2b02385e 100644 --- a/crates/editor/src/hover_links.rs +++ b/crates/editor/src/hover_links.rs @@ -1,7 +1,7 @@ use crate::{ hover_popover::{self, InlayHover}, - Anchor, Editor, EditorSnapshot, GoToDefinition, GoToTypeDefinition, InlayId, PointForPosition, - SelectPhase, + Anchor, Editor, EditorSnapshot, FindAllReferences, GoToDefinition, GoToTypeDefinition, InlayId, + PointForPosition, SelectPhase, }; use gpui::{px, AsyncWindowContext, Model, Modifiers, Task, ViewContext}; use language::{Bias, ToOffset}; @@ -11,9 +11,10 @@ use project::{ HoverBlock, HoverBlockKind, InlayHintLabelPartTooltip, InlayHintTooltip, LocationLink, ResolveState, }; -use std::ops::Range; +use std::{cmp, ops::Range}; +use text::Point; use theme::ActiveTheme as _; -use util::{maybe, TryFutureExt}; +use util::{maybe, ResultExt, TryFutureExt}; #[derive(Debug)] pub struct HoveredLinkState { @@ -131,6 +132,47 @@ impl Editor { modifiers: Modifiers, cx: &mut ViewContext, ) { + let selection_before_revealing = self.selections.newest::(cx); + let multi_buffer_snapshot = self.buffer().read(cx).snapshot(cx); + let before_revealing_head = selection_before_revealing.head(); + let before_revealing_tail = selection_before_revealing.tail(); + let before_revealing = match before_revealing_tail.cmp(&before_revealing_head) { + cmp::Ordering::Equal | cmp::Ordering::Less => { + multi_buffer_snapshot.anchor_after(before_revealing_head) + ..multi_buffer_snapshot.anchor_before(before_revealing_tail) + } + cmp::Ordering::Greater => { + multi_buffer_snapshot.anchor_before(before_revealing_tail) + ..multi_buffer_snapshot.anchor_after(before_revealing_head) + } + }; + drop(multi_buffer_snapshot); + + let reveal_task = self.cmd_click_reveal_task(point, modifiers, cx); + cx.spawn(|editor, mut cx| async move { + let definition_revealed = reveal_task.await.log_err().unwrap_or(false); + let find_references = editor + .update(&mut cx, |editor, cx| { + if definition_revealed && revealed_elsewhere(editor, before_revealing, cx) { + return None; + } + editor.find_all_references(&FindAllReferences, cx) + }) + .ok() + .flatten(); + if let Some(find_references) = find_references { + find_references.await.log_err(); + } + }) + .detach(); + } + + fn cmd_click_reveal_task( + &mut self, + point: PointForPosition, + modifiers: Modifiers, + cx: &mut ViewContext, + ) -> Task> { if let Some(hovered_link_state) = self.hovered_link_state.take() { self.hide_hovered_link(cx); if !hovered_link_state.links.is_empty() { @@ -138,8 +180,12 @@ impl Editor { cx.focus(&self.focus_handle); } - self.navigate_to_hover_links(None, hovered_link_state.links, modifiers.alt, cx); - return; + return self.navigate_to_hover_links( + None, + hovered_link_state.links, + modifiers.alt, + cx, + ); } } @@ -160,10 +206,52 @@ impl Editor { } else { self.go_to_definition(&GoToDefinition, cx) } + } else { + Task::ready(Ok(false)) } } } +fn revealed_elsewhere( + editor: &mut Editor, + before_revealing: Range, + cx: &mut ViewContext<'_, Editor>, +) -> bool { + let multi_buffer_snapshot = editor.buffer().read(cx).snapshot(cx); + + let selection_after_revealing = editor.selections.newest::(cx); + let after_revealing_head = selection_after_revealing.head(); + let after_revealing_tail = selection_after_revealing.tail(); + let after_revealing = match after_revealing_tail.cmp(&after_revealing_head) { + cmp::Ordering::Equal | cmp::Ordering::Less => { + multi_buffer_snapshot.anchor_after(after_revealing_tail) + ..multi_buffer_snapshot.anchor_before(after_revealing_head) + } + cmp::Ordering::Greater => { + multi_buffer_snapshot.anchor_after(after_revealing_head) + ..multi_buffer_snapshot.anchor_before(after_revealing_tail) + } + }; + + let before_intersects_after_range = (before_revealing + .start + .cmp(&after_revealing.start, &multi_buffer_snapshot) + .is_ge() + && before_revealing + .start + .cmp(&after_revealing.end, &multi_buffer_snapshot) + .is_le()) + || (before_revealing + .end + .cmp(&after_revealing.start, &multi_buffer_snapshot) + .is_ge() + && before_revealing + .end + .cmp(&after_revealing.end, &multi_buffer_snapshot) + .is_le()); + !before_intersects_after_range +} + pub fn update_inlay_link_and_hover_points( snapshot: &EditorSnapshot, point_for_position: PointForPosition, @@ -473,10 +561,11 @@ pub fn show_link_definition( )), }; - this.update(&mut cx, |this, cx| { + this.update(&mut cx, |editor, cx| { // Clear any existing highlights - this.clear_highlights::(cx); - let Some(hovered_link_state) = this.hovered_link_state.as_mut() else { + editor.clear_highlights::(cx); + let Some(hovered_link_state) = editor.hovered_link_state.as_mut() else { + editor.hide_hovered_link(cx); return; }; hovered_link_state.preferred_kind = preferred_kind; @@ -485,43 +574,12 @@ pub fn show_link_definition( .and_then(|(symbol_range, _)| symbol_range.clone()); if let Some((symbol_range, definitions)) = result { - hovered_link_state.links = definitions.clone(); + hovered_link_state.links = definitions; - let buffer_snapshot = buffer.read(cx).snapshot(); + let underline_hovered_link = hovered_link_state.links.len() > 0 + || hovered_link_state.symbol_range.is_some(); - // Only show highlight if there exists a definition to jump to that doesn't contain - // the current location. - let any_definition_does_not_contain_current_location = - definitions.iter().any(|definition| { - match &definition { - HoverLink::Text(link) => { - if link.target.buffer == buffer { - let range = &link.target.range; - // Expand range by one character as lsp definition ranges include positions adjacent - // but not contained by the symbol range - let start = buffer_snapshot.clip_offset( - range - .start - .to_offset(&buffer_snapshot) - .saturating_sub(1), - Bias::Left, - ); - let end = buffer_snapshot.clip_offset( - range.end.to_offset(&buffer_snapshot) + 1, - Bias::Right, - ); - let offset = buffer_position.to_offset(&buffer_snapshot); - !(start <= offset && end >= offset) - } else { - true - } - } - HoverLink::InlayHint(_, _) => true, - HoverLink::Url(_) => true, - } - }); - - if any_definition_does_not_contain_current_location { + if underline_hovered_link { let style = gpui::HighlightStyle { underline: Some(gpui::UnderlineStyle { thickness: px(1.), @@ -547,15 +605,14 @@ pub fn show_link_definition( }); match highlight_range { - RangeInEditor::Text(text_range) => { - this.highlight_text::(vec![text_range], style, cx) - } - RangeInEditor::Inlay(highlight) => this + RangeInEditor::Text(text_range) => editor + .highlight_text::(vec![text_range], style, cx), + RangeInEditor::Inlay(highlight) => editor .highlight_inlays::(vec![highlight], style, cx), } - } else { - this.hide_hovered_link(cx); } + } else { + editor.hide_hovered_link(cx); } })?; @@ -643,7 +700,10 @@ mod tests { use gpui::Modifiers; use indoc::indoc; use language::language_settings::InlayHintSettings; - use lsp::request::{GotoDefinition, GotoTypeDefinition}; + use lsp::{ + request::{GotoDefinition, GotoTypeDefinition}, + References, + }; use util::assert_set_eq; use workspace::item::Item; @@ -1208,4 +1268,147 @@ mod tests { cx.simulate_click(screen_coord, Modifiers::command()); assert_eq!(cx.opened_url(), Some("https://zed.dev/releases".into())); } + + #[gpui::test] + async fn test_cmd_click_back_and_forth(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorLspTestContext::new_rust(lsp::ServerCapabilities::default(), cx).await; + cx.set_state(indoc! {" + fn test() { + do_work(); + }ˇ + + fn do_work() { + test(); + } + "}); + + // cmd-click on `test` definition and usage, and expect Zed to allow going back and forth, + // because cmd-click first searches for definitions to go to, and then fall backs to symbol usages to go to. + let definition_hover_point = cx.pixel_position(indoc! {" + fn testˇ() { + do_work(); + } + + fn do_work() { + test(); + } + "}); + let definition_display_point = cx.display_point(indoc! {" + fn testˇ() { + do_work(); + } + + fn do_work() { + test(); + } + "}); + let definition_range = cx.lsp_range(indoc! {" + fn «test»() { + do_work(); + } + + fn do_work() { + test(); + } + "}); + let reference_hover_point = cx.pixel_position(indoc! {" + fn test() { + do_work(); + } + + fn do_work() { + testˇ(); + } + "}); + let reference_display_point = cx.display_point(indoc! {" + fn test() { + do_work(); + } + + fn do_work() { + testˇ(); + } + "}); + let reference_range = cx.lsp_range(indoc! {" + fn test() { + do_work(); + } + + fn do_work() { + «test»(); + } + "}); + let expected_uri = cx.buffer_lsp_url.clone(); + cx.lsp + .handle_request::(move |params, _| { + let expected_uri = expected_uri.clone(); + async move { + assert_eq!( + params.text_document_position_params.text_document.uri, + expected_uri + ); + let position = params.text_document_position_params.position; + Ok(Some(lsp::GotoDefinitionResponse::Link( + if position.line == reference_display_point.row() + && position.character == reference_display_point.column() + { + vec![lsp::LocationLink { + origin_selection_range: None, + target_uri: params.text_document_position_params.text_document.uri, + target_range: definition_range, + target_selection_range: definition_range, + }] + } else { + // We cannot navigate to the definition outside of its reference point + Vec::new() + }, + ))) + } + }); + let expected_uri = cx.buffer_lsp_url.clone(); + cx.lsp.handle_request::(move |params, _| { + let expected_uri = expected_uri.clone(); + async move { + assert_eq!( + params.text_document_position.text_document.uri, + expected_uri + ); + let position = params.text_document_position.position; + // Zed should not look for references if GotoDefinition works or returns non-empty result + assert_eq!(position.line, definition_display_point.row()); + assert_eq!(position.character, definition_display_point.column()); + Ok(Some(vec![lsp::Location { + uri: params.text_document_position.text_document.uri, + range: reference_range, + }])) + } + }); + + for _ in 0..5 { + cx.simulate_click(definition_hover_point, Modifiers::command()); + cx.background_executor.run_until_parked(); + cx.assert_editor_state(indoc! {" + fn test() { + do_work(); + } + + fn do_work() { + «testˇ»(); + } + "}); + + cx.simulate_click(reference_hover_point, Modifiers::command()); + cx.background_executor.run_until_parked(); + cx.assert_editor_state(indoc! {" + fn «testˇ»() { + do_work(); + } + + fn do_work() { + test(); + } + "}); + } + } } diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 8cbd83a50c..a76f9d8d4c 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -97,7 +97,7 @@ pub(crate) struct PerformRename { pub push_to_history: bool, } -pub(crate) struct GetDefinition { +pub struct GetDefinition { pub position: PointUtf16, }