From d824baeeced6c386f7329fca0c7f81c2cc1fd70e Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Fri, 20 Dec 2024 17:59:10 +0100 Subject: [PATCH] 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. --- crates/language/src/language.rs | 32 +++++++++++---- crates/languages/src/css.rs | 43 +++++++++++++++------ crates/languages/src/json.rs | 44 +++++++++++++++------ crates/languages/src/python.rs | 44 ++++++++++++++------- crates/languages/src/tailwind.rs | 45 ++++++++++++++++------ crates/languages/src/typescript.rs | 62 ++++++++++++++++++++---------- crates/languages/src/yaml.rs | 43 +++++++++++++++------ 7 files changed, 228 insertions(+), 85 deletions(-) diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index ec49185d6b1e0..9832c50482a65 100644 --- 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 { + None + } + async fn fetch_server_binary( &self, latest_version: Box, @@ -516,14 +525,23 @@ async fn try_fetch_server_binary .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 index 536f339664793..148f6acced679 100644 --- 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 { let latest_version = latest_version.downcast::().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 { + let version = version.downcast_ref::().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 index 9c6315d2e272b..41ff7e0c8329e 100644 --- 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) -> Self { Self { node, @@ -142,31 +144,51 @@ impl LspAdapter for JsonLspAdapter { ) -> Result> { 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, - container_dir: PathBuf, + version: &(dyn 'static + Send + Any), + container_dir: &PathBuf, _: &dyn LspAdapterDelegate, - ) -> Result { - let latest_version = latest_version.downcast::().unwrap(); + ) -> Option { + let version = version.downcast_ref::().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, + container_dir: PathBuf, + _: &dyn LspAdapterDelegate, + ) -> Result { + let latest_version = latest_version.downcast::().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 index a0465745b52d4..607140e33af0f 100644 --- 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::().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 { + let version = version.downcast_ref::().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 index e2ced0f67f399..02c3dbefc2c1c 100644 --- 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> { 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 { let latest_version = latest_version.downcast::().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 { + let version = version.downcast_ref::().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/typescript.rs b/crates/languages/src/typescript.rs index 076d8d3374893..225ca63538cb2 100644 --- a/crates/languages/src/typescript.rs +++ b/crates/languages/src/typescript.rs @@ -73,6 +73,7 @@ impl TypeScriptLspAdapter { const NEW_SERVER_PATH: &'static str = "node_modules/typescript-language-server/lib/cli.mjs"; const SERVER_NAME: LanguageServerName = LanguageServerName::new_static("typescript-language-server"); + const PACKAGE_NAME: &str = "typescript"; pub fn new(node: NodeRuntime) -> Self { TypeScriptLspAdapter { node } } @@ -114,40 +115,61 @@ impl LspAdapter for TypeScriptLspAdapter { }) as Box<_>) } - async fn fetch_server_binary( + async fn check_if_version_installed( &self, - latest_version: Box, - container_dir: PathBuf, + version: &(dyn 'static + Send + Any), + container_dir: &PathBuf, _: &dyn LspAdapterDelegate, - ) -> Result { - let latest_version = latest_version.downcast::().unwrap(); + ) -> Option { + println!("{:?}", version.type_id()); + let version = version.downcast_ref::().unwrap(); let server_path = container_dir.join(Self::NEW_SERVER_PATH); - let package_name = "typescript"; let should_install_language_server = self .node .should_install_npm_package( - package_name, + Self::PACKAGE_NAME, &server_path, &container_dir, - latest_version.typescript_version.as_str(), + version.typescript_version.as_str(), ) .await; if should_install_language_server { - self.node - .npm_install_packages( - &container_dir, - &[ - (package_name, latest_version.typescript_version.as_str()), - ( - "typescript-language-server", - latest_version.server_version.as_str(), - ), - ], - ) - .await?; + None + } else { + Some(LanguageServerBinary { + path: self.node.binary_path().await.ok()?, + env: None, + arguments: typescript_server_binary_arguments(&server_path), + }) } + } + + async fn fetch_server_binary( + &self, + latest_version: Box, + container_dir: PathBuf, + _: &dyn LspAdapterDelegate, + ) -> Result { + let latest_version = latest_version.downcast::().unwrap(); + let server_path = container_dir.join(Self::NEW_SERVER_PATH); + + self.node + .npm_install_packages( + &container_dir, + &[ + ( + Self::PACKAGE_NAME, + latest_version.typescript_version.as_str(), + ), + ( + "typescript-language-server", + latest_version.server_version.as_str(), + ), + ], + ) + .await?; Ok(LanguageServerBinary { path: self.node.binary_path().await?, diff --git a/crates/languages/src/yaml.rs b/crates/languages/src/yaml.rs index 6d34d9816c7d7..2f412d3102e66 100644 --- a/crates/languages/src/yaml.rs +++ b/crates/languages/src/yaml.rs @@ -31,6 +31,7 @@ pub struct YamlLspAdapter { impl YamlLspAdapter { const SERVER_NAME: LanguageServerName = LanguageServerName::new_static("yaml-language-server"); + const PACKAGE_NAME: &str = "yaml-language-server"; pub fn new(node: NodeRuntime) -> Self { YamlLspAdapter { node } } @@ -61,18 +62,13 @@ impl LspAdapter for YamlLspAdapter { ) -> Result { let latest_version = latest_version.downcast::().unwrap(); let server_path = container_dir.join(SERVER_PATH); - let package_name = "yaml-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?, @@ -81,6 +77,31 @@ impl LspAdapter for YamlLspAdapter { }) } + async fn check_if_version_installed( + &self, + version: &(dyn 'static + Send + Any), + container_dir: &PathBuf, + _: &dyn LspAdapterDelegate, + ) -> Option { + let version = version.downcast_ref::().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,