From efe8b8b6d0ddeed8bba92a38e04f8b7f32e387a9 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Jul 2023 20:46:45 +0300 Subject: [PATCH 1/2] Revert "Fix language servers improper restarts" This reverts commit 91832c8cd8de4743a5c8dad87005a67d9601d7e5. --- crates/language/src/language.rs | 28 +--------------------------- crates/project/src/project.rs | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 642f5469cdc16133ecd2d8556215dcd6b359cdf7..976d8062ea8042c6c95a885b3c943c31f8a2d7be 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -90,8 +90,7 @@ pub struct LanguageServerName(pub Arc); /// once at startup, and caches the results. pub struct CachedLspAdapter { pub name: LanguageServerName, - initialization_options: Option, - initialization_overrides: Mutex>, + pub initialization_options: Option, pub disk_based_diagnostic_sources: Vec, pub disk_based_diagnostics_progress_token: Option, pub language_ids: HashMap, @@ -110,7 +109,6 @@ impl CachedLspAdapter { Arc::new(CachedLspAdapter { name, initialization_options, - initialization_overrides: Mutex::new(None), disk_based_diagnostic_sources, disk_based_diagnostics_progress_token, language_ids, @@ -210,30 +208,6 @@ impl CachedLspAdapter { ) -> Option { self.adapter.label_for_symbol(name, kind, language).await } - - pub fn update_initialization_overrides(&self, new: Option<&Value>) -> bool { - let mut current = self.initialization_overrides.lock(); - if current.as_ref() != new { - *current = new.cloned(); - true - } else { - false - } - } - - pub fn initialization_options(&self) -> Option { - let initialization_options = self.initialization_options.as_ref(); - let override_options = self.initialization_overrides.lock().clone(); - match (initialization_options, override_options) { - (None, override_options) => override_options, - (initialization_options, None) => initialization_options.cloned(), - (Some(initialization_options), Some(override_options)) => { - let mut initialization_options = initialization_options.clone(); - merge_json_value_into(override_options, &mut initialization_options); - Some(initialization_options) - } - } - } } pub trait LspAdapterDelegate: Send + Sync { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index dc4c8852ddc48751adc4f041d1cc216bcbf2eb92..81db0c7ed7d3b6ffe61df7104e53796d9dd43b54 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -78,8 +78,8 @@ use std::{ use terminals::Terminals; use text::Anchor; use util::{ - debug_panic, defer, http::HttpClient, paths::LOCAL_SETTINGS_RELATIVE_PATH, post_inc, ResultExt, - TryFutureExt as _, + debug_panic, defer, http::HttpClient, merge_json_value_into, + paths::LOCAL_SETTINGS_RELATIVE_PATH, post_inc, ResultExt, TryFutureExt as _, }; pub use fs::*; @@ -800,7 +800,7 @@ impl Project { .lsp .get(&adapter.name.0) .and_then(|s| s.initialization_options.as_ref()); - if adapter.update_initialization_overrides(new_lsp_settings) { + if adapter.initialization_options.as_ref() != new_lsp_settings { language_servers_to_restart.push((worktree, Arc::clone(language))); } } @@ -2545,13 +2545,20 @@ impl Project { let project_settings = settings::get::(cx); let lsp = project_settings.lsp.get(&adapter.name.0); let override_options = lsp.map(|s| s.initialization_options.clone()).flatten(); - adapter.update_initialization_overrides(override_options.as_ref()); + + let mut initialization_options = adapter.initialization_options.clone(); + match (&mut initialization_options, override_options) { + (Some(initialization_options), Some(override_options)) => { + merge_json_value_into(override_options, initialization_options); + } + (None, override_options) => initialization_options = override_options, + _ => {} + } let server_id = pending_server.server_id; let container_dir = pending_server.container_dir.clone(); let state = LanguageServerState::Starting({ let adapter = adapter.clone(); - let initialization_options = adapter.initialization_options(); let server_name = adapter.name.0.clone(); let languages = self.languages.clone(); let language = language.clone(); From 4b4d049b0a7b5ce278051202c82dec09c898ae50 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Jul 2023 21:29:47 +0300 Subject: [PATCH 2/2] Refactor LSP restart logic Instead of storing `initialization_options` in every LSP adapter as before, store previous LSP settings in `Project` entirely. This way, we can later have use multiple different project configurations per single LSP with its associated adapter. co-authored-by: Max Brunsfeld --- crates/command_palette/src/command_palette.rs | 1 + crates/project/src/project.rs | 30 ++++++++++++++----- crates/terminal_view/src/terminal_view.rs | 1 + crates/workspace/src/pane.rs | 1 + 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index aec876bd78ade4312493c2853f51ac09f8dcb489..77dde09875910fdad88abd1a8ca04dc25e412f86 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -369,6 +369,7 @@ mod tests { editor::init(cx); workspace::init(app_state.clone(), cx); init(cx); + Project::init_settings(cx); app_state }) } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 81db0c7ed7d3b6ffe61df7104e53796d9dd43b54..364b19e3a9fc26f51186e99b51184d47dfedbaec 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -50,7 +50,7 @@ use lsp::{ }; use lsp_command::*; use postage::watch; -use project_settings::ProjectSettings; +use project_settings::{LspSettings, ProjectSettings}; use rand::prelude::*; use search::SearchQuery; use serde::Serialize; @@ -149,6 +149,7 @@ pub struct Project { _maintain_workspace_config: Task<()>, terminals: Terminals, copilot_enabled: bool, + current_lsp_settings: HashMap, LspSettings>, } struct DelayedDebounced { @@ -614,6 +615,7 @@ impl Project { local_handles: Vec::new(), }, copilot_enabled: Copilot::global(cx).is_some(), + current_lsp_settings: settings::get::(cx).lsp.clone(), } }) } @@ -706,6 +708,7 @@ impl Project { local_handles: Vec::new(), }, copilot_enabled: Copilot::global(cx).is_some(), + current_lsp_settings: settings::get::(cx).lsp.clone(), }; for worktree in worktrees { let _ = this.add_worktree(&worktree, cx); @@ -779,7 +782,9 @@ impl Project { let mut language_servers_to_stop = Vec::new(); let mut language_servers_to_restart = Vec::new(); let languages = self.languages.to_vec(); - let project_settings = settings::get::(cx).clone(); + + let new_lsp_settings = settings::get::(cx).lsp.clone(); + let current_lsp_settings = &self.current_lsp_settings; for (worktree_id, started_lsp_name) in self.language_server_ids.keys() { let language = languages.iter().find_map(|l| { let adapter = l @@ -796,16 +801,25 @@ impl Project { if !language_settings(Some(language), file.as_ref(), cx).enable_language_server { language_servers_to_stop.push((*worktree_id, started_lsp_name.clone())); } else if let Some(worktree) = worktree { - let new_lsp_settings = project_settings - .lsp - .get(&adapter.name.0) - .and_then(|s| s.initialization_options.as_ref()); - if adapter.initialization_options.as_ref() != new_lsp_settings { - language_servers_to_restart.push((worktree, Arc::clone(language))); + let server_name = &adapter.name.0; + match ( + current_lsp_settings.get(server_name), + new_lsp_settings.get(server_name), + ) { + (None, None) => {} + (Some(_), None) | (None, Some(_)) => { + language_servers_to_restart.push((worktree, Arc::clone(language))); + } + (Some(current_lsp_settings), Some(new_lsp_settings)) => { + if current_lsp_settings != new_lsp_settings { + language_servers_to_restart.push((worktree, Arc::clone(language))); + } + } } } } } + self.current_lsp_settings = new_lsp_settings; // Stop all newly-disabled language servers. for (worktree_id, adapter_name) in language_servers_to_stop { diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index c40a1a7ccd3d493e16e6a4b03a2d36e21a0b0c14..f7963f6e5f34b007850efc2d9cdf777f8055fccf 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -907,6 +907,7 @@ mod tests { let params = cx.update(AppState::test); cx.update(|cx| { theme::init((), cx); + Project::init_settings(cx); language::init(cx); }); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 6a20fab9a275571bd52a55b804e7d6b6407016e6..8e6e10748824b373fc93ab8a6af943c9df836e0d 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -2316,6 +2316,7 @@ mod tests { cx.set_global(SettingsStore::test(cx)); theme::init((), cx); crate::init_settings(cx); + Project::init_settings(cx); }); }