diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c5ea6d4ee20cd123f793129b88396c407a49ceca..007fdd52d799208c1428bcf1faa3bc2196d8d7d2 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -175,16 +175,14 @@ use settings::{Settings, SettingsLocation, SettingsStore, update_settings_file}; use smallvec::{SmallVec, smallvec}; use snippet::Snippet; use std::{ - any::TypeId, + any::{Any, TypeId}, borrow::Cow, - cell::OnceCell, - cell::RefCell, + cell::{OnceCell, RefCell}, cmp::{self, Ordering, Reverse}, iter::Peekable, mem, num::NonZeroU32, - ops::Not, - ops::{ControlFlow, Deref, DerefMut, Range, RangeInclusive}, + ops::{ControlFlow, Deref, DerefMut, Not, Range, RangeInclusive}, path::{Path, PathBuf}, rc::Rc, sync::Arc, @@ -850,7 +848,7 @@ pub struct ResolvedTasks { #[derive(Copy, Clone, Debug, PartialEq, PartialOrd)] struct BufferOffset(usize); -// Addons allow storing per-editor state in other crates (e.g. Vim) +/// Addons allow storing per-editor state in other crates (e.g. Vim) pub trait Addon: 'static { fn extend_key_context(&self, _: &mut KeyContext, _: &App) {} @@ -1178,6 +1176,7 @@ pub struct Editor { next_color_inlay_id: usize, colors: Option, folding_newlines: Task<()>, + pub lookup_key: Option>, } #[derive(Copy, Clone, Debug, PartialEq, Eq, Default)] @@ -2257,6 +2256,7 @@ impl Editor { mode, selection_drag_state: SelectionDragState::None, folding_newlines: Task::ready(()), + lookup_key: None, }; if is_minimap { @@ -16396,7 +16396,7 @@ impl Editor { anyhow::Ok(Navigated::from_bool(opened)) } else if locations.is_empty() { - // If there is one definition, just open it directly + // If there is one url or file, open it directly match first_url_or_file { Some(Either::Left(url)) => { acx.update(|_, cx| cx.open_url(&url))?; @@ -16423,8 +16423,6 @@ impl Editor { 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)); let range = editor.range_for_match(&range); let range = collapse_multiline_range(range); @@ -16434,6 +16432,7 @@ impl Editor { { editor.go_to_singleton_buffer_range(range, window, cx); } else { + let pane = workspace.read(cx).active_pane().clone(); window.defer(cx, move |window, cx| { let target_editor: Entity = workspace.update(cx, |workspace, cx| { @@ -16607,13 +16606,17 @@ impl Editor { return; } - // If there are multiple definitions, open them in a multibuffer locations.sort_by_key(|location| location.buffer.read(cx).remote_id()); + let mut locations = locations.into_iter().peekable(); let mut ranges: Vec> = Vec::new(); let capability = workspace.project().read(cx).capability(); + // a key to find existing multibuffer editors with the same set of locations + // to prevent us from opening more and more multibuffer tabs for searches and the like + let mut key = (title.clone(), vec![]); let excerpt_buffer = cx.new(|cx| { + let key = &mut key.1; let mut multibuffer = MultiBuffer::new(capability); while let Some(location) = locations.next() { let buffer = location.buffer.read(cx); @@ -16621,16 +16624,17 @@ impl Editor { let range = location.range.to_point(buffer); ranges_for_buffer.push(range.clone()); - while let Some(next_location) = locations.peek() { - if next_location.buffer == location.buffer { - ranges_for_buffer.push(next_location.range.to_point(buffer)); - locations.next(); - } else { - break; - } + while let Some(next_location) = + locations.next_if(|next_location| next_location.buffer == location.buffer) + { + ranges_for_buffer.push(next_location.range.to_point(buffer)); } ranges_for_buffer.sort_by_key(|range| (range.start, Reverse(range.end))); + key.push(( + location.buffer.read(cx).remote_id(), + ranges_for_buffer.clone(), + )); let (new_ranges, _) = multibuffer.set_excerpts_for_path( PathKey::for_buffer(&location.buffer, cx), location.buffer.clone(), @@ -16643,14 +16647,31 @@ impl Editor { multibuffer.with_title(title) }); - - let editor = cx.new(|cx| { - Editor::for_multibuffer( - excerpt_buffer, - Some(workspace.project().clone()), - window, - cx, - ) + let existing = workspace.active_pane().update(cx, |pane, cx| { + pane.items() + .filter_map(|item| item.downcast::()) + .find(|editor| { + editor + .read(cx) + .lookup_key + .as_ref() + .and_then(|it| { + it.downcast_ref::<(String, Vec<(BufferId, Vec>)>)>() + }) + .is_some_and(|it| *it == key) + }) + }); + let editor = existing.unwrap_or_else(|| { + cx.new(|cx| { + let mut editor = Editor::for_multibuffer( + excerpt_buffer, + Some(workspace.project().clone()), + window, + cx, + ); + editor.lookup_key = Some(Box::new(key)); + editor + }) }); editor.update(cx, |editor, cx| { match multibuffer_selection_mode { diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 4e3c5012e14f7919a8994bb2951c71cd75d0e404..131b03c61b9a36ca73bf91fbc3bdf35b0000e3ee 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -21402,6 +21402,159 @@ async fn test_goto_definition_no_fallback(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_find_all_references_editor_reuse(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + references_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + cx, + ) + .await; + + cx.set_state( + &r#" + fn one() { + let mut a = two(); + } + + fn ˇtwo() {}"# + .unindent(), + ); + cx.lsp + .set_request_handler::(move |params, _| async move { + Ok(Some(vec![ + lsp::Location { + uri: params.text_document_position.text_document.uri.clone(), + range: lsp::Range::new(lsp::Position::new(0, 16), lsp::Position::new(0, 19)), + }, + lsp::Location { + uri: params.text_document_position.text_document.uri, + range: lsp::Range::new(lsp::Position::new(4, 4), lsp::Position::new(4, 7)), + }, + ])) + }); + let navigated = cx + .update_editor(|editor, window, cx| { + editor.find_all_references(&FindAllReferences, window, cx) + }) + .unwrap() + .await + .expect("Failed to navigate to references"); + assert_eq!( + navigated, + Navigated::Yes, + "Should have navigated to references from the FindAllReferences response" + ); + cx.assert_editor_state( + &r#"fn one() { + let mut a = two(); + } + + fn ˇtwo() {}"# + .unindent(), + ); + + let editors = cx.update_workspace(|workspace, _, cx| { + workspace.items_of_type::(cx).collect::>() + }); + cx.update_editor(|_, _, _| { + assert_eq!(editors.len(), 2, "We should have opened a new multibuffer"); + }); + + cx.set_state( + &r#"fn one() { + let mut a = ˇtwo(); + } + + fn two() {}"# + .unindent(), + ); + let navigated = cx + .update_editor(|editor, window, cx| { + editor.find_all_references(&FindAllReferences, window, cx) + }) + .unwrap() + .await + .expect("Failed to navigate to references"); + assert_eq!( + navigated, + Navigated::Yes, + "Should have navigated to references from the FindAllReferences response" + ); + cx.assert_editor_state( + &r#"fn one() { + let mut a = ˇtwo(); + } + + fn two() {}"# + .unindent(), + ); + let editors = cx.update_workspace(|workspace, _, cx| { + workspace.items_of_type::(cx).collect::>() + }); + cx.update_editor(|_, _, _| { + assert_eq!( + editors.len(), + 2, + "should have re-used the previous multibuffer" + ); + }); + + cx.set_state( + &r#"fn one() { + let mut a = ˇtwo(); + } + fn three() {} + fn two() {}"# + .unindent(), + ); + cx.lsp + .set_request_handler::(move |params, _| async move { + Ok(Some(vec![ + lsp::Location { + uri: params.text_document_position.text_document.uri.clone(), + range: lsp::Range::new(lsp::Position::new(0, 16), lsp::Position::new(0, 19)), + }, + lsp::Location { + uri: params.text_document_position.text_document.uri, + range: lsp::Range::new(lsp::Position::new(5, 4), lsp::Position::new(5, 7)), + }, + ])) + }); + let navigated = cx + .update_editor(|editor, window, cx| { + editor.find_all_references(&FindAllReferences, window, cx) + }) + .unwrap() + .await + .expect("Failed to navigate to references"); + assert_eq!( + navigated, + Navigated::Yes, + "Should have navigated to references from the FindAllReferences response" + ); + cx.assert_editor_state( + &r#"fn one() { + let mut a = ˇtwo(); + } + fn three() {} + fn two() {}"# + .unindent(), + ); + let editors = cx.update_workspace(|workspace, _, cx| { + workspace.items_of_type::(cx).collect::>() + }); + cx.update_editor(|_, _, _| { + assert_eq!( + editors.len(), + 3, + "should have used a new multibuffer as offsets changed" + ); + }); +} #[gpui::test] async fn test_find_enclosing_node_with_task(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 8fa8c2c08c25aa6f003365556594ed0719f9861e..5637f9af1a8ec8d3a064b29c4f8a18a6aff074a4 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -2092,17 +2092,14 @@ impl MultiBuffer { let snapshot = self.read(cx); let buffers = self.buffers.borrow(); let mut cursor = snapshot.excerpts.cursor::>(&()); - for locator in buffers - .get(&buffer_id) - .map(|state| &state.excerpts) - .into_iter() - .flatten() - { - cursor.seek_forward(&Some(locator), Bias::Left); - if let Some(excerpt) = cursor.item() - && excerpt.locator == *locator - { - excerpts.push((excerpt.id, excerpt.range.clone())); + if let Some(locators) = buffers.get(&buffer_id).map(|state| &state.excerpts) { + for locator in locators { + cursor.seek_forward(&Some(locator), Bias::Left); + if let Some(excerpt) = cursor.item() + && excerpt.locator == *locator + { + excerpts.push((excerpt.id, excerpt.range.clone())); + } } } diff --git a/crates/text/src/locator.rs b/crates/text/src/locator.rs index 9b89cf21c74eccfe6cbb93fd2dec5bc849f2170d..7363d5bdd3424b202bd92b4e6d98e2d678f0829f 100644 --- a/crates/text/src/locator.rs +++ b/crates/text/src/locator.rs @@ -1,9 +1,5 @@ -use smallvec::{SmallVec, smallvec}; +use smallvec::SmallVec; use std::iter; -use std::sync::LazyLock; - -static MIN: LazyLock = LazyLock::new(Locator::min); -static MAX: LazyLock = LazyLock::new(Locator::max); /// An identifier for a position in a ordered collection. /// @@ -16,20 +12,22 @@ static MAX: LazyLock = LazyLock::new(Locator::max); pub struct Locator(SmallVec<[u64; 4]>); impl Locator { - pub fn min() -> Self { - Self(smallvec![u64::MIN]) + pub const fn min() -> Self { + // SAFETY: 1 is <= 4 + Self(unsafe { SmallVec::from_const_with_len_unchecked([u64::MIN; 4], 1) }) } - pub fn max() -> Self { - Self(smallvec![u64::MAX]) + pub const fn max() -> Self { + // SAFETY: 1 is <= 4 + Self(unsafe { SmallVec::from_const_with_len_unchecked([u64::MAX; 4], 1) }) } - pub fn min_ref() -> &'static Self { - &MIN + pub const fn min_ref() -> &'static Self { + const { &Self::min() } } - pub fn max_ref() -> &'static Self { - &MAX + pub const fn max_ref() -> &'static Self { + const { &Self::max() } } pub fn assign(&mut self, other: &Self) {