From 4e1962503195af463875f1cb9c882ab97e41fbb9 Mon Sep 17 00:00:00 2001 From: woojiq Date: Tue, 8 Aug 2023 20:47:36 +0300 Subject: [PATCH 1/3] fix: all lsp terminates if at least one failed to start --- helix-lsp/src/lib.rs | 13 ++++++++----- helix-view/src/editor.rs | 26 ++++++++++++++++---------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index d29a21440d39..eb2aca6ca691 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -755,7 +755,7 @@ impl Registry { doc_path: Option<&std::path::PathBuf>, root_dirs: &[PathBuf], enable_snippets: bool, - ) -> Result>> { + ) -> HashMap>> { language_config .language_servers .iter() @@ -764,19 +764,22 @@ impl Registry { 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( + let client = match self.start_client( name.clone(), language_config, doc_path, root_dirs, enable_snippets, - )?; + ) { + Ok(client) => client, + Err(err) => return (name.to_owned(), Err(err)), + }; let clients = self.inner.entry(name.clone()).or_default(); clients.push(client.clone()); - Ok((name.clone(), client)) + (name.clone(), Ok(client)) }) .collect() } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 2265633dfced..4f00661acf61 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1204,21 +1204,27 @@ impl Editor { 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 - ) + .into_iter() + .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 { + if !language_servers.is_empty() { 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 From 8290ae1b138d5efbe5691c54f70a245f10944686 Mon Sep 17 00:00:00 2001 From: woojiq Date: Mon, 25 Sep 2023 11:55:56 +0300 Subject: [PATCH 2/3] remove option; return iterator --- helix-lsp/src/lib.rs | 22 ++++++------ helix-view/src/editor.rs | 73 +++++++++++++++++++++------------------- 2 files changed, 48 insertions(+), 47 deletions(-) diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index eb2aca6ca691..0568c4c5de84 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -749,17 +749,15 @@ 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, - ) -> HashMap>> { - 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) @@ -780,8 +778,8 @@ impl Registry { let clients = self.inner.entry(name.clone()).or_default(); clients.push(client.clone()); (name.clone(), Ok(client)) - }) - .collect() + }, + ) } pub fn iter_clients(&self) -> impl Iterator> { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 4f00661acf61..7af28ccc680a 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1188,18 +1188,22 @@ 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; @@ -1208,7 +1212,6 @@ impl Editor { 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) - .into_iter() .filter_map(|(lang, client)| match client { Ok(client) => Some((lang, client)), Err(err) => { @@ -1224,41 +1227,41 @@ impl Editor { .collect::>() }); - if !language_servers.is_empty() { - 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) { @@ -1460,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 }; From adbff52396196b5636b38f9c2bb7f6c43db3a3a1 Mon Sep 17 00:00:00 2001 From: woojiq Date: Tue, 26 Sep 2023 21:00:19 +0300 Subject: [PATCH 3/3] move all returns to match --- helix-lsp/src/lib.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 0568c4c5de84..a4be923b2f89 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -765,19 +765,22 @@ impl Registry { return (name.to_owned(), Ok(client.clone())); } } - let client = match self.start_client( + match self.start_client( name.clone(), language_config, doc_path, root_dirs, enable_snippets, ) { - Ok(client) => client, - Err(err) => return (name.to_owned(), Err(err)), - }; - let clients = self.inner.entry(name.clone()).or_default(); - clients.push(client.clone()); - (name.clone(), Ok(client)) + Ok(client) => { + self.inner + .entry(name.to_owned()) + .or_default() + .push(client.clone()); + (name.clone(), Ok(client)) + } + Err(err) => (name.to_owned(), Err(err)), + } }, ) }