Skip to content

Commit 89efb4f

Browse files
lsp: Resolve completion item asynchronously on idle-timeout (#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.
1 parent eada6d5 commit 89efb4f

File tree

3 files changed

+52
-14
lines changed

3 files changed

+52
-14
lines changed

helix-lsp/src/client.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -650,12 +650,11 @@ impl Client {
650650
self.call::<lsp::request::Completion>(params)
651651
}
652652

653-
pub async fn resolve_completion_item(
653+
pub fn resolve_completion_item(
654654
&self,
655655
completion_item: lsp::CompletionItem,
656-
) -> Result<lsp::CompletionItem> {
657-
self.request::<lsp::request::ResolveCompletionItem>(completion_item)
658-
.await
656+
) -> impl Future<Output = Result<Value>> {
657+
self.call::<lsp::request::ResolveCompletionItem>(completion_item)
659658
}
660659

661660
pub fn text_document_signature_help(

helix-term/src/ui/completion.rs

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ impl Completion {
257257
let future = language_server.resolve_completion_item(completion_item);
258258
let response = helix_lsp::block_on(future);
259259
match response {
260-
Ok(completion_item) => Some(completion_item),
260+
Ok(value) => serde_json::from_value(value).ok(),
261261
Err(err) => {
262262
log::error!("execute LSP command: {}", err);
263263
None
@@ -304,6 +304,12 @@ impl Completion {
304304
self.popup.contents().is_empty()
305305
}
306306

307+
fn replace_item(&mut self, old_item: lsp::CompletionItem, new_item: lsp::CompletionItem) {
308+
self.popup.contents_mut().replace_option(old_item, new_item);
309+
}
310+
311+
/// Asynchronously requests that the currently selection completion item is
312+
/// resolved through LSP `completionItem/resolve`.
307313
pub fn ensure_item_resolved(&mut self, cx: &mut commands::Context) -> bool {
308314
// > If computing full completion items is expensive, servers can additionally provide a
309315
// > handler for the completion item resolve request. ...
@@ -313,16 +319,38 @@ impl Completion {
313319
// > 'completionItem/resolve' request is sent with the selected completion item as a parameter.
314320
// > The returned completion item should have the documentation property filled in.
315321
// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion
316-
match self.popup.contents_mut().selection_mut() {
317-
Some(item) if item.documentation.is_none() => {
318-
let doc = doc!(cx.editor);
319-
if let Some(resolved_item) = Self::resolve_completion_item(doc, item.clone()) {
320-
*item = resolved_item;
322+
let current_item = match self.popup.contents().selection() {
323+
Some(item) if item.documentation.is_none() => item.clone(),
324+
_ => return false,
325+
};
326+
327+
let language_server = match doc!(cx.editor).language_server() {
328+
Some(language_server) => language_server,
329+
None => return false,
330+
};
331+
332+
// This method should not block the compositor so we handle the response asynchronously.
333+
let future = language_server.resolve_completion_item(current_item.clone());
334+
335+
cx.callback(
336+
future,
337+
move |_editor, compositor, response: Option<lsp::CompletionItem>| {
338+
let resolved_item = match response {
339+
Some(item) => item,
340+
None => return,
341+
};
342+
343+
if let Some(completion) = &mut compositor
344+
.find::<crate::ui::EditorView>()
345+
.unwrap()
346+
.completion
347+
{
348+
completion.replace_item(current_item, resolved_item);
321349
}
322-
true
323-
}
324-
_ => false,
325-
}
350+
},
351+
);
352+
353+
true
326354
}
327355
}
328356

helix-term/src/ui/menu.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,17 @@ impl<T: Item> Menu<T> {
224224
}
225225
}
226226

227+
impl<T: Item + PartialEq> Menu<T> {
228+
pub fn replace_option(&mut self, old_option: T, new_option: T) {
229+
for option in &mut self.options {
230+
if old_option == *option {
231+
*option = new_option;
232+
break;
233+
}
234+
}
235+
}
236+
}
237+
227238
use super::PromptEvent as MenuEvent;
228239

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

0 commit comments

Comments
 (0)