completion: fully revert state before apply & insertText common prefix

4 files changed, 71 insertions, 44 deletions

diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs
index 0e49fbe3..dfc18fbe 100644
--- a/helix-core/src/transaction.rs
+++ b/helix-core/src/transaction.rs
@@ -468,6 +468,13 @@ impl Transaction {
         }
     }
 
+    pub fn compose(mut self, other: Self) -> Self {
+        self.changes = self.changes.compose(other.changes);
+        // Other selection takes precedence
+        self.selection = other.selection;
+        self
+    }
+
     pub fn with_selection(mut self, selection: Selection) -> Self {
         self.selection = Some(selection);
         self
diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs
index c75b24f1..44879fcf 100644
--- a/helix-term/src/ui/completion.rs
+++ b/helix-term/src/ui/completion.rs
@@ -5,7 +5,7 @@ use tui::buffer::Buffer as Surface;
 use std::borrow::Cow;
 
 use helix_core::Transaction;
-use helix_view::{graphics::Rect, Document, Editor, View};
+use helix_view::{graphics::Rect, Document, Editor};
 
 use crate::commands;
 use crate::ui::{menu, Markdown, Menu, Popup, PromptEvent};
@@ -83,13 +83,13 @@ impl Completion {
         start_offset: usize,
         trigger_offset: usize,
     ) -> Self {
-        // let items: Vec<CompletionItem> = Vec::new();
         let menu = Menu::new(items, move |editor: &mut Editor, item, event| {
             fn item_to_transaction(
                 doc: &Document,
-                view: &View,
                 item: &CompletionItem,
                 offset_encoding: helix_lsp::OffsetEncoding,
+                start_offset: usize,
+                trigger_offset: usize,
             ) -> Transaction {
                 if let Some(edit) = &item.text_edit {
                     let edit = match edit {
@@ -105,63 +105,52 @@ impl Completion {
                     )
                 } else {
                     let text = item.insert_text.as_ref().unwrap_or(&item.label);
-                    let cursor = doc
-                        .selection(view.id)
-                        .primary()
-                        .cursor(doc.text().slice(..));
+                    // Some LSPs just give you an insertText with no offset ¯\_(ツ)_/¯
+                    // in these cases we need to check for a common prefix and remove it
+                    let prefix = Cow::from(doc.text().slice(start_offset..trigger_offset));
+                    let text = text.trim_start_matches::<&str>(&prefix);
                     Transaction::change(
                         doc.text(),
-                        vec![(cursor, cursor, Some(text.as_str().into()))].into_iter(),
+                        vec![(trigger_offset, trigger_offset, Some(text.into()))].into_iter(),
                     )
                 }
             }
 
+            let (view, doc) = current!(editor);
+
+            // if more text was entered, remove it
+            doc.restore(view.id);
+
             match event {
                 PromptEvent::Abort => {}
                 PromptEvent::Update => {
-                    let (view, doc) = current!(editor);
-
                     // always present here
                     let item = item.unwrap();
 
-                    // if more text was entered, remove it
-                    // TODO: ideally to undo we should keep the last completion tx revert, and map it over new changes
-                    let cursor = doc
-                        .selection(view.id)
-                        .primary()
-                        .cursor(doc.text().slice(..));
-                    if trigger_offset < cursor {
-                        let remove = Transaction::change(
-                            doc.text(),
-                            vec![(trigger_offset, cursor, None)].into_iter(),
-                        );
-                        doc.apply(&remove, view.id);
-                    }
+                    let transaction = item_to_transaction(
+                        doc,
+                        item,
+                        offset_encoding,
+                        start_offset,
+                        trigger_offset,
+                    );
+
+                    // initialize a savepoint
+                    doc.savepoint();
 
-                    let transaction = item_to_transaction(doc, view, item, offset_encoding);
                     doc.apply(&transaction, view.id);
                 }
                 PromptEvent::Validate => {
-                    let (view, doc) = current!(editor);
-
                     // always present here
                     let item = item.unwrap();
 
-                    // if more text was entered, remove it
-                    // TODO: ideally to undo we should keep the last completion tx revert, and map it over new changes
-                    let cursor = doc
-                        .selection(view.id)
-                        .primary()
-                        .cursor(doc.text().slice(..));
-                    if trigger_offset < cursor {
-                        let remove = Transaction::change(
-                            doc.text(),
-                            vec![(trigger_offset, cursor, None)].into_iter(),
-                        );
-                        doc.apply(&remove, view.id);
-                    }
-
-                    let transaction = item_to_transaction(doc, view, item, offset_encoding);
+                    let transaction = item_to_transaction(
+                        doc,
+                        item,
+                        offset_encoding,
+                        start_offset,
+                        trigger_offset,
+                    );
                     doc.apply(&transaction, view.id);
 
                     if let Some(additional_edits) = &item.additional_text_edits {
diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs
index 692696a6..850fec0f 100644
--- a/helix-term/src/ui/editor.rs
+++ b/helix-term/src/ui/editor.rs
@@ -13,7 +13,7 @@ use helix_core::{
     syntax::{self, HighlightEvent},
     unicode::segmentation::UnicodeSegmentation,
     unicode::width::UnicodeWidthStr,
-    LineEnding, Position, Range, Selection,
+    LineEnding, Position, Range, Selection, Transaction,
 };
 use helix_view::{
     document::Mode,
@@ -721,7 +721,7 @@ impl EditorView {
 
     pub fn set_completion(
         &mut self,
-        editor: &Editor,
+        editor: &mut Editor,
         items: Vec<helix_lsp::lsp::CompletionItem>,
         offset_encoding: helix_lsp::OffsetEncoding,
         start_offset: usize,
@@ -736,6 +736,9 @@ impl EditorView {
             return;
         }
 
+        // Immediately initialize a savepoint
+        doc_mut!(editor).savepoint();
+
         // TODO : propagate required size on resize to completion too
         completion.required_size((size.width, size.height));
         self.completion = Some(completion);
@@ -945,6 +948,9 @@ impl Component for EditorView {
                                     if callback.is_some() {
                                         // assume close_fn
                                         self.completion = None;
+                                        // Clear any savepoints
+                                        let (_, doc) = current!(cxt.editor);
+                                        doc.savepoint = None;
                                         cxt.editor.clear_idle_timer(); // don't retrigger
                                     }
                                 }
@@ -959,6 +965,9 @@ impl Component for EditorView {
                                     completion.update(&mut cxt);
                                     if completion.is_empty() {
                                         self.completion = None;
+                                        // Clear any savepoints
+                                        let (_, doc) = current!(cxt.editor);
+                                        doc.savepoint = None;
                                         cxt.editor.clear_idle_timer(); // don't retrigger
                                     }
                                 }
diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index 8804681b..23c2dbc6 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -97,6 +97,9 @@ pub struct Document {
     // it back as it separated from the edits. We could split out the parts manually but that will
     // be more troublesome.
     history: Cell<History>,
+
+    pub savepoint: Option<Transaction>,
+
     last_saved_revision: usize,
     version: i32, // should be usize?
 
@@ -328,6 +331,7 @@ impl Document {
             text,
             selections: HashMap::default(),
             indent_style: DEFAULT_INDENT,
+            line_ending: DEFAULT_LINE_ENDING,
             mode: Mode::Normal,
             restore_cursor: false,
             syntax: None,
@@ -337,9 +341,9 @@ impl Document {
             diagnostics: Vec::new(),
             version: 0,
             history: Cell::new(History::default()),
+            savepoint: None,
             last_saved_revision: 0,
             language_server: None,
-            line_ending: DEFAULT_LINE_ENDING,
         }
     }
 
@@ -635,6 +639,14 @@ impl Document {
         if !transaction.changes().is_empty() {
             self.version += 1;
 
+            // generate revert to savepoint
+            if self.savepoint.is_some() {
+                take_with(&mut self.savepoint, |prev_revert| {
+                    let revert = transaction.invert(&old_doc);
+                    Some(revert.compose(prev_revert.unwrap()))
+                });
+            }
+
             // update tree-sitter syntax tree
             if let Some(syntax) = &mut self.syntax {
                 // TODO: no unwrap
@@ -724,6 +736,16 @@ impl Document {
         }
     }
 
+    pub fn savepoint(&mut self) {
+        self.savepoint = Some(Transaction::new(self.text()));
+    }
+
+    pub fn restore(&mut self, view_id: ViewId) {
+        if let Some(revert) = self.savepoint.take() {
+            self.apply(&revert, view_id);
+        }
+    }
+
     /// Undo modifications to the [`Document`] according to `uk`.
     pub fn earlier(&mut self, view_id: ViewId, uk: helix_core::history::UndoKind) {
         let txns = self.history.get_mut().earlier(uk);