Make transactions serializable to enable edits on behalf of other users

Antonio Scandurra and Nathan Sobo created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

crates/editor/src/editor.rs       |   6 
crates/editor/src/multi_buffer.rs |  13 
crates/language/src/buffer.rs     |  74 +++++-----
crates/language/src/proto.rs      | 110 +++++++-------
crates/project/src/project.rs     | 115 ++++++++-------
crates/project/src/worktree.rs    |  14 
crates/rpc/proto/zed.proto        |  24 +-
crates/text/src/tests.rs          |  24 +-
crates/text/src/text.rs           | 232 ++++++++++++++++++--------------
9 files changed, 323 insertions(+), 289 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -2081,10 +2081,10 @@ impl Editor {
             project.apply_code_action(buffer, action, true, cx)
         });
         Some(cx.spawn(|workspace, mut cx| async move {
-            let buffers = apply_code_actions.await?;
+            let project_transaction = apply_code_actions.await?;
             // TODO: replace this with opening a single tab that is a multibuffer
             workspace.update(&mut cx, |workspace, cx| {
-                for (buffer, _) in buffers {
+                for (buffer, _) in project_transaction.0 {
                     workspace.open_item(BufferItemHandle(buffer), cx);
                 }
             });
@@ -4825,7 +4825,7 @@ impl View for Editor {
         self.focused = true;
         self.blink_cursors(self.blink_epoch, cx);
         self.buffer.update(cx, |buffer, cx| {
-            buffer.avoid_grouping_next_transaction(cx);
+            buffer.finalize_last_transaction(cx);
             buffer.set_active_selections(&self.selections, cx)
         });
     }

crates/editor/src/multi_buffer.rs 🔗

@@ -27,7 +27,6 @@ use text::{
     AnchorRangeExt as _, Edit, Point, PointUtf16, TextSummary,
 };
 use theme::SyntaxTheme;
-use util::post_inc;
 
 const NEWLINES: &'static [u8] = &[b'\n'; u8::MAX as usize];
 
@@ -43,7 +42,7 @@ pub struct MultiBuffer {
 }
 
 struct History {
-    next_transaction_id: usize,
+    next_transaction_id: TransactionId,
     undo_stack: Vec<Transaction>,
     redo_stack: Vec<Transaction>,
     transaction_depth: usize,
@@ -59,7 +58,7 @@ pub enum CharKind {
 }
 
 struct Transaction {
-    id: usize,
+    id: TransactionId,
     buffer_transactions: HashSet<(usize, text::TransactionId)>,
     first_edit_at: Instant,
     last_edit_at: Instant,
@@ -472,9 +471,11 @@ impl MultiBuffer {
         }
     }
 
-    pub fn avoid_grouping_next_transaction(&mut self, cx: &mut ModelContext<Self>) {
+    pub fn finalize_last_transaction(&mut self, cx: &mut ModelContext<Self>) {
         for BufferState { buffer, .. } in self.buffers.borrow().values() {
-            buffer.update(cx, |buffer, _| buffer.avoid_grouping_next_transaction());
+            buffer.update(cx, |buffer, _| {
+                buffer.finalize_last_transaction();
+            });
         }
     }
 
@@ -2092,7 +2093,7 @@ impl History {
     fn start_transaction(&mut self, now: Instant) -> Option<TransactionId> {
         self.transaction_depth += 1;
         if self.transaction_depth == 1 {
-            let id = post_inc(&mut self.next_transaction_id);
+            let id = self.next_transaction_id.tick();
             self.undo_stack.push(Transaction {
                 id,
                 buffer_transactions: Default::default(),

crates/language/src/buffer.rs 🔗

@@ -38,7 +38,7 @@ use text::{operation_queue::OperationQueue, rope::TextDimension};
 pub use text::{Buffer as TextBuffer, Operation as _, *};
 use theme::SyntaxTheme;
 use tree_sitter::{InputEdit, QueryCursor, Tree};
-use util::{post_inc, TryFutureExt as _};
+use util::{post_inc, ResultExt, TryFutureExt as _};
 
 #[cfg(any(test, feature = "test-support"))]
 pub use tree_sitter_rust;
@@ -214,7 +214,7 @@ pub trait File {
         buffer_id: u64,
         completion: Completion<Anchor>,
         cx: &mut MutableAppContext,
-    ) -> Task<Result<Vec<clock::Local>>>;
+    ) -> Task<Result<Option<Transaction>>>;
 
     fn code_actions(
         &self,
@@ -307,7 +307,7 @@ impl File for FakeFile {
         _: u64,
         _: Completion<Anchor>,
         _: &mut MutableAppContext,
-    ) -> Task<Result<Vec<clock::Local>>> {
+    ) -> Task<Result<Option<Transaction>>> {
         Task::ready(Ok(Default::default()))
     }
 
@@ -1339,16 +1339,12 @@ impl Buffer {
         }
     }
 
-    pub fn push_transaction(
-        &mut self,
-        edit_ids: impl IntoIterator<Item = clock::Local>,
-        now: Instant,
-    ) {
-        self.text.push_transaction(edit_ids, now);
+    pub fn push_transaction(&mut self, transaction: Transaction, now: Instant) {
+        self.text.push_transaction(transaction, now);
     }
 
-    pub fn avoid_grouping_next_transaction(&mut self) {
-        self.text.avoid_grouping_next_transaction();
+    pub fn finalize_last_transaction(&mut self) -> Option<&Transaction> {
+        self.text.finalize_last_transaction()
     }
 
     pub fn forget_transaction(&mut self, transaction_id: TransactionId) {
@@ -1536,7 +1532,7 @@ impl Buffer {
         edits: impl IntoIterator<Item = lsp::TextEdit>,
         version: Option<i32>,
         cx: &mut ModelContext<Self>,
-    ) -> Result<Vec<(Range<Anchor>, clock::Local)>> {
+    ) -> Result<()> {
         let mut anchored_edits = Vec::new();
         let snapshot =
             if let Some((version, language_server)) = version.zip(self.language_server.as_mut()) {
@@ -1560,14 +1556,11 @@ impl Buffer {
         }
 
         self.start_transaction();
-        let edit_ids = anchored_edits
-            .into_iter()
-            .filter_map(|(range, new_text)| {
-                Some((range.clone(), self.edit([range], new_text, cx)?))
-            })
-            .collect();
+        for (range, new_text) in anchored_edits {
+            self.edit([range], new_text, cx);
+        }
         self.end_transaction(cx);
-        Ok(edit_ids)
+        Ok(())
     }
 
     fn did_edit(
@@ -1941,7 +1934,7 @@ impl Buffer {
         completion: Completion<Anchor>,
         push_to_history: bool,
         cx: &mut ModelContext<Self>,
-    ) -> Task<Result<Vec<clock::Local>>> {
+    ) -> Task<Result<Option<Transaction>>> {
         let file = if let Some(file) = self.file.as_ref() {
             file
         } else {
@@ -1961,20 +1954,22 @@ impl Buffer {
                     .await?;
                 if let Some(additional_edits) = resolved_completion.additional_text_edits {
                     this.update(&mut cx, |this, cx| {
-                        if !push_to_history {
-                            this.avoid_grouping_next_transaction();
-                        }
+                        this.finalize_last_transaction();
                         this.start_transaction();
-                        let edits = this.apply_lsp_edits(additional_edits, None, cx);
-                        if let Some(transaction_id) = this.end_transaction(cx) {
+                        this.apply_lsp_edits(additional_edits, None, cx).log_err();
+                        let transaction = if this.end_transaction(cx).is_some() {
+                            let transaction = this.finalize_last_transaction().unwrap().clone();
                             if !push_to_history {
-                                this.forget_transaction(transaction_id);
+                                this.forget_transaction(transaction.id);
                             }
-                        }
-                        Ok(edits?.into_iter().map(|(_, edit_id)| edit_id).collect())
+                            Some(transaction)
+                        } else {
+                            None
+                        };
+                        Ok(transaction)
                     })
                 } else {
-                    Ok(Default::default())
+                    Ok(None)
                 }
             })
         } else {
@@ -1984,17 +1979,20 @@ impl Buffer {
                 cx.as_mut(),
             );
             cx.spawn(|this, mut cx| async move {
-                let edit_ids = apply_edits.await?;
-                this.update(&mut cx, |this, _| {
-                    this.wait_for_edits(edit_ids.iter().copied())
-                })
-                .await;
-                if push_to_history {
+                if let Some(transaction) = apply_edits.await? {
                     this.update(&mut cx, |this, _| {
-                        this.push_transaction(edit_ids.iter().copied(), Instant::now());
-                    });
+                        this.wait_for_edits(transaction.edit_ids.iter().copied())
+                    })
+                    .await;
+                    if push_to_history {
+                        this.update(&mut cx, |this, _| {
+                            this.push_transaction(transaction.clone(), Instant::now());
+                        });
+                    }
+                    Ok(Some(transaction))
+                } else {
+                    Ok(None)
                 }
-                Ok(edit_ids)
             })
         }
     }

crates/language/src/proto.rs 🔗

@@ -25,14 +25,7 @@ pub fn serialize_operation(operation: &Operation) -> proto::Operation {
                 replica_id: undo.id.replica_id as u32,
                 local_timestamp: undo.id.value,
                 lamport_timestamp: lamport_timestamp.value,
-                ranges: undo
-                    .ranges
-                    .iter()
-                    .map(|r| proto::Range {
-                        start: r.start.0 as u64,
-                        end: r.end.0 as u64,
-                    })
-                    .collect(),
+                ranges: undo.ranges.iter().map(serialize_range).collect(),
                 counts: undo
                     .counts
                     .iter()
@@ -75,20 +68,12 @@ pub fn serialize_operation(operation: &Operation) -> proto::Operation {
 }
 
 pub fn serialize_edit_operation(operation: &EditOperation) -> proto::operation::Edit {
-    let ranges = operation
-        .ranges
-        .iter()
-        .map(|range| proto::Range {
-            start: range.start.0 as u64,
-            end: range.end.0 as u64,
-        })
-        .collect();
     proto::operation::Edit {
         replica_id: operation.timestamp.replica_id as u32,
         local_timestamp: operation.timestamp.local,
         lamport_timestamp: operation.timestamp.lamport,
         version: From::from(&operation.version),
-        ranges,
+        ranges: operation.ranges.iter().map(serialize_range).collect(),
         new_text: operation.new_text.clone(),
     }
 }
@@ -211,11 +196,7 @@ pub fn deserialize_operation(message: proto::Operation) -> Result<Operation> {
                             )
                         })
                         .collect(),
-                    ranges: undo
-                        .ranges
-                        .into_iter()
-                        .map(|r| FullOffset(r.start as usize)..FullOffset(r.end as usize))
-                        .collect(),
+                    ranges: undo.ranges.into_iter().map(deserialize_range).collect(),
                     version: undo.version.into(),
                 },
             }),
@@ -263,11 +244,6 @@ pub fn deserialize_operation(message: proto::Operation) -> Result<Operation> {
 }
 
 pub fn deserialize_edit_operation(edit: proto::operation::Edit) -> EditOperation {
-    let ranges = edit
-        .ranges
-        .into_iter()
-        .map(|range| FullOffset(range.start as usize)..FullOffset(range.end as usize))
-        .collect();
     EditOperation {
         timestamp: InsertionTimestamp {
             replica_id: edit.replica_id as ReplicaId,
@@ -275,7 +251,7 @@ pub fn deserialize_edit_operation(edit: proto::operation::Edit) -> EditOperation
             lamport: edit.lamport_timestamp,
         },
         version: edit.version.into(),
-        ranges,
+        ranges: edit.ranges.into_iter().map(deserialize_range).collect(),
         new_text: edit.new_text,
     }
 }
@@ -469,42 +445,64 @@ pub fn deserialize_code_action(action: proto::CodeAction) -> Result<CodeAction<A
     })
 }
 
-pub fn serialize_code_action_edit(
-    edit_id: clock::Local,
-    old_range: &Range<Anchor>,
-) -> proto::CodeActionEdit {
-    proto::CodeActionEdit {
-        id: Some(serialize_edit_id(edit_id)),
-        old_start: Some(serialize_anchor(&old_range.start)),
-        old_end: Some(serialize_anchor(&old_range.end)),
+pub fn serialize_transaction(transaction: &Transaction) -> proto::Transaction {
+    proto::Transaction {
+        id: Some(serialize_local_timestamp(transaction.id)),
+        edit_ids: transaction
+            .edit_ids
+            .iter()
+            .copied()
+            .map(serialize_local_timestamp)
+            .collect(),
+        start: (&transaction.start).into(),
+        end: (&transaction.end).into(),
+        ranges: transaction.ranges.iter().map(serialize_range).collect(),
     }
 }
 
-pub fn deserialize_code_action_edit(
-    edit: proto::CodeActionEdit,
-) -> Result<(Range<Anchor>, clock::Local)> {
-    let old_start = edit
-        .old_start
-        .and_then(deserialize_anchor)
-        .ok_or_else(|| anyhow!("invalid old_start"))?;
-    let old_end = edit
-        .old_end
-        .and_then(deserialize_anchor)
-        .ok_or_else(|| anyhow!("invalid old_end"))?;
-    let edit_id = deserialize_edit_id(edit.id.ok_or_else(|| anyhow!("invalid edit_id"))?);
-    Ok((old_start..old_end, edit_id))
+pub fn deserialize_transaction(transaction: proto::Transaction) -> Result<Transaction> {
+    Ok(Transaction {
+        id: deserialize_local_timestamp(
+            transaction
+                .id
+                .ok_or_else(|| anyhow!("missing transaction id"))?,
+        ),
+        edit_ids: transaction
+            .edit_ids
+            .into_iter()
+            .map(deserialize_local_timestamp)
+            .collect(),
+        start: transaction.start.into(),
+        end: transaction.end.into(),
+        ranges: transaction
+            .ranges
+            .into_iter()
+            .map(deserialize_range)
+            .collect(),
+    })
 }
 
-pub fn serialize_edit_id(edit_id: clock::Local) -> proto::EditId {
-    proto::EditId {
-        replica_id: edit_id.replica_id as u32,
-        local_timestamp: edit_id.value,
+pub fn serialize_local_timestamp(timestamp: clock::Local) -> proto::LocalTimestamp {
+    proto::LocalTimestamp {
+        replica_id: timestamp.replica_id as u32,
+        value: timestamp.value,
     }
 }
 
-pub fn deserialize_edit_id(edit_id: proto::EditId) -> clock::Local {
+pub fn deserialize_local_timestamp(timestamp: proto::LocalTimestamp) -> clock::Local {
     clock::Local {
-        replica_id: edit_id.replica_id as ReplicaId,
-        value: edit_id.local_timestamp,
+        replica_id: timestamp.replica_id as ReplicaId,
+        value: timestamp.value,
+    }
+}
+
+pub fn serialize_range(range: &Range<FullOffset>) -> proto::Range {
+    proto::Range {
+        start: range.start.0 as u64,
+        end: range.end.0 as u64,
     }
 }
+
+pub fn deserialize_range(range: proto::Range) -> Range<FullOffset> {
+    FullOffset(range.start as usize)..FullOffset(range.end as usize)
+}

crates/project/src/project.rs 🔗

@@ -109,6 +109,9 @@ pub struct Definition {
     pub target_range: Range<language::Anchor>,
 }
 
+#[derive(Default)]
+pub struct ProjectTransaction(pub HashMap<ModelHandle<Buffer>, language::Transaction>);
+
 impl DiagnosticSummary {
     fn new<'a, T: 'a>(diagnostics: impl IntoIterator<Item = &'a DiagnosticEntry<T>>) -> Self {
         let mut this = Self {
@@ -1174,8 +1177,7 @@ impl Project {
         mut action: CodeAction<language::Anchor>,
         push_to_history: bool,
         cx: &mut ModelContext<Self>,
-    ) -> Task<Result<HashMap<ModelHandle<Buffer>, Vec<(Range<language::Anchor>, clock::Local)>>>>
-    {
+    ) -> Task<Result<ProjectTransaction>> {
         if self.is_local() {
             let buffer = buffer_handle.read(cx);
             let lang_name = if let Some(lang) = buffer.language() {
@@ -1237,7 +1239,7 @@ impl Project {
                     }
                 }
 
-                let mut edited_buffers = HashMap::default();
+                let mut project_transaction = ProjectTransaction::default();
                 for operation in operations {
                     match operation {
                         lsp::DocumentChangeOperation::Op(lsp::ResourceOp::Create(op)) => {
@@ -1298,34 +1300,38 @@ impl Project {
                                     )
                                 })
                                 .await?;
-                            let edits = buffer_to_edit.update(&mut cx, |buffer, cx| {
+                            let transaction = buffer_to_edit.update(&mut cx, |buffer, cx| {
                                 let edits = op.edits.into_iter().map(|edit| match edit {
                                     lsp::OneOf::Left(edit) => edit,
                                     lsp::OneOf::Right(edit) => edit.text_edit,
                                 });
-                                if !push_to_history {
-                                    buffer.avoid_grouping_next_transaction();
-                                }
+
+                                buffer.finalize_last_transaction();
                                 buffer.start_transaction();
-                                let edits =
-                                    buffer.apply_lsp_edits(edits, op.text_document.version, cx);
-                                if let Some(transaction_id) = buffer.end_transaction(cx) {
+                                buffer
+                                    .apply_lsp_edits(edits, op.text_document.version, cx)
+                                    .log_err();
+                                let transaction = if buffer.end_transaction(cx).is_some() {
+                                    let transaction =
+                                        buffer.finalize_last_transaction().unwrap().clone();
                                     if !push_to_history {
-                                        buffer.forget_transaction(transaction_id);
+                                        buffer.forget_transaction(transaction.id);
                                     }
-                                }
+                                    Some(transaction)
+                                } else {
+                                    None
+                                };
 
-                                edits
-                            })?;
-                            edited_buffers
-                                .entry(buffer_to_edit)
-                                .or_insert(Vec::new())
-                                .extend(edits);
+                                transaction
+                            });
+                            if let Some(transaction) = transaction {
+                                project_transaction.0.insert(buffer_to_edit, transaction);
+                            }
                         }
                     }
                 }
 
-                Ok(edited_buffers)
+                Ok(project_transaction)
             })
         } else if let Some(project_id) = self.remote_id() {
             let client = self.client.clone();
@@ -1335,35 +1341,34 @@ impl Project {
                 action: Some(language::proto::serialize_code_action(&action)),
             };
             cx.spawn(|this, mut cx| async move {
-                let response = client.request(request).await?;
-                let mut edited_buffers = HashMap::default();
-                for buffer_edit in response.buffer_edits {
-                    let buffer = buffer_edit
-                        .buffer
-                        .ok_or_else(|| anyhow!("invalid buffer"))?;
+                let response = client
+                    .request(request)
+                    .await?
+                    .transaction
+                    .ok_or_else(|| anyhow!("missing transaction"))?;
+                let mut project_transaction = ProjectTransaction::default();
+                for (buffer, transaction) in response.buffers.into_iter().zip(response.transactions)
+                {
                     let buffer = this.update(&mut cx, |this, cx| {
                         this.deserialize_remote_buffer(buffer, cx)
                     })?;
-
-                    let buffer_edits = edited_buffers.entry(buffer.clone()).or_insert(Vec::new());
-                    for edit in buffer_edit.edits {
-                        buffer_edits.push(language::proto::deserialize_code_action_edit(edit)?);
-                    }
+                    let transaction = language::proto::deserialize_transaction(transaction)?;
 
                     buffer
                         .update(&mut cx, |buffer, _| {
-                            buffer.wait_for_edits(buffer_edits.iter().map(|e| e.1))
+                            buffer.wait_for_edits(transaction.edit_ids.iter().copied())
                         })
                         .await;
 
                     if push_to_history {
                         buffer.update(&mut cx, |buffer, _| {
-                            buffer
-                                .push_transaction(buffer_edits.iter().map(|e| e.1), Instant::now());
+                            buffer.push_transaction(transaction.clone(), Instant::now());
                         });
                     }
+
+                    project_transaction.0.insert(buffer, transaction);
                 }
-                Ok(edited_buffers)
+                Ok(project_transaction)
             })
         } else {
             Task::ready(Err(anyhow!("project does not have a remote id")))
@@ -1975,13 +1980,12 @@ impl Project {
                 })
                 .await
             {
-                Ok(edit_ids) => rpc.respond(
+                Ok(transaction) => rpc.respond(
                     receipt,
                     proto::ApplyCompletionAdditionalEditsResponse {
-                        additional_edits: edit_ids
-                            .into_iter()
-                            .map(language::proto::serialize_edit_id)
-                            .collect(),
+                        transaction: transaction
+                            .as_ref()
+                            .map(language::proto::serialize_transaction),
                     },
                 ),
                 Err(error) => rpc.respond_with_error(
@@ -2062,20 +2066,25 @@ impl Project {
         let apply_code_action = self.apply_code_action(buffer, action, false, cx);
         cx.spawn(|this, mut cx| async move {
             match apply_code_action.await {
-                Ok(edited_buffers) => this.update(&mut cx, |this, cx| {
-                    let buffer_edits = edited_buffers
-                        .into_iter()
-                        .map(|(buffer, edits)| proto::CodeActionBufferEdits {
-                            buffer: Some(this.serialize_buffer_for_peer(&buffer, sender_id, cx)),
-                            edits: edits
-                                .into_iter()
-                                .map(|(range, edit_id)| {
-                                    language::proto::serialize_code_action_edit(edit_id, &range)
-                                })
-                                .collect(),
-                        })
-                        .collect();
-                    rpc.respond(receipt, proto::ApplyCodeActionResponse { buffer_edits })
+                Ok(project_transaction) => this.update(&mut cx, |this, cx| {
+                    let mut serialized_transaction = proto::ProjectTransaction {
+                        buffers: Default::default(),
+                        transactions: Default::default(),
+                    };
+                    for (buffer, transaction) in project_transaction.0 {
+                        serialized_transaction
+                            .buffers
+                            .push(this.serialize_buffer_for_peer(&buffer, sender_id, cx));
+                        serialized_transaction
+                            .transactions
+                            .push(language::proto::serialize_transaction(&transaction));
+                    }
+                    rpc.respond(
+                        receipt,
+                        proto::ApplyCodeActionResponse {
+                            transaction: Some(serialized_transaction),
+                        },
+                    )
                 }),
                 Err(error) => rpc.respond_with_error(
                     receipt,

crates/project/src/worktree.rs 🔗

@@ -15,7 +15,7 @@ use gpui::{
     Task,
 };
 use language::{
-    Anchor, Buffer, Completion, DiagnosticEntry, Language, Operation, PointUtf16, Rope,
+    Anchor, Buffer, Completion, DiagnosticEntry, Language, Operation, PointUtf16, Rope, Transaction,
 };
 use lazy_static::lazy_static;
 use parking_lot::Mutex;
@@ -1446,7 +1446,7 @@ impl language::File for File {
         buffer_id: u64,
         completion: Completion<Anchor>,
         cx: &mut MutableAppContext,
-    ) -> Task<Result<Vec<clock::Local>>> {
+    ) -> Task<Result<Option<Transaction>>> {
         let worktree = self.worktree.read(cx);
         let worktree = if let Some(worktree) = worktree.as_remote() {
             worktree
@@ -1466,11 +1466,11 @@ impl language::File for File {
                 })
                 .await?;
 
-            Ok(response
-                .additional_edits
-                .into_iter()
-                .map(language::proto::deserialize_edit_id)
-                .collect())
+            if let Some(transaction) = response.transaction {
+                Ok(Some(language::proto::deserialize_transaction(transaction)?))
+            } else {
+                Ok(None)
+            }
         })
     }
 

crates/rpc/proto/zed.proto 🔗

@@ -228,7 +228,7 @@ message ApplyCompletionAdditionalEdits {
 }
 
 message ApplyCompletionAdditionalEditsResponse {
-    repeated EditId additional_edits = 1;
+    Transaction transaction = 1;
 }
 
 message Completion {
@@ -255,7 +255,7 @@ message ApplyCodeAction {
 }
 
 message ApplyCodeActionResponse {
-    repeated CodeActionBufferEdits buffer_edits = 1;
+    ProjectTransaction transaction = 1;
 }
 
 message CodeAction {
@@ -263,20 +263,22 @@ message CodeAction {
     bytes lsp_action = 2;
 }
 
-message CodeActionBufferEdits {
-    Buffer buffer = 1;
-    repeated CodeActionEdit edits = 2;
+message ProjectTransaction {
+    repeated Buffer buffers = 1;
+    repeated Transaction transactions = 2;
 }
 
-message CodeActionEdit {
-    EditId id = 1;
-    Anchor old_start = 2;
-    Anchor old_end = 3;
+message Transaction {
+    LocalTimestamp id = 1;
+    repeated LocalTimestamp edit_ids = 2;
+    repeated VectorClockEntry start = 3;
+    repeated VectorClockEntry end = 4;
+    repeated Range ranges = 5;
 }
 
-message EditId {
+message LocalTimestamp {
     uint32 replica_id = 1;
-    uint32 local_timestamp = 2;
+    uint32 value = 2;
 }
 
 message UpdateDiagnosticSummary {

crates/text/src/tests.rs 🔗

@@ -431,28 +431,28 @@ fn test_undo_redo() {
     buffer.edit(vec![3..5], "cd");
     assert_eq!(buffer.text(), "1abcdef234");
 
-    let transactions = buffer.history.undo_stack.clone();
-    assert_eq!(transactions.len(), 3);
+    let entries = buffer.history.undo_stack.clone();
+    assert_eq!(entries.len(), 3);
 
-    buffer.undo_or_redo(transactions[0].clone()).unwrap();
+    buffer.undo_or_redo(entries[0].transaction.clone()).unwrap();
     assert_eq!(buffer.text(), "1cdef234");
-    buffer.undo_or_redo(transactions[0].clone()).unwrap();
+    buffer.undo_or_redo(entries[0].transaction.clone()).unwrap();
     assert_eq!(buffer.text(), "1abcdef234");
 
-    buffer.undo_or_redo(transactions[1].clone()).unwrap();
+    buffer.undo_or_redo(entries[1].transaction.clone()).unwrap();
     assert_eq!(buffer.text(), "1abcdx234");
-    buffer.undo_or_redo(transactions[2].clone()).unwrap();
+    buffer.undo_or_redo(entries[2].transaction.clone()).unwrap();
     assert_eq!(buffer.text(), "1abx234");
-    buffer.undo_or_redo(transactions[1].clone()).unwrap();
+    buffer.undo_or_redo(entries[1].transaction.clone()).unwrap();
     assert_eq!(buffer.text(), "1abyzef234");
-    buffer.undo_or_redo(transactions[2].clone()).unwrap();
+    buffer.undo_or_redo(entries[2].transaction.clone()).unwrap();
     assert_eq!(buffer.text(), "1abcdef234");
 
-    buffer.undo_or_redo(transactions[2].clone()).unwrap();
+    buffer.undo_or_redo(entries[2].transaction.clone()).unwrap();
     assert_eq!(buffer.text(), "1abyzef234");
-    buffer.undo_or_redo(transactions[0].clone()).unwrap();
+    buffer.undo_or_redo(entries[0].transaction.clone()).unwrap();
     assert_eq!(buffer.text(), "1yzef234");
-    buffer.undo_or_redo(transactions[1].clone()).unwrap();
+    buffer.undo_or_redo(entries[1].transaction.clone()).unwrap();
     assert_eq!(buffer.text(), "1234");
 }
 
@@ -510,7 +510,7 @@ fn test_avoid_grouping_next_transaction() {
     buffer.end_transaction_at(now);
     assert_eq!(buffer.text(), "12cd56");
 
-    buffer.avoid_grouping_next_transaction();
+    buffer.finalize_last_transaction();
     buffer.start_transaction_at(now);
     buffer.edit(vec![4..5], "e");
     buffer.end_transaction_at(now).unwrap();

crates/text/src/text.rs 🔗

@@ -40,7 +40,7 @@ pub use subscription::*;
 pub use sum_tree::Bias;
 use sum_tree::{FilterCursor, SumTree};
 
-pub type TransactionId = usize;
+pub type TransactionId = clock::Local;
 
 pub struct Buffer {
     snapshot: BufferSnapshot,
@@ -67,28 +67,33 @@ pub struct BufferSnapshot {
 }
 
 #[derive(Clone, Debug)]
-pub struct Transaction {
-    id: TransactionId,
-    start: clock::Global,
-    end: clock::Global,
-    edits: Vec<clock::Local>,
-    ranges: Vec<Range<FullOffset>>,
+pub struct HistoryEntry {
+    transaction: Transaction,
     first_edit_at: Instant,
     last_edit_at: Instant,
     suppress_grouping: bool,
 }
 
-impl Transaction {
+#[derive(Clone, Debug)]
+pub struct Transaction {
+    pub id: TransactionId,
+    pub edit_ids: Vec<clock::Local>,
+    pub start: clock::Global,
+    pub end: clock::Global,
+    pub ranges: Vec<Range<FullOffset>>,
+}
+
+impl HistoryEntry {
     fn push_edit(&mut self, edit: &EditOperation) {
-        self.edits.push(edit.timestamp.local());
-        self.end.observe(edit.timestamp.local());
+        self.transaction.edit_ids.push(edit.timestamp.local());
+        self.transaction.end.observe(edit.timestamp.local());
 
         let mut other_ranges = edit.ranges.iter().peekable();
         let mut new_ranges = Vec::new();
         let insertion_len = edit.new_text.as_ref().map_or(0, |t| t.len());
         let mut delta = 0;
 
-        for mut self_range in self.ranges.iter().cloned() {
+        for mut self_range in self.transaction.ranges.iter().cloned() {
             self_range.start += delta;
             self_range.end += delta;
 
@@ -122,7 +127,7 @@ impl Transaction {
             delta += insertion_len;
         }
 
-        self.ranges = new_ranges;
+        self.transaction.ranges = new_ranges;
     }
 }
 
@@ -131,11 +136,10 @@ pub struct History {
     // TODO: Turn this into a String or Rope, maybe.
     pub base_text: Arc<str>,
     operations: HashMap<clock::Local, Operation>,
-    undo_stack: Vec<Transaction>,
-    redo_stack: Vec<Transaction>,
+    undo_stack: Vec<HistoryEntry>,
+    redo_stack: Vec<HistoryEntry>,
     transaction_depth: usize,
     group_interval: Duration,
-    next_transaction_id: TransactionId,
 }
 
 impl History {
@@ -147,7 +151,6 @@ impl History {
             redo_stack: Vec::new(),
             transaction_depth: 0,
             group_interval: Duration::from_millis(300),
-            next_transaction_id: 0,
         }
     }
 
@@ -155,17 +158,23 @@ impl History {
         self.operations.insert(op.local_timestamp(), op);
     }
 
-    fn start_transaction(&mut self, start: clock::Global, now: Instant) -> Option<TransactionId> {
+    fn start_transaction(
+        &mut self,
+        start: clock::Global,
+        now: Instant,
+        local_clock: &mut clock::Local,
+    ) -> 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(),
-                ranges: Vec::new(),
+            let id = local_clock.tick();
+            self.undo_stack.push(HistoryEntry {
+                transaction: Transaction {
+                    id,
+                    start: start.clone(),
+                    end: start,
+                    edit_ids: Default::default(),
+                    ranges: Default::default(),
+                },
                 first_edit_at: now,
                 last_edit_at: now,
                 suppress_grouping: false,
@@ -176,17 +185,24 @@ impl History {
         }
     }
 
-    fn end_transaction(&mut self, now: Instant) -> Option<&Transaction> {
+    fn end_transaction(&mut self, now: Instant) -> Option<&HistoryEntry> {
         assert_ne!(self.transaction_depth, 0);
         self.transaction_depth -= 1;
         if self.transaction_depth == 0 {
-            if self.undo_stack.last().unwrap().ranges.is_empty() {
+            if self
+                .undo_stack
+                .last()
+                .unwrap()
+                .transaction
+                .ranges
+                .is_empty()
+            {
                 self.undo_stack.pop();
                 None
             } else {
-                let transaction = self.undo_stack.last_mut().unwrap();
-                transaction.last_edit_at = now;
-                Some(transaction)
+                let entry = self.undo_stack.last_mut().unwrap();
+                entry.last_edit_at = now;
+                Some(entry)
             }
         } else {
             None
@@ -195,16 +211,15 @@ impl History {
 
     fn group(&mut self) -> Option<TransactionId> {
         let mut new_len = self.undo_stack.len();
-        let mut transactions = self.undo_stack.iter_mut();
-
-        if let Some(mut transaction) = transactions.next_back() {
-            while let Some(prev_transaction) = transactions.next_back() {
-                if !prev_transaction.suppress_grouping
-                    && transaction.first_edit_at - prev_transaction.last_edit_at
-                        <= self.group_interval
-                    && transaction.start == prev_transaction.end
+        let mut entries = self.undo_stack.iter_mut();
+
+        if let Some(mut entry) = entries.next_back() {
+            while let Some(prev_entry) = entries.next_back() {
+                if !prev_entry.suppress_grouping
+                    && entry.first_edit_at - prev_entry.last_edit_at <= self.group_interval
+                    && entry.transaction.start == prev_entry.transaction.end
                 {
-                    transaction = prev_transaction;
+                    entry = prev_entry;
                     new_len -= 1;
                 } else {
                     break;
@@ -212,45 +227,39 @@ impl History {
             }
         }
 
-        let (transactions_to_keep, transactions_to_merge) = self.undo_stack.split_at_mut(new_len);
-        if let Some(last_transaction) = transactions_to_keep.last_mut() {
-            for transaction in &*transactions_to_merge {
-                for edit_id in &transaction.edits {
-                    last_transaction.push_edit(self.operations[edit_id].as_edit().unwrap());
+        let (entries_to_keep, entries_to_merge) = self.undo_stack.split_at_mut(new_len);
+        if let Some(last_entry) = entries_to_keep.last_mut() {
+            for entry in &*entries_to_merge {
+                for edit_id in &entry.transaction.edit_ids {
+                    last_entry.push_edit(self.operations[edit_id].as_edit().unwrap());
                 }
             }
 
-            if let Some(transaction) = transactions_to_merge.last_mut() {
-                last_transaction.last_edit_at = transaction.last_edit_at;
-                last_transaction.end = transaction.end.clone();
+            if let Some(entry) = entries_to_merge.last_mut() {
+                last_entry.last_edit_at = entry.last_edit_at;
+                last_entry.transaction.end = entry.transaction.end.clone();
             }
         }
 
         self.undo_stack.truncate(new_len);
-        self.undo_stack.last().map(|t| t.id)
+        self.undo_stack.last().map(|e| e.transaction.id)
     }
 
-    fn avoid_grouping_next_transaction(&mut self) {
-        if let Some(transaction) = self.undo_stack.last_mut() {
-            transaction.suppress_grouping = true;
-        }
+    fn finalize_last_transaction(&mut self) -> Option<&Transaction> {
+        self.undo_stack.last_mut().map(|entry| {
+            entry.suppress_grouping = true;
+            &entry.transaction
+        })
     }
 
-    fn push_transaction(&mut self, edit_ids: impl IntoIterator<Item = clock::Local>, now: Instant) {
+    fn push_transaction(&mut self, transaction: Transaction, now: Instant) {
         assert_eq!(self.transaction_depth, 0);
-        let mut edit_ids = edit_ids.into_iter().peekable();
-
-        if let Some(first_edit) = edit_ids
-            .peek()
-            .and_then(|e| self.operations.get(&e)?.as_edit())
-        {
-            let version = first_edit.version.clone();
-            self.start_transaction(version, now);
-            for edit_id in edit_ids {
-                self.push_undo(edit_id);
-            }
-            self.end_transaction(now);
-        }
+        self.undo_stack.push(HistoryEntry {
+            transaction,
+            first_edit_at: now,
+            last_edit_at: now,
+            suppress_grouping: false,
+        });
     }
 
     fn push_undo(&mut self, op_id: clock::Local) {
@@ -261,21 +270,25 @@ impl History {
         }
     }
 
-    fn pop_undo(&mut self) -> Option<&Transaction> {
+    fn pop_undo(&mut self) -> Option<&HistoryEntry> {
         assert_eq!(self.transaction_depth, 0);
-        if let Some(transaction) = self.undo_stack.pop() {
-            self.redo_stack.push(transaction);
+        if let Some(entry) = self.undo_stack.pop() {
+            self.redo_stack.push(entry);
             self.redo_stack.last()
         } else {
             None
         }
     }
 
-    fn remove_from_undo(&mut self, transaction_id: TransactionId) -> Option<&Transaction> {
+    fn remove_from_undo(&mut self, transaction_id: TransactionId) -> Option<&HistoryEntry> {
         assert_eq!(self.transaction_depth, 0);
-        if let Some(transaction_ix) = self.undo_stack.iter().rposition(|t| t.id == transaction_id) {
-            let transaction = self.undo_stack.remove(transaction_ix);
-            self.redo_stack.push(transaction);
+        if let Some(entry_ix) = self
+            .undo_stack
+            .iter()
+            .rposition(|entry| entry.transaction.id == transaction_id)
+        {
+            let entry = self.undo_stack.remove(entry_ix);
+            self.redo_stack.push(entry);
             self.redo_stack.last()
         } else {
             None
@@ -284,30 +297,40 @@ impl History {
 
     fn forget(&mut self, transaction_id: TransactionId) {
         assert_eq!(self.transaction_depth, 0);
-        if let Some(transaction_ix) = self.undo_stack.iter().rposition(|t| t.id == transaction_id) {
-            self.undo_stack.remove(transaction_ix);
-        } else if let Some(transaction_ix) =
-            self.redo_stack.iter().rposition(|t| t.id == transaction_id)
+        if let Some(entry_ix) = self
+            .undo_stack
+            .iter()
+            .rposition(|entry| entry.transaction.id == transaction_id)
+        {
+            self.undo_stack.remove(entry_ix);
+        } else if let Some(entry_ix) = self
+            .redo_stack
+            .iter()
+            .rposition(|entry| entry.transaction.id == transaction_id)
         {
-            self.undo_stack.remove(transaction_ix);
+            self.undo_stack.remove(entry_ix);
         }
     }
 
-    fn pop_redo(&mut self) -> Option<&Transaction> {
+    fn pop_redo(&mut self) -> Option<&HistoryEntry> {
         assert_eq!(self.transaction_depth, 0);
-        if let Some(transaction) = self.redo_stack.pop() {
-            self.undo_stack.push(transaction);
+        if let Some(entry) = self.redo_stack.pop() {
+            self.undo_stack.push(entry);
             self.undo_stack.last()
         } else {
             None
         }
     }
 
-    fn remove_from_redo(&mut self, transaction_id: TransactionId) -> Option<&Transaction> {
+    fn remove_from_redo(&mut self, transaction_id: TransactionId) -> Option<&HistoryEntry> {
         assert_eq!(self.transaction_depth, 0);
-        if let Some(transaction_ix) = self.redo_stack.iter().rposition(|t| t.id == transaction_id) {
-            let transaction = self.redo_stack.remove(transaction_ix);
-            self.undo_stack.push(transaction);
+        if let Some(entry_ix) = self
+            .redo_stack
+            .iter()
+            .rposition(|entry| entry.transaction.id == transaction_id)
+        {
+            let entry = self.redo_stack.remove(entry_ix);
+            self.undo_stack.push(entry);
             self.undo_stack.last()
         } else {
             None
@@ -1123,7 +1146,7 @@ impl Buffer {
         }
     }
 
-    pub fn peek_undo_stack(&self) -> Option<&Transaction> {
+    pub fn peek_undo_stack(&self) -> Option<&HistoryEntry> {
         self.history.undo_stack.last()
     }
 
@@ -1132,7 +1155,8 @@ impl Buffer {
     }
 
     pub fn start_transaction_at(&mut self, now: Instant) -> Option<TransactionId> {
-        self.history.start_transaction(self.version.clone(), now)
+        self.history
+            .start_transaction(self.version.clone(), now, &mut self.local_clock)
     }
 
     pub fn end_transaction(&mut self) -> Option<(TransactionId, clock::Global)> {
@@ -1140,8 +1164,8 @@ impl Buffer {
     }
 
     pub fn end_transaction_at(&mut self, now: Instant) -> Option<(TransactionId, clock::Global)> {
-        if let Some(transaction) = self.history.end_transaction(now) {
-            let since = transaction.start.clone();
+        if let Some(entry) = self.history.end_transaction(now) {
+            let since = entry.transaction.start.clone();
             let id = self.history.group().unwrap();
             Some((id, since))
         } else {
@@ -1149,8 +1173,8 @@ impl Buffer {
         }
     }
 
-    pub fn avoid_grouping_next_transaction(&mut self) {
-        self.history.avoid_grouping_next_transaction()
+    pub fn finalize_last_transaction(&mut self) -> Option<&Transaction> {
+        self.history.finalize_last_transaction()
     }
 
     pub fn base_text(&self) -> &Arc<str> {
@@ -1169,7 +1193,8 @@ impl Buffer {
     }
 
     pub fn undo(&mut self) -> Option<(TransactionId, Operation)> {
-        if let Some(transaction) = self.history.pop_undo().cloned() {
+        if let Some(entry) = self.history.pop_undo() {
+            let transaction = entry.transaction.clone();
             let transaction_id = transaction.id;
             let op = self.undo_or_redo(transaction).unwrap();
             Some((transaction_id, op))
@@ -1179,7 +1204,8 @@ impl Buffer {
     }
 
     pub fn undo_transaction(&mut self, transaction_id: TransactionId) -> Option<Operation> {
-        if let Some(transaction) = self.history.remove_from_undo(transaction_id).cloned() {
+        if let Some(entry) = self.history.remove_from_undo(transaction_id) {
+            let transaction = entry.transaction.clone();
             let op = self.undo_or_redo(transaction).unwrap();
             Some(op)
         } else {
@@ -1192,7 +1218,8 @@ impl Buffer {
     }
 
     pub fn redo(&mut self) -> Option<(TransactionId, Operation)> {
-        if let Some(transaction) = self.history.pop_redo().cloned() {
+        if let Some(entry) = self.history.pop_redo() {
+            let transaction = entry.transaction.clone();
             let transaction_id = transaction.id;
             let op = self.undo_or_redo(transaction).unwrap();
             Some((transaction_id, op))
@@ -1202,7 +1229,8 @@ impl Buffer {
     }
 
     pub fn redo_transaction(&mut self, transaction_id: TransactionId) -> Option<Operation> {
-        if let Some(transaction) = self.history.remove_from_redo(transaction_id).cloned() {
+        if let Some(entry) = self.history.remove_from_redo(transaction_id) {
+            let transaction = entry.transaction.clone();
             let op = self.undo_or_redo(transaction).unwrap();
             Some(op)
         } else {
@@ -1212,7 +1240,7 @@ impl Buffer {
 
     fn undo_or_redo(&mut self, transaction: Transaction) -> Result<Operation> {
         let mut counts = HashMap::default();
-        for edit_id in transaction.edits {
+        for edit_id in transaction.edit_ids {
             counts.insert(edit_id, self.undo_map.undo_count(edit_id) + 1);
         }
 
@@ -1232,12 +1260,9 @@ impl Buffer {
         Ok(operation)
     }
 
-    pub fn push_transaction(
-        &mut self,
-        edit_ids: impl IntoIterator<Item = clock::Local>,
-        now: Instant,
-    ) {
-        self.history.push_transaction(edit_ids, now);
+    pub fn push_transaction(&mut self, transaction: Transaction, now: Instant) {
+        self.history.push_transaction(transaction, now);
+        self.history.finalize_last_transaction();
     }
 
     pub fn subscribe(&mut self) -> Subscription {
@@ -1364,7 +1389,8 @@ impl Buffer {
 
         let mut ops = Vec::new();
         for _ in 0..rng.gen_range(1..=5) {
-            if let Some(transaction) = self.history.undo_stack.choose(rng).cloned() {
+            if let Some(entry) = self.history.undo_stack.choose(rng) {
+                let transaction = entry.transaction.clone();
                 log::info!(
                     "undoing buffer {} transaction {:?}",
                     self.replica_id,