Simplify `undo_to_transaction` and `redo_to_transaction`

Antonio Scandurra created

We don't need to mutate the history anymore now that we render pending renames
with a block decoration.

Change summary

crates/editor/src/multi_buffer.rs | 59 --------------------------
crates/language/src/buffer.rs     |  6 -
crates/text/src/text.rs           | 70 +++++++++++---------------------
crates/util/src/lib.rs            | 35 ----------------
4 files changed, 29 insertions(+), 141 deletions(-)

Detailed changes

crates/editor/src/multi_buffer.rs 🔗

@@ -27,7 +27,6 @@ use text::{
     AnchorRangeExt as _, Edit, Point, PointUtf16, TextSummary,
 };
 use theme::SyntaxTheme;
-use util::CowMut;
 
 const NEWLINES: &'static [u8] = &[b'\n'; u8::MAX as usize];
 
@@ -566,7 +565,7 @@ impl MultiBuffer {
                         if let Some(entry) = buffer.peek_undo_stack() {
                             *buffer_transaction_id = entry.transaction_id();
                         }
-                        buffer.undo_to_transaction(undo_to, true, cx)
+                        buffer.undo_to_transaction(undo_to, cx)
                     });
                 }
             }
@@ -579,35 +578,6 @@ impl MultiBuffer {
         None
     }
 
-    pub fn undo_to_transaction(
-        &mut self,
-        transaction_id: TransactionId,
-        push_redo: bool,
-        cx: &mut ModelContext<Self>,
-    ) -> bool {
-        if let Some(buffer) = self.as_singleton() {
-            return buffer.update(cx, |buffer, cx| {
-                buffer.undo_to_transaction(transaction_id, push_redo, cx)
-            });
-        }
-
-        let mut undone = false;
-        for transaction in &mut *self.history.remove_from_undo(transaction_id, push_redo) {
-            for (buffer_id, buffer_transaction_id) in &mut transaction.buffer_transactions {
-                if let Some(BufferState { buffer, .. }) = self.buffers.borrow().get(&buffer_id) {
-                    undone |= buffer.update(cx, |buffer, cx| {
-                        let undo_to = *buffer_transaction_id;
-                        if let Some(entry) = buffer.peek_undo_stack() {
-                            *buffer_transaction_id = entry.transaction_id();
-                        }
-                        buffer.undo_to_transaction(undo_to, true, cx)
-                    });
-                }
-            }
-        }
-        undone
-    }
-
     pub fn redo(&mut self, cx: &mut ModelContext<Self>) -> Option<TransactionId> {
         if let Some(buffer) = self.as_singleton() {
             return buffer.update(cx, |buffer, cx| buffer.redo(cx));
@@ -622,7 +592,7 @@ impl MultiBuffer {
                         if let Some(entry) = buffer.peek_redo_stack() {
                             *buffer_transaction_id = entry.transaction_id();
                         }
-                        buffer.redo_to_transaction(redo_to, true, cx)
+                        buffer.redo_to_transaction(redo_to, cx)
                     });
                 }
             }
@@ -2345,31 +2315,6 @@ impl History {
         }
     }
 
