Skip to content

Commit

Permalink
Revert PR zed-industries#6924 - go to reference when there's only one (…
Browse files Browse the repository at this point in the history 
…zed-industries#10094)

This PR reverts zed-industries#6924, for the reasons stated in
zed-industries#6924 (comment).

It also fixes an issue were the `find_all_references_task_sources`
wasn't cleaned up in all cases.

Release Notes:

- N/A

---------

Co-authored-by: Marshall <[email protected]>
Co-authored-by: Marshall Bowers <[email protected]>
  • Loading branch information
@maxbrunsfeld @maxdeviant
3 people authored Apr 2, 2024
1 parent 65cde17 commit 7d5048e
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 323 deletions.
99 changes: 22 additions & 77 deletions crates/editor/src/editor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ use ui::{
h_flex, prelude::*, ButtonSize, ButtonStyle, IconButton, IconName, IconSize, ListItem, Popover,
Tooltip,
};
use util::{maybe, post_inc, RangeExt, ResultExt, TryFutureExt};
use util::{defer, maybe, post_inc, RangeExt, ResultExt, TryFutureExt};
use workspace::Toast;
use workspace::{
searchable::SearchEvent, ItemNavHistory, SplitDirection, ViewId, Workspace, WorkspaceId,
Expand Down Expand Up @@ -7853,9 +7853,10 @@ impl Editor {
Bias::Left
},
);

match self
.find_all_references_task_sources
.binary_search_by(|task_anchor| task_anchor.cmp(&head_anchor, &multi_buffer_snapshot))
.binary_search_by(|anchor| anchor.cmp(&head_anchor, &multi_buffer_snapshot))
{
Ok(_) => {
log::info!(
Expand All @@ -7873,66 +7874,27 @@ impl Editor {
let workspace = self.workspace()?;
let project = workspace.read(cx).project().clone();
let references = project.update(cx, |project, cx| project.references(&buffer, head, cx));
let open_task = cx.spawn(|editor, mut cx| async move {
let mut locations = references.await?;
let snapshot = buffer.update(&mut cx, |buffer, _| buffer.snapshot())?;
let head_offset = text::ToOffset::to_offset(&head, &snapshot);

// LSP may return references that contain the item itself we requested `find_all_references` for (eg. rust-analyzer)
// So we will remove it from locations
// If there is only one reference, we will not do this filter cause it may make locations empty
if locations.len() > 1 {
cx.update(|cx| {
locations.retain(|location| {
// fn foo(x : i64) {
// ^
// println!(x);
// }
// It is ok to find reference when caret being at ^ (the end of the word)
// So we turn offset into inclusive to include the end of the word
!location
.range
.to_offset(location.buffer.read(cx))
.to_inclusive()
.contains(&head_offset)
Some(cx.spawn(|editor, mut cx| async move {
let _cleanup = defer({
let mut cx = cx.clone();
move || {
let _ = editor.update(&mut cx, |editor, _| {
if let Ok(i) =
editor
.find_all_references_task_sources
.binary_search_by(|anchor| {
anchor.cmp(&head_anchor, &multi_buffer_snapshot)
})
{
editor.find_all_references_task_sources.remove(i);
}
});
})?;
}
}
});

let locations = references.await?;
if locations.is_empty() {
return Ok(());
}

// If there is one reference, just open it directly
if 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() {
editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
s.select_ranges([range]);
});
} else {
cx.window_context().defer(move |cx| {
let target_editor: View<Self> =
workspace.update(cx, |workspace, cx| {
workspace.open_project_item(
workspace.active_pane().clone(),
target.buffer.clone(),
cx,
)
});
target_editor.update(cx, |target_editor, cx| {
target_editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
s.select_ranges([range]);
})
})
})
}
});
return anyhow::Ok(());
}

workspace.update(&mut cx, |workspace, cx| {
Expand All @@ -7952,24 +7914,7 @@ impl Editor {
Self::open_locations_in_multibuffer(
workspace, locations, replica_id, title, false, cx,
);
})?;

Ok(())
});
Some(cx.spawn(|editor, mut cx| async move {
open_task.await?;
editor.update(&mut cx, |editor, _| {
if let Ok(i) =
editor
.find_all_references_task_sources
.binary_search_by(|task_anchor| {
task_anchor.cmp(&head_anchor, &multi_buffer_snapshot)
})
{
editor.find_all_references_task_sources.remove(i);
}
})?;
anyhow::Ok(())
})
}))
}

