Skip to content

Commit

Permalink
discard stale completion requests
Browse files Browse the repository at this point in the history
Completion requests are computed asynchronously to avoid common micro
freezes while editing. This means that once a completion request
completes, the state of the editor might have changed. Currently,
there is a check to ensure we are still in insert mode. However,
we also need to ensure that the view and document hasn't changed
to avoid accidentally using a savepoint with the wrong view/document.

Furthermore, the editor might request a new completion while the
previous completion request hasn't complemented yet. This can
lead to weird flickering or an outdated completion request replacing
a newer completion that has already completed (the LSP server
is not required to process completion requests in order). This change
also needed to ensure determinism/linear ordering so that completion
popup always correspond to the last completion request.
  • Loading branch information
pascalkuthe committed Mar 4, 2023
1 parent 616226d commit 56bf37a
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 3 deletions.
31 changes: 29 additions & 2 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub(crate) mod typed;
pub use dap::*;
use helix_vcs::Hunk;
pub use lsp::*;
use tokio::sync::oneshot;
use tui::widgets::Row;
pub use typed::*;

Expand Down Expand Up @@ -4173,6 +4174,24 @@ pub fn completion(cx: &mut Context) {
None => return,
};

// setup a chanel that allows the request to be canceled
let (tx, rx) = oneshot::channel();
// set completion_request so that this request can be canceled
// by setting completion_request, the old channel stored there is dropped
// and the associated request is automatically dropped
cx.editor.completion_request_handle = Some(tx);
let future = async move {
tokio::select! {
biased;
_ = rx => {
Ok(serde_json::Value::Null)
}
res = future => {
res
}
}
};

let trigger_offset = cursor;

// TODO: trigger_offset should be the cursor offset but we also need a starting offset from where we want to apply
Expand All @@ -4185,11 +4204,19 @@ pub fn completion(cx: &mut Context) {
let start_offset = cursor.saturating_sub(offset);
let savepoint = doc.savepoint(view);

let trigger_doc = doc.id();
let trigger_view = view.id;

cx.callback(
future,
move |editor, compositor, response: Option<lsp::CompletionResponse>| {
if editor.mode != Mode::Insert {
// we're not in insert mode anymore
let (view, doc) = current_ref!(editor);
// check if the completion request is stale.
//
// Completions are completed asynchrounsly and therefore the user could
//switch document/view or leave insert mode. In all of thoise cases the
// completion should be discarded
if editor.mode != Mode::Insert || view.id != trigger_view || doc.id() != trigger_doc {
return;
}

Expand Down
1 change: 1 addition & 0 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ impl EditorView {
(Mode::Insert, Mode::Normal) => {
// if exiting insert mode, remove completion
self.completion = None;
cxt.editor.completion_request_handle = None;

// TODO: Use an on_mode_change hook to remove signature help
cxt.jobs.callback(async {
Expand Down
11 changes: 10 additions & 1 deletion helix-view/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use std::{
use tokio::{
sync::{
mpsc::{unbounded_channel, UnboundedReceiver, UnboundedSender},
Notify, RwLock,
oneshot, Notify, RwLock,
},
time::{sleep, Duration, Instant, Sleep},
};
Expand Down Expand Up @@ -881,6 +881,14 @@ pub struct Editor {
/// avoid calculating the cursor position multiple
/// times during rendering and should not be set by other functions.
pub cursor_cache: Cell<Option<Option<Position>>>,
/// When a new completion request is sent to the server old
/// unifinished request must be dropped. Each completion
/// request is associated with a channel that cancels
/// when the channel is dropped. That channel is stored
/// here. When a new completion request is sent this
/// field is set and any old requests are automatically
/// canceled as a result
pub completion_request_handle: Option<oneshot::Sender<()>>,
}

pub type RedrawHandle = (Arc<Notify>, Arc<RwLock<()>>);
Expand Down Expand Up @@ -979,6 +987,7 @@ impl Editor {
redraw_handle: Default::default(),
needs_redraw: false,
cursor_cache: Cell::new(None),
completion_request_handle: None,
}
}

Expand Down

0 comments on commit 56bf37a

Please sign in to comment.