From 4db5666f8a40517509f5e4b9ca52e02af4e0a553 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 19 Mar 2026 15:28:30 +0100 Subject: [PATCH] editor: Open a singleton buffer when go to action results fit into a single excerpt (#51461) Closes https://github.com/zed-industries/zed/issues/44203 Release Notes: - Go to definition (and similar actions) will now no longer open a multi buffer if there are multiple results that all fit into a single excerpt --- crates/editor/src/editor.rs | 77 +++++++++++++-- crates/editor/src/editor_tests.rs | 157 ++++++++++++++++++++++++++++++ 2 files changed, 227 insertions(+), 7 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9fb18d414f1b1551390bb71a3b4bd6dc01ae4d27..141f8254ef357d0676ee1cdf29b6fa9c8eb6ac17 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -18119,6 +18119,7 @@ impl Editor { let workspace = self.workspace(); + let excerpt_context_lines = multi_buffer::excerpt_context_lines(cx); cx.spawn_in(window, async move |editor, cx| { let locations: Vec = future::join_all(definitions) .await @@ -18139,7 +18140,11 @@ impl Editor { for ranges in locations.values_mut() { ranges.sort_by_key(|range| (range.start, Reverse(range.end))); ranges.dedup(); - num_locations += ranges.len(); + let fits_in_one_excerpt = ranges + .iter() + .tuple_windows() + .all(|(a, b)| b.start.row - a.end.row <= 2 * excerpt_context_lines); + num_locations += if fits_in_one_excerpt { 1 } else { ranges.len() }; } if num_locations > 1 { @@ -18244,16 +18249,43 @@ impl Editor { } } else { let (target_buffer, target_ranges) = locations.into_iter().next().unwrap(); - let target_range = target_ranges.first().unwrap().clone(); editor.update_in(cx, |editor, window, cx| { - let range = editor.range_for_match(&target_range); - let range = collapse_multiline_range(range); - + let target_ranges = target_ranges + .into_iter() + .map(|r| editor.range_for_match(&r)) + .map(collapse_multiline_range) + .collect::>(); if !split && Some(&target_buffer) == editor.buffer.read(cx).as_singleton().as_ref() { - editor.go_to_singleton_buffer_range(range, window, cx); + let multibuffer = editor.buffer.read(cx); + let target_ranges = target_ranges + .into_iter() + .filter_map(|r| { + let start = multibuffer.buffer_point_to_anchor( + &target_buffer, + r.start, + cx, + )?; + let end = multibuffer.buffer_point_to_anchor( + &target_buffer, + r.end, + cx, + )?; + Some(start..end) + }) + .collect::>(); + if target_ranges.is_empty() { + return Navigated::No; + } + + editor.change_selections( + SelectionEffects::default().nav_history(true), + window, + cx, + |s| s.select_anchor_ranges(target_ranges), + ); let target = editor.navigation_entry(editor.selections.newest_anchor().head(), cx); @@ -18302,7 +18334,37 @@ impl Editor { // When selecting a definition in a different buffer, disable the nav history // to avoid creating a history entry at the previous cursor location. pane.update(cx, |pane, _| pane.disable_history()); - target_editor.go_to_singleton_buffer_range(range, window, cx); + + let multibuffer = target_editor.buffer.read(cx); + let Some(target_buffer) = multibuffer.as_singleton() else { + return Navigated::No; + }; + let target_ranges = target_ranges + .into_iter() + .filter_map(|r| { + let start = multibuffer.buffer_point_to_anchor( + &target_buffer, + r.start, + cx, + )?; + let end = multibuffer.buffer_point_to_anchor( + &target_buffer, + r.end, + cx, + )?; + Some(start..end) + }) + .collect::>(); + if target_ranges.is_empty() { + return Navigated::No; + } + + target_editor.change_selections( + SelectionEffects::default().nav_history(true), + window, + cx, + |s| s.select_anchor_ranges(target_ranges), + ); let nav_data = target_editor.navigation_data( target_editor.selections.newest_anchor().head(), @@ -18314,6 +18376,7 @@ impl Editor { ))); nav_history.push_tag(origin, target); pane.update(cx, |pane, _| pane.enable_history()); + Navigated::Yes }); }); } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index d813dcc62c38d2c1bb5a121ed6821d0513f655d1..f6fd13e10da09fc4fa0259d5a4922fa36118631d 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -24798,6 +24798,163 @@ async fn test_goto_definition_no_fallback(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_goto_definition_close_ranges_open_singleton(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + definition_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + cx, + ) + .await; + + // File content: 10 lines with functions defined on lines 3, 5, and 7 (0-indexed). + // With the default excerpt_context_lines of 2, ranges that are within + // 2 * 2 = 4 rows of each other should be grouped into one excerpt. + cx.set_state( + &r#"fn caller() { + let _ = ˇtarget(); + } + fn target_a() {} + + fn target_b() {} + + fn target_c() {} + "# + .unindent(), + ); + + // Return two definitions that are close together (lines 3 and 5, gap of 2 rows) + cx.set_request_handler::(move |url, _, _| async move { + Ok(Some(lsp::GotoDefinitionResponse::Array(vec![ + lsp::Location { + uri: url.clone(), + range: lsp::Range::new(lsp::Position::new(3, 3), lsp::Position::new(3, 11)), + }, + lsp::Location { + uri: url, + range: lsp::Range::new(lsp::Position::new(5, 3), lsp::Position::new(5, 11)), + }, + ]))) + }); + + let navigated = cx + .update_editor(|editor, window, cx| editor.go_to_definition(&GoToDefinition, window, cx)) + .await + .expect("Failed to navigate to definitions"); + assert_eq!(navigated, Navigated::Yes); + + let editors = cx.update_workspace(|workspace, _, cx| { + workspace.items_of_type::(cx).collect::>() + }); + cx.update_editor(|_, _, _| { + assert_eq!( + editors.len(), + 1, + "Close ranges should navigate in-place without opening a new editor" + ); + }); + + // Both target ranges should be selected + cx.assert_editor_state( + &r#"fn caller() { + let _ = target(); + } + fn «target_aˇ»() {} + + fn «target_bˇ»() {} + + fn target_c() {} + "# + .unindent(), + ); +} + +#[gpui::test] +async fn test_goto_definition_far_ranges_open_multibuffer(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + definition_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + cx, + ) + .await; + + // Create a file with definitions far apart (more than 2 * excerpt_context_lines rows). + cx.set_state( + &r#"fn caller() { + let _ = ˇtarget(); + } + fn target_a() {} + + + + + + + + + + + + + + + + fn target_b() {} + "# + .unindent(), + ); + + // Return two definitions that are far apart (lines 3 and 19, gap of 16 rows) + cx.set_request_handler::(move |url, _, _| async move { + Ok(Some(lsp::GotoDefinitionResponse::Array(vec![ + lsp::Location { + uri: url.clone(), + range: lsp::Range::new(lsp::Position::new(3, 3), lsp::Position::new(3, 11)), + }, + lsp::Location { + uri: url, + range: lsp::Range::new(lsp::Position::new(19, 3), lsp::Position::new(19, 11)), + }, + ]))) + }); + + let navigated = cx + .update_editor(|editor, window, cx| editor.go_to_definition(&GoToDefinition, window, cx)) + .await + .expect("Failed to navigate to definitions"); + assert_eq!(navigated, Navigated::Yes); + + let editors = cx.update_workspace(|workspace, _, cx| { + workspace.items_of_type::(cx).collect::>() + }); + cx.update_editor(|_, _, test_editor_cx| { + assert_eq!( + editors.len(), + 2, + "Far apart ranges should open a new multibuffer editor" + ); + let multibuffer_editor = editors + .into_iter() + .find(|editor| *editor != test_editor_cx.entity()) + .expect("Should have a multibuffer editor"); + let multibuffer_text = multibuffer_editor.read(test_editor_cx).text(test_editor_cx); + assert!( + multibuffer_text.contains("target_a"), + "Multibuffer should contain the first definition" + ); + assert!( + multibuffer_text.contains("target_b"), + "Multibuffer should contain the second definition" + ); + }); +} + #[gpui::test] async fn test_find_all_references_editor_reuse(cx: &mut TestAppContext) { init_test(cx, |_| {});