Refactor doc language servers to a HashMap, and the config to use a Vec to retain order

2 files changed, 54 insertions, 52 deletions

diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index 49eb13a0..27f5d279 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -180,7 +180,7 @@ pub struct Document {
     pub(crate) modified_since_accessed: bool,
 
     diagnostics: Vec<Diagnostic>,
-    language_servers: Vec<Arc<helix_lsp::Client>>,
+    pub(crate) language_servers: HashMap<LanguageServerName, Arc<Client>>,
 
     diff_handle: Option<DiffHandle>,
     version_control_head: Option<Arc<ArcSwap<Box<str>>>>,
@@ -580,7 +580,7 @@ where
     *mut_ref = f(mem::take(mut_ref));
 }
 
-use helix_lsp::{lsp, Client, OffsetEncoding};
+use helix_lsp::{lsp, Client, LanguageServerName, OffsetEncoding};
 use url::Url;
 
 impl Document {
@@ -616,7 +616,7 @@ impl Document {
             last_saved_time: SystemTime::now(),
             last_saved_revision: 0,
             modified_since_accessed: false,
-            language_servers: Vec::new(),
+            language_servers: HashMap::new(),
             diff_handle: None,
             config,
             version_control_head: None,
@@ -850,7 +850,7 @@ impl Document {
                 text: text.clone(),
             };
 
-            for language_server in language_servers {
+            for (_, language_server) in language_servers {
                 if !language_server.is_initialized() {
                     return Ok(event);
                 }
@@ -1006,11 +1006,6 @@ impl Document {
         Ok(())
     }
 
-    /// Set the LSP.
-    pub fn set_language_servers(&mut self, language_servers: Vec<Arc<helix_lsp::Client>>) {
-        self.language_servers = language_servers;
-    }
-
     /// Select text within the [`Document`].
     pub fn set_selection(&mut self, view_id: ViewId, selection: Selection) {
         // TODO: use a transaction?
@@ -1437,16 +1432,17 @@ impl Document {
     }
 
     pub fn language_servers(&self) -> impl Iterator<Item = &helix_lsp::Client> {
-        self.language_servers
-            .iter()
-            .filter_map(|l| if l.is_initialized() { Some(&**l) } else { None })
+        self.language_servers.values().filter_map(|l| {
+            if l.is_initialized() {
+                Some(&**l)
+            } else {
+                None
+            }
+        })
     }
 
     pub fn remove_language_server_by_name(&mut self, name: &str) -> Option<Arc<Client>> {
-        match self.language_servers.iter().position(|l| l.name() == name) {
-            Some(index) => Some(self.language_servers.remove(index)),
-            None => None,
-        }
+        self.language_servers.remove(name)
     }
 
     // TODO filter also based on LSP capabilities?
@@ -1454,12 +1450,15 @@ impl Document {
         &self,
         feature: LanguageServerFeature,
     ) -> impl Iterator<Item = &helix_lsp::Client> {
-        self.language_servers().filter(move |server| {
-            self.language_config()
-                .and_then(|config| config.language_servers.get(server.name()))
-                .map_or(false, |server_features| {
-                    server_features.has_feature(feature)
-                })
+        self.language_config().into_iter().flat_map(move |config| {
+            config.language_servers.iter().filter_map(move |features| {
+                let ls = &**self.language_servers.get(&features.name)?;
+                if ls.is_initialized() && features.has_feature(feature) {
+                    Some(ls)
+                } else {
+                    None
+                }
+            })
         })
     }
 
@@ -1610,7 +1609,10 @@ impl Document {
                 .find(|ls| ls.id() == d.language_server_id)
                 .and_then(|ls| {
                     let config = self.language_config()?;
-                    let features = config.language_servers.get(ls.name())?;
+                    let features = config
+                        .language_servers
+                        .iter()
+                        .find(|features| features.name == ls.name())?;
                     Some(features.has_feature(LanguageServerFeature::Diagnostics))
                 })
                 == Some(true)
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index bca97815..ca2144fd 100644
--- a/helix-view/src/editor.rs
+++ b/helix-view/src/editor.rs
@@ -689,7 +689,7 @@ pub struct WhitespaceCharacters {
 impl Default for WhitespaceCharacters {
     fn default() -> Self {
         Self {
-            space: '·',   // U+00B7
+            space: '·',    // U+00B7
             nbsp: '⍽',    // U+237D
             tab: '→',     // U+2192
             newline: '⏎', // U+23CE
@@ -1103,9 +1103,9 @@ impl Editor {
         if !self.config().lsp.enable {
             return None;
         }
-
         // if doc doesn't have a URL it's a scratch buffer, ignore it
-        let doc = self.document(doc_id)?;
+        let doc = self.documents.get_mut(&doc_id)?;
+        let doc_url = doc.url()?;
         let (lang, path) = (doc.language.clone(), doc.path().cloned());
         let config = doc.config.load();
         let root_dirs = &config.workspace_lsp_roots;
@@ -1124,37 +1124,37 @@ impl Editor {
                 .ok()
         });
 
-        let doc = self.document_mut(doc_id)?;
-        let doc_url = doc.url()?;
-
         if let Some(language_servers) = language_servers {
-            // only spawn new lang servers if the servers aren't the same
-            // TODO simplify?
-            let doc_language_servers = doc.language_servers().collect::<Vec<_>>();
-            let spawn_new_servers = language_servers.len() != doc_language_servers.len()
-                || language_servers
-                    .iter()
-                    .zip(doc_language_servers.iter())
-                    .any(|(l, dl)| l.id() != dl.id());
-            if spawn_new_servers {
-                for doc_language_server in doc_language_servers {
-                    tokio::spawn(doc_language_server.text_document_did_close(doc.identifier()));
-                }
+            let language_id = doc.language_id().map(ToOwned::to_owned).unwrap_or_default();
 
-                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
 
-                for language_server in &language_servers {
-                    // 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(),
-                    ));
-                }
+            let doc_language_servers_not_in_registry =
+                doc.language_servers.iter().filter(|(name, doc_ls)| {
+                    !language_servers.contains_key(*name)
+                        || language_servers[*name].id() != doc_ls.id()
+                });
 
-                doc.set_language_servers(language_servers);
+            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.contains_key(*name)
+                    || doc.language_servers[*name].id() != 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(),
+                ));
+            }
+
+            doc.language_servers = language_servers;
         }
         Some(())
     }