From fcfe4e2c1407c6c4d17b3c817d22bbd9ca94e9c5 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 14 May 2025 18:24:17 +0200 Subject: [PATCH] Reuse existing language servers for invisible worktrees (#30707) Closes https://github.com/zed-industries/zed/issues/20767 Before: https://github.com/user-attachments/assets/6438eb26-796a-4586-9b20-f49d9a133624 After: https://github.com/user-attachments/assets/b3fc2f8b-2873-443f-8d80-ab4a35cf0c09 Release Notes: - Fixed external files spawning extra language servers --- crates/editor/src/editor_tests.rs | 152 ++++++++- crates/gpui/src/platform/test/platform.rs | 2 +- crates/project/src/lsp_store.rs | 311 ++++++++++++------ crates/project/src/manifest_tree.rs | 4 +- .../project/src/manifest_tree/server_tree.rs | 36 +- crates/workspace/src/pane.rs | 2 +- 6 files changed, 400 insertions(+), 107 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 4038676457..2502579b21 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -52,7 +52,7 @@ use util::{ uri, }; use workspace::{ - CloseAllItems, CloseInactiveItems, NavigationEntry, ViewId, + CloseActiveItem, CloseAllItems, CloseInactiveItems, NavigationEntry, OpenOptions, ViewId, item::{FollowEvent, FollowableItem, Item, ItemHandle}, }; @@ -19867,6 +19867,156 @@ async fn test_html_linked_edits_on_completion(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_invisible_worktree_servers(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/root"), + json!({ + "a": { + "main.rs": "fn main() {}", + }, + "foo": { + "bar": { + "external_file.rs": "pub mod external {}", + } + } + }), + ) + .await; + + let project = Project::test(fs, [path!("/root/a").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + let _fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + ..FakeLspAdapter::default() + }, + ); + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let worktree_id = workspace.update(cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }) + }); + + let assert_language_servers_count = + |expected: usize, context: &str, cx: &mut VisualTestContext| { + project.update(cx, |project, cx| { + let current = project + .lsp_store() + .read(cx) + .as_local() + .unwrap() + .language_servers + .len(); + assert_eq!(expected, current, "{context}"); + }); + }; + + assert_language_servers_count( + 0, + "No servers should be running before any file is open", + cx, + ); + let pane = workspace.update(cx, |workspace, _| workspace.active_pane().clone()); + let main_editor = workspace + .update_in(cx, |workspace, window, cx| { + workspace.open_path( + (worktree_id, "main.rs"), + Some(pane.downgrade()), + true, + window, + cx, + ) + }) + .unwrap() + .await + .downcast::() + .unwrap(); + pane.update(cx, |pane, cx| { + let open_editor = pane.active_item().unwrap().downcast::().unwrap(); + open_editor.update(cx, |editor, cx| { + assert_eq!( + editor.display_text(cx), + "fn main() {}", + "Original main.rs text on initial open", + ); + }); + assert_eq!(open_editor, main_editor); + }); + assert_language_servers_count(1, "First *.rs file starts a language server", cx); + + let external_editor = workspace + .update_in(cx, |workspace, window, cx| { + workspace.open_abs_path( + PathBuf::from("/root/foo/bar/external_file.rs"), + OpenOptions::default(), + window, + cx, + ) + }) + .await + .expect("opening external file") + .downcast::() + .expect("downcasted external file's open element to editor"); + pane.update(cx, |pane, cx| { + let open_editor = pane.active_item().unwrap().downcast::().unwrap(); + open_editor.update(cx, |editor, cx| { + assert_eq!( + editor.display_text(cx), + "pub mod external {}", + "External file is open now", + ); + }); + assert_eq!(open_editor, external_editor); + }); + assert_language_servers_count( + 1, + "Second, external, *.rs file should join the existing server", + cx, + ); + + pane.update_in(cx, |pane, window, cx| { + pane.close_active_item(&CloseActiveItem::default(), window, cx) + }) + .unwrap() + .await + .unwrap(); + pane.update_in(cx, |pane, window, cx| { + pane.navigate_backward(window, cx); + }); + cx.run_until_parked(); + pane.update(cx, |pane, cx| { + let open_editor = pane.active_item().unwrap().downcast::().unwrap(); + open_editor.update(cx, |editor, cx| { + assert_eq!( + editor.display_text(cx), + "pub mod external {}", + "External file is open now", + ); + }); + }); + assert_language_servers_count( + 1, + "After closing and reopening (with navigate back) of an external file, no extra language servers should appear", + cx, + ); + + cx.update(|_, cx| { + workspace::reload(&workspace::Reload::default(), cx); + }); + assert_language_servers_count( + 1, + "After reloading the worktree with local and external files opened, only one project should be started", + cx, + ); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(DisplayRow(row as u32), column as u32); point..point diff --git a/crates/gpui/src/platform/test/platform.rs b/crates/gpui/src/platform/test/platform.rs index 2c1f7e4ad5..bc3c2b89a8 100644 --- a/crates/gpui/src/platform/test/platform.rs +++ b/crates/gpui/src/platform/test/platform.rs @@ -236,7 +236,7 @@ impl Platform for TestPlatform { fn quit(&self) {} fn restart(&self, _: Option) { - unimplemented!() + // } fn activate(&self, _ignoring_other_apps: bool) { diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 7102f8bc5e..fa7169906f 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -9,7 +9,9 @@ use crate::{ environment::ProjectEnvironment, lsp_command::{self, *}, lsp_store, - manifest_tree::{AdapterQuery, LanguageServerTree, LaunchDisposition, ManifestTree}, + manifest_tree::{ + AdapterQuery, LanguageServerTree, LanguageServerTreeNode, LaunchDisposition, ManifestTree, + }, prettier_store::{self, PrettierStore, PrettierStoreEvent}, project_settings::{LspSettings, ProjectSettings}, relativize_path, resolve_path, @@ -36,7 +38,7 @@ use http_client::HttpClient; use itertools::Itertools as _; use language::{ Bias, BinaryStatus, Buffer, BufferSnapshot, CachedLspAdapter, CodeLabel, Diagnostic, - DiagnosticEntry, DiagnosticSet, Diff, File as _, Language, LanguageRegistry, + DiagnosticEntry, DiagnosticSet, Diff, File as _, Language, LanguageName, LanguageRegistry, LanguageToolchainStore, LocalFile, LspAdapter, LspAdapterDelegate, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction, Unclipped, language_settings::{ @@ -73,7 +75,7 @@ use std::{ any::Any, borrow::Cow, cell::RefCell, - cmp::Ordering, + cmp::{Ordering, Reverse}, convert::TryInto, ffi::OsStr, iter, mem, @@ -1032,7 +1034,7 @@ impl LocalLspStore { .read(cx) .worktree_for_id(project_path.worktree_id, cx) else { - return vec![]; + return Vec::new(); }; let delegate = LocalLspAdapterDelegate::from_local_lsp(self, &worktree, cx); let root = self.lsp_tree.update(cx, |this, cx| { @@ -2284,19 +2286,37 @@ impl LocalLspStore { else { return; }; - let delegate = LocalLspAdapterDelegate::from_local_lsp(self, &worktree, cx); - let servers = self.lsp_tree.clone().update(cx, |this, cx| { - this.get( - ProjectPath { worktree_id, path }, - AdapterQuery::Language(&language.name()), - delegate.clone(), - cx, - ) - .collect::>() - }); + let language_name = language.name(); + let (reused, delegate, servers) = self + .lsp_tree + .update(cx, |lsp_tree, cx| { + self.reuse_existing_language_server(lsp_tree, &worktree, &language_name, cx) + }) + .map(|(delegate, servers)| (true, delegate, servers)) + .unwrap_or_else(|| { + let delegate = LocalLspAdapterDelegate::from_local_lsp(self, &worktree, cx); + let servers = self + .lsp_tree + .clone() + .update(cx, |language_server_tree, cx| { + language_server_tree + .get( + ProjectPath { worktree_id, path }, + AdapterQuery::Language(&language.name()), + delegate.clone(), + cx, + ) + .collect::>() + }); + (false, delegate, servers) + }); let servers = servers .into_iter() .filter_map(|server_node| { + if reused && server_node.server_id().is_none() { + return None; + } + let server_id = server_node.server_id_or_init( |LaunchDisposition { server_name, @@ -2435,6 +2455,63 @@ impl LocalLspStore { } } + fn reuse_existing_language_server( + &self, + server_tree: &mut LanguageServerTree, + worktree: &Entity, + language_name: &LanguageName, + cx: &mut App, + ) -> Option<(Arc, Vec)> { + if worktree.read(cx).is_visible() { + return None; + } + + let worktree_store = self.worktree_store.read(cx); + let servers = server_tree + .instances + .iter() + .filter(|(worktree_id, _)| { + worktree_store + .worktree_for_id(**worktree_id, cx) + .is_some_and(|worktree| worktree.read(cx).is_visible()) + }) + .flat_map(|(worktree_id, servers)| { + servers + .roots + .iter() + .flat_map(|(_, language_servers)| language_servers) + .map(move |(_, (server_node, server_languages))| { + (worktree_id, server_node, server_languages) + }) + .filter(|(_, _, server_languages)| server_languages.contains(language_name)) + .map(|(worktree_id, server_node, _)| { + ( + *worktree_id, + LanguageServerTreeNode::from(Arc::downgrade(server_node)), + ) + }) + }) + .fold(HashMap::default(), |mut acc, (worktree_id, server_node)| { + acc.entry(worktree_id) + .or_insert_with(Vec::new) + .push(server_node); + acc + }) + .into_values() + .max_by_key(|servers| servers.len())?; + + for server_node in &servers { + server_tree.register_reused( + worktree.read(cx).id(), + language_name.clone(), + server_node.clone(), + ); + } + + let delegate = LocalLspAdapterDelegate::from_local_lsp(self, worktree, cx); + Some((delegate, servers)) + } + pub(crate) fn unregister_old_buffer_from_language_servers( &mut self, buffer: &Entity, @@ -4018,6 +4095,16 @@ impl LspStore { buffers_with_unknown_injections.push(handle); } } + + // Deprioritize the invisible worktrees so main worktrees' language servers can be started first, + // and reused later in the invisible worktrees. + plain_text_buffers.sort_by_key(|buffer| { + Reverse( + crate::File::from_dyn(buffer.read(cx).file()) + .map(|file| file.worktree.read(cx).is_visible()), + ) + }); + for buffer in plain_text_buffers { this.detect_language_for_buffer(&buffer, cx); if let Some(local) = this.as_local_mut() { @@ -4355,8 +4442,13 @@ impl LspStore { }; let mut rebase = lsp_tree.rebase(); - for buffer in buffer_store.read(cx).buffers().collect::>() { - let buffer = buffer.read(cx); + for buffer_handle in buffer_store.read(cx).buffers().sorted_by_key(|buffer| { + Reverse( + crate::File::from_dyn(buffer.read(cx).file()) + .map(|file| file.worktree.read(cx).is_visible()), + ) + }) { + let buffer = buffer_handle.read(cx); if !local.registered_buffers.contains_key(&buffer.remote_id()) { continue; } @@ -4372,43 +4464,81 @@ impl LspStore { else { continue; }; - let path: Arc = file - .path() - .parent() - .map(Arc::from) - .unwrap_or_else(|| file.path().clone()); - let worktree_path = ProjectPath { worktree_id, path }; - let Some(delegate) = adapters - .entry(worktree_id) - .or_insert_with(|| get_adapter(worktree_id, cx)) - .clone() + let Some((reused, delegate, nodes)) = local + .reuse_existing_language_server( + rebase.server_tree(), + &worktree, + &language, + cx, + ) + .map(|(delegate, servers)| (true, delegate, servers)) + .or_else(|| { + let delegate = adapters + .entry(worktree_id) + .or_insert_with(|| get_adapter(worktree_id, cx)) + .clone()?; + let path = file + .path() + .parent() + .map(Arc::from) + .unwrap_or_else(|| file.path().clone()); + let worktree_path = ProjectPath { worktree_id, path }; + + let nodes = rebase.get( + worktree_path, + AdapterQuery::Language(&language), + delegate.clone(), + cx, + ); + + Some((false, delegate, nodes.collect())) + }) else { continue; }; - let nodes = rebase.get( - worktree_path, - AdapterQuery::Language(&language), - delegate.clone(), - cx, - ); + for node in nodes { - node.server_id_or_init( - |LaunchDisposition { - server_name, - attach, - path, - settings, - }| match attach { - language::Attach::InstancePerRoot => { - // todo: handle instance per root proper. - if let Some(server_ids) = local - .language_server_ids - .get(&(worktree_id, server_name.clone())) - { - server_ids.iter().cloned().next().unwrap() - } else { - local.start_language_server( + if !reused { + node.server_id_or_init( + |LaunchDisposition { + server_name, + attach, + path, + settings, + }| match attach { + language::Attach::InstancePerRoot => { + // todo: handle instance per root proper. + if let Some(server_ids) = local + .language_server_ids + .get(&(worktree_id, server_name.clone())) + { + server_ids.iter().cloned().next().unwrap() + } else { + local.start_language_server( + &worktree, + delegate.clone(), + local + .languages + .lsp_adapters(&language) + .into_iter() + .find(|adapter| { + &adapter.name() == server_name + }) + .expect("To find LSP adapter"), + settings, + cx, + ) + } + } + language::Attach::Shared => { + let uri = Url::from_file_path( + worktree.read(cx).abs_path().join(&path.path), + ); + let key = (worktree_id, server_name.clone()); + local.language_server_ids.remove(&key); + + let server_id = local.start_language_server( &worktree, delegate.clone(), local @@ -4419,38 +4549,19 @@ impl LspStore { .expect("To find LSP adapter"), settings, cx, - ) + ); + if let Some(state) = + local.language_servers.get(&server_id) + { + if let Ok(uri) = uri { + state.add_workspace_folder(uri); + }; + } + server_id } - } - language::Attach::Shared => { - let uri = Url::from_file_path( - worktree.read(cx).abs_path().join(&path.path), - ); - let key = (worktree_id, server_name.clone()); - local.language_server_ids.remove(&key); - - let server_id = local.start_language_server( - &worktree, - delegate.clone(), - local - .languages - .lsp_adapters(&language) - .into_iter() - .find(|adapter| &adapter.name() == server_name) - .expect("To find LSP adapter"), - settings, - cx, - ); - if let Some(state) = local.language_servers.get(&server_id) - { - if let Ok(uri) = uri { - state.add_workspace_folder(uri); - }; - } - server_id - } - }, - ); + }, + ); + } } } } @@ -6365,26 +6476,30 @@ impl LspStore { let Some(local) = self.as_local_mut() else { return; }; - let worktree_id = worktree.read(cx).id(); - let path = ProjectPath { - worktree_id, - path: Arc::from("".as_ref()), - }; - let delegate = LocalLspAdapterDelegate::from_local_lsp(local, &worktree, cx); - local.lsp_tree.update(cx, |this, cx| { - for node in this.get( - path, - AdapterQuery::Adapter(&language_server_name), - delegate, - cx, - ) { - node.server_id_or_init(|disposition| { - assert_eq!(disposition.server_name, &language_server_name); - language_server_id - }); - } - }); + let worktree_id = worktree.read(cx).id(); + if worktree.read(cx).is_visible() { + let path = ProjectPath { + worktree_id, + path: Arc::from("".as_ref()), + }; + let delegate = LocalLspAdapterDelegate::from_local_lsp(local, &worktree, cx); + local.lsp_tree.update(cx, |language_server_tree, cx| { + for node in language_server_tree.get( + path, + AdapterQuery::Adapter(&language_server_name), + delegate, + cx, + ) { + node.server_id_or_init(|disposition| { + assert_eq!(disposition.server_name, &language_server_name); + + language_server_id + }); + } + }); + } + local .language_server_ids .entry((worktree_id, language_server_name)) diff --git a/crates/project/src/manifest_tree.rs b/crates/project/src/manifest_tree.rs index 64b36212a9..2104067a03 100644 --- a/crates/project/src/manifest_tree.rs +++ b/crates/project/src/manifest_tree.rs @@ -27,7 +27,9 @@ use crate::{ worktree_store::{WorktreeStore, WorktreeStoreEvent}, }; -pub(crate) use server_tree::{AdapterQuery, LanguageServerTree, LaunchDisposition}; +pub(crate) use server_tree::{ + AdapterQuery, LanguageServerTree, LanguageServerTreeNode, LaunchDisposition, +}; struct WorktreeRoots { roots: RootPathTrie, diff --git a/crates/project/src/manifest_tree/server_tree.rs b/crates/project/src/manifest_tree/server_tree.rs index 0a8cbbedb4..cc41f3dff2 100644 --- a/crates/project/src/manifest_tree/server_tree.rs +++ b/crates/project/src/manifest_tree/server_tree.rs @@ -28,8 +28,8 @@ use crate::{LanguageServerId, ProjectPath, project_settings::LspSettings}; use super::{ManifestTree, ManifestTreeEvent}; #[derive(Debug, Default)] -struct ServersForWorktree { - roots: BTreeMap< +pub(crate) struct ServersForWorktree { + pub(crate) roots: BTreeMap< Arc, BTreeMap, BTreeSet)>, >, @@ -37,7 +37,7 @@ struct ServersForWorktree { pub struct LanguageServerTree { manifest_tree: Entity, - instances: BTreeMap, + pub(crate) instances: BTreeMap, attach_kind_cache: HashMap, languages: Arc, _subscriptions: Subscription, @@ -47,7 +47,7 @@ pub struct LanguageServerTree { /// - A language server that has already been initialized/updated for a given project /// - A soon-to-be-initialized language server. #[derive(Clone)] -pub(crate) struct LanguageServerTreeNode(Weak); +pub struct LanguageServerTreeNode(Weak); /// Describes a request to launch a language server. #[derive(Debug)] @@ -96,7 +96,7 @@ impl From> for LanguageServerTreeNode { } #[derive(Debug)] -struct InnerTreeNode { +pub struct InnerTreeNode { id: OnceLock, name: LanguageServerName, attach: Attach, @@ -336,6 +336,28 @@ impl LanguageServerTree { } } } + + pub(crate) fn register_reused( + &mut self, + worktree_id: WorktreeId, + language_name: LanguageName, + reused: LanguageServerTreeNode, + ) { + let Some(node) = reused.0.upgrade() else { + return; + }; + + self.instances + .entry(worktree_id) + .or_default() + .roots + .entry(Arc::from(Path::new(""))) + .or_default() + .entry(node.name.clone()) + .or_insert_with(|| (node, BTreeSet::new())) + .1 + .insert(language_name); + } } pub(crate) struct ServerTreeRebase<'a> { @@ -441,4 +463,8 @@ impl<'tree> ServerTreeRebase<'tree> { .filter(|(id, _)| !self.rebased_server_ids.contains(id)) .collect() } + + pub(crate) fn server_tree(&mut self) -> &mut LanguageServerTree { + &mut self.new_tree + } } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 6074ebfee9..a4afba20d7 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -712,7 +712,7 @@ impl Pane { !self.nav_history.0.lock().forward_stack.is_empty() } - fn navigate_backward(&mut self, window: &mut Window, cx: &mut Context) { + pub fn navigate_backward(&mut self, window: &mut Window, cx: &mut Context) { if let Some(workspace) = self.workspace.upgrade() { let pane = cx.entity().downgrade(); window.defer(cx, move |window, cx| {