Filter out language servers which fail to spawn (#8374)

2 files changed, 74 insertions, 61 deletions

diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs
index d29a2144..a4be923b 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<HashMap<LanguageServerName, Arc<Client>>> {
-        language_config
-            .language_servers
-            .iter()
-            .map(|LanguageServerFeatures { name, .. }| {
+    ) -> impl Iterator<Item = (LanguageServerName, Result<Arc<Client>>)> + '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<Item = &Arc<Client>> {
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index 2265633d..7af28ccc 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::<HashMap<_, _>>()
         });
 
-        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
         };