From 23248c69df3455fa03f2a2114a2fb9d351c45188 Mon Sep 17 00:00:00 2001 From: kyfanc Date: Wed, 28 Feb 2024 18:30:55 +0800 Subject: [PATCH 1/4] Merge hover results from multiple configured LSPs --- helix-term/src/commands/lsp.rs | 98 ++++++++++++++++++++-------------- helix-term/src/ui/markdown.rs | 4 ++ 2 files changed, 62 insertions(+), 40 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index a1f7bf17dc88..21bf08d2278e 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -950,52 +950,70 @@ pub fn signature_help(cx: &mut Context) { pub fn hover(cx: &mut Context) { let (view, doc) = current!(cx.editor); - // TODO support multiple language servers (merge UI somehow) - let language_server = - language_server_with_feature!(cx.editor, doc, LanguageServerFeature::Hover); - // TODO: factor out a doc.position_identifier() that returns lsp::TextDocumentPositionIdentifier - let pos = doc.position(view.id, language_server.offset_encoding()); - let future = language_server - .text_document_hover(doc.identifier(), pos, None) - .unwrap(); + let requests: Vec<_> = doc + .language_servers_with_feature(LanguageServerFeature::Hover) + .map(|language_server| { + // TODO: factor out a doc.position_identifier() that returns lsp::TextDocumentPositionIdentifier + let pos = doc.position(view.id, language_server.offset_encoding()); + language_server + .text_document_hover(doc.identifier(), pos, None) + .unwrap() + }) + .collect(); - cx.callback( - future, - move |editor, compositor, response: Option| { - if let Some(hover) = response { - // hover.contents / .range <- used for visualizing - - fn marked_string_to_markdown(contents: lsp::MarkedString) -> String { - match contents { - lsp::MarkedString::String(contents) => contents, - lsp::MarkedString::LanguageString(string) => { - if string.language == "markdown" { - string.value - } else { - format!("```{}\n{}\n```", string.language, string.value) + // no configured language server support hover + if requests.is_empty() { + cx.editor.set_status(format!( + "No configured language server supports {}", + LanguageServerFeature::Hover + )); + return; + } + + for future in requests { + cx.callback( + future, + move |editor, compositor, response: Option| { + if let Some(hover) = response { + // hover.contents / .range <- used for visualizing + + fn marked_string_to_markdown(contents: lsp::MarkedString) -> String { + match contents { + lsp::MarkedString::String(contents) => contents, + lsp::MarkedString::LanguageString(string) => { + if string.language == "markdown" { + string.value + } else { + format!("```{}\n{}\n```", string.language, string.value) + } } } } - } - let contents = match hover.contents { - lsp::HoverContents::Scalar(contents) => marked_string_to_markdown(contents), - lsp::HoverContents::Array(contents) => contents - .into_iter() - .map(marked_string_to_markdown) - .collect::>() - .join("\n\n"), - lsp::HoverContents::Markup(contents) => contents.value, - }; - - // skip if contents empty + let contents = match hover.contents { + lsp::HoverContents::Scalar(contents) => marked_string_to_markdown(contents), + lsp::HoverContents::Array(contents) => contents + .into_iter() + .map(marked_string_to_markdown) + .collect::>() + .join("\n\n"), + lsp::HoverContents::Markup(contents) => contents.value, + }; - let contents = ui::Markdown::new(contents, editor.syn_loader.clone()); - let popup = Popup::new("hover", contents).auto_close(true); - compositor.replace_or_push("hover", popup); - } - }, - ); + if let Some(popup) = compositor.find_id::>("hover") { + // append to existing popup + let markdown_content = popup.contents_mut().contents_mut(); + *markdown_content = format!("{}\n\n---\n\n{}", markdown_content, contents); + } else { + // create new popup + let contents = ui::Markdown::new(contents, editor.syn_loader.clone()); + let popup = Popup::new("hover", contents).auto_close(true); + compositor.replace_or_push("hover", popup); + }; + } + }, + ); + } } pub fn rename_symbol(cx: &mut Context) { diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index 749d58508b4e..820ed206bc05 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -338,6 +338,10 @@ impl Markdown { Text::from(lines) } + + pub fn contents_mut(&mut self) -> &mut String { + &mut self.contents + } } impl Component for Markdown { From 4c1ef4815edbe037186073ba3bbf89bf9d6b5add Mon Sep 17 00:00:00 2001 From: kyfanc Date: Wed, 28 Feb 2024 18:32:44 +0800 Subject: [PATCH 2/4] Display LSP name in hover contents when multiple LSPs are configured --- helix-term/src/commands/lsp.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 21bf08d2278e..2020ba07146e 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -955,9 +955,11 @@ pub fn hover(cx: &mut Context) { .map(|language_server| { // TODO: factor out a doc.position_identifier() that returns lsp::TextDocumentPositionIdentifier let pos = doc.position(view.id, language_server.offset_encoding()); - language_server + let name = language_server.name().to_string(); + let request = language_server .text_document_hover(doc.identifier(), pos, None) - .unwrap() + .unwrap(); + (name, request) }) .collect(); @@ -970,7 +972,9 @@ pub fn hover(cx: &mut Context) { return; } - for future in requests { + let lsp_count = requests.len(); + + for (name, future) in requests { cx.callback( future, move |editor, compositor, response: Option| { @@ -990,7 +994,7 @@ pub fn hover(cx: &mut Context) { } } - let contents = match hover.contents { + let mut contents = match hover.contents { lsp::HoverContents::Scalar(contents) => marked_string_to_markdown(contents), lsp::HoverContents::Array(contents) => contents .into_iter() @@ -1000,6 +1004,11 @@ pub fn hover(cx: &mut Context) { lsp::HoverContents::Markup(contents) => contents.value, }; + // attach lsp name if more than one is available + if lsp_count > 1 { + contents = format!("# {}:\n\n{}", name, contents); + } + if let Some(popup) = compositor.find_id::>("hover") { // append to existing popup let markdown_content = popup.contents_mut().contents_mut(); From 0aaed7d16344a65113ca4ccda4b2b5e0853f70ea Mon Sep 17 00:00:00 2001 From: kyfanc Date: Fri, 1 Mar 2024 15:42:05 +0800 Subject: [PATCH 3/4] Respect configured LSPs order for hover request with deduplication --- helix-term/src/commands/lsp.rs | 145 +++++++++++++++++++-------------- helix-term/src/ui/markdown.rs | 4 - 2 files changed, 84 insertions(+), 65 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 2020ba07146e..b44b731ddd02 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1,4 +1,4 @@ -use futures_util::{stream::FuturesUnordered, FutureExt}; +use futures_util::{stream::FuturesOrdered, stream::FuturesUnordered, FutureExt}; use helix_lsp::{ block_on, lsp::{ @@ -948,23 +948,12 @@ pub fn signature_help(cx: &mut Context) { } pub fn hover(cx: &mut Context) { - let (view, doc) = current!(cx.editor); - - let requests: Vec<_> = doc + let doc = doc!(cx.editor); + if doc .language_servers_with_feature(LanguageServerFeature::Hover) - .map(|language_server| { - // TODO: factor out a doc.position_identifier() that returns lsp::TextDocumentPositionIdentifier - let pos = doc.position(view.id, language_server.offset_encoding()); - let name = language_server.name().to_string(); - let request = language_server - .text_document_hover(doc.identifier(), pos, None) - .unwrap(); - (name, request) - }) - .collect(); - - // no configured language server support hover - if requests.is_empty() { + .count() + == 0 + { cx.editor.set_status(format!( "No configured language server supports {}", LanguageServerFeature::Hover @@ -972,57 +961,91 @@ pub fn hover(cx: &mut Context) { return; } - let lsp_count = requests.len(); + let get_hovers = move |editor: &mut Editor| { + let (view, doc) = current!(editor); - for (name, future) in requests { - cx.callback( - future, - move |editor, compositor, response: Option| { - if let Some(hover) = response { - // hover.contents / .range <- used for visualizing - - fn marked_string_to_markdown(contents: lsp::MarkedString) -> String { - match contents { - lsp::MarkedString::String(contents) => contents, - lsp::MarkedString::LanguageString(string) => { - if string.language == "markdown" { - string.value - } else { - format!("```{}\n{}\n```", string.language, string.value) - } - } + let mut seen_language_servers = HashSet::new(); + let mut futures: FuturesOrdered<_> = doc + .language_servers_with_feature(LanguageServerFeature::Hover) + .filter(|ls| seen_language_servers.insert(ls.id())) + .map(|language_server| { + let lsp_name = language_server.name().to_string(); + // TODO: factor out a doc.position_identifier() that returns lsp::TextDocumentPositionIdentifier + let pos = doc.position(view.id, language_server.offset_encoding()); + let request = language_server + .text_document_hover(doc.identifier(), pos, None) + .unwrap(); + + async move { + let json = request.await?; + let response = serde_json::from_value::>(json)?; + anyhow::Ok((lsp_name, response)) + } + }) + .collect(); + + let lsp_count = futures.len(); + let format_contents = move |lsp_name: String, hover: lsp::Hover| { + // hover.contents / .range <- used for visualizing + + fn marked_string_to_markdown(contents: lsp::MarkedString) -> String { + match contents { + lsp::MarkedString::String(contents) => contents, + lsp::MarkedString::LanguageString(string) => { + if string.language == "markdown" { + string.value + } else { + format!("```{}\n{}\n```", string.language, string.value) } } + } + } - let mut contents = match hover.contents { - lsp::HoverContents::Scalar(contents) => marked_string_to_markdown(contents), - lsp::HoverContents::Array(contents) => contents - .into_iter() - .map(marked_string_to_markdown) - .collect::>() - .join("\n\n"), - lsp::HoverContents::Markup(contents) => contents.value, - }; + let mut contents = match hover.contents { + lsp::HoverContents::Scalar(contents) => marked_string_to_markdown(contents), + lsp::HoverContents::Array(contents) => contents + .into_iter() + .map(marked_string_to_markdown) + .collect::>() + .join("\n\n"), + lsp::HoverContents::Markup(contents) => contents.value, + }; - // attach lsp name if more than one is available - if lsp_count > 1 { - contents = format!("# {}:\n\n{}", name, contents); - } + // attach lsp name if more than one is available + if lsp_count > 1 { + contents = format!("# {}:\n\n{}", lsp_name, contents); + } - if let Some(popup) = compositor.find_id::>("hover") { - // append to existing popup - let markdown_content = popup.contents_mut().contents_mut(); - *markdown_content = format!("{}\n\n---\n\n{}", markdown_content, contents); - } else { - // create new popup - let contents = ui::Markdown::new(contents, editor.syn_loader.clone()); - let popup = Popup::new("hover", contents).auto_close(true); - compositor.replace_or_push("hover", popup); - }; + contents + }; + + async move { + let mut contents: Vec = Vec::new(); + + while let Some((lsp_name, hover)) = futures.try_next().await? { + if let Some(hover) = hover { + contents.push(format_contents(lsp_name, hover)); } - }, - ); - } + } + + let contents = contents.join("\n\n---\n\n"); + anyhow::Ok(contents) + } + .boxed() + }; + + let contents = get_hovers(cx.editor); + + cx.jobs.callback(async move { + let contents = contents.await?; + let call = move |editor: &mut Editor, compositor: &mut Compositor| { + // create new popup + let contents = ui::Markdown::new(contents, editor.syn_loader.clone()); + let popup = Popup::new("hover", contents).auto_close(true); + compositor.replace_or_push("hover", popup); + }; + Ok(Callback::EditorCompositor(Box::new(call))) + }); } pub fn rename_symbol(cx: &mut Context) { diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index 820ed206bc05..749d58508b4e 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -338,10 +338,6 @@ impl Markdown { Text::from(lines) } - - pub fn contents_mut(&mut self) -> &mut String { - &mut self.contents - } } impl Component for Markdown { From 84375f75fd4b65d814d6822ab55bf1a67b8db0a2 Mon Sep 17 00:00:00 2001 From: kyfanc Date: Mon, 25 Mar 2024 10:48:03 +0800 Subject: [PATCH 4/4] improve separator visibility with [LSP] prefix --- helix-term/src/commands/lsp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index b44b731ddd02..efaf1b010da3 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1013,7 +1013,7 @@ pub fn hover(cx: &mut Context) { // attach lsp name if more than one is available if lsp_count > 1 { - contents = format!("# {}:\n\n{}", lsp_name, contents); + contents = format!("# [**LSP**] {}:\n\n{}", lsp_name, contents); } contents