-    fn remove_from_undo(
-        &mut self,
-        transaction_id: TransactionId,
-        push_redo: bool,
-    ) -> CowMut<[Transaction]> {
-        assert_eq!(self.transaction_depth, 0);
-
-        if let Some(entry_ix) = self
-            .undo_stack
-            .iter()
-            .rposition(|transaction| transaction.id == transaction_id)
-        {
-            let transactions = self.undo_stack.drain(entry_ix..).rev();
-            if push_redo {
-                let redo_stack_start_len = self.redo_stack.len();
-                self.redo_stack.extend(transactions);
-                CowMut::Borrowed(&mut self.redo_stack[redo_stack_start_len..])
-            } else {
-                CowMut::Owned(transactions.collect())
-            }
-        } else {
-            CowMut::Owned(Default::default())
-        }
-    }
-
     fn pop_redo(&mut self) -> Option<&mut Transaction> {
         assert_eq!(self.transaction_depth, 0);
         if let Some(transaction) = self.redo_stack.pop() {

crates/language/src/buffer.rs 🔗

@@ -1738,13 +1738,12 @@ impl Buffer {
     pub fn undo_to_transaction(
         &mut self,
         transaction_id: TransactionId,
-        push_redo: bool,
         cx: &mut ModelContext<Self>,
     ) -> bool {
         let was_dirty = self.is_dirty();
         let old_version = self.version.clone();
 
-        let operations = self.text.undo_to_transaction(transaction_id, push_redo);
+        let operations = self.text.undo_to_transaction(transaction_id);
         let undone = !operations.is_empty();
         for operation in operations {
             self.send_operation(Operation::Buffer(operation), cx);
@@ -1771,13 +1770,12 @@ impl Buffer {
     pub fn redo_to_transaction(
         &mut self,
         transaction_id: TransactionId,
-        push_undo: bool,
         cx: &mut ModelContext<Self>,
     ) -> bool {
         let was_dirty = self.is_dirty();
         let old_version = self.version.clone();
 
-        let operations = self.text.redo_to_transaction(transaction_id, push_undo);
+        let operations = self.text.redo_to_transaction(transaction_id);
         let redone = !operations.is_empty();
         for operation in operations {
             self.send_operation(Operation::Buffer(operation), cx);

crates/text/src/text.rs 🔗

@@ -285,31 +285,19 @@ impl History {
         }
     }
 
-    fn remove_from_undo(
-        &mut self,
-        transaction_id: TransactionId,
-        push_redo: bool,
-    ) -> Vec<Transaction> {
+    fn remove_from_undo(&mut self, transaction_id: TransactionId) -> &[HistoryEntry] {
         assert_eq!(self.transaction_depth, 0);
 
-        let mut transactions = Vec::new();
+        let redo_stack_start_len = self.redo_stack.len();
         if let Some(entry_ix) = self
             .undo_stack
             .iter()
             .rposition(|entry| entry.transaction.id == transaction_id)
         {
-            transactions.extend(
-                self.undo_stack[entry_ix..]
-                    .iter()
-                    .rev()
-                    .map(|entry| entry.transaction.clone()),
-            );
-            let transactions = self.undo_stack.drain(entry_ix..).rev();
-            if push_redo {
-                self.redo_stack.extend(transactions);
-            }
+            self.redo_stack
+                .extend(self.undo_stack.drain(entry_ix..).rev());
         }
-        transactions
+        &self.redo_stack[redo_stack_start_len..]
     }
 
     fn forget(&mut self, transaction_id: TransactionId) {
@@ -339,31 +327,19 @@ impl History {
         }
     }
 
-    fn remove_from_redo(
-        &mut self,
-        transaction_id: TransactionId,
-        push_undo: bool,
-    ) -> Vec<Transaction> {
+    fn remove_from_redo(&mut self, transaction_id: TransactionId) -> &[HistoryEntry] {
         assert_eq!(self.transaction_depth, 0);
 
-        let mut transactions = Vec::new();
+        let undo_stack_start_len = self.undo_stack.len();
         if let Some(entry_ix) = self
             .redo_stack
             .iter()
             .rposition(|entry| entry.transaction.id == transaction_id)
         {
-            transactions.extend(
-                self.redo_stack[entry_ix..]
-                    .iter()
-                    .rev()
-                    .map(|entry| entry.transaction.clone()),
-            );
-            if push_undo {
-                self.undo_stack
-                    .extend(self.redo_stack.drain(entry_ix..).rev());
-            }
+            self.undo_stack
+                .extend(self.redo_stack.drain(entry_ix..).rev());
         }
-        transactions
+        &self.undo_stack[undo_stack_start_len..]
     }
 }
 
@@ -1239,12 +1215,13 @@ impl Buffer {
         }
     }
 
-    pub fn undo_to_transaction(
-        &mut self,
-        transaction_id: TransactionId,
-        push_redo: bool,
-    ) -> Vec<Operation> {
-        let transactions = self.history.remove_from_undo(transaction_id, push_redo);
+    pub fn undo_to_transaction(&mut self, transaction_id: TransactionId) -> Vec<Operation> {
+        let transactions = self
+            .history
+            .remove_from_undo(transaction_id)
+            .iter()
+            .map(|entry| entry.transaction.clone())
+            .collect::<Vec<_>>();
         transactions
             .into_iter()
             .map(|transaction| self.undo_or_redo(transaction).unwrap())
@@ -1266,12 +1243,13 @@ impl Buffer {
         }
     }
 
-    pub fn redo_to_transaction(
-        &mut self,
-        transaction_id: TransactionId,
-        push_undo: bool,
-    ) -> Vec<Operation> {
-        let transactions = self.history.remove_from_redo(transaction_id, push_undo);
+    pub fn redo_to_transaction(&mut self, transaction_id: TransactionId) -> Vec<Operation> {
+        let transactions = self
+            .history
+            .remove_from_redo(transaction_id)
+            .iter()
+            .map(|entry| entry.transaction.clone())
+            .collect::<Vec<_>>();
         transactions
             .into_iter()
             .map(|transaction| self.undo_or_redo(transaction).unwrap())

crates/util/src/lib.rs 🔗

@@ -3,9 +3,8 @@ pub mod test;
 
 use futures::Future;
 use std::{
-    borrow::{Borrow, BorrowMut},
     cmp::Ordering,
-    ops::{AddAssign, Deref, DerefMut},
+    ops::AddAssign,
     pin::Pin,
     task::{Context, Poll},
 };
@@ -124,38 +123,6 @@ where
     }
 }
 
-pub enum CowMut<'a, T: ?Sized + ToOwned> {
-    Borrowed(&'a mut T),
-    Owned(T::Owned),
-}
-
-impl<'a, T> Deref for CowMut<'a, T>
-where
-    T: ?Sized + ToOwned,
-{
-    type Target = T;
-
-    fn deref(&self) -> &Self::Target {
-        match self {
-            CowMut::Borrowed(value) => value,
-            CowMut::Owned(value) => value.borrow(),
-        }
-    }
-}
-
-impl<'a, T> DerefMut for CowMut<'a, T>
-where
-    T: ?Sized + ToOwned,
-    T::Owned: BorrowMut<T>,
-{
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        match self {
-            CowMut::Borrowed(value) => value,
-            CowMut::Owned(value) => value.borrow_mut(),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;