Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor undo cursor #377

Merged
merged 2 commits into from
Jun 6, 2024
Merged

Refactor undo cursor #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffa4037
refactor: make cursor transformation in undo/redo better
zxch3n Jun 6, 2024
6490a73
chore: add release info
zxch3n Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/sharp-rules-report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"loro-wasm": patch
"loro-crdt": patch
---

Make cursors transformation better in undo/redo loop
86 changes: 79 additions & 7 deletions crates/loro-internal/src/undo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use loro_common::{
ContainerID, Counter, CounterSpan, HasCounterSpan, HasIdSpan, IdSpan, LoroError, LoroResult,
LoroValue, PeerID,
};
use tracing::{debug_span, info_span, instrument, trace};
use tracing::{debug_span, info_span, instrument};

use crate::{
change::get_sys_timestamp,
Expand Down Expand Up @@ -71,7 +71,7 @@ fn transform_cursor(
container_remap: &FxHashMap<ContainerID, ContainerID>,
) {
let mut cid = &cursor_with_pos.cursor.container;
while let Some(new_cid) = container_remap.get(&cid) {
while let Some(new_cid) = container_remap.get(cid) {
cid = new_cid;
}

Expand Down Expand Up @@ -140,6 +140,8 @@ impl UndoOrRedo {
}
}

/// When a undo/redo item is pushed, the undo manager will call the on_push callback to get the meta data of the undo item.
/// The returned cursors will be recorded for a new pushed undo item.
pub type OnPush = Box<dyn Fn(UndoOrRedo, CounterSpan) -> UndoItemMeta + Send + Sync>;
pub type OnPop = Box<dyn Fn(UndoOrRedo, CounterSpan, UndoItemMeta) + Send + Sync>;

Expand All @@ -152,6 +154,7 @@ struct UndoManagerInner {
merge_interval: i64,
max_stack_size: usize,
exclude_origin_prefixes: Vec<Box<str>>,
last_popped_selection: Option<Vec<CursorWithPos>>,
on_push: Option<OnPush>,
on_pop: Option<OnPop>,
}
Expand Down Expand Up @@ -187,13 +190,13 @@ struct StackItem {
///
/// The cursors inside the metadata will be transformed by remote operations as well.
/// So that when the item is popped, users can restore the cursors position correctly.
#[derive(Debug, Default)]
#[derive(Debug, Default, Clone)]
pub struct UndoItemMeta {
pub value: LoroValue,
pub cursors: Vec<CursorWithPos>,
}

#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct CursorWithPos {
pub cursor: Cursor,
pub pos: AbsolutePosition,
Expand Down Expand Up @@ -370,6 +373,7 @@ impl UndoManagerInner {
last_undo_time: 0,
max_stack_size: usize::MAX,
exclude_origin_prefixes: vec![],
last_popped_selection: None,
on_pop: None,
on_push: None,
}
Expand All @@ -388,6 +392,7 @@ impl UndoManagerInner {
.as_ref()
.map(|x| x(UndoOrRedo::Undo, span))
.unwrap_or_default();

if !self.undo_stack.is_empty() && now - self.last_undo_time < self.merge_interval {
self.undo_stack.push_with_merge(span, meta, true);
} else {
Expand Down Expand Up @@ -522,6 +527,60 @@ impl UndoManager {
get_opposite: impl Fn(&mut UndoManagerInner) -> &mut Stack,
kind: UndoOrRedo,
) -> LoroResult<bool> {
// When in the undo/redo loop, the new undo/redo stack item should restore the selection
// to the state it was in before the item that was popped two steps ago from the stack.
//
// ┌────────────┐
// │Selection 1 │
// └─────┬──────┘
// │ Some
// ▼ ops
// ┌────────────┐
// │Selection 2 │
// └─────┬──────┘
// │ Some
// ▼ ops
// ┌────────────┐
// │Selection 3 │◁ ─ ─ ─ ─ ─ ─ ─ Restore ─ ─ ─
// └─────┬──────┘ │
// │
// │ │
// │ ┌ ─ ─ ─ ─ ─ ─ ─
// Enter the │ Undo ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─▶ Push Redo │
// undo/redo ─ ─ ─ ▶ ▼ └ ─ ─ ─ ─ ─ ─ ─
// loop ┌────────────┐ │
// │Selection 2 │◁─ ─ ─ Restore ─
// └─────┬──────┘ │ │
// │
// │ │ │
// │ ┌ ─ ─ ─ ─ ─ ─ ─
// │ Undo ─ ─ ─ ─ ▶ Push Redo │ │
// ▼ └ ─ ─ ─ ─ ─ ─ ─
// ┌────────────┐ │ │
// │Selection 1 │
// └─────┬──────┘ │ │
// │ Redo ◀ ─ ─ ─ ─ ─ ─ ─ ─
// ▼ │
// ┌────────────┐
// ┌ Restore ─ ▷│Selection 2 │ │
// └─────┬──────┘
// │ │ │
// ┌ ─ ─ ─ ─ ─ ─ ─ │
// Push Undo │◀─ ─ ─ ─ ─ ─ ─ │ Redo ◀ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┘
// └ ─ ─ ─ ─ ─ ─ ─ ▼
// │ ┌────────────┐
// │Selection 3 │
// │ └─────┬──────┘
// ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ ▶ │ Undo
// ▼
// ┌────────────┐
// │Selection 2 │
// └────────────┘
//
// Because users may change the selections during the undo/redo loop, it's
// more stable to keep the selection stored in the last stack item
// rather than using the current selection directly.

self.record_new_checkpoint(doc)?;
let end_counter = get_counter_end(doc, self.peer);
let mut top = {
Expand All @@ -532,6 +591,7 @@ impl UndoManager {

let mut executed = false;
while let Some((mut span, remote_diff)) = top {
let mut next_push_selection = None;
{
let inner = self.inner.clone();
// We need to clone this because otherwise <transform_delta> will be applied to the same remote diff
Expand All @@ -552,7 +612,8 @@ impl UndoManager {
},
)?;
drop(commit);
if let Some(x) = self.inner.try_lock().unwrap().on_pop.as_ref() {
let mut inner = self.inner.try_lock().unwrap();
if let Some(x) = inner.on_pop.as_ref() {
for cursor in span.meta.cursors.iter_mut() {
// <cursor_transform> We need to transform cursor here.
// Note that right now <transform_delta> is already done,
Expand All @@ -564,17 +625,28 @@ impl UndoManager {
&self.container_remap,
);
}
x(kind, span.span, span.meta)

x(kind, span.span, span.meta.clone());
let take = inner.last_popped_selection.take();
next_push_selection = take;
inner.last_popped_selection = Some(span.meta.cursors);
}
}
let new_counter = get_counter_end(doc, self.peer);
if end_counter != new_counter {
let mut inner = self.inner.try_lock().unwrap();
let meta = inner
let mut meta = inner
.on_push
.as_ref()
.map(|x| x(kind.opposite(), CounterSpan::new(end_counter, new_counter)))
.unwrap_or_default();
if matches!(kind, UndoOrRedo::Undo) && get_opposite(&mut inner).is_empty() {
// If it's the first undo, we use the cursors from the users
} else if let Some(inner) = next_push_selection.take() {
// Otherwise, we use the cursors from the undo/redo loop
meta.cursors = inner;
}

get_opposite(&mut inner).push(CounterSpan::new(end_counter, new_counter), meta);
inner.latest_counter = new_counter;
executed = true;
Expand Down
8 changes: 8 additions & 0 deletions crates/loro-wasm/deno.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading