From 8cb7cdfd7a01b9cb50b9142e9a5d133bd1e23256 Mon Sep 17 00:00:00 2001
From: Pascal Kuthe <pascal.kuthe@semimod.de>
Date: Thu, 2 Mar 2023 23:12:50 +0100
Subject: [PATCH] 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.
---
 helix-term/src/commands.rs  | 31 +++++++++++++++++++++++++++++--
 helix-term/src/ui/editor.rs |  1 +
 helix-view/src/editor.rs    | 11 ++++++++++-
 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index 01673c895..574e1edf6 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -5,6 +5,7 @@
 pub use dap::*;
 use helix_vcs::Hunk;
 pub use lsp::*;
+use tokio::sync::oneshot;
 use tui::widgets::Row;
 pub use typed::*;
 
@@ -4171,6 +4172,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
@@ -4183,11 +4202,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;
             }
 
diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs
index c81ae635a..859176fb4 100644
--- a/helix-term/src/ui/editor.rs
+++ b/helix-term/src/ui/editor.rs
@@ -820,6 +820,7 @@ fn handle_keymap_event(
                 (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 {
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index 5b819b333..c6541105a 100644
--- a/helix-view/src/editor.rs
+++ b/helix-view/src/editor.rs
@@ -31,7 +31,7 @@
 use tokio::{
     sync::{
         mpsc::{unbounded_channel, UnboundedReceiver, UnboundedSender},
-        Notify, RwLock,
+        oneshot, Notify, RwLock,
     },
     time::{sleep, Duration, Instant, Sleep},
 };
@@ -852,6 +852,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<()>>);
@@ -950,6 +958,7 @@ pub fn new(
             redraw_handle: Default::default(),
             needs_redraw: false,
             cursor_cache: Cell::new(None),
+            completion_request_handle: None,
         }
     }