From c0e17dd324f016401d56d66b7c113dada0644155 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= <blaz@mxxn.io>
Date: Fri, 23 Oct 2020 11:32:25 +0900
Subject: [PATCH] Fix undo/redo not updating the syntax tree.

---
 helix-core/src/history.rs  | 46 +++++++++++++++++++++-----------------
 helix-view/src/commands.rs |  8 +++++--
 helix-view/src/document.rs |  1 -
 3 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/helix-core/src/history.rs b/helix-core/src/history.rs
index e6d9a738c..664455251 100644
--- a/helix-core/src/history.rs
+++ b/helix-core/src/history.rs
@@ -57,37 +57,31 @@ impl History {
         self.cursor == 0
     }
 
-    pub fn undo(&mut self, state: &mut State) {
+    // TODO: I'd like to pass Transaction by reference but it fights with the borrowck
+
+    pub fn undo(&mut self) -> Option<Transaction> {
         if self.at_root() {
             // We're at the root of undo, nothing to do.
-            return;
+            return None;
         }
 
         let current_revision = &self.revisions[self.cursor];
 
-        // TODO: pass the return value through? It should always succeed
-        let success = current_revision.revert.apply(state);
-
-        if !success {
-            panic!("Failed to apply undo!");
-        }
-
         self.cursor = current_revision.parent;
+
+        Some(current_revision.revert.clone())
     }
 
-    pub fn redo(&mut self, state: &mut State) {
+    pub fn redo(&mut self) -> Option<Transaction> {
         let current_revision = &self.revisions[self.cursor];
 
         // for now, simply pick the latest child (linear undo / redo)
         if let Some((index, transaction)) = current_revision.children.last() {
-            let success = transaction.apply(state);
-
-            if !success {
-                panic!("Failed to apply redo!");
-            }
-
             self.cursor = *index;
+
+            return Some(transaction.clone());
         }
+        None
     }
 }
 
@@ -120,17 +114,27 @@ mod test {
         assert_eq!("hello 世界!", state.doc());
 
         // ---
+        fn undo(history: &mut History, state: &mut State) {
+            if let Some(transaction) = history.undo() {
+                transaction.apply(state);
+            }
+        }
+        fn redo(history: &mut History, state: &mut State) {
+            if let Some(transaction) = history.redo() {
+                transaction.apply(state);
+            }
+        }
 
-        history.undo(&mut state);
+        undo(&mut history, &mut state);
         assert_eq!("hello world!", state.doc());
-        history.redo(&mut state);
+        redo(&mut history, &mut state);
         assert_eq!("hello 世界!", state.doc());
-        history.undo(&mut state);
-        history.undo(&mut state);
+        undo(&mut history, &mut state);
+        undo(&mut history, &mut state);
         assert_eq!("hello", state.doc());
 
         // undo at root is a no-op
-        history.undo(&mut state);
+        undo(&mut history, &mut state);
         assert_eq!("hello", state.doc());
     }
 }
diff --git a/helix-view/src/commands.rs b/helix-view/src/commands.rs
index b5350ff46..6bf89040f 100644
--- a/helix-view/src/commands.rs
+++ b/helix-view/src/commands.rs
@@ -509,13 +509,17 @@ pub fn insert_char_prompt(prompt: &mut Prompt, c: char) {
 // Undo / Redo
 
 pub fn undo(view: &mut View, _count: usize) {
-    view.doc.history.undo(&mut view.doc.state);
+    if let Some(revert) = view.doc.history.undo() {
+        view.doc.apply(&revert);
+    }
 
     // TODO: each command could simply return a Option<transaction>, then the higher level handles storing it?
 }
 
 pub fn redo(view: &mut View, _count: usize) {
-    view.doc.history.redo(&mut view.doc.state);
+    if let Some(transaction) = view.doc.history.redo() {
+        view.doc.apply(&transaction);
+    }
 }
 
 // Yank / Paste
diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs
index c4b9d0819..04018ed6a 100644
--- a/helix-view/src/document.rs
+++ b/helix-view/src/document.rs
@@ -108,7 +108,6 @@ impl Document {
         };
     }
 
-    // TODO: needs to run on undo/redo
     pub fn apply(&mut self, transaction: &Transaction) -> bool {
         let old_doc = self.text().clone();