Return optional transaction ids when starting/ending a transaction

Nathan Sobo created

If the transaction was nested, we return None. Otherwise we return the transaction id in preparation for editors to maintain their own selection state.

Change summary

crates/editor/src/editor.rs       | 11 +++------
crates/editor/src/multi_buffer.rs |  6 ++--
crates/language/src/buffer.rs     | 38 +++++++++++++++++---------------
crates/language/src/tests.rs      | 16 ++++++------
crates/text/src/tests.rs          | 10 ++++----
crates/text/src/text.rs           | 34 ++++++++++++++++++++--------
6 files changed, 64 insertions(+), 51 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -25,11 +25,10 @@ use language::{
     BracketPair, Buffer, Diagnostic, DiagnosticSeverity, Language, Point, Selection, SelectionGoal,
     SelectionSetId,
 };
+pub use multi_buffer::MultiBuffer;
 use multi_buffer::{
-    Anchor, AnchorRangeExt, MultiBufferChunks, MultiBufferSnapshot,
-    SelectionSet, ToOffset, ToPoint,
+    Anchor, AnchorRangeExt, MultiBufferChunks, MultiBufferSnapshot, SelectionSet, ToOffset, ToPoint,
 };
-pub use multi_buffer::MultiBuffer;
 use serde::{Deserialize, Serialize};
 use smallvec::SmallVec;
 use smol::Timer;
