From 351914f4bd783fa2dc4fcddf02f54c51afedeb8d Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 23 Jan 2024 11:34:51 +0200 Subject: [PATCH 1/3] Clean up LSP servers on worktree release --- crates/gpui/src/text_system.rs | 4 ++ crates/project/src/prettier_support.rs | 6 ++- crates/project/src/project.rs | 53 +++++++++++++++++++++++++- crates/workspace/src/workspace.rs | 3 +- 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index 6cc56e306b..a43b96ef90 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -70,6 +70,10 @@ impl TextSystem { /// Get a list of all available font names from the operating system. pub fn all_font_names(&self) -> Vec { + eprintln!( + "~~~~~~~~~~~~~ all_font_names called {}", + std::backtrace::Backtrace::capture() + ); let mut names: BTreeSet<_> = self .platform_text_system .all_font_names() diff --git a/crates/project/src/prettier_support.rs b/crates/project/src/prettier_support.rs index c176c79a91..098fe2b39a 100644 --- a/crates/project/src/prettier_support.rs +++ b/crates/project/src/prettier_support.rs @@ -16,7 +16,7 @@ use language::{ language_settings::{Formatter, LanguageSettings}, Buffer, Language, LanguageServerName, LocalFile, }; -use lsp::LanguageServerId; +use lsp::{LanguageServer, LanguageServerId}; use node_runtime::NodeRuntime; use prettier::Prettier; use util::{paths::DEFAULT_PRETTIER_DIR, ResultExt, TryFutureExt}; @@ -212,6 +212,10 @@ impl PrettierInstance { }, }) } + + pub async fn server(&self) -> Option> { + self.prettier.clone()?.await.ok()?.server().cloned() + } } fn start_default_prettier( diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index de69a93ba6..1f5a5a8c64 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -632,7 +632,7 @@ impl Project { let copilot_lsp_subscription = Copilot::global(cx).map(|copilot| subscribe_for_copilot_events(&copilot, cx)); Self { - worktrees: Default::default(), + worktrees: Vec::new(), buffer_ordered_messages_tx: tx, collaborators: Default::default(), next_buffer_id: 0, @@ -973,7 +973,7 @@ impl Project { } // Start all the newly-enabled language servers. - for (worktree, language) in language_servers_to_start { + for (worktree, language) in dbg!(language_servers_to_start) { let worktree_path = worktree.read(cx).abs_path(); self.start_language_servers(&worktree, worktree_path, language, cx); } @@ -6370,6 +6370,55 @@ impl Project { } pub fn remove_worktree(&mut self, id_to_remove: WorktreeId, cx: &mut ModelContext) { + let mut servers_to_remove = HashMap::default(); + let mut servers_to_preserve = HashSet::default(); + for ((worktree_id, server_name), &server_id) in &self.language_server_ids { + if worktree_id == &id_to_remove { + servers_to_remove.insert(server_id, server_name.clone()); + } else { + servers_to_preserve.insert(server_id); + } + } + servers_to_remove.retain(|server_id, _| !servers_to_preserve.contains(server_id)); + for (server_id_to_remove, server_name) in servers_to_remove { + self.language_server_ids + .remove(&(id_to_remove, server_name)); + self.language_server_statuses.remove(&server_id_to_remove); + self.last_workspace_edits_by_language_server + .remove(&server_id_to_remove); + self.language_servers.remove(&server_id_to_remove); + cx.emit(Event::LanguageServerRemoved(server_id_to_remove)); + } + + let mut prettier_instances_to_clean = FuturesUnordered::new(); + if let Some(prettier_paths) = self.prettiers_per_worktree.remove(&id_to_remove) { + for path in prettier_paths.iter().flatten() { + if let Some(prettier_instance) = self.prettier_instances.remove(path) { + prettier_instances_to_clean.push(async move { + prettier_instance + .server() + .await + .map(|server| server.server_id()) + }); + } + } + } + cx.spawn(|project, mut cx| async move { + while let Some(prettier_server_id) = prettier_instances_to_clean.next().await { + if let Some(prettier_server_id) = prettier_server_id { + project + .update(&mut cx, |project, cx| { + project + .supplementary_language_servers + .remove(&prettier_server_id); + cx.emit(Event::LanguageServerRemoved(prettier_server_id)); + }) + .ok(); + } + } + }) + .detach(); + self.worktrees.retain(|worktree| { if let Some(worktree) = worktree.upgrade() { let id = worktree.read(cx).id(); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index a552d9c5af..c51852b511 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1355,7 +1355,7 @@ impl Workspace { Some(visible) => match this .update(&mut cx, |this, cx| { Workspace::project_path_for_path( - this.project.clone(), + dbg!(this.project.clone()), abs_path, visible, cx, @@ -1368,6 +1368,7 @@ impl Workspace { }, None => None, }; + dbg!(&project_path); let this = this.clone(); let abs_path = abs_path.clone(); From ab8585ee7ec0f69ce40755b71b9deb0b3ba62936 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 23 Jan 2024 11:58:17 +0200 Subject: [PATCH 2/3] Remove the `async` modifier from the `workspace_configuration` method --- crates/language/src/language.rs | 12 ++++-------- crates/project/src/project.rs | 21 +++++++++------------ crates/zed/src/languages/json.rs | 10 ++++------ crates/zed/src/languages/tailwind.rs | 16 ++++------------ crates/zed/src/languages/typescript.rs | 13 +++---------- crates/zed/src/languages/yaml.rs | 22 +++++++--------------- 6 files changed, 31 insertions(+), 63 deletions(-) diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index c7327b63d7..1033198bab 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -23,7 +23,7 @@ use async_trait::async_trait; use collections::{HashMap, HashSet}; use futures::{ channel::{mpsc, oneshot}, - future::{BoxFuture, Shared}, + future::Shared, FutureExt, TryFutureExt as _, }; use gpui::{AppContext, AsyncAppContext, BackgroundExecutor, Task}; @@ -216,11 +216,7 @@ impl CachedLspAdapter { self.adapter.code_action_kinds() } - pub fn workspace_configuration( - &self, - workspace_root: &Path, - cx: &mut AppContext, - ) -> BoxFuture<'static, Value> { + pub fn workspace_configuration(&self, workspace_root: &Path, cx: &mut AppContext) -> Value { self.adapter.workspace_configuration(workspace_root, cx) } @@ -345,8 +341,8 @@ pub trait LspAdapter: 'static + Send + Sync { None } - fn workspace_configuration(&self, _: &Path, _: &mut AppContext) -> BoxFuture<'static, Value> { - futures::future::ready(serde_json::json!({})).boxed() + fn workspace_configuration(&self, _: &Path, _: &mut AppContext) -> Value { + serde_json::json!({}) } /// Returns a list of code actions supported by a given LspAdapter diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 1f5a5a8c64..285d86742b 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -664,7 +664,7 @@ impl Project { next_diagnostic_group_id: Default::default(), supplementary_language_servers: HashMap::default(), language_servers: Default::default(), - language_server_ids: Default::default(), + language_server_ids: HashMap::default(), language_server_statuses: Default::default(), last_workspace_edits_by_language_server: Default::default(), buffers_being_formatted: Default::default(), @@ -752,7 +752,7 @@ impl Project { }, supplementary_language_servers: HashMap::default(), language_servers: Default::default(), - language_server_ids: Default::default(), + language_server_ids: HashMap::default(), language_server_statuses: response .payload .language_servers @@ -2700,7 +2700,7 @@ impl Project { }); cx.spawn(move |this, mut cx| async move { - while let Some(_) = settings_changed_rx.next().await { + while let Some(()) = settings_changed_rx.next().await { let servers: Vec<_> = this.update(&mut cx, |this, _| { this.language_servers .values() @@ -2714,9 +2714,8 @@ impl Project { })?; for (adapter, server) in servers { - let workspace_config = cx - .update(|cx| adapter.workspace_configuration(server.root_path(), cx))? - .await; + let workspace_config = + cx.update(|cx| adapter.workspace_configuration(server.root_path(), cx))?; server .notify::( lsp::DidChangeConfigurationParams { @@ -3020,9 +3019,8 @@ impl Project { server_id: LanguageServerId, cx: &mut AsyncAppContext, ) -> Result> { - let workspace_config = cx - .update(|cx| adapter.workspace_configuration(worktree_path, cx))? - .await; + let workspace_config = + cx.update(|cx| adapter.workspace_configuration(worktree_path, cx))?; let language_server = pending_server.task.await?; language_server @@ -3056,9 +3054,8 @@ impl Project { let adapter = adapter.clone(); let worktree_path = worktree_path.clone(); async move { - let workspace_config = cx - .update(|cx| adapter.workspace_configuration(&worktree_path, cx))? - .await; + let workspace_config = + cx.update(|cx| adapter.workspace_configuration(&worktree_path, cx))?; Ok(params .items .into_iter() diff --git a/crates/zed/src/languages/json.rs b/crates/zed/src/languages/json.rs index 162d4c9fdb..19af72f7c6 100644 --- a/crates/zed/src/languages/json.rs +++ b/crates/zed/src/languages/json.rs @@ -2,7 +2,7 @@ use anyhow::{anyhow, Result}; use async_trait::async_trait; use collections::HashMap; use feature_flags::FeatureFlagAppExt; -use futures::{future::BoxFuture, FutureExt, StreamExt}; +use futures::StreamExt; use gpui::AppContext; use language::{LanguageRegistry, LanguageServerName, LspAdapter, LspAdapterDelegate}; use lsp::LanguageServerBinary; @@ -13,7 +13,6 @@ use smol::fs; use std::{ any::Any, ffi::OsString, - future, path::{Path, PathBuf}, sync::Arc, }; @@ -107,7 +106,7 @@ impl LspAdapter for JsonLspAdapter { &self, _workspace_root: &Path, cx: &mut AppContext, - ) -> BoxFuture<'static, serde_json::Value> { + ) -> serde_json::Value { let action_names = cx.all_action_names(); let staff_mode = cx.is_staff(); let language_names = &self.languages.language_names(); @@ -119,7 +118,7 @@ impl LspAdapter for JsonLspAdapter { cx, ); - future::ready(serde_json::json!({ + serde_json::json!({ "json": { "format": { "enable": true, @@ -138,8 +137,7 @@ impl LspAdapter for JsonLspAdapter { } ] } - })) - .boxed() + }) } async fn language_ids(&self) -> HashMap { diff --git a/crates/zed/src/languages/tailwind.rs b/crates/zed/src/languages/tailwind.rs index 0dfa700b01..5c39ee0efd 100644 --- a/crates/zed/src/languages/tailwind.rs +++ b/crates/zed/src/languages/tailwind.rs @@ -1,10 +1,7 @@ use anyhow::{anyhow, Result}; use async_trait::async_trait; use collections::HashMap; -use futures::{ - future::{self, BoxFuture}, - FutureExt, StreamExt, -}; +use futures::StreamExt; use gpui::AppContext; use language::{LanguageServerName, LspAdapter, LspAdapterDelegate}; use lsp::LanguageServerBinary; @@ -107,17 +104,12 @@ impl LspAdapter for TailwindLspAdapter { })) } - fn workspace_configuration( - &self, - _workspace_root: &Path, - _: &mut AppContext, - ) -> BoxFuture<'static, Value> { - future::ready(json!({ + fn workspace_configuration(&self, _workspace_root: &Path, _: &mut AppContext) -> Value { + json!({ "tailwindCSS": { "emmetCompletions": true, } - })) - .boxed() + }) } async fn language_ids(&self) -> HashMap { diff --git a/crates/zed/src/languages/typescript.rs b/crates/zed/src/languages/typescript.rs index de25f2ead8..133e9095c0 100644 --- a/crates/zed/src/languages/typescript.rs +++ b/crates/zed/src/languages/typescript.rs @@ -3,7 +3,6 @@ use async_compression::futures::bufread::GzipDecoder; use async_tar::Archive; use async_trait::async_trait; use collections::HashMap; -use futures::{future::BoxFuture, FutureExt}; use gpui::AppContext; use language::{LanguageServerName, LspAdapter, LspAdapterDelegate}; use lsp::{CodeActionKind, LanguageServerBinary}; @@ -13,7 +12,6 @@ use smol::{fs, io::BufReader, stream::StreamExt}; use std::{ any::Any, ffi::OsString, - future, path::{Path, PathBuf}, sync::Arc, }; @@ -212,12 +210,8 @@ impl EsLintLspAdapter { #[async_trait] impl LspAdapter for EsLintLspAdapter { - fn workspace_configuration( - &self, - workspace_root: &Path, - _: &mut AppContext, - ) -> BoxFuture<'static, Value> { - future::ready(json!({ + fn workspace_configuration(&self, workspace_root: &Path, _: &mut AppContext) -> Value { + json!({ "": { "validate": "on", "rulesCustomizations": [], @@ -230,8 +224,7 @@ impl LspAdapter for EsLintLspAdapter { .unwrap_or_else(|| workspace_root.as_os_str()), }, } - })) - .boxed() + }) } async fn name(&self) -> LanguageServerName { diff --git a/crates/zed/src/languages/yaml.rs b/crates/zed/src/languages/yaml.rs index fbed9ba78f..6f29f62f8e 100644 --- a/crates/zed/src/languages/yaml.rs +++ b/crates/zed/src/languages/yaml.rs @@ -1,6 +1,6 @@ use anyhow::{anyhow, Result}; use async_trait::async_trait; -use futures::{future::BoxFuture, FutureExt, StreamExt}; +use futures::StreamExt; use gpui::AppContext; use language::{ language_settings::all_language_settings, LanguageServerName, LspAdapter, LspAdapterDelegate, @@ -12,7 +12,6 @@ use smol::fs; use std::{ any::Any, ffi::OsString, - future, path::{Path, PathBuf}, sync::Arc, }; @@ -93,24 +92,17 @@ impl LspAdapter for YamlLspAdapter { ) -> Option { get_cached_server_binary(container_dir, &*self.node).await } - fn workspace_configuration( - &self, - _workspace_root: &Path, - cx: &mut AppContext, - ) -> BoxFuture<'static, Value> { - let tab_size = all_language_settings(None, cx) - .language(Some("YAML")) - .tab_size; - - future::ready(serde_json::json!({ + fn workspace_configuration(&self, _workspace_root: &Path, cx: &mut AppContext) -> Value { + serde_json::json!({ "yaml": { "keyOrdering": false }, "[yaml]": { - "editor.tabSize": tab_size, + "editor.tabSize": all_language_settings(None, cx) + .language(Some("YAML")) + .tab_size, } - })) - .boxed() + }) } } From 5e0cabc3949df87bb6fad5cd2d52ab6f0267abb0 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 23 Jan 2024 13:10:30 +0200 Subject: [PATCH 3/3] Create a special worktree for settings files To avoid LSP server restarts/leaks when those are being opened co-authored-by: Piotr --- crates/gpui/src/text_system.rs | 4 ---- crates/project/src/project.rs | 2 +- crates/workspace/src/workspace.rs | 3 +-- crates/zed/src/zed.rs | 39 +++++++++++++++++++++++++------ 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/crates/gpui/src/text_system.rs b/crates/gpui/src/text_system.rs index a43b96ef90..6cc56e306b 100644 --- a/crates/gpui/src/text_system.rs +++ b/crates/gpui/src/text_system.rs @@ -70,10 +70,6 @@ impl TextSystem { /// Get a list of all available font names from the operating system. pub fn all_font_names(&self) -> Vec { - eprintln!( - "~~~~~~~~~~~~~ all_font_names called {}", - std::backtrace::Backtrace::capture() - ); let mut names: BTreeSet<_> = self .platform_text_system .all_font_names() diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 285d86742b..266acc8299 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -973,7 +973,7 @@ impl Project { } // Start all the newly-enabled language servers. - for (worktree, language) in dbg!(language_servers_to_start) { + for (worktree, language) in language_servers_to_start { let worktree_path = worktree.read(cx).abs_path(); self.start_language_servers(&worktree, worktree_path, language, cx); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index c51852b511..a552d9c5af 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1355,7 +1355,7 @@ impl Workspace { Some(visible) => match this .update(&mut cx, |this, cx| { Workspace::project_path_for_path( - dbg!(this.project.clone()), + this.project.clone(), abs_path, visible, cx, @@ -1368,7 +1368,6 @@ impl Workspace { }, None => None, }; - dbg!(&project_path); let this = this.clone(); let abs_path = abs_path.clone(); diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index e2b64a1c93..b94881d1b0 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -20,9 +20,10 @@ use assets::Assets; use futures::{channel::mpsc, select_biased, StreamExt}; use project_panel::ProjectPanel; use quick_action_bar::QuickActionBar; +use rope::Rope; use search::project_search::ProjectSearchBar; use settings::{initial_local_settings_content, KeymapFile, Settings, SettingsStore}; -use std::{borrow::Cow, ops::Deref, sync::Arc}; +use std::{borrow::Cow, ops::Deref, path::Path, sync::Arc}; use terminal_view::terminal_panel::{self, TerminalPanel}; use util::{ asset_str, @@ -256,16 +257,16 @@ pub fn initialize_workspace(app_state: Arc, cx: &mut AppContext) { ) .register_action( move |_: &mut Workspace, _: &OpenKeymap, cx: &mut ViewContext| { - create_and_open_local_file(&paths::KEYMAP, cx, Default::default) - .detach_and_log_err(cx); + open_settings_file(&paths::KEYMAP, Rope::default, cx); }, ) .register_action( move |_: &mut Workspace, _: &OpenSettings, cx: &mut ViewContext| { - create_and_open_local_file(&paths::SETTINGS, cx, || { - settings::initial_user_settings_content().as_ref().into() - }) - .detach_and_log_err(cx); + open_settings_file( + &paths::SETTINGS, + || settings::initial_user_settings_content().as_ref().into(), + cx, + ); }, ) .register_action(open_local_settings_file) @@ -723,6 +724,30 @@ fn open_bundled_file( .detach_and_log_err(cx); } +fn open_settings_file( + abs_path: &'static Path, + default_content: impl FnOnce() -> Rope + Send + 'static, + cx: &mut ViewContext, +) { + cx.spawn(|workspace, mut cx| async move { + let (worktree_creation_task, settings_open_task) = + workspace.update(&mut cx, |workspace, cx| { + let worktree_creation_task = workspace.project().update(cx, |project, cx| { + // Set up a dedicated worktree for settings, since otherwise we're dropping and re-starting LSP servers for each file inside on every settings file close/open + // TODO: Do note that all other external files (e.g. drag and drop from OS) still have their worktrees released on file close, causing LSP servers' restarts. + project.find_or_create_local_worktree(paths::CONFIG_DIR.as_path(), false, cx) + }); + let settings_open_task = create_and_open_local_file(&abs_path, cx, default_content); + (worktree_creation_task, settings_open_task) + })?; + + let _ = worktree_creation_task.await?; + let _ = settings_open_task.await?; + anyhow::Ok(()) + }) + .detach_and_log_err(cx); +} + #[cfg(test)] mod tests { use super::*;