Skip to content

Commit

Permalink
lsp: Resolve completion item asynchronously on idle-timeout (helix-ed…
Browse files Browse the repository at this point in the history
…itor#4781)

d7d0d5f resolves completion items on
the idle-timeout event. The `Completion::resolve_completion_item`
function blocks on the LSP request though, which blocks the compositor
and in turn blocks the event loop. So until the language server returns
the resolved completion item, Helix is unable to respond to keypresses
or other LSP messages.

This is typically ok since the resolution request is fast but for some
language servers this can be problematic, and ideally we shouldn't be
blocking like this anyways.

When receiving a `completionItem/resolve` request, the Volar server
sends a `workspace/configuration` request to Helix and blocks itself
on the response, leading to a deadlock. Eventually the resolve request
times out within Helix but Helix is locked up and unresponsive in that
window.

This change resolves the completion item without blocking the
compositor.
  • Loading branch information
the-mikedavis authored and Frederik Vestre committed Feb 6, 2023
1 parent 49c2bed commit 1ec0c65
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 14 deletions.
7 changes: 3 additions & 4 deletions helix-lsp/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,12 +650,11 @@ impl Client {
self.call::<lsp::request::Completion>(params)
}

pub async fn resolve_completion_item(
pub fn resolve_completion_item(
&self,
completion_item: lsp::CompletionItem,
) -> Result<lsp::CompletionItem> {
self.request::<lsp::request::ResolveCompletionItem>(completion_item)
.await
) -> impl Future<Output = Result<Value>> {
self.call::<lsp::request::ResolveCompletionItem>(completion_item)
}

pub fn text_document_signature_help(
Expand Down
48 changes: 38 additions & 10 deletions helix-term/src/ui/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ impl Completion {
let future = language_server.resolve_completion_item(completion_item);
let response = helix_lsp::block_on(future);
match response {
Ok(completion_item) => Some(completion_item),
Ok(value) => serde_json::from_value(value).ok(),
Err(err) => {
log::error!("execute LSP command: {}", err);
None
Expand Down Expand Up @@ -304,6 +304,12 @@ impl Completion {
self.popup.contents().is_empty()
}

fn replace_item(&mut self, old_item: lsp::CompletionItem, new_item: lsp::CompletionItem) {
self.popup.contents_mut().replace_option(old_item, new_item);
}

/// Asynchronously requests that the currently selection completion item is
/// resolved through LSP `completionItem/resolve`.
pub fn ensure_item_resolved(&mut self, cx: &mut commands::Context) -> bool {
// > If computing full completion items is expensive, servers can additionally provide a
// > handler for the completion item resolve request. ...
Expand All @@ -313,16 +319,38 @@ impl Completion {
// > 'completionItem/resolve' request is sent with the selected completion item as a parameter.
// > The returned completion item should have the documentation property filled in.
// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion
match self.popup.contents_mut().selection_mut() {
Some(item) if item.documentation.is_none() => {
let doc = doc!(cx.editor);
if let Some(resolved_item) = Self::resolve_completion_item(doc, item.clone()) {
*item = resolved_item;
let current_item = match self.popup.contents().selection() {
Some(item) if item.documentation.is_none() => item.clone(),
_ => return false,
};

let language_server = match doc!(cx.editor).language_server() {
Some(language_server) => language_server,
None => return false,
};

// This method should not block the compositor so we handle the response asynchronously.
let future = language_server.resolve_completion_item(current_item.clone());

cx.callback(
future,
move |_editor, compositor, response: Option<lsp::CompletionItem>| {
let resolved_item = match response {
Some(item) => item,
None => return,
};

if let Some(completion) = &mut compositor
.find::<crate::ui::EditorView>()
.unwrap()
.completion
{
completion.replace_item(current_item, resolved_item);
}
true
}
_ => false,
}
},
);

true
}
}

Expand Down
11 changes: 11 additions & 0 deletions helix-term/src/ui/menu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,17 @@ impl<T: Item> Menu<T> {
}
}

impl<T: Item + PartialEq> Menu<T> {
pub fn replace_option(&mut self, old_option: T, new_option: T) {
for option in &mut self.options {
if old_option == *option {
*option = new_option;
break;
}
}
}
}

use super::PromptEvent as MenuEvent;

impl<T: Item + 'static> Component for Menu<T> {
Expand Down

0 comments on commit 1ec0c65

Please sign in to comment.