mirror of
https://github.com/zed-industries/zed.git
synced 2026-08-20 22:44:35 +00:00
Fix language server path displayed in LSP status tooltip (#62919)
Some checks are pending
Congratsbot / check-author (push) Waiting to run
Congratsbot / congrats (push) Blocked by required conditions
deploy_nightly_docs / deploy_docs (push) Waiting to run
run_tests / orchestrate (push) Waiting to run
run_tests / check_style (push) Waiting to run
run_tests / clippy_windows (push) Blocked by required conditions
run_tests / clippy_linux (push) Blocked by required conditions
run_tests / clippy_mac (push) Blocked by required conditions
run_tests / clippy_mac_x86_64 (push) Blocked by required conditions
run_tests / run_tests_windows (push) Blocked by required conditions
run_tests / run_tests_linux (push) Blocked by required conditions
run_tests / run_tests_mac (push) Blocked by required conditions
run_tests / miri_scheduler (push) Blocked by required conditions
run_tests / doctests (push) Blocked by required conditions
run_tests / check_workspace_binaries (push) Blocked by required conditions
run_tests / build_visual_tests_binary (push) Blocked by required conditions
run_tests / check_wasm (push) Blocked by required conditions
run_tests / check_dependencies (push) Blocked by required conditions
run_tests / check_docs (push) Blocked by required conditions
run_tests / check_licenses (push) Blocked by required conditions
run_tests / check_scripts (push) Blocked by required conditions
run_tests / check_postgres_and_protobuf_migrations (push) Blocked by required conditions
run_tests / extension_tests (push) Blocked by required conditions
run_tests / tests_pass (push) Blocked by required conditions
Some checks are pending
Congratsbot / check-author (push) Waiting to run
Congratsbot / congrats (push) Blocked by required conditions
deploy_nightly_docs / deploy_docs (push) Waiting to run
run_tests / orchestrate (push) Waiting to run
run_tests / check_style (push) Waiting to run
run_tests / clippy_windows (push) Blocked by required conditions
run_tests / clippy_linux (push) Blocked by required conditions
run_tests / clippy_mac (push) Blocked by required conditions
run_tests / clippy_mac_x86_64 (push) Blocked by required conditions
run_tests / run_tests_windows (push) Blocked by required conditions
run_tests / run_tests_linux (push) Blocked by required conditions
run_tests / run_tests_mac (push) Blocked by required conditions
run_tests / miri_scheduler (push) Blocked by required conditions
run_tests / doctests (push) Blocked by required conditions
run_tests / check_workspace_binaries (push) Blocked by required conditions
run_tests / build_visual_tests_binary (push) Blocked by required conditions
run_tests / check_wasm (push) Blocked by required conditions
run_tests / check_dependencies (push) Blocked by required conditions
run_tests / check_docs (push) Blocked by required conditions
run_tests / check_licenses (push) Blocked by required conditions
run_tests / check_scripts (push) Blocked by required conditions
run_tests / check_postgres_and_protobuf_migrations (push) Blocked by required conditions
run_tests / extension_tests (push) Blocked by required conditions
run_tests / tests_pass (push) Blocked by required conditions
# Objective Closes #52822 When hovering over the LSP status tooltip for a running language server, the binary path is displayed. Currently, this tooltip shows `LanguageServerBinary.path`. However, many language servers are executed through runtimes such as Node or Python. For example, Zed-managed `Basedpyright` produces a `LanguageServerBinary` like: ```Rust LanguageServerBinary { path: "/usr/bin/node", arguments: [ "/home/xin/.local/share/zed/languages/basedpyright/node_modules/basedpyright/langserver.index.js", "--stdio", ], env: ... } ``` In this case, only `"/usr/bin/node"` is displayed, which doesn't convey useful information about the actual language server script being executed. ## Solution There was a PR #53076 in which I was involved, and the solution there was to populate every language server adapter with a special marker for the path to be shown in the tooltip. That solution is accurate, but in order to make this solution work for extension-provided servers, the final diff became huge for a simple fix. So here, as commented in https://github.com/zed-industries/zed/pull/53076#pullrequestreview-4204849892, a guess is performed by a newly introduced function `tooltip_for_server_binary()` to get the real path to be shown. It may have some edge cases, but works for current cases and is simple to implement. ## Testing Added new unit tests, built and tested locally, with the comparision attached in the Showcase section. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase For the mentioned `Basedpyright` language server case, the comparision is shown below: | Before | After | |:--:|:--:| | <img width="431" height="187" alt="before" src="https://github.com/user-attachments/assets/0611f2fe-687b-4d67-ba78-380d89ed0212" /> | <img width="582" height="185" alt="after" src="https://github.com/user-attachments/assets/6ad08720-ac53-4b2d-a72c-3febb441e2f1" /> | --- Release Notes: - Improved the LSP status tooltip to display the target script path for runtime-managed language servers
This commit is contained in:
parent
53b39e8e89
commit
b0e37a6c18
3 changed files with 135 additions and 29 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -9949,6 +9949,7 @@ dependencies = [
|
|||
"language",
|
||||
"lsp",
|
||||
"menu",
|
||||
"path",
|
||||
"project",
|
||||
"proto",
|
||||
"release_channel",
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ itertools.workspace = true
|
|||
language.workspace = true
|
||||
lsp.workspace = true
|
||||
menu.workspace = true
|
||||
path.workspace = true
|
||||
project.workspace = true
|
||||
proto.workspace = true
|
||||
serde_json.workspace = true
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ use editor::{Editor, EditorEvent};
|
|||
use gpui::{Action as _, Anchor, App, Entity, Subscription, Task, TaskExt, WeakEntity, actions};
|
||||
use language::{BinaryStatus, BufferId, ServerHealth};
|
||||
use lsp::{LanguageServerId, LanguageServerName, LanguageServerSelector};
|
||||
use path::PathStyle;
|
||||
use project::{
|
||||
LspStore, LspStoreEvent, Worktree, lsp_store::log_store::GlobalLogStore,
|
||||
project_settings::ProjectSettings, trusted_worktrees::TrustedWorktrees,
|
||||
|
|
@ -188,6 +189,13 @@ struct ServerInfo {
|
|||
message: Option<SharedString>,
|
||||
}
|
||||
|
||||
#[derive(Default, Clone)]
|
||||
struct ServerMetadata {
|
||||
server_version: Option<SharedString>,
|
||||
binary_display_path: Option<SharedString>,
|
||||
process_id: Option<u32>,
|
||||
}
|
||||
|
||||
impl ServerInfo {
|
||||
fn server_selector(&self) -> LanguageServerSelector {
|
||||
LanguageServerSelector::Id(self.id)
|
||||
|
|
@ -260,24 +268,32 @@ impl LanguageServerState {
|
|||
);
|
||||
}
|
||||
|
||||
let server_metadata = self
|
||||
.lsp_store
|
||||
.update(cx, |lsp_store, _| {
|
||||
lsp_store
|
||||
.language_server_statuses()
|
||||
.map(|(server_id, status)| {
|
||||
(
|
||||
server_id,
|
||||
let path_style = self
|
||||
.workspace
|
||||
.upgrade()
|
||||
.map(|workspace| workspace.read(cx).path_style(cx))
|
||||
.unwrap_or(PathStyle::local());
|
||||
|
||||
let server_metadata =
|
||||
self.lsp_store
|
||||
.update(cx, |lsp_store, _| {
|
||||
lsp_store
|
||||
.language_server_statuses()
|
||||
.map(|(server_id, status)| {
|
||||
(
|
||||
status.server_readable_version.clone(),
|
||||
status.binary.as_ref().map(|b| b.path.clone()),
|
||||
status.process_id,
|
||||
),
|
||||
)
|
||||
})
|
||||
.collect::<HashMap<_, _>>()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
server_id,
|
||||
ServerMetadata {
|
||||
server_version: status.server_readable_version.clone(),
|
||||
binary_display_path: status.binary.as_ref().map(|binary| {
|
||||
tooltip_for_server_binary(binary, path_style)
|
||||
}),
|
||||
process_id: status.process_id,
|
||||
},
|
||||
)
|
||||
})
|
||||
.collect::<HashMap<_, _>>()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
|
||||
let process_memory_cache = self.process_memory_cache.clone();
|
||||
|
||||
|
|
@ -360,17 +376,14 @@ impl LanguageServerState {
|
|||
.or_else(|| server_info.binary_status.as_ref()?.message.as_ref())
|
||||
.cloned();
|
||||
|
||||
let (server_version, binary_path, process_id) = server_metadata
|
||||
let ServerMetadata {
|
||||
server_version,
|
||||
binary_display_path,
|
||||
process_id,
|
||||
} = server_metadata
|
||||
.get(&server_info.id)
|
||||
.map(|(version, path, process_id)| {
|
||||
(
|
||||
version.clone(),
|
||||
path.as_ref()
|
||||
.map(|p| SharedString::from(p.compact().to_string_lossy().to_string())),
|
||||
*process_id,
|
||||
)
|
||||
})
|
||||
.unwrap_or((None, None, None));
|
||||
.cloned()
|
||||
.unwrap_or_default();
|
||||
|
||||
let server_message = message.clone();
|
||||
|
||||
|
|
@ -581,7 +594,7 @@ impl LanguageServerState {
|
|||
}
|
||||
|
||||
submenu = submenu.separator().custom_row({
|
||||
let binary_path = binary_path.clone();
|
||||
let binary_display_path = binary_display_path.clone();
|
||||
let server_version = server_version.clone();
|
||||
let server_message = server_message.clone();
|
||||
let process_memory_cache = process_memory_cache.clone();
|
||||
|
|
@ -659,7 +672,7 @@ impl LanguageServerState {
|
|||
.size(LabelSize::Small),
|
||||
)
|
||||
})
|
||||
.when_some(binary_path.clone(), |el, path| {
|
||||
.when_some(binary_display_path.clone(), |el, path| {
|
||||
el.tooltip(Tooltip::text(path))
|
||||
})
|
||||
.into_any_element()
|
||||
|
|
@ -675,6 +688,34 @@ impl LanguageServerState {
|
|||
}
|
||||
}
|
||||
|
||||
fn tooltip_for_server_binary(
|
||||
server_binary: &lsp::LanguageServerBinary,
|
||||
path_style: PathStyle,
|
||||
) -> SharedString {
|
||||
let runtime = path_style.file_name(&server_binary.path).and_then(|name| {
|
||||
["node", "python"]
|
||||
.into_iter()
|
||||
.find(|runtime| name.starts_with(runtime))
|
||||
});
|
||||
|
||||
let target_path = runtime
|
||||
.and_then(|_runtime| {
|
||||
server_binary
|
||||
.arguments
|
||||
.iter()
|
||||
.find(|arg| !arg.to_string_lossy().starts_with('-'))
|
||||
})
|
||||
.map(Path::new)
|
||||
.unwrap_or(&server_binary.path);
|
||||
|
||||
let display_path = path_style.normalize(&target_path.compact().to_string_lossy());
|
||||
|
||||
match runtime {
|
||||
Some(runtime) => format!("{display_path} ({runtime})").into(),
|
||||
None => display_path.into(),
|
||||
}
|
||||
}
|
||||
|
||||
impl LanguageServers {
|
||||
fn update_binary_status(
|
||||
&mut self,
|
||||
|
|
@ -1585,4 +1626,67 @@ mod tests {
|
|||
"the new server's health entry is present",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tooltip_for_server_binary_handles_runtime_and_standalone_servers() {
|
||||
let node_server = lsp::LanguageServerBinary {
|
||||
path: "/usr/bin/node".into(),
|
||||
arguments: vec![
|
||||
"/zed/languages/basedpyright/langserver.index.js".into(),
|
||||
"--stdio".into(),
|
||||
],
|
||||
env: None,
|
||||
};
|
||||
assert_eq!(
|
||||
tooltip_for_server_binary(&node_server, PathStyle::Unix),
|
||||
"/zed/languages/basedpyright/langserver.index.js (node)"
|
||||
);
|
||||
|
||||
let node_server_windows = lsp::LanguageServerBinary {
|
||||
path: "C:\\Program Files\\nodejs\\node.exe".into(),
|
||||
arguments: vec![
|
||||
"C:\\Users\\Zed\\languages\\basedpyright\\node_modules/basedpyright/langserver.index.js".into(),
|
||||
"--stdio".into(),
|
||||
],
|
||||
env: None
|
||||
};
|
||||
assert_eq!(
|
||||
tooltip_for_server_binary(&node_server_windows, PathStyle::Windows),
|
||||
"C:\\Users\\Zed\\languages\\basedpyright\\node_modules\\basedpyright\\langserver.index.js (node)"
|
||||
);
|
||||
|
||||
let python_server = lsp::LanguageServerBinary {
|
||||
path: "/usr/bin/python3".into(),
|
||||
arguments: vec!["/zed/languages/pylsp/pylsp".into(), "--stdio".into()],
|
||||
env: None,
|
||||
};
|
||||
assert_eq!(
|
||||
tooltip_for_server_binary(&python_server, PathStyle::Unix),
|
||||
"/zed/languages/pylsp/pylsp (python)"
|
||||
);
|
||||
|
||||
let standalone_server = lsp::LanguageServerBinary {
|
||||
path: "/usr/bin/ty".into(),
|
||||
arguments: vec!["server".into()],
|
||||
env: None,
|
||||
};
|
||||
assert_eq!(
|
||||
tooltip_for_server_binary(&standalone_server, PathStyle::Unix),
|
||||
"/usr/bin/ty"
|
||||
);
|
||||
|
||||
let flagged_node_server = lsp::LanguageServerBinary {
|
||||
path: "/usr/bin/node".into(),
|
||||
arguments: vec![
|
||||
"--max-old-space-size=8192".into(),
|
||||
"/zed/languages/eslint/server.js".into(),
|
||||
"--stdio".into(),
|
||||
],
|
||||
env: None,
|
||||
};
|
||||
assert_eq!(
|
||||
tooltip_for_server_binary(&flagged_node_server, PathStyle::Unix),
|
||||
"/zed/languages/eslint/server.js (node)"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue