From 080a085fa796fe2b71b5932b704d465695fba824 Mon Sep 17 00:00:00 2001 From: woojiq <122799969+woojiq@users.noreply.github.com> Date: Tue, 26 Sep 2023 23:12:19 +0300 Subject: [PATCH] Filter out language servers which fail to spawn (#8374) --- helix-lsp/src/lib.rs | 40 +++++++++-------- helix-view/src/editor.rs | 95 ++++++++++++++++++++++------------------ 2 files changed, 74 insertions(+), 61 deletions(-) diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index d29a21440..a4be923b2 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -749,36 +749,40 @@ impl Registry { } } - pub fn get( - &mut self, - language_config: &LanguageConfiguration, - doc_path: Option<&std::path::PathBuf>, - root_dirs: &[PathBuf], + pub fn get<'a>( + &'a mut self, + language_config: &'a LanguageConfiguration, + doc_path: Option<&'a std::path::PathBuf>, + root_dirs: &'a [PathBuf], enable_snippets: bool, - ) -> Result>> { - language_config - .language_servers - .iter() - .map(|LanguageServerFeatures { name, .. }| { + ) -> impl Iterator>)> + 'a { + language_config.language_servers.iter().map( + move |LanguageServerFeatures { name, .. }| { if let Some(clients) = self.inner.get(name) { if let Some((_, client)) = clients.iter().enumerate().find(|(i, client)| { client.try_add_doc(&language_config.roots, root_dirs, doc_path, *i == 0) }) { - return Ok((name.to_owned(), client.clone())); + return (name.to_owned(), Ok(client.clone())); } } - let client = self.start_client( + match self.start_client( name.clone(), language_config, doc_path, root_dirs, enable_snippets, - )?; - let clients = self.inner.entry(name.clone()).or_default(); - clients.push(client.clone()); - Ok((name.clone(), client)) - }) - .collect() + ) { + Ok(client) => { + self.inner + .entry(name.to_owned()) + .or_default() + .push(client.clone()); + (name.clone(), Ok(client)) + } + Err(err) => (name.to_owned(), Err(err)), + } + }, + ) } pub fn iter_clients(&self) -> impl Iterator> { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 2265633df..7af28ccc6 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1188,71 +1188,80 @@ impl Editor { } /// Refreshes the language server for a given document - pub fn refresh_language_servers(&mut self, doc_id: DocumentId) -> Option<()> { + pub fn refresh_language_servers(&mut self, doc_id: DocumentId) { self.launch_language_servers(doc_id) } /// Launch a language server for a given document - fn launch_language_servers(&mut self, doc_id: DocumentId) -> Option<()> { + fn launch_language_servers(&mut self, doc_id: DocumentId) { if !self.config().lsp.enable { - return None; + return; } // if doc doesn't have a URL it's a scratch buffer, ignore it - let doc = self.documents.get_mut(&doc_id)?; - let doc_url = doc.url()?; + let Some(doc) = self.documents.get_mut(&doc_id) else { + return; + }; + let Some(doc_url) = doc.url() else { + return; + }; let (lang, path) = (doc.language.clone(), doc.path().cloned()); let config = doc.config.load(); let root_dirs = &config.workspace_lsp_roots; - // try to find language servers based on the language name - let language_servers = lang.as_ref().and_then(|language| { + // store only successfully started language servers + let language_servers = lang.as_ref().map_or_else(HashMap::default, |language| { self.language_servers .get(language, path.as_ref(), root_dirs, config.lsp.snippets) - .map_err(|e| { - log::error!( - "Failed to initialize the language servers for `{}` {{ {} }}", - language.scope(), - e - ) + .filter_map(|(lang, client)| match client { + Ok(client) => Some((lang, client)), + Err(err) => { + log::error!( + "Failed to initialize the language servers for `{}` - `{}` {{ {} }}", + language.scope(), + lang, + err + ); + None + } }) - .ok() + .collect::>() }); - if let Some(language_servers) = language_servers { - let language_id = doc.language_id().map(ToOwned::to_owned).unwrap_or_default(); - - // only spawn new language servers if the servers aren't the same - - let doc_language_servers_not_in_registry = - doc.language_servers.iter().filter(|(name, doc_ls)| { - language_servers - .get(*name) - .map_or(true, |ls| ls.id() != doc_ls.id()) - }); + if language_servers.is_empty() { + return; + } - for (_, language_server) in doc_language_servers_not_in_registry { - tokio::spawn(language_server.text_document_did_close(doc.identifier())); - } + let language_id = doc.language_id().map(ToOwned::to_owned).unwrap_or_default(); - let language_servers_not_in_doc = language_servers.iter().filter(|(name, ls)| { - doc.language_servers + // only spawn new language servers if the servers aren't the same + let doc_language_servers_not_in_registry = + doc.language_servers.iter().filter(|(name, doc_ls)| { + language_servers .get(*name) - .map_or(true, |doc_ls| ls.id() != doc_ls.id()) + .map_or(true, |ls| ls.id() != doc_ls.id()) }); - for (_, language_server) in language_servers_not_in_doc { - // TODO: this now races with on_init code if the init happens too quickly - tokio::spawn(language_server.text_document_did_open( - doc_url.clone(), - doc.version(), - doc.text(), - language_id.clone(), - )); - } + for (_, language_server) in doc_language_servers_not_in_registry { + tokio::spawn(language_server.text_document_did_close(doc.identifier())); + } + + let language_servers_not_in_doc = language_servers.iter().filter(|(name, ls)| { + doc.language_servers + .get(*name) + .map_or(true, |doc_ls| ls.id() != doc_ls.id()) + }); - doc.language_servers = language_servers; + for (_, language_server) in language_servers_not_in_doc { + // TODO: this now races with on_init code if the init happens too quickly + tokio::spawn(language_server.text_document_did_open( + doc_url.clone(), + doc.version(), + doc.text(), + language_id.clone(), + )); } - Some(()) + + doc.language_servers = language_servers; } fn _refresh(&mut self) { @@ -1454,7 +1463,7 @@ impl Editor { doc.set_version_control_head(self.diff_providers.get_current_head_name(&path)); let id = self.new_document(doc); - let _ = self.launch_language_servers(id); + self.launch_language_servers(id); id };