ssh remoting: Do not double-register LspAdapters (#18132)

Thorsten Ball created 1 year ago

This fixes the bug with hover tooltips appearing multiple times.

Turns out everytime we receive the `CreateLanguageServer` message we'd
add a new adapter but only have a single server running for all of them.

And we send a `CreateLanguageServer` message everytime you open a
buffer.

What this does is to only add a new adapter if it hasn't already been
registered, which is also what we do locally.


Release Notes:

- N/A

Change summary

crates/language/src/language_registry.rs | 34 ++++++++++++++++++-
crates/project/src/lsp_store.rs          | 44 +++++++++++++-------------
2 files changed, 54 insertions(+), 24 deletions(-)

Detailed changes

crates/language/src/language_registry.rs

@@ -326,13 +326,43 @@ impl LanguageRegistry {
         Some(load_lsp_adapter())
     }
 
-    pub fn register_lsp_adapter(&self, language_name: LanguageName, adapter: Arc<dyn LspAdapter>) {
+    pub fn register_lsp_adapter(
+        &self,
+        language_name: LanguageName,
+        adapter: Arc<dyn LspAdapter>,
+    ) -> Arc<CachedLspAdapter> {
+        let cached = CachedLspAdapter::new(adapter);
         self.state
             .write()
             .lsp_adapters
             .entry(language_name)
             .or_default()
-            .push(CachedLspAdapter::new(adapter));
+            .push(cached.clone());
+        cached
+    }
+
+    pub fn get_or_register_lsp_adapter(
+        &self,
+        language_name: LanguageName,
+        server_name: LanguageServerName,
+        build_adapter: impl FnOnce() -> Arc<dyn LspAdapter> + 'static,
+    ) -> Arc<CachedLspAdapter> {
+        let registered = self
+            .state
+            .write()
+            .lsp_adapters
+            .entry(language_name.clone())
+            .or_default()
+            .iter()
+            .find(|cached_adapter| cached_adapter.name == server_name)
+            .cloned();
+
+        if let Some(found) = registered {
+            found
+        } else {
+            let adapter = build_adapter();
+            self.register_lsp_adapter(language_name, adapter)
+        }
     }
 
     /// Register a fake language server and adapter

crates/project/src/lsp_store.rs

@@ -4475,7 +4475,7 @@ impl LspStore {
         mut cx: AsyncAppContext,
     ) -> Result<proto::Ack> {
         let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id);
-        let name = LanguageServerName::from_proto(envelope.payload.name);
+        let server_name = LanguageServerName::from_proto(envelope.payload.name);
 
         let binary = envelope
             .payload
@@ -4494,6 +4494,14 @@ impl LspStore {
         let matcher: LanguageMatcher = serde_json::from_str(&language.matcher)?;
 
         this.update(&mut cx, |this, cx| {
+            let Some(worktree) = this
+                .worktree_store
+                .read(cx)
+                .worktree_for_id(worktree_id, cx)
+            else {
+                return Err(anyhow!("worktree not found"));
+            };
+
             this.languages
                 .register_language(language_name.clone(), None, matcher.clone(), {
                     let language_name = language_name.clone();
@@ -4513,28 +4521,20 @@ impl LspStore {
                 .spawn(this.languages.language_for_name(language_name.0.as_ref()))
                 .detach();
 
-            let adapter = Arc::new(SshLspAdapter::new(
-                name,
-                binary,
-                envelope.payload.initialization_options,
-                envelope.payload.code_action_kinds,
-            ));
-
-            this.languages
-                .register_lsp_adapter(language_name.clone(), adapter.clone());
-            let Some(worktree) = this
-                .worktree_store
-                .read(cx)
-                .worktree_for_id(worktree_id, cx)
-            else {
-                return Err(anyhow!("worktree not found"));
-            };
-            this.start_language_server(
-                &worktree,
-                CachedLspAdapter::new(adapter),
-                language_name,
-                cx,
+            let adapter = this.languages.get_or_register_lsp_adapter(
+                language_name.clone(),
+                server_name.clone(),
+                || {
+                    Arc::new(SshLspAdapter::new(
+                        server_name,
+                        binary,
+                        envelope.payload.initialization_options,
+                        envelope.payload.code_action_kinds,
+                    ))
+                },
             );
+
+            this.start_language_server(&worktree, adapter, language_name, cx);
             Ok(())
         })??;
         Ok(proto::Ack {})