Skip to content

Commit 70a3bf3

Browse files
synoetzxch3n
andauthored
feat: group api for UndoManager (#720)
Adds a new api to `UndoManager` to support grouping together changes into *ideally* one undo item. `UndoManager::group_start()`: will start a new group of changes, all subsequent changes will be merged into a new item on the undo stack. If we receive remote changes, we determine wether or not they are conflicting. If the remote changes are conflicting we split the undo item and close the group. If there are no conflict in changed container ids we continue the group merge. `UndoManager::group_end()`: ends the current group, calling `UndoManager::undo()` after this will undo all changes that occurred during the group. - Introduces new semantics for handling remote imports. If a remote import happens we compare `ContainerID`'s from the remote import and the last item on the undo stack, if there is no overlap we deem it safe to continue merging into the last undo item regardless of the remote import. If there are conflicts it behaves as it did previously. ``` ┌────────────────┐ │ group_start() │ └────────────────┘ │ UndoStack │ ┌─────────────────┐ make local change │ ┌─────────────┐ │ push new item ┌────────────────┐ │ │ undo_1 │◀│─ ─ ─ to undo stack ─ ─ ─ ─│ commit() │ │ └─────────────┘ │ └────────────────┘ ┌───────────┐ │ ▲ │ │ │group_end()│◀ ─ ─ ─ ─ ┐ │ │ │ make local change └───────────┘ │ │ │ merge with ┌────────────────┐ │ │ └ ─ ─ ─ ─│─ ─ ─ previous item ─ ─ ─ ─│ commit() │ │ │ │ in same group └────────────────┘ Λ │ │ │ │ ╱ ╲ │ │ │ │ ─▶▕ Y ▏─ ┘ │ │ import remote change ┌ ─ ─ ─ ─ ─ ─ ─ ┐ │ ╲ ╱ │ │ │ ┌────────────────┐ does import V │ │ │ import() │─ ─ ─▶ │ affect last │─ ┤ │ │ │ └────────────────┘ undo item? Λ │ │ Λ │ └ ─ ─ ─ ─ ─ ─ ─ ┘ │ ╱ ╲ │ │ │ ╱ ╲ │ ─▶▕ N ▏ │ ─ ─ ─ ─ ┼ ─▕ Y ▏◀─ ─ ─ ─ ─ ▼ ╲ ╱ │ │ ╲ ╱ │ make local change V │ │ V ┌ ─ ─ ─ ─ ─ ─ ─ ┐ ┌────────────────┐ │ │ is group_active ─│ commit() │ │ │ └ ─ ─ ─ ─ ─ ─ ─ ┘ └────────────────┘ │ ┌─────────────┐ │ │ │ │ undo_2 │◀┼───┐ Λ │ └─────────────┘ │ │ ╱ ╲ │ │ │ └─────▕ N ▏◀ ─ │ push new item ╲ ╱ │ │ V └─────────────────┘ ``` Example usage from tests: ```typescript const doc = new LoroDoc() const undoManager = new UndoManager(doc, {}); const text = doc.getText("text"); undoManager.groupStart(); text.update("hello", undefined); doc.commit(); text.update("world", undefined); doc.commit(); undoManager.groupEnd(); undoManager.undo(); expect(text.toString()).toBe(""); ``` --------- Co-authored-by: Zixuan Chen <remch183@outlook.com>
1 parent c2263d2 commit 70a3bf3

File tree

7 files changed

+430
-24
lines changed

7 files changed

+430
-24
lines changed

.changeset/late-jeans-beg.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"loro-crdt": patch
3+
---
4+
5+
Feat: support `group_start` and `group_end` for UndoManager #720

crates/loro-common/src/error.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ pub enum LoroError {
7272
UndoInvalidIdSpan(ID),
7373
#[error("PeerID cannot be changed. Expected: {expected:?}, Actual: {actual:?}")]
7474
UndoWithDifferentPeerId { expected: PeerID, actual: PeerID },
75-
#[error("The input JSON schema is invalid")]
75+
#[error("There is already an active undo group, call `group_end` first")]
76+
UndoGroupAlreadyStarted,
77+
#[error("There is no active undo group, call `group_start` first")]
7678
InvalidJsonSchema,
7779
#[error("Cannot insert or delete utf-8 in the middle of the codepoint in Unicode")]
7880
UTF8InUnicodeCodePoint { pos: usize },

crates/loro-internal/VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.5.6
1+
1.5.6

crates/loro-internal/src/undo.rs

Lines changed: 123 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ use std::{collections::VecDeque, sync::Arc};
22

33
use crate::sync::{AtomicU64, Mutex};
44
use either::Either;
5-
use fxhash::FxHashMap;
5+
use fxhash::{FxHashMap, FxHashSet};
66
use loro_common::{
7-
ContainerID, Counter, CounterSpan, HasIdSpan, IdSpan, LoroResult, LoroValue, PeerID,
7+
ContainerID, Counter, CounterSpan, HasIdSpan, IdSpan, LoroError, LoroResult, LoroValue, PeerID,
88
};
99
use tracing::{debug_span, info_span, instrument};
1010

@@ -199,6 +199,7 @@ struct UndoManagerInner {
199199
last_popped_selection: Option<Vec<CursorWithPos>>,
200200
on_push: Option<OnPush>,
201201
on_pop: Option<OnPop>,
202+
group: Option<UndoGroup>,
202203
}
203204

204205
impl std::fmt::Debug for UndoManagerInner {
@@ -212,10 +213,26 @@ impl std::fmt::Debug for UndoManagerInner {
212213
.field("merge_interval", &self.merge_interval_in_ms)
213214
.field("max_stack_size", &self.max_stack_size)
214215
.field("exclude_origin_prefixes", &self.exclude_origin_prefixes)
216+
.field("group", &self.group)
215217
.finish()
216218
}
217219
}
218220

221+
#[derive(Debug, Clone, Default)]
222+
struct UndoGroup {
223+
start_counter: Counter,
224+
affected_cids: FxHashSet<ContainerID>,
225+
}
226+
227+
impl UndoGroup {
228+
pub fn new(start_counter: Counter) -> Self {
229+
Self {
230+
start_counter,
231+
affected_cids: Default::default(),
232+
}
233+
}
234+
}
235+
219236
#[derive(Debug)]
220237
struct Stack {
221238
stack: VecDeque<(VecDeque<StackItem>, Arc<Mutex<DiffBatch>>)>,
@@ -308,35 +325,58 @@ impl Stack {
308325
}
309326

310327
pub fn push(&mut self, span: CounterSpan, meta: UndoItemMeta) {
311-
self.push_with_merge(span, meta, false)
328+
self.push_with_merge(span, meta, false, None)
312329
}
313330

314-
pub fn push_with_merge(&mut self, span: CounterSpan, meta: UndoItemMeta, can_merge: bool) {
331+
pub fn push_with_merge(
332+
&mut self,
333+
span: CounterSpan,
334+
meta: UndoItemMeta,
335+
can_merge: bool,
336+
group: Option<&UndoGroup>,
337+
) {
315338
let last = self.stack.back_mut().unwrap();
316339
let last_remote_diff = last.1.lock().unwrap();
317-
if !last_remote_diff.cid_to_events.is_empty() {
318-
// If the remote diff is not empty, we cannot merge
340+
341+
// Check if the remote diff is disjoint with the current undo group
342+
let is_disjoint_group = group.is_some_and(|g| {
343+
g.affected_cids.iter().all(|cid| {
344+
last_remote_diff
345+
.cid_to_events
346+
.get(cid)
347+
.is_none_or(|diff| diff.is_empty())
348+
})
349+
});
350+
351+
// Can't merge if remote diffs exist and it's not disjoint with the current undo group
352+
let should_create_new_entry =
353+
!last_remote_diff.cid_to_events.is_empty() && !is_disjoint_group;
354+
355+
if should_create_new_entry {
356+
// Create a new entry in the stack
319357
drop(last_remote_diff);
320358
let mut v = VecDeque::new();
321359
v.push_back(StackItem { span, meta });
322360
self.stack
323361
.push_back((v, Arc::new(Mutex::new(DiffBatch::default()))));
324-
325362
self.size += 1;
326-
} else {
327-
if can_merge {
328-
if let Some(last_span) = last.0.back_mut() {
329-
if last_span.span.end == span.start {
330-
// merge the span
331-
last_span.span.end = span.end;
332-
return;
333-
}
363+
return;
364+
}
365+
366+
// Try to merge with the previous entry if allowed
367+
if can_merge {
368+
if let Some(last_span) = last.0.back_mut() {
369+
if last_span.span.end == span.start {
370+
// Merge spans by extending the end of the last span
371+
last_span.span.end = span.end;
372+
return;
334373
}
335374
}
336-
337-
self.size += 1;
338-
last.0.push_back(StackItem { span, meta });
339375
}
376+
377+
// Add as a new item to the existing entry
378+
self.size += 1;
379+
last.0.push_back(StackItem { span, meta });
340380
}
341381

342382
pub fn compose_remote_event(&mut self, diff: &[&ContainerDiff]) {
@@ -415,10 +455,23 @@ impl UndoManagerInner {
415455
last_popped_selection: None,
416456
on_pop: None,
417457
on_push: None,
458+
group: None,
418459
}
419460
}
420461

462+
/// Returns true if a given container diff is disjoint with the current group.
463+
/// They are disjoint if they have no overlap in changed container ids.
464+
fn is_disjoint_with_group(&self, diff: &[&ContainerDiff]) -> bool {
465+
let Some(group) = &self.group else {
466+
return false;
467+
};
468+
469+
diff.iter().all(|d| !group.affected_cids.contains(&d.id))
470+
}
471+
421472
fn record_checkpoint(&mut self, latest_counter: Counter, event: Option<DiffEvent>) {
473+
let previous_counter = self.next_counter;
474+
422475
if Some(latest_counter) == self.next_counter {
423476
return;
424477
}
@@ -428,7 +481,14 @@ impl UndoManagerInner {
428481
return;
429482
}
430483

431-
assert!(self.next_counter.unwrap() < latest_counter);
484+
if let Some(group) = &mut self.group {
485+
event.iter().for_each(|e| {
486+
e.events.iter().for_each(|e| {
487+
group.affected_cids.insert(e.id.clone());
488+
})
489+
});
490+
}
491+
432492
let now = get_sys_timestamp() as Timestamp;
433493
let span = CounterSpan::new(self.next_counter.unwrap(), latest_counter);
434494
let meta = self
@@ -437,8 +497,25 @@ impl UndoManagerInner {
437497
.map(|x| x(UndoOrRedo::Undo, span, event))
438498
.unwrap_or_default();
439499

440-
if !self.undo_stack.is_empty() && now - self.last_undo_time < self.merge_interval_in_ms {
441-
self.undo_stack.push_with_merge(span, meta, true);
500+
// Wether the change is within the accepted merge interval
501+
let in_merge_interval = now - self.last_undo_time < self.merge_interval_in_ms;
502+
503+
// If group is active, but there is nothing in the group, don't merge
504+
// If the group is active and it's not the first push in the group, merge
505+
let group_should_merge = self.group.is_some()
506+
&& match (
507+
previous_counter,
508+
self.group.as_ref().map(|g| g.start_counter),
509+
) {
510+
(Some(previous), Some(active)) => previous != active,
511+
_ => true,
512+
};
513+
514+
let should_merge = !self.undo_stack.is_empty() && (in_merge_interval || group_should_merge);
515+
516+
if should_merge {
517+
self.undo_stack
518+
.push_with_merge(span, meta, true, self.group.as_ref());
442519
} else {
443520
self.last_undo_time = now;
444521
self.undo_stack.push(span, meta);
@@ -526,8 +603,16 @@ impl UndoManager {
526603
}
527604
}
528605

606+
let is_import_disjoint = inner.is_disjoint_with_group(event.events);
607+
529608
inner.undo_stack.compose_remote_event(event.events);
530609
inner.redo_stack.compose_remote_event(event.events);
610+
611+
// If the import is not disjoint, we end the active group
612+
// all subsequent changes will be new undo items
613+
if !is_import_disjoint {
614+
inner.group = None;
615+
}
531616
}
532617
EventTriggerKind::Checkout => {
533618
let mut inner = inner_clone.lock().unwrap();
@@ -556,6 +641,22 @@ impl UndoManager {
556641
}
557642
}
558643

644+
pub fn group_start(&mut self) -> LoroResult<()> {
645+
let mut inner = self.inner.lock().unwrap();
646+
647+
if inner.group.is_some() {
648+
return Err(LoroError::UndoGroupAlreadyStarted);
649+
}
650+
651+
inner.group = Some(UndoGroup::new(inner.next_counter.unwrap()));
652+
653+
Ok(())
654+
}
655+
656+
pub fn group_end(&mut self) {
657+
self.inner.lock().unwrap().group = None;
658+
}
659+
559660
pub fn peer(&self) -> PeerID {
560661
self.peer.load(std::sync::atomic::Ordering::Relaxed)
561662
}
@@ -726,6 +827,7 @@ impl UndoManager {
726827
)
727828
})
728829
.unwrap_or_default();
830+
729831
if matches!(kind, UndoOrRedo::Undo) && get_opposite(&mut inner).is_empty() {
730832
// If it's the first undo, we use the cursors from the users
731833
} else if let Some(inner) = next_push_selection.take() {

0 commit comments

Comments
 (0)