Skip to content

Commit 56bf37a

Browse files
committed
discard stale completion requests
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.
1 parent 616226d commit 56bf37a

File tree

3 files changed

+40
-3
lines changed

3 files changed

+40
-3
lines changed

helix-term/src/commands.rs

+29-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ pub(crate) mod typed;
55
pub use dap::*;
66
use helix_vcs::Hunk;
77
pub use lsp::*;
8+
use tokio::sync::oneshot;
89
use tui::widgets::Row;
910
pub use typed::*;
1011

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

4177+
// setup a chanel that allows the request to be canceled
4178+
let (tx, rx) = oneshot::channel();
4179+
// set completion_request so that this request can be canceled
4180+
// by setting completion_request, the old channel stored there is dropped
4181+
// and the associated request is automatically dropped
4182+
cx.editor.completion_request_handle = Some(tx);
4183+
let future = async move {
4184+
tokio::select! {
4185+
biased;
4186+
_ = rx => {
4187+
Ok(serde_json::Value::Null)
4188+
}
4189+
res = future => {
4190+
res
4191+
}
4192+
}
4193+
};
4194+
41764195
let trigger_offset = cursor;
41774196

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

4207+
let trigger_doc = doc.id();
4208+
let trigger_view = view.id;
4209+
41884210
cx.callback(
41894211
future,
41904212
move |editor, compositor, response: Option<lsp::CompletionResponse>| {
4191-
if editor.mode != Mode::Insert {
4192-
// we're not in insert mode anymore
4213+
let (view, doc) = current_ref!(editor);
4214+
// check if the completion request is stale.
4215+
//
4216+
// Completions are completed asynchrounsly and therefore the user could
4217+
//switch document/view or leave insert mode. In all of thoise cases the
4218+
// completion should be discarded
4219+
if editor.mode != Mode::Insert || view.id != trigger_view || doc.id() != trigger_doc {
41934220
return;
41944221
}
41954222

helix-term/src/ui/editor.rs

+1
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,7 @@ impl EditorView {
820820
(Mode::Insert, Mode::Normal) => {
821821
// if exiting insert mode, remove completion
822822
self.completion = None;
823+
cxt.editor.completion_request_handle = None;
823824

824825
// TODO: Use an on_mode_change hook to remove signature help
825826
cxt.jobs.callback(async {

helix-view/src/editor.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use std::{
3131
use tokio::{
3232
sync::{
3333
mpsc::{unbounded_channel, UnboundedReceiver, UnboundedSender},
34-
Notify, RwLock,
34+
oneshot, Notify, RwLock,
3535
},
3636
time::{sleep, Duration, Instant, Sleep},
3737
};
@@ -881,6 +881,14 @@ pub struct Editor {
881881
/// avoid calculating the cursor position multiple
882882
/// times during rendering and should not be set by other functions.
883883
pub cursor_cache: Cell<Option<Option<Position>>>,
884+
/// When a new completion request is sent to the server old
885+
/// unifinished request must be dropped. Each completion
886+
/// request is associated with a channel that cancels
887+
/// when the channel is dropped. That channel is stored
888+
/// here. When a new completion request is sent this
889+
/// field is set and any old requests are automatically
890+
/// canceled as a result
891+
pub completion_request_handle: Option<oneshot::Sender<()>>,
884892
}
885893

886894
pub type RedrawHandle = (Arc<Notify>, Arc<RwLock<()>>);
@@ -979,6 +987,7 @@ impl Editor {
979987
redraw_handle: Default::default(),
980988
needs_redraw: false,
981989
cursor_cache: Cell::new(None),
990+
completion_request_handle: None,
982991
}
983992
}
984993

0 commit comments

Comments
 (0)