Fix lang servers status set to Downloading when checking version (#22292

) This message has confused me many times too: we printed the status as "Downloading" when we were only checking whether we need to install a given version of a language server. This fixes the issue for Node-based language servers where we had the same check in all implementations. Closes #22241 Release Notes: - Fixed some language servers reporting status as "Downloading..." when only a version check was being done.
zed-industries · Dec 20, 2024 · d824bae · d824bae
1 parent 8a858fe
commit d824bae
Show file tree

Hide file tree

Showing 7 changed files with 228 additions and 85 deletions.
diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs
@@ -385,6 +385,15 @@ pub trait LspAdapter: 'static + Send + Sync {
         None
     }
 
+    async fn check_if_version_installed(
+        &self,
+        _version: &(dyn 'static + Send + Any),
+        _container_dir: &PathBuf,
+        _delegate: &dyn LspAdapterDelegate,
+    ) -> Option<LanguageServerBinary> {
+        None
+    }
+
     async fn fetch_server_binary(
         &self,
         latest_version: Box<dyn 'static + Send + Any>,
@@ -516,14 +525,23 @@ async fn try_fetch_server_binary<L: LspAdapter + 'static + Send + Sync + ?Sized>
         .fetch_latest_server_version(delegate.as_ref())
         .await?;
 
-    log::info!("downloading language server {:?}", name.0);
-    delegate.update_status(adapter.name(), LanguageServerBinaryStatus::Downloading);
-    let binary = adapter
-        .fetch_server_binary(latest_version, container_dir, delegate.as_ref())
-        .await;
+    if let Some(binary) = adapter
+        .check_if_version_installed(latest_version.as_ref(), &container_dir, delegate.as_ref())
+        .await
+    {
+        log::info!("language server {:?} is already installed", name.0);
+        delegate.update_status(name.clone(), LanguageServerBinaryStatus::None);
+        Ok(binary)
+    } else {
+        log::info!("downloading language server {:?}", name.0);
+        delegate.update_status(adapter.name(), LanguageServerBinaryStatus::Downloading);
+        let binary = adapter
+            .fetch_server_binary(latest_version, container_dir, delegate.as_ref())
+            .await;
 
-    delegate.update_status(name.clone(), LanguageServerBinaryStatus::None);
-    binary
+        delegate.update_status(name.clone(), LanguageServerBinaryStatus::None);
+        binary
+    }
 }
 
 #[derive(Clone, Debug, Default, PartialEq, Eq)]

diff --git a/crates/languages/src/css.rs b/crates/languages/src/css.rs
@@ -26,6 +26,7 @@ pub struct CssLspAdapter {
 }
 
 impl CssLspAdapter {
+    const PACKAGE_NAME: &str = "vscode-langservers-extracted";
     pub fn new(node: NodeRuntime) -> Self {
         CssLspAdapter { node }
     }
@@ -56,18 +57,13 @@ impl LspAdapter for CssLspAdapter {
     ) -> Result<LanguageServerBinary> {
         let latest_version = latest_version.downcast::<String>().unwrap();
         let server_path = container_dir.join(SERVER_PATH);
-        let package_name = "vscode-langservers-extracted";
 
-        let should_install_language_server = self
-            .node
-            .should_install_npm_package(package_name, &server_path, &container_dir, &latest_version)
-            .await;
-
-        if should_install_language_server {
-            self.node
-                .npm_install_packages(&container_dir, &[(package_name, latest_version.as_str())])
-                .await?;
-        }
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[(Self::PACKAGE_NAME, latest_version.as_str())],
+            )
+            .await?;
 
         Ok(LanguageServerBinary {
             path: self.node.binary_path().await?,
@@ -76,6 +72,31 @@ impl LspAdapter for CssLspAdapter {
         })
     }
 
+    async fn check_if_version_installed(
+        &self,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Option<LanguageServerBinary> {
+        let version = version.downcast_ref::<String>().unwrap();
+        let server_path = container_dir.join(SERVER_PATH);
+
+        let should_install_language_server = self
+            .node
+            .should_install_npm_package(Self::PACKAGE_NAME, &server_path, &container_dir, &version)
+            .await;
+
+        if should_install_language_server {
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: server_binary_arguments(&server_path),
+            })
+        }
+    }
+
     async fn cached_server_binary(
         &self,
         container_dir: PathBuf,

diff --git a/crates/languages/src/json.rs b/crates/languages/src/json.rs
@@ -64,6 +64,8 @@ pub struct JsonLspAdapter {
 }
 
 impl JsonLspAdapter {
+    const PACKAGE_NAME: &str = "vscode-langservers-extracted";
+
     pub fn new(node: NodeRuntime, languages: Arc<LanguageRegistry>) -> Self {
         Self {
             node,
@@ -142,31 +144,51 @@ impl LspAdapter for JsonLspAdapter {
     ) -> Result<Box<dyn 'static + Send + Any>> {
         Ok(Box::new(
             self.node
-                .npm_package_latest_version("vscode-langservers-extracted")
+                .npm_package_latest_version(Self::PACKAGE_NAME)
                 .await?,
         ) as Box<_>)
     }
 
-    async fn fetch_server_binary(
+    async fn check_if_version_installed(
         &self,
-        latest_version: Box<dyn 'static + Send + Any>,
-        container_dir: PathBuf,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
         _: &dyn LspAdapterDelegate,
-    ) -> Result<LanguageServerBinary> {
-        let latest_version = latest_version.downcast::<String>().unwrap();
+    ) -> Option<LanguageServerBinary> {
+        let version = version.downcast_ref::<String>().unwrap();
         let server_path = container_dir.join(SERVER_PATH);
-        let package_name = "vscode-langservers-extracted";
 
         let should_install_language_server = self
             .node
-            .should_install_npm_package(package_name, &server_path, &container_dir, &latest_version)
+            .should_install_npm_package(Self::PACKAGE_NAME, &server_path, &container_dir, &version)
             .await;
 
         if should_install_language_server {
-            self.node
-                .npm_install_packages(&container_dir, &[(package_name, latest_version.as_str())])
-                .await?;
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: server_binary_arguments(&server_path),
+            })
         }
+    }
+
+    async fn fetch_server_binary(
+        &self,
+        latest_version: Box<dyn 'static + Send + Any>,
+        container_dir: PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Result<LanguageServerBinary> {
+        let latest_version = latest_version.downcast::<String>().unwrap();
+        let server_path = container_dir.join(SERVER_PATH);
+
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[(Self::PACKAGE_NAME, latest_version.as_str())],
+            )
+            .await?;
 
         Ok(LanguageServerBinary {
             path: self.node.binary_path().await?,

diff --git a/crates/languages/src/python.rs b/crates/languages/src/python.rs
@@ -117,30 +117,48 @@ impl LspAdapter for PythonLspAdapter {
         let latest_version = latest_version.downcast::<String>().unwrap();
         let server_path = container_dir.join(SERVER_PATH);
 
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[(Self::SERVER_NAME.as_ref(), latest_version.as_str())],
+            )
+            .await?;
+
+        Ok(LanguageServerBinary {
+            path: self.node.binary_path().await?,
+            env: None,
+            arguments: server_binary_arguments(&server_path),
+        })
+    }
+
+    async fn check_if_version_installed(
+        &self,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Option<LanguageServerBinary> {
+        let version = version.downcast_ref::<String>().unwrap();
+        let server_path = container_dir.join(SERVER_PATH);
+
         let should_install_language_server = self
             .node
             .should_install_npm_package(
                 Self::SERVER_NAME.as_ref(),
                 &server_path,
                 &container_dir,
-                &latest_version,
+                &version,
             )
             .await;
 
         if should_install_language_server {
-            self.node
-                .npm_install_packages(
-                    &container_dir,
-                    &[(Self::SERVER_NAME.as_ref(), latest_version.as_str())],
-                )
-                .await?;
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: server_binary_arguments(&server_path),
+            })
         }
-
-        Ok(LanguageServerBinary {
-            path: self.node.binary_path().await?,
-            env: None,
-            arguments: server_binary_arguments(&server_path),
-        })
     }
 
     async fn cached_server_binary(

diff --git a/crates/languages/src/tailwind.rs b/crates/languages/src/tailwind.rs
@@ -34,6 +34,7 @@ pub struct TailwindLspAdapter {
 impl TailwindLspAdapter {
     const SERVER_NAME: LanguageServerName =
         LanguageServerName::new_static("tailwindcss-language-server");
+    const PACKAGE_NAME: &str = "@tailwindcss/language-server";
 
     pub fn new(node: NodeRuntime) -> Self {
         TailwindLspAdapter { node }
@@ -52,7 +53,7 @@ impl LspAdapter for TailwindLspAdapter {
     ) -> Result<Box<dyn 'static + Any + Send>> {
         Ok(Box::new(
             self.node
-                .npm_package_latest_version("@tailwindcss/language-server")
+                .npm_package_latest_version(Self::PACKAGE_NAME)
                 .await?,
         ) as Box<_>)
     }
@@ -65,18 +66,13 @@ impl LspAdapter for TailwindLspAdapter {
     ) -> Result<LanguageServerBinary> {
         let latest_version = latest_version.downcast::<String>().unwrap();
         let server_path = container_dir.join(SERVER_PATH);
-        let package_name = "@tailwindcss/language-server";
 
-        let should_install_language_server = self
-            .node
-            .should_install_npm_package(package_name, &server_path, &container_dir, &latest_version)
-            .await;
-
-        if should_install_language_server {
-            self.node
-                .npm_install_packages(&container_dir, &[(package_name, latest_version.as_str())])
-                .await?;
-        }
+        self.node
+            .npm_install_packages(
+                &container_dir,
+                &[(Self::PACKAGE_NAME, latest_version.as_str())],
+            )
+            .await?;
 
         Ok(LanguageServerBinary {
             path: self.node.binary_path().await?,
@@ -85,6 +81,31 @@ impl LspAdapter for TailwindLspAdapter {
         })
     }
 
+    async fn check_if_version_installed(
+        &self,
+        version: &(dyn 'static + Send + Any),
+        container_dir: &PathBuf,
+        _: &dyn LspAdapterDelegate,
+    ) -> Option<LanguageServerBinary> {
+        let version = version.downcast_ref::<String>().unwrap();
+        let server_path = container_dir.join(SERVER_PATH);
+
+        let should_install_language_server = self
+            .node
+            .should_install_npm_package(Self::PACKAGE_NAME, &server_path, &container_dir, &version)
+            .await;
+
+        if should_install_language_server {
+            None
+        } else {
+            Some(LanguageServerBinary {
+                path: self.node.binary_path().await.ok()?,
+                env: None,
+                arguments: server_binary_arguments(&server_path),
+            })
+        }
+    }
+
     async fn cached_server_binary(
         &self,
         container_dir: PathBuf,