From 5e3e1f939dc92c4c4a9bcedecf38c83707e53a60 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:38:51 +0800 Subject: [PATCH] fix(lsp): Run jdtls on a JDK 21+ and offer Install in the server popup (#2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(lsp): Run jdtls on a JDK 21+ and offer Install in the server popup jdtls refuses to start on Java < 21, and its launcher took whatever JAVA_HOME / PATH pointed at, so it crash-looped with a Python traceback when that was an older JDK (#1). Pick the first detected JDK 21+ and pass it via JAVA_HOME; projects still compile against their own runtimes. When none is found, report a clear message instead of launching, and rescan on restart so a newly installed JDK is picked up. JDK detection now also covers the java on PATH and Windows vendor directories. The Language Servers status popup showed the manual install hint for missing servers; it now has an Install button that runs tools.install, which opens the Language Servers settings page for tools the IDE cannot install itself. Fixes #1 Claude-Session: https://claude.ai/code/session_01B65qLEKnkeYTGdbwhLpjCu * fix(lsp): Address review of the jdtls runtime check - Cache the JDK scan and rescan at most every 10s while no JDK 21+ is found, refreshing the shared list instead of discarding it; skip duplicate Windows Program Files roots. - Check `launch_error` in the manager (inside the blocking launch task) rather than in ProcessLauncher, and skip such servers in batch inspection. - Keep the install hint visible next to the Install button; when the tool list failed to load, try the install so the error is reported. - Don't let a server snapshot requested before a status event overwrite that newer status (left jdtls showing "Starting…" while missing). --- crates/ide-language/src/lib.rs | 4 + crates/ide-lsp/src/inspect.rs | 8 +- crates/ide-lsp/src/lib.rs | 16 ++- crates/ide-lsp/tests/mock_server.rs | 16 +++ crates/ide-lsp/tests/real_servers.rs | 2 + crates/ide-sdk/src/java.rs | 135 ++++++++++++++++++-- crates/ide-sdk/src/tests.rs | 1 + src-tauri/src/commands/build.rs | 3 +- src-tauri/src/commands/java.rs | 4 +- src-tauri/src/state.rs | 8 +- src/features/code-insight/ServerStatus.tsx | 32 +++-- src/features/code-insight/servers.test.ts | 42 ++++++ src/features/code-insight/servers.ts | 15 ++- src/features/tools/__tests__/tools.test.tsx | 21 +++ src/features/tools/index.ts | 15 ++- 15 files changed, 284 insertions(+), 38 deletions(-) create mode 100644 src/features/code-insight/servers.test.ts diff --git a/crates/ide-language/src/lib.rs b/crates/ide-language/src/lib.rs index 6cdaaf8ce..67f736cde 100644 --- a/crates/ide-language/src/lib.rs +++ b/crates/ide-language/src/lib.rs @@ -75,6 +75,10 @@ pub struct LanguageServerConfig { /// basedpyright): only used while none of those is installed. #[serde(default)] pub fallback: bool, + /// Set by the settings hook when the server can't run here (jdtls without + /// a JDK 21+); reported as the failure instead of launching it. + #[serde(skip)] + pub launch_error: Option, } #[derive(Clone, Debug, Serialize, Deserialize)] diff --git a/crates/ide-lsp/src/inspect.rs b/crates/ide-lsp/src/inspect.rs index 5d6693d2a..3aa6c645f 100644 --- a/crates/ide-lsp/src/inspect.rs +++ b/crates/ide-lsp/src/inspect.rs @@ -60,9 +60,9 @@ pub enum FileInspection { } impl LspManager { - /// Whether an installed language server is configured for `path` (by file - /// name). Missing servers are not started, so a batch run doesn't prompt - /// to install every language it meets. + /// Whether an installed language server that can run here is configured + /// for `path` (by file name). Missing servers are not started, so a batch + /// run doesn't prompt to install every language it meets. pub fn serves_path(&self, path: &Path) -> bool { let lang = self.languages.detect(path, None); let Some(config) = self.languages.get(&lang) else { return false }; @@ -71,7 +71,7 @@ impl LspManager { if let Some(hook) = self.config_hook.read().clone() { hook(&mut cfg, &self.project_root); } - self.launcher.available(&cfg, &self.project_root) + cfg.launch_error.is_none() && self.launcher.available(&cfg, &self.project_root) }) } diff --git a/crates/ide-lsp/src/lib.rs b/crates/ide-lsp/src/lib.rs index c5eee5a0e..bbc8e018f 100644 --- a/crates/ide-lsp/src/lib.rs +++ b/crates/ide-lsp/src/lib.rs @@ -329,13 +329,19 @@ impl LspManager { async fn run_instance(self: Arc, server: Arc, generation: u64) { server.set_status(&self.sink, ide_core::event::ServerStatus::Starting, None); let handler = Arc::new(handler::ClientHandler::new(self.me.clone(), server.clone(), generation)); - let launcher = self.launcher.clone(); - let cfg = self.configured(&server); - server.set_effective_config(cfg.clone()); - let root = server.root.clone(); + let me = self.clone(); let tail = server.clone(); + // The settings hook may touch the filesystem (e.g. scanning for JDKs). let launched = tokio::task::spawn_blocking(move || { - launcher.launch(&cfg, &root, handler, Box::new(move |line| tail.push_stderr(line))) + let cfg = me.configured(&tail); + tail.set_effective_config(cfg.clone()); + let root = tail.root.clone(); + if let Some(message) = &cfg.launch_error { + // A missing binary still asks to be installed first. + let missing = !me.launcher.available(&cfg, &root); + return Err(if missing { LaunchError::Missing } else { LaunchError::Failed(message.clone()) }); + } + me.launcher.launch(&cfg, &root, handler, Box::new(move |line| tail.push_stderr(line))) }) .await .unwrap_or_else(|e| Err(LaunchError::Failed(e.to_string()))); diff --git a/crates/ide-lsp/tests/mock_server.rs b/crates/ide-lsp/tests/mock_server.rs index 6747ef2b3..c5f655217 100644 --- a/crates/ide-lsp/tests/mock_server.rs +++ b/crates/ide-lsp/tests/mock_server.rs @@ -193,6 +193,7 @@ fn server_cfg(name: &str) -> LanguageServerConfig { initialization_options: Some(toml::Value::Table(toml::toml! { flag = true nested = { deep = 1 } })), install_hint: Some(format!("install {name}")), fallback: false, + launch_error: None, } } @@ -703,6 +704,21 @@ async fn missing_server_reports_once() { assert!(h.mgr.semantic_tokens(doc).await.unwrap().is_none()); } +#[tokio::test(flavor = "multi_thread")] +async fn launch_error_fails_without_starting_the_server() { + let h = harness(&[("mock", incremental_caps())]); + h.mgr.set_config_hook(Box::new(|cfg, _| cfg.launch_error = Some("needs JDK 21".into()))); + assert!(!h.mgr.serves_path(&h.root.join("a.mk")), "batch inspection skips it"); + h.open("a.mk", "x"); + h.wait_status("mock", ServerStatus::Crashed).await; + assert_eq!(h.launcher.state("mock").launches.load(Ordering::SeqCst), 0); + let crashed = h.events.lock().iter().find_map(|e| match e { + IdeEvent::LanguageServer { status: ServerStatus::Crashed, message, .. } => message.clone(), + _ => None, + }); + assert_eq!(crashed.as_deref(), Some("needs JDK 21")); +} + #[tokio::test(flavor = "multi_thread")] async fn fallback_servers_only_run_when_the_preferred_one_is_missing() { // Preferred installed: the alternative is never started. diff --git a/crates/ide-lsp/tests/real_servers.rs b/crates/ide-lsp/tests/real_servers.rs index 136f346ce..582253b2b 100644 --- a/crates/ide-lsp/tests/real_servers.rs +++ b/crates/ide-lsp/tests/real_servers.rs @@ -72,6 +72,7 @@ async fn missing_binary_is_reported_with_install_hint() { initialization_options: None, install_hint: Some("npm i -g nope-ls".into()), fallback: false, + launch_error: None, }; let (mgr, events) = manager(dir.path(), LanguageRegistry::from_configs(vec![lang("nope", "nope", server)])); let path = dir.path().join("a.nope"); @@ -198,6 +199,7 @@ async fn typescript_language_server_end_to_end() { initialization_options: None, install_hint: None, fallback: false, + launch_error: None, }; let (mgr, events) = manager(&root, LanguageRegistry::from_configs(vec![lang("typescript", "ts", server)])); let doc = DocId::next(); diff --git a/crates/ide-sdk/src/java.rs b/crates/ide-sdk/src/java.rs index ecf589cbe..1f16029e2 100644 --- a/crates/ide-sdk/src/java.rs +++ b/crates/ide-sdk/src/java.rs @@ -6,7 +6,8 @@ use ide_language::LanguageServerConfig; use serde_json::json; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; -use std::sync::OnceLock; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; /// A JDK home and its feature version (8, 11, 17, 21…). #[derive(Clone, Debug, PartialEq, Eq)] @@ -42,13 +43,26 @@ fn children(dir: &Path) -> Vec { out } +/// Vendor directories JDK installers use on Windows, under `Program Files` +/// and (per-user installs) `%LOCALAPPDATA%\Programs`. +const WINDOWS_VENDORS: &[&str] = + &["Java", "Eclipse Adoptium", "Microsoft", "Zulu", "Amazon Corretto", "BellSoft", "Semeru", "OpenJDK"]; + /// JDK homes in the usual install locations (macOS `JavaVirtualMachines`, -/// `/usr/lib/jvm`, SDKMAN, IDEA's `~/.jdks`, Homebrew), `JAVA_HOME` first. +/// `/usr/lib/jvm`, SDKMAN, IDEA's `~/.jdks`, Homebrew, Windows vendor +/// directories), `JAVA_HOME` first, then the `java` on `PATH`. fn scan() -> Vec { let mut homes: Vec = Vec::new(); if let Some(h) = std::env::var_os("JAVA_HOME") { homes.push(PathBuf::from(h)); } + let path = std::env::var_os("PATH").unwrap_or_default(); + if let Some(java) = std::env::split_paths(&path).map(|d| d.join(crate::exe("java"))).find(|j| j.is_file()) { + // Through symlinks (`/usr/bin/java`, Homebrew, SDKMAN `current`) to `/bin/java`. + if let Some(home) = dunce::canonicalize(java).ok().and_then(|j| j.parent()?.parent().map(Path::to_path_buf)) { + homes.push(home); + } + } let home = home_dir(); let mut roots: Vec = vec![ PathBuf::from("/Library/Java/JavaVirtualMachines"), @@ -59,8 +73,21 @@ fn scan() -> Vec { if let Some(h) = &home { roots.extend([h.join("Library/Java/JavaVirtualMachines"), h.join(".jdks"), h.join(".sdkman/candidates/java")]); } - for root in roots { - for dir in children(&root) { + if cfg!(windows) { + let bases = ["ProgramFiles", "ProgramW6432", "LOCALAPPDATA"].into_iter().filter_map(|v| { + let dir = PathBuf::from(std::env::var_os(v)?); + Some(if v == "LOCALAPPDATA" { dir.join("Programs") } else { dir }) + }); + for base in bases { + roots.extend(WINDOWS_VENDORS.iter().map(|v| base.join(v))); + } + } + for (i, root) in roots.iter().enumerate() { + // `ProgramFiles` and `ProgramW6432` are usually the same directory. + if roots[..i].contains(root) { + continue; + } + for dir in children(root) { homes.extend([dir.join("Contents/Home"), dir.join("libexec/openjdk.jdk/Contents/Home"), dir]); } } @@ -75,10 +102,47 @@ fn scan() -> Vec { out } -/// Installed JDKs (scanned once per process). -pub fn installed_jdks() -> &'static [Jdk] { - static JDKS: OnceLock> = OnceLock::new(); - JDKS.get_or_init(scan) +/// How long a scan that found no JDK able to run jdtls is reused before +/// [`jdtls_jdks`] scans again. +const RESCAN_AFTER: Duration = Duration::from_secs(10); + +struct Scan { + at: Instant, + jdks: Arc<[Jdk]>, +} + +static JDKS: Mutex> = Mutex::new(None); + +fn rescan(slot: &mut Option) -> Arc<[Jdk]> { + slot.insert(Scan { at: Instant::now(), jdks: scan().into() }).jdks.clone() +} + +/// Installed JDKs (scanned on first use; [`jdtls_jdks`] may refresh them). +pub fn installed_jdks() -> Arc<[Jdk]> { + let mut slot = JDKS.lock().unwrap_or_else(|e| e.into_inner()); + match &*slot { + Some(s) => s.jdks.clone(), + None => rescan(&mut slot), + } +} + +/// Like [`installed_jdks`], but while none can run jdtls the list is rescanned +/// (at most every [`RESCAN_AFTER`]), so a JDK installed after startup is picked +/// up when the server is restarted. +pub fn jdtls_jdks() -> Arc<[Jdk]> { + let mut slot = JDKS.lock().unwrap_or_else(|e| e.into_inner()); + match &*slot { + Some(s) if jdtls_runtime(&s.jdks).is_some() || s.at.elapsed() < RESCAN_AFTER => s.jdks.clone(), + _ => rescan(&mut slot), + } +} + +/// Oldest Java version jdtls itself runs on (projects may target older ones). +pub const JDTLS_MIN_JAVA: u32 = 21; + +/// The JDK to run jdtls with: the first found that is new enough (`JAVA_HOME` wins). +pub fn jdtls_runtime(jdks: &[Jdk]) -> Option<&Jdk> { + jdks.iter().find(|j| j.major >= JDTLS_MIN_JAVA) } /// jdtls `java.configuration.runtimes`: one JDK per Java version (the first @@ -114,8 +178,9 @@ const METADATA_PROPERTY: &str = "java.import.generatesMetadataFilesAtProjectRoot /// jdtls: its Eclipse workspace (`-data`) lives under the IDE's data /// directory, one per server root, and no Eclipse metadata is written into -/// the project; installed JDKs are registered as runtimes and `bundles` -/// (e.g. the java-debug plugin jar) are loaded into the server. +/// the project; it runs on a JDK 21+ (via `JAVA_HOME`, which its launcher +/// reads), installed JDKs are registered as runtimes and `bundles` (e.g. the +/// java-debug plugin jar) are loaded into the server. pub fn configure_jdtls( cfg: &mut LanguageServerConfig, server_root: &Path, @@ -135,6 +200,14 @@ pub fn configure_jdtls( if !cfg.args.iter().any(|a| a.contains(METADATA_PROPERTY)) { cfg.args.push(format!("--jvm-arg=-D{METADATA_PROPERTY}=false")); } + if !cfg.env.contains_key("JAVA_HOME") { + match jdtls_runtime(jdks) { + Some(jdk) => { + cfg.env.insert("JAVA_HOME".to_owned(), jdk.home.to_string_lossy().into_owned()); + } + None => cfg.launch_error = Some(missing_runtime_message(jdks)), + } + } let runtimes = runtimes(jdks); if !runtimes.is_empty() { merge_init_options(cfg, json!({ "settings": { "java": { "configuration": { "runtimes": runtimes } } } })); @@ -145,6 +218,22 @@ pub fn configure_jdtls( } } +fn missing_runtime_message(jdks: &[Jdk]) -> String { + let mut found: Vec = jdks.iter().map(|j| j.major).collect(); + found.sort_unstable(); + found.dedup(); + let found = if found.is_empty() { + "no JDK was found".to_owned() + } else { + let list: Vec = found.iter().map(u32::to_string).collect(); + format!("found JDK {}", list.join(", ")) + }; + format!( + "jdtls needs JDK {JDTLS_MIN_JAVA} or newer to run ({found}). Install one (or point JAVA_HOME at it) and \ + restart the server; the project can still target an older Java version." + ) +} + /// The `java` launcher of a JDK home. pub fn java_executable(home: &Path) -> PathBuf { home.join("bin").join(crate::exe("java")) @@ -251,6 +340,8 @@ mod tests { assert_eq!(rt[1], json!({ "name": "JavaSE-21", "path": "/jdk21a" })); assert_eq!(rt[2], json!({ "name": "JavaSE-25", "path": "/jdk25" })); assert_eq!(opts["bundles"], json!(["/tools/java-debug/bin/com.microsoft.java.debug.plugin.jar"])); + assert_eq!(cfg.env["JAVA_HOME"], "/jdk25", "first JDK new enough to run jdtls"); + assert_eq!(cfg.launch_error, None); // Deterministic (reconfigure compares configs) and idempotent. let before = cfg.clone(); @@ -262,6 +353,30 @@ mod tests { assert!(other.args.is_empty() && other.initialization_options.is_none()); } + #[test] + fn jdtls_needs_a_jdk_21_to_run() { + let jdtls = || -> LanguageServerConfig { toml::from_str("name = \"jdtls\"\ncommand = \"jdtls\"").unwrap() }; + let old = [Jdk { major: 17, home: PathBuf::from("/jdk17") }, Jdk { major: 8, home: PathBuf::from("/jdk8") }]; + let mut cfg = jdtls(); + configure_jdtls(&mut cfg, Path::new("/p"), Path::new("/data"), &old, &[]); + assert!(!cfg.env.contains_key("JAVA_HOME")); + let err = cfg.launch_error.unwrap(); + assert!(err.contains("JDK 21 or newer") && err.contains("found JDK 8, 17"), "{err}"); + + let mut cfg = jdtls(); + configure_jdtls(&mut cfg, Path::new("/p"), Path::new("/data"), &[], &[]); + assert!(cfg.launch_error.unwrap().contains("no JDK was found")); + + // An explicit JAVA_HOME is the user's call. + let mut cfg = jdtls(); + cfg.env.insert("JAVA_HOME".into(), "/custom".into()); + configure_jdtls(&mut cfg, Path::new("/p"), Path::new("/data"), &old, &[]); + assert_eq!((cfg.env["JAVA_HOME"].as_str(), cfg.launch_error), ("/custom", None)); + + let mixed = [old[0].clone(), Jdk { major: 21, home: PathBuf::from("/jdk21") }]; + assert_eq!(jdtls_runtime(&mixed).unwrap().major, 21); + } + #[test] fn picks_the_jdk_matching_the_project() { let jdks = [ diff --git a/crates/ide-sdk/src/tests.rs b/crates/ide-sdk/src/tests.rs index d58a6df87..4cac9ba4d 100644 --- a/crates/ide-sdk/src/tests.rs +++ b/crates/ide-sdk/src/tests.rs @@ -27,6 +27,7 @@ fn server(name: &str) -> LanguageServerConfig { initialization_options: None, install_hint: None, fallback: false, + launch_error: None, } } diff --git a/src-tauri/src/commands/build.rs b/src-tauri/src/commands/build.rs index 310deeb00..737ea4cc7 100644 --- a/src-tauri/src/commands/build.rs +++ b/src-tauri/src/commands/build.rs @@ -16,7 +16,8 @@ fn join_err(e: tauri::Error) -> IdeError { /// `JAVA_HOME` of the installed JDK matching `release` (none when the project names no version). fn java_env(release: Option) -> BTreeMap { - let jdk = release.and_then(|r| ide_sdk::java::pick_jdk(ide_sdk::java::installed_jdks(), Some(r))); + let jdks = ide_sdk::java::installed_jdks(); + let jdk = release.and_then(|r| ide_sdk::java::pick_jdk(&jdks, Some(r))); jdk.map(|j| BTreeMap::from([("JAVA_HOME".to_owned(), j.home.to_string_lossy().into_owned())])).unwrap_or_default() } diff --git a/src-tauri/src/commands/java.rs b/src-tauri/src/commands/java.rs index b9b197dea..053e54ac2 100644 --- a/src-tauri/src/commands/java.rs +++ b/src-tauri/src/commands/java.rs @@ -43,8 +43,8 @@ pub async fn java_resolve_launch( let from_jdtls = info.java_home.as_deref().map(ide_sdk::java::java_executable).filter(|p| p.is_file()); let java = from_jdtls.or_else(|| { let release = ide_sdk::java::project_java_version(&module); - ide_sdk::java::pick_jdk(ide_sdk::java::installed_jdks(), release) - .map(|j| ide_sdk::java::java_executable(&j.home)) + let jdks = ide_sdk::java::installed_jdks(); + ide_sdk::java::pick_jdk(&jdks, release).map(|j| ide_sdk::java::java_executable(&j.home)) }); info.java_exec = Some(java.map(|p| p.to_string_lossy().into_owned()).unwrap_or_else(|| "java".to_owned())); Ok(info) diff --git a/src-tauri/src/state.rs b/src-tauri/src/state.rs index 88f5899f7..bc03077ed 100644 --- a/src-tauri/src/state.rs +++ b/src-tauri/src/state.rs @@ -243,8 +243,12 @@ impl AppState { let data_dir = self.data_dir.clone(); lsp.set_config_hook(Box::new(move |cfg, server_root| { ide_sdk::apply::configure_server(cfg, &root, server_root, &settings.effective()); - let bundles: Vec = ide_tools::managed_bundle(&data_dir, "java-debug").into_iter().collect(); - ide_sdk::java::configure_jdtls(cfg, server_root, &data_dir, ide_sdk::java::installed_jdks(), &bundles); + if cfg.name == "jdtls" { + let bundles: Vec = + ide_tools::managed_bundle(&data_dir, "java-debug").into_iter().collect(); + let jdks = ide_sdk::java::jdtls_jdks(); + ide_sdk::java::configure_jdtls(cfg, server_root, &data_dir, &jdks, &bundles); + } })); } { diff --git a/src/features/code-insight/ServerStatus.tsx b/src/features/code-insight/ServerStatus.tsx index a49d7e040..7233a1cb3 100644 --- a/src/features/code-insight/ServerStatus.tsx +++ b/src/features/code-insight/ServerStatus.tsx @@ -1,7 +1,8 @@ /** Status bar widget listing language servers, with a popup to restart them. */ import { useMemo, useRef, useState } from "react"; import clsx from "clsx"; -import { BarButton, IconButton, Popup } from "@/ui"; +import { getAction, executeAction } from "@/platform/actions"; +import { BarButton, Button, IconButton, Popup } from "@/ui"; import { relativePath, useProject } from "@/platform/project"; import { STATUS_LABEL, restartServer, serversSummary, useServers } from "./servers"; import styles from "./ServerStatus.module.css"; @@ -13,6 +14,8 @@ export function ServerStatus() { const [open, setOpen] = useState(false); const ref = useRef(null); const summary = serversSummary(servers); + // features/tools owns installing; without it, fall back to the manual hint text. + const canInstall = !!getAction("tools.install"); if (!summary) return null; return ( <> @@ -43,13 +46,26 @@ export function ServerStatus() { {s.name} {STATUS_LABEL[s.status]} - void restartServer(s.id)} - /> + {s.status === "missing" && canInstall ? ( + + ) : ( + void restartServer(s.id)} + /> + )} {(s.languages.length > 0 || s.root) && (
diff --git a/src/features/code-insight/servers.test.ts b/src/features/code-insight/servers.test.ts new file mode 100644 index 000000000..53ecc29f5 --- /dev/null +++ b/src/features/code-insight/servers.test.ts @@ -0,0 +1,42 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { ServerId, ServerInfo } from "@/bindings"; + +const pending = vi.hoisted(() => [] as ((data: ServerInfo[]) => void)[]); + +vi.mock("@/platform/ipc", async (importOriginal) => { + const ipc = await importOriginal(); + return { + ...ipc, + isTauri: () => true, + commands: { + ...ipc.commands, + lspServers: () => + new Promise((resolve) => { + pending.push((data) => resolve({ status: "ok", data })); + }), + }, + }; +}); + +const { emitLocalIdeEvent } = await import("@/platform/ipc"); +const { useProject } = await import("@/platform/project"); +const { installServerEvents, useServers } = await import("./servers"); + +describe("server status", () => { + let dispose = () => {}; + afterEach(() => dispose()); + + it("keeps a status that arrived after the snapshot was requested", async () => { + useProject.setState({ project: { name: "p", root: "/p", isGit: false, settings: {} } as never }); + dispose = installServerEvents(); + const id = 1 as ServerId; + const info: ServerInfo = { id, name: "jdtls", languages: [], root: "/p", status: "starting", pid: null }; + // The first event requests a snapshot; the server reports missing before it answers. + emitLocalIdeEvent({ type: "languageServer", server: id, name: "jdtls", status: "starting", message: null }); + emitLocalIdeEvent({ type: "languageServer", server: id, name: "jdtls", status: "missing", message: "install jdtls" }); + pending.splice(0).forEach((resolve) => resolve([info])); + await vi.waitFor(() => expect(useServers.getState().servers.get(id)?.languages).toEqual([])); + await new Promise((r) => setTimeout(r, 0)); + expect(useServers.getState().servers.get(id)).toMatchObject({ status: "missing", message: "install jdtls" }); + }); +}); diff --git a/src/features/code-insight/servers.ts b/src/features/code-insight/servers.ts index 23e2cad95..aa91614d0 100644 --- a/src/features/code-insight/servers.ts +++ b/src/features/code-insight/servers.ts @@ -23,6 +23,10 @@ export const useServers = create(() => ({ servers: new Map() })); const notifiedMissing = new Set(); +/** Event count at each server's latest event, so a snapshot taken earlier doesn't undo it. */ +let eventSeq = 0; +const lastEvent = new Map(); + function notifyMissing(e: ServerEntry) { if (notifiedMissing.has(e.name)) return; notifiedMissing.add(e.name); @@ -57,18 +61,22 @@ export async function refreshServers() { } if (!isTauri()) return; try { + const since = eventSeq; const list: ServerInfo[] = await call(commands.lspServers()); const prev = useServers.getState().servers; const map = new Map(); - for (const s of list) + for (const s of list) { + const p = prev.get(s.id); + const newer = p && (lastEvent.get(s.id) ?? 0) > since; map.set(s.id, { id: s.id, name: s.name, - status: s.status, + status: newer ? p.status : s.status, languages: s.languages, root: s.root, - message: prev.get(s.id)?.message ?? null, + message: p?.message ?? null, }); + } useServers.setState({ servers: map }); for (const e of map.values()) if (e.status === "missing") notifyMissing(e); } catch { @@ -92,6 +100,7 @@ export async function restartServer(id: ServerId) { export function installServerEvents(): () => void { const off = onIdeEvent("languageServer", (e) => { const known = useServers.getState().servers.has(e.server); + lastEvent.set(e.server, ++eventSeq); upsertServer({ id: e.server, name: e.name, status: e.status, message: e.message }); if (!known) void refreshServers(); }); diff --git a/src/features/tools/__tests__/tools.test.tsx b/src/features/tools/__tests__/tools.test.tsx index cf1b8b241..ffb645d67 100644 --- a/src/features/tools/__tests__/tools.test.tsx +++ b/src/features/tools/__tests__/tools.test.tsx @@ -167,3 +167,24 @@ describe("LanguageServersPage", () => { expect(await screen.findByRole("alert")).toHaveTextContent("boom"); }); }); + +describe("tools.install action", () => { + it("installs installable tools and opens the page for the rest", async () => { + const { executeAction, registerAction } = await import("@/platform/actions"); + const { activate } = await import("../index"); + const opened: unknown[] = []; + const disposers = [ + registerAction({ id: "file.settings", title: "Settings", run: ({ arg }) => void opened.push(arg) }), + activate(), + ]; + try { + await executeAction("tools.install", "gopls"); + expect(useTools.getState().tools.find((t) => t.name === "gopls")?.status).toBe("installed"); + expect(opened).toEqual([]); + await executeAction("tools.install", "solargraph"); + expect(opened).toEqual(["languages.servers"]); + } finally { + disposers.reverse().forEach((d) => d()); + } + }); +}); diff --git a/src/features/tools/index.ts b/src/features/tools/index.ts index bb4a6ff9d..075736a2f 100644 --- a/src/features/tools/index.ts +++ b/src/features/tools/index.ts @@ -8,7 +8,7 @@ import { lazyComponent } from "@/ui"; import { executeAction, registerActions } from "@/platform/actions"; import { registerSettingsPage } from "@/workbench/contributions"; import { installMissingServerPrompts } from "./missing"; -import { installTool, SETTINGS_PAGE_ID } from "./store"; +import { installTool, refreshTools, SETTINGS_PAGE_ID, toolByName, useTools } from "./store"; /** The Language Servers page (also used by the Settings window). */ export function activateSettingsPages(): () => void { @@ -39,9 +39,18 @@ export function activate(): () => void { title: "Install Language Server / Debugger", category: "Tools", hidden: true, + // Tools the IDE can't install (no recipe, missing npm/curl) open the page with the manual hint. run: async ({ arg }) => { - if (typeof arg === "string") await installTool(arg); - else await executeAction("file.settings", SETTINGS_PAGE_ID); + if (typeof arg === "string") { + if (!useTools.getState().loaded) await refreshTools(); + const tool = toolByName(arg); + // Unknown because the list failed to load: try anyway, so the failure is reported. + if (tool ? tool.canInstall : useTools.getState().error !== null) { + await installTool(arg); + return; + } + } + await executeAction("file.settings", SETTINGS_PAGE_ID); }, }, ]),