Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use a hook for resolving completion items #9668

Merged
merged 1 commit into from
Feb 23, 2024
Merged

Use a hook for resolving completion items #9668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions helix-lsp/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@ impl Client {
pub fn resolve_completion_item(
&self,
completion_item: lsp::CompletionItem,
) -> Option<impl Future<Output = Result<Value>>> {
) -> Option<impl Future<Output = Result<lsp::CompletionItem>>> {
let capabilities = self.capabilities.get().unwrap();

// Return early if the server does not support resolving completion items.
Expand All @@ -1029,7 +1029,8 @@ impl Client {
_ => return None,
}

Some(self.call::<lsp::request::ResolveCompletionItem>(completion_item))
let res = self.call::<lsp::request::ResolveCompletionItem>(completion_item);
Some(async move { Ok(serde_json::from_value(res.await?)?) })
}

pub fn resolve_code_action(
Expand Down
156 changes: 97 additions & 59 deletions helix-term/src/ui/completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use crate::{
compositor::{Component, Context, Event, EventResult},
handlers::trigger_auto_completion,
job,
};
use helix_event::AsyncHook;
use helix_view::{
document::SavePoint,
editor::CompleteAction,
Expand All @@ -10,14 +12,14 @@ use helix_view::{
theme::{Modifier, Style},
ViewId,
};
use tokio::time::Instant;
use tui::{buffer::Buffer as Surface, text::Span};

use std::{borrow::Cow, sync::Arc};
use std::{borrow::Cow, sync::Arc, time::Duration};

use helix_core::{chars, Change, Transaction};
use helix_view::{graphics::Rect, Document, Editor};

use crate::commands;
use crate::ui::{menu, Markdown, Menu, Popup, PromptEvent};

use helix_lsp::{lsp, util, OffsetEncoding};
Expand Down Expand Up @@ -102,6 +104,7 @@ pub struct Completion {
#[allow(dead_code)]
trigger_offset: usize,
filter: String,
resolve_handler: tokio::sync::mpsc::Sender<CompletionItem>,
}

impl Completion {
Expand Down Expand Up @@ -368,6 +371,7 @@ impl Completion {
// TODO: expand nucleo api to allow moving straight to a Utf32String here
// and avoid allocation during matching
filter: String::from(fragment),
resolve_handler: ResolveHandler::default().spawn(),
};

// need to recompute immediately in case start_offset != trigger_offset
Expand All @@ -379,14 +383,16 @@ impl Completion {
completion
}

/// Synchronously resolve the given completion item. This is used when
/// accepting a completion.
fn resolve_completion_item(
language_server: &helix_lsp::Client,
completion_item: lsp::CompletionItem,
) -> Option<lsp::CompletionItem> {
let future = language_server.resolve_completion_item(completion_item)?;
let response = helix_lsp::block_on(future);
match response {
Ok(value) => serde_json::from_value(value).ok(),
Ok(item) => Some(item),
Err(err) => {
log::error!("Failed to resolve completion item: {}", err);
None
Expand Down Expand Up @@ -420,62 +426,6 @@ impl Completion {
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. ...
// > A typical use case is for example: the `textDocument/completion` request doesn't fill
// > in the `documentation` property for returned completion items since it is expensive
// > to compute. When the item is selected in the user interface then a
// > '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
let current_item = match self.popup.contents().selection() {
Some(item) if !item.resolved => item.clone(),
_ => return false,
};

let Some(language_server) = cx
.editor
.language_server_by_id(current_item.language_server_id)
else {
return false;
};

// This method should not block the compositor so we handle the response asynchronously.
let Some(future) = language_server.resolve_completion_item(current_item.item.clone())
else {
return false;
};

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
{
let resolved_item = CompletionItem {
item: resolved_item,
language_server_id: current_item.language_server_id,
resolved: true,
};

completion.replace_item(current_item, resolved_item);
}
},
);

true
}

pub fn area(&mut self, viewport: Rect, editor: &Editor) -> Rect {
self.popup.area(viewport, editor)
}
Expand All @@ -498,6 +448,9 @@ impl Component for Completion {
Some(option) => option,
None => return,
};
if !option.resolved {
helix_event::send_blocking(&self.resolve_handler, option.clone());
}
// need to render:
// option.detail
// ---
Expand Down Expand Up @@ -599,3 +552,88 @@ impl Component for Completion {
markdown_doc.render(doc_area, surface, cx);
}
}

/// A hook for resolving incomplete completion items.
///
/// From the [LSP spec](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion):
///
/// > If computing full completion items is expensive, servers can additionally provide a
/// > handler for the completion item resolve request. ...
/// > A typical use case is for example: the `textDocument/completion` request doesn't fill
/// > in the `documentation` property for returned completion items since it is expensive
/// > to compute. When the item is selected in the user interface then a
/// > 'completionItem/resolve' request is sent with the selected completion item as a parameter.
/// > The returned completion item should have the documentation property filled in.
#[derive(Debug, Default)]
struct ResolveHandler {
trigger: Option<CompletionItem>,
request: Option<helix_event::CancelTx>,
}

impl AsyncHook for ResolveHandler {
type Event = CompletionItem;

fn handle_event(
&mut self,
item: Self::Event,
timeout: Option<tokio::time::Instant>,
) -> Option<tokio::time::Instant> {
if self
.trigger
.as_ref()
.is_some_and(|trigger| trigger == &item)
{
timeout
} else {
self.trigger = Some(item);
self.request = None;
Some(Instant::now() + Duration::from_millis(150))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that you don't want to introduce 10 different settings, but what about low_latency = true which will set all these annoying debounces to zero?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be fine to have a config section dedicated to timeouts. As we tackle the items in #9629 we'll be hard-coding a lot of debounce constants. Something like:

[editor.timeout] # or maybe 'editor.debounce'
lsp.auto-complete = 250
lsp.signature-help = 120
lsp.resolve-completion-item = 150
highlight-picker-preview = 150

seems fine to me but I'm not sure how @archseer feels about it. In any case we would always advise setting the constant to something non-zero, and would probably enforce that in the code

}
}

fn finish_debounce(&mut self) {
let Some(item) = self.trigger.take() else { return };
let (tx, rx) = helix_event::cancelation();
self.request = Some(tx);
job::dispatch_blocking(move |editor, _| resolve_completion_item(editor, item, rx))
}
}

fn resolve_completion_item(
editor: &mut Editor,
item: CompletionItem,
cancel: helix_event::CancelRx,
) {
let Some(language_server) = editor.language_server_by_id(item.language_server_id) else {
return;
};

let Some(future) = language_server.resolve_completion_item(item.item.clone()) else {
return;
};

tokio::spawn(async move {
match helix_event::cancelable_future(future, cancel).await {
Some(Ok(resolved_item)) => {
job::dispatch(move |_, compositor| {
if let Some(completion) = &mut compositor
.find::<crate::ui::EditorView>()
.unwrap()
.completion
{
let resolved_item = CompletionItem {
item: resolved_item,
language_server_id: item.language_server_id,
resolved: true,
};

completion.replace_item(item, resolved_item);
};
})
.await
}
Some(Err(err)) => log::error!("completion resolve request failed: {err}"),
None => (),
}
});
}
8 changes: 0 additions & 8 deletions helix-term/src/ui/editor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,14 +1027,6 @@ impl EditorView {
pub fn handle_idle_timeout(&mut self, cx: &mut commands::Context) -> EventResult {
commands::compute_inlay_hints_for_all_views(cx.editor, cx.jobs);

if let Some(completion) = &mut self.completion {
return if completion.ensure_item_resolved(cx) {
EventResult::Consumed(None)
} else {
EventResult::Ignored(None)
};
}

EventResult::Ignored(None)
}
}
Expand Down
Loading