@@ -3209,15 +3208,13 @@ impl Editor {
     fn start_transaction(&mut self, cx: &mut ViewContext<Self>) {
         self.end_selection(cx);
         self.buffer.update(cx, |buffer, cx| {
-            buffer
-                .start_transaction([self.selection_set_id], cx)
-                .unwrap()
+            buffer.start_transaction([self.selection_set_id], cx);
         });
     }
 
     fn end_transaction(&self, cx: &mut ViewContext<Self>) {
         self.buffer.update(cx, |buffer, cx| {
-            buffer.end_transaction([self.selection_set_id], cx).unwrap()
+            buffer.end_transaction([self.selection_set_id], cx);
         });
     }
 

crates/editor/src/multi_buffer.rs 🔗

@@ -8,7 +8,7 @@ use collections::HashMap;
 use gpui::{AppContext, Entity, ModelContext, ModelHandle, MutableAppContext, Task};
 use language::{
     Buffer, BufferChunks, BufferSnapshot, Chunk, DiagnosticEntry, Event, File, Language,
-    ToOffset as _, ToPoint as _,
+    ToOffset as _, ToPoint as _, TransactionId,
 };
 pub use selection::SelectionSet;
 use std::{
@@ -206,7 +206,7 @@ impl MultiBuffer {
         &mut self,
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
         cx: &mut ModelContext<Self>,
-    ) -> Result<()> {
+    ) -> Option<TransactionId> {
         // TODO
         self.as_singleton()
             .unwrap()
@@ -217,7 +217,7 @@ impl MultiBuffer {
         &mut self,
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
         cx: &mut ModelContext<Self>,
-    ) -> Result<()> {
+    ) -> Option<TransactionId> {
         // TODO
         self.as_singleton().unwrap().update(cx, |buffer, cx| {
             buffer.end_transaction(selection_set_ids, cx)

crates/language/src/buffer.rs 🔗

@@ -977,8 +977,7 @@ impl Buffer {
             .flat_map(|req| req.selection_set_ids.clone())
             .collect::<HashSet<_>>();
 
-        self.start_transaction(selection_set_ids.iter().copied())
-            .unwrap();
+        self.start_transaction(selection_set_ids.iter().copied());
         for (row, indent_column) in &indent_columns {
             self.set_indent_column_for_line(*row, *indent_column, cx);
         }
@@ -1014,8 +1013,7 @@ impl Buffer {
             }
         }
 
-        self.end_transaction(selection_set_ids.iter().copied(), cx)
-            .unwrap();
+        self.end_transaction(selection_set_ids.iter().copied(), cx);
     }
 
     fn set_indent_column_for_line(&mut self, row: u32, column: u32, cx: &mut ModelContext<Self>) {
@@ -1055,7 +1053,7 @@ impl Buffer {
 
     pub(crate) fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext<Self>) -> bool {
         if self.version == diff.base_version {
-            self.start_transaction(None).unwrap();
+            self.start_transaction(None);
             let mut offset = 0;
             for (tag, len) in diff.changes {
                 let range = offset..(offset + len);
@@ -1068,7 +1066,7 @@ impl Buffer {
                     }
                 }
             }
-            self.end_transaction(None, cx).unwrap();
+            self.end_transaction(None, cx);
             true
         } else {
             false
@@ -1095,7 +1093,7 @@ impl Buffer {
     pub fn start_transaction(
         &mut self,
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
-    ) -> Result<()> {
+    ) -> Option<TransactionId> {
         self.start_transaction_at(selection_set_ids, Instant::now())
     }
 
@@ -1103,7 +1101,7 @@ impl Buffer {
         &mut self,
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
         now: Instant,
-    ) -> Result<()> {
+    ) -> Option<TransactionId> {
         self.text.start_transaction_at(selection_set_ids, now)
     }
 
@@ -1111,7 +1109,7 @@ impl Buffer {
         &mut self,
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
         cx: &mut ModelContext<Self>,
-    ) -> Result<()> {
+    ) -> Option<TransactionId> {
         self.end_transaction_at(selection_set_ids, Instant::now(), cx)
     }
 
@@ -1120,12 +1118,16 @@ impl Buffer {
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
         now: Instant,
         cx: &mut ModelContext<Self>,
-    ) -> Result<()> {
-        if let Some(start_version) = self.text.end_transaction_at(selection_set_ids, now) {
+    ) -> Option<TransactionId> {
+        if let Some((transaction_id, start_version)) =
+            self.text.end_transaction_at(selection_set_ids, now)
+        {
             let was_dirty = start_version != self.saved_version;
             self.did_edit(&start_version, was_dirty, cx);
+            Some(transaction_id)
+        } else {
+            None
         }
-        Ok(())
     }
 
     fn update_language_server(&mut self) {
@@ -1210,7 +1212,7 @@ impl Buffer {
             return;
         }
 
-        self.start_transaction(None).unwrap();
+        self.start_transaction(None);
         self.pending_autoindent.take();
         let autoindent_request = if autoindent && self.language.is_some() {
             let before_edit = self.snapshot();
@@ -1268,7 +1270,7 @@ impl Buffer {
             }));
         }
 
-        self.end_transaction(None, cx).unwrap();
+        self.end_transaction(None, cx);
         self.send_operation(Operation::Buffer(text::Operation::Edit(edit)), cx);
     }
 
@@ -1474,18 +1476,18 @@ impl Buffer {
     ) where
         T: rand::Rng,
     {
-        self.start_transaction(None).unwrap();
+        self.start_transaction(None);
         self.text.randomly_edit(rng, old_range_count);
-        self.end_transaction(None, cx).unwrap();
+        self.end_transaction(None, cx);
     }
 
     pub fn randomly_mutate<T>(&mut self, rng: &mut T, cx: &mut ModelContext<Self>)
     where
         T: rand::Rng,
     {
-        self.start_transaction(None).unwrap();
+        self.start_transaction(None);
         self.text.randomly_mutate(rng);
-        self.end_transaction(None, cx).unwrap();
+        self.end_transaction(None, cx);
     }
 }
 

crates/language/src/tests.rs 🔗

@@ -92,15 +92,15 @@ fn test_edit_events(cx: &mut gpui::MutableAppContext) {
         buffer.edit(Some(2..4), "XYZ", cx);
 
         // An empty transaction does not emit any events.
-        buffer.start_transaction(None).unwrap();
-        buffer.end_transaction(None, cx).unwrap();
+        buffer.start_transaction(None);
+        buffer.end_transaction(None, cx);
 
         // A transaction containing two edits emits one edited event.
         now += Duration::from_secs(1);
-        buffer.start_transaction_at(None, now).unwrap();
+        buffer.start_transaction_at(None, now);
         buffer.edit(Some(5..5), "u", cx);
         buffer.edit(Some(6..6), "w", cx);
-        buffer.end_transaction_at(None, now, cx).unwrap();
+        buffer.end_transaction_at(None, now, cx);
 
         // Undoing a transaction emits one edited event.
         buffer.undo(cx);
@@ -167,7 +167,7 @@ async fn test_reparse(mut cx: gpui::TestAppContext) {
     // Perform some edits (add parameter and variable reference)
     // Parsing doesn't begin until the transaction is complete
     buffer.update(&mut cx, |buf, cx| {
-        buf.start_transaction(None).unwrap();
+        buf.start_transaction(None);
 
         let offset = buf.text().find(")").unwrap();
         buf.edit(vec![offset..offset], "b: C", cx);
@@ -177,7 +177,7 @@ async fn test_reparse(mut cx: gpui::TestAppContext) {
         buf.edit(vec![offset..offset], " d; ", cx);
         assert!(!buf.is_parsing());
 
-        buf.end_transaction(None, cx).unwrap();
+        buf.end_transaction(None, cx);
         assert_eq!(buf.text(), "fn a(b: C) { d; }");
         assert!(buf.is_parsing());
     });
@@ -342,7 +342,7 @@ fn test_autoindent_moves_selections(cx: &mut MutableAppContext) {
             Buffer::new(0, text, cx).with_language(Some(Arc::new(rust_lang())), None, cx);
 
         let selection_set_id = buffer.add_selection_set::<usize>(&[], cx);
-        buffer.start_transaction(Some(selection_set_id)).unwrap();
+        buffer.start_transaction(Some(selection_set_id));
         buffer.edit_with_autoindent([5..5, 9..9], "\n\n", cx);
         buffer
             .update_selection_set(
@@ -370,7 +370,7 @@ fn test_autoindent_moves_selections(cx: &mut MutableAppContext) {
 
         // Ending the transaction runs the auto-indent. The selection
         // at the start of the auto-indented row is pushed to the right.
-        buffer.end_transaction(Some(selection_set_id), cx).unwrap();
+        buffer.end_transaction(Some(selection_set_id), cx);
         assert_eq!(buffer.text(), "fn a(\n    \n) {}\n\n");
         let selection_ranges = buffer
             .selection_set(selection_set_id)

crates/text/src/tests.rs 🔗

@@ -467,13 +467,13 @@ fn test_history() {
     } else {
         unreachable!()
     };
-    buffer.start_transaction_at(Some(set_id), now).unwrap();
+    buffer.start_transaction_at(Some(set_id), now);
     buffer.edit(vec![2..4], "cd");
-    buffer.end_transaction_at(Some(set_id), now).unwrap();
+    buffer.end_transaction_at(Some(set_id), now);
     assert_eq!(buffer.text(), "12cd56");
     assert_eq!(buffer.selection_ranges(set_id).unwrap(), vec![4..4]);
 
-    buffer.start_transaction_at(Some(set_id), now).unwrap();
+    buffer.start_transaction_at(Some(set_id), now);
     buffer
         .update_selection_set(set_id, &buffer.selections_from_ranges(vec![1..3]).unwrap())
         .unwrap();
@@ -483,7 +483,7 @@ fn test_history() {
     assert_eq!(buffer.selection_ranges(set_id).unwrap(), vec![1..3]);
 
     now += buffer.history.group_interval + Duration::from_millis(1);
-    buffer.start_transaction_at(Some(set_id), now).unwrap();
+    buffer.start_transaction_at(Some(set_id), now);
     buffer
         .update_selection_set(set_id, &buffer.selections_from_ranges(vec![2..2]).unwrap())
         .unwrap();
@@ -515,7 +515,7 @@ fn test_history() {
     assert_eq!(buffer.text(), "ab2cde6");
     assert_eq!(buffer.selection_ranges(set_id).unwrap(), vec![3..3]);
 
-    buffer.start_transaction_at(None, now).unwrap();
+    buffer.start_transaction_at(None, now);
     assert!(buffer.end_transaction_at(None, now).is_none());
     buffer.undo();
     assert_eq!(buffer.text(), "12cde6");

crates/text/src/text.rs 🔗

@@ -38,6 +38,8 @@ pub use subscription::*;
 pub use sum_tree::Bias;
 use sum_tree::{FilterCursor, SumTree};
 
+pub type TransactionId = usize;
+
 pub struct Buffer {
     snapshot: BufferSnapshot,
     last_edit: clock::Local,
@@ -64,6 +66,7 @@ pub struct BufferSnapshot {
 
 #[derive(Clone, Debug)]
 pub struct Transaction {
+    id: TransactionId,
     start: clock::Global,
     end: clock::Global,
     edits: Vec<clock::Local>,
@@ -135,6 +138,7 @@ pub struct History {
     redo_stack: Vec<Transaction>,
     transaction_depth: usize,
     group_interval: Duration,
+    next_transaction_id: TransactionId,
 }
 
 impl History {
@@ -146,6 +150,7 @@ impl History {
             redo_stack: Vec::new(),
             transaction_depth: 0,
             group_interval: Duration::from_millis(300),
+            next_transaction_id: 0,
         }
     }
 
@@ -158,10 +163,13 @@ impl History {
         start: clock::Global,
         selections_before: HashMap<SelectionSetId, Arc<[Selection<Anchor>]>>,
         now: Instant,
-    ) {
+    ) -> Option<TransactionId> {
         self.transaction_depth += 1;
         if self.transaction_depth == 1 {
+            let id = self.next_transaction_id;
+            self.next_transaction_id += 1;
             self.undo_stack.push(Transaction {
+                id,
                 start: start.clone(),
                 end: start,
                 edits: Vec::new(),
@@ -171,6 +179,9 @@ impl History {
                 first_edit_at: now,
                 last_edit_at: now,
             });
+            Some(id)
+        } else {
+            None
         }
     }
 
@@ -547,7 +558,7 @@ impl Buffer {
             None
         };
 
-        self.start_transaction(None).unwrap();
+        self.start_transaction(None);
         let timestamp = InsertionTimestamp {
             replica_id: self.replica_id,
             local: self.local_clock.tick().value,
@@ -1141,7 +1152,7 @@ impl Buffer {
     pub fn start_transaction(
         &mut self,
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
-    ) -> Result<()> {
+    ) -> Option<TransactionId> {
         self.start_transaction_at(selection_set_ids, Instant::now())
     }
 
@@ -1149,7 +1160,7 @@ impl Buffer {
         &mut self,
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
         now: Instant,
-    ) -> Result<()> {
+    ) -> Option<TransactionId> {
         let selections = selection_set_ids
             .into_iter()
             .map(|set_id| {
@@ -1161,19 +1172,21 @@ impl Buffer {
             })
             .collect();
         self.history
-            .start_transaction(self.version.clone(), selections, now);
-        Ok(())
+            .start_transaction(self.version.clone(), selections, now)
     }
 
-    pub fn end_transaction(&mut self, selection_set_ids: impl IntoIterator<Item = SelectionSetId>) {
-        self.end_transaction_at(selection_set_ids, Instant::now());
+    pub fn end_transaction(
+        &mut self,
+        selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
+    ) -> Option<(TransactionId, clock::Global)> {
+        self.end_transaction_at(selection_set_ids, Instant::now())
     }
 
     pub fn end_transaction_at(
         &mut self,
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
         now: Instant,
-    ) -> Option<clock::Global> {
+    ) -> Option<(TransactionId, clock::Global)> {
         let selections = selection_set_ids
             .into_iter()
             .map(|set_id| {
@@ -1186,9 +1199,10 @@ impl Buffer {
             .collect();
 
         if let Some(transaction) = self.history.end_transaction(selections, now) {
+            let id = transaction.id;
             let since = transaction.start.clone();
             self.history.group();
-            Some(since)
+            Some((id, since))
         } else {
             None
         }