From 7d8641afb6d367d99785e3476ceddb039cf8e177 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 8 Feb 2022 19:46:12 +0100 Subject: [PATCH] Make transactions serializable to enable edits on behalf of other users Co-Authored-By: Nathan Sobo --- 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 | 121 ++++++++-------- crates/project/src/worktree.rs | 14 +- crates/rpc/proto/zed.proto | 24 ++-- crates/text/src/tests.rs | 24 ++-- crates/text/src/text.rs | 230 +++++++++++++++++------------- 9 files changed, 325 insertions(+), 291 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 61a1e47a5c..5c2f00e2b3 100644 --- a/crates/editor/src/editor.rs +++ b/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) }); } diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 5e37d25c19..d8071ebbda 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/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, redo_stack: Vec, 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) { + pub fn finalize_last_transaction(&mut self, cx: &mut ModelContext) { 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 { 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(), diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 127b6831f4..eb49e3ca95 100644 --- a/crates/language/src/buffer.rs +++ b/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, cx: &mut MutableAppContext, - ) -> Task>>; + ) -> Task>>; fn code_actions( &self, @@ -307,7 +307,7 @@ impl File for FakeFile { _: u64, _: Completion, _: &mut MutableAppContext, - ) -> Task>> { + ) -> Task>> { Task::ready(Ok(Default::default())) } @@ -1339,16 +1339,12 @@ impl Buffer { } } - pub fn push_transaction( - &mut self, - edit_ids: impl IntoIterator, - 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, version: Option, cx: &mut ModelContext, - ) -> Result, 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, push_to_history: bool, cx: &mut ModelContext, - ) -> Task>> { + ) -> Task>> { 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) }) } } diff --git a/crates/language/src/proto.rs b/crates/language/src/proto.rs index 62691583fb..296594ae4c 100644 --- a/crates/language/src/proto.rs +++ b/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 { ) }) .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 { } 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, -) -> 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, 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 { + 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) -> 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(range.start as usize)..FullOffset(range.end as usize) +} diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 1e71e44e48..f7fb3ce37f 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -109,6 +109,9 @@ pub struct Definition { pub target_range: Range, } +#[derive(Default)] +pub struct ProjectTransaction(pub HashMap, language::Transaction>); + impl DiagnosticSummary { fn new<'a, T: 'a>(diagnostics: impl IntoIterator>) -> Self { let mut this = Self { @@ -1174,8 +1177,7 @@ impl Project { mut action: CodeAction, push_to_history: bool, cx: &mut ModelContext, - ) -> Task, Vec<(Range, clock::Local)>>>> - { + ) -> Task> { 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.start_transaction(); - let edits = - buffer.apply_lsp_edits(edits, op.text_document.version, cx); - if let Some(transaction_id) = buffer.end_transaction(cx) { - if !push_to_history { - buffer.forget_transaction(transaction_id); - } - } - edits - })?; - edited_buffers - .entry(buffer_to_edit) - .or_insert(Vec::new()) - .extend(edits); + buffer.finalize_last_transaction(); + buffer.start_transaction(); + 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); + } + Some(transaction) + } else { + None + }; + + 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, diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index f30f8f868b..904835c1b5 100644 --- a/crates/project/src/worktree.rs +++ b/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, cx: &mut MutableAppContext, - ) -> Task>> { + ) -> Task>> { 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) + } }) } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 5e92c7b22f..a71cd37dcd 100644 --- a/crates/rpc/proto/zed.proto +++ b/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 { diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index 0c6c95cb4a..e45011c4ac 100644 --- a/crates/text/src/tests.rs +++ b/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(); diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index 37e5c72402..a1b1bd36d0 100644 --- a/crates/text/src/text.rs +++ b/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, - ranges: Vec>, +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, + pub start: clock::Global, + pub end: clock::Global, + pub ranges: Vec>, +} + +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, operations: HashMap, - undo_stack: Vec, - redo_stack: Vec, + undo_stack: Vec, + redo_stack: Vec, 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 { + fn start_transaction( + &mut self, + start: clock::Global, + now: Instant, + local_clock: &mut clock::Local, + ) -> Option { 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 { let mut new_len = self.undo_stack.len(); - let mut transactions = self.undo_stack.iter_mut(); + let mut entries = 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 + 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, 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(transaction_ix); + 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(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 { - 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 { @@ -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 { - 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 { - 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 { 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, - 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,