editor: Honor lsp_results_location in the cmd-click references fallback (#61187)
Follow-up to #59838, implementing what was discussed at the end of #59829: cmd-click navigation now respects `lsp_results_location` when go-to-definition falls back to find-all-references (invited in https://github.com/zed-industries/zed/issues/59829#issuecomment-4989966493: "It would! Feel free to hook that up if you'd like to!"). ## Problem Cmd-clicking a symbol's definition falls back to find-all-references, but the results always open in a multibuffer even with `"lsp_results_location": "picker"`. The hover-link click path calls the editor navigation methods directly, so the action handlers registered by `lsp_locations` never get a chance to intercept. ## Solution - `handle_click_hovered_link`'s fallback now dispatches the `FindAllReferences` action (with `open_results_in: None`, deferring to the global setting) instead of calling the method, so the `lsp_locations` handler can intercept it, or propagate to the editor's built-in handler when the setting is `multi_buffer`, preserving today's behavior exactly. - The plain cmd-click arm of `cmd_click_reveal_task` now runs the definition query via `go_to_definition_of_kind` (no internal references fallback) instead of `go_to_definition`, so the click path has a single fallback decision point: the dispatching one. Without this, the method's baked-in fallback opened a multibuffer before the dispatch could run. - `go_to_definition_of_kind` visibility widened to `pub(crate)` for the call from `hover_links.rs`. Shift/alt click variants (type definition, splits) are untouched. Keyboard invocations were already intercepted and are unchanged. ## Testing - New test `test_cmd_click_fallback_honors_lsp_results_location` in `lsp_locations`, following the module's existing test patterns: fake LSP returning no definition and two references, `lsp_results_location: picker`, simulated cmd-click at the cursor's pixel position, asserts the picker opens. The test fails without this change. - `cargo nextest run -p lsp_locations`: 6/6. - `cargo nextest run -p editor -E 'test(hover) or test(fallback) or test(go_to_definition) or test(references)'`: 54/54. - `cargo fmt` and `./script/clippy` clean. - Verified manually in a release build: with the setting on, cmd-clicking a definition opens the picker; with it off, behavior is unchanged. Per the contributing guidelines' note on AI assistance: this change was developed with heavy AI assistance (Claude Code). I have reviewed and understand the full diff and the reasoning behind each hunk, and I'm the one answering review feedback. Release Notes: - Fixed cmd-click go-to-definition falling back to a references multibuffer even when `lsp_results_location` is set to `picker`.
This commit is contained in:
1 parent
7d29f184f0
commit
731db7357d
3 files changed
+64
-24
No files matched your search
@@ -1,11 +1,10 @@
|
||||
use crate::{
|
||||
Anchor, Editor, EditorSettings, EditorSnapshot, FindAllReferences, GoToDefinition,
|
||||
GoToDefinitionSplit, GoToTypeDefinition, GoToTypeDefinitionSplit, GotoDefinitionKind,
|
||||
HighlightKey, Navigated, PointForPosition, SelectPhase,
|
||||
editor_settings::GoToDefinitionFallback, scroll::ScrollAmount,
|
||||
Anchor, Editor, EditorSettings, EditorSnapshot, FindAllReferences, GoToDefinitionSplit,
|
||||
GoToTypeDefinition, GoToTypeDefinitionSplit, GotoDefinitionKind, HighlightKey, Navigated,
|
||||
PointForPosition, SelectPhase, editor_settings::GoToDefinitionFallback, scroll::ScrollAmount,
|
||||
};
|
||||
use gpui::{
|
||||
App, AsyncWindowContext, Context, Entity, HighlightStyle, Modifiers, Pixels, Task,
|
||||
App, AsyncWindowContext, Context, Entity, Focusable, HighlightStyle, Modifiers, Pixels, Task,
|
||||
UnderlineStyle, Window, px,
|
||||
};
|
||||
use language::{Bias, ToOffset};
|
||||
@@ -207,26 +206,22 @@ impl Editor {
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Editor>,
|
||||
) {
|
||||
let focus_handle = self.focus_handle(cx);
|
||||
let reveal_task = self.cmd_click_reveal_task(point, modifiers, window, cx);
|
||||
cx.spawn_in(window, async move |editor, cx| {
|
||||
cx.spawn_in(window, async move |_, cx| {
|
||||
let definition_revealed = reveal_task.await.log_err().unwrap_or(Navigated::No);
|
||||
let find_references = editor
|
||||
.update_in(cx, |editor, window, cx| {
|
||||
if definition_revealed == Navigated::Yes {
|
||||
return None;
|
||||
}
|
||||
match EditorSettings::get_global(cx).go_to_definition_fallback {
|
||||
GoToDefinitionFallback::None => None,
|
||||
GoToDefinitionFallback::FindAllReferences => {
|
||||
editor.find_all_references(&FindAllReferences::default(), window, cx)
|
||||
}
|
||||
}
|
||||
})
|
||||
.ok()
|
||||
.flatten();
|
||||
if let Some(find_references) = find_references {
|
||||
find_references.await.log_err();
|
||||
if definition_revealed == Navigated::Yes {
|
||||
return;
|
||||
}
|
||||
cx.update(|window, cx| {
|
||||
match EditorSettings::get_global(cx).go_to_definition_fallback {
|
||||
GoToDefinitionFallback::None => {}
|
||||
GoToDefinitionFallback::FindAllReferences => {
|
||||
focus_handle.dispatch_action(&FindAllReferences::default(), window, cx);
|
||||
}
|
||||
}
|
||||
})
|
||||
.ok();
|
||||
})
|
||||
.detach();
|
||||
}
|
||||
@@ -328,7 +323,9 @@ impl Editor {
|
||||
}
|
||||
(true, false) => self.go_to_type_definition(&GoToTypeDefinition, window, cx),
|
||||
(false, true) => self.go_to_definition_split(&GoToDefinitionSplit, window, cx),
|
||||
(false, false) => self.go_to_definition(&GoToDefinition::default(), window, cx),
|
||||
(false, false) => {
|
||||
self.go_to_definition_of_kind(GotoDefinitionKind::Symbol, false, window, cx)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
Task::ready(Ok(Navigated::No))
|
||||
|
||||
@@ -2372,7 +2372,7 @@ impl Editor {
|
||||
})
|
||||
}
|
||||
|
||||
fn go_to_definition_of_kind(
|
||||
pub(crate) fn go_to_definition_of_kind(
|
||||
&mut self,
|
||||
kind: GotoDefinitionKind,
|
||||
split: bool,
|
||||
|
||||
@@ -778,6 +778,7 @@ mod tests {
|
||||
use editor::test::editor_lsp_test_context::EditorLspTestContext;
|
||||
use gpui::TestAppContext;
|
||||
use indoc::indoc;
|
||||
use workspace::Item as _;
|
||||
|
||||
async fn rust_cx(
|
||||
capabilities: lsp::ServerCapabilities,
|
||||
@@ -969,4 +970,46 @@ mod tests {
|
||||
assert_eq!(matches(&mut cx, "zzzz"), 0);
|
||||
assert_eq!(matches(&mut cx, ""), 2);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_cmd_click_fallback_honors_lsp_results_location(cx: &mut TestAppContext) {
|
||||
cx.update(crate::init);
|
||||
let mut cx = rust_cx(
|
||||
lsp::ServerCapabilities {
|
||||
definition_provider: Some(lsp::OneOf::Left(true)),
|
||||
references_provider: Some(lsp::OneOf::Left(true)),
|
||||
..Default::default()
|
||||
},
|
||||
cx,
|
||||
)
|
||||
.await;
|
||||
cx.update(|_window, cx| {
|
||||
cx.update_global::<settings::SettingsStore, _>(|settings, cx| {
|
||||
settings.update_user_settings(cx, |settings| {
|
||||
settings.editor.lsp_results_location = Some(OpenResultsIn::Picker);
|
||||
});
|
||||
});
|
||||
});
|
||||
cx.set_state(SOURCE);
|
||||
cx.lsp
|
||||
.set_request_handler::<lsp::request::GotoDefinition, _, _>(async move |_params, _| {
|
||||
Ok(None)
|
||||
});
|
||||
cx.lsp
|
||||
.set_request_handler::<lsp::request::References, _, _>(async move |params, _| {
|
||||
let uri = params.text_document_position.text_document.uri;
|
||||
Ok(Some(references(uri, &[(1, 8, 11), (2, 14, 17)])))
|
||||
});
|
||||
|
||||
let screen_coord = cx
|
||||
.editor(|editor, _, cx| editor.pixel_position_of_cursor(cx))
|
||||
.unwrap();
|
||||
cx.simulate_click(screen_coord, gpui::Modifiers::secondary_key());
|
||||
cx.run_until_parked();
|
||||
|
||||
assert!(
|
||||
active_picker(&mut cx).is_some(),
|
||||
"the cmd-click go-to-definition fallback should open the references picker when lsp_results_location is picker"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user