Expand Down
99 changes: 0 additions & 99 deletions crates/editor/src/editor_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8498,105 +8498,6 @@ async fn test_document_format_with_prettier(cx: &mut gpui::TestAppContext) {
);
}

#[gpui::test]
async fn test_find_all_references(cx: &mut gpui::TestAppContext) {
init_test(cx, |_| {});

let mut cx = EditorLspTestContext::new_rust(
lsp::ServerCapabilities {
document_formatting_provider: Some(lsp::OneOf::Left(true)),
..Default::default()
},
cx,
)
.await;

cx.set_state(indoc! {"
fn foo(«paramˇ»: i64) {
println!(param);
}
"});

cx.lsp
.handle_request::<lsp::request::References, _, _>(move |_, _| async move {
Ok(Some(vec![
lsp::Location {
uri: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
range: lsp::Range::new(lsp::Position::new(0, 7), lsp::Position::new(0, 12)),
},
lsp::Location {
uri: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
range: lsp::Range::new(lsp::Position::new(1, 13), lsp::Position::new(1, 18)),
},
]))
});

let references = cx
.update_editor(|editor, cx| editor.find_all_references(&FindAllReferences, cx))
.unwrap();

cx.executor().run_until_parked();

cx.executor().start_waiting();
references.await.unwrap();

cx.assert_editor_state(indoc! {"
fn foo(param: i64) {
println!(«paramˇ»);
}
"});

let references = cx
.update_editor(|editor, cx| editor.find_all_references(&FindAllReferences, cx))
.unwrap();

cx.executor().run_until_parked();

cx.executor().start_waiting();
references.await.unwrap();

cx.assert_editor_state(indoc! {"
fn foo(«paramˇ»: i64) {
println!(param);
}
"});

cx.set_state(indoc! {"
fn foo(param: i64) {
let a = param;
let aˇ = param;
let a = param;
println!(param);
}
"});

cx.lsp
.handle_request::<lsp::request::References, _, _>(move |_, _| async move {
Ok(Some(vec![lsp::Location {
uri: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
range: lsp::Range::new(lsp::Position::new(2, 8), lsp::Position::new(2, 9)),
}]))
});

let references = cx
.update_editor(|editor, cx| editor.find_all_references(&FindAllReferences, cx))
.unwrap();

cx.executor().run_until_parked();

cx.executor().start_waiting();
references.await.unwrap();

cx.assert_editor_state(indoc! {"
fn foo(param: i64) {
let a = param;
let «aˇ» = param;
let a = param;
println!(param);
}
"});
}

#[gpui::test]
async fn test_addition_reverts(cx: &mut gpui::TestAppContext) {
init_test(cx, |_| {});
Expand Down
148 changes: 1 addition & 147 deletions crates/editor/src/hover_links.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -702,10 +702,7 @@ mod tests {
use gpui::Modifiers;
use indoc::indoc;
use language::language_settings::InlayHintSettings;
use lsp::{
request::{GotoDefinition, GotoTypeDefinition},
References,
};
use lsp::request::{GotoDefinition, GotoTypeDefinition};
use util::assert_set_eq;
use workspace::item::Item;

Expand Down Expand Up @@ -1270,147 +1267,4 @@ mod tests {
cx.simulate_click(screen_coord, Modifiers::secondary_key());
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::<GotoDefinition, _, _>(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::<References, _, _>(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::secondary_key());
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::secondary_key());
cx.background_executor.run_until_parked();
cx.assert_editor_state(indoc! {"
fn «testˇ»() {
do_work();
}
fn do_work() {
test();
}
"});
}
}
}
Loading

0 comments on commit 7d5048e

Please sign in to comment.