Skip to content

fix: restore selection when undoing the last history item - #1102

Open
fendermoon wants to merge 1 commit into
loro-dev:mainfrom
fendermoon:codex/fix-undo-last-item-selection
Open

fendermoon wants to merge 1 commit into
loro-dev:mainfrom
fendermoon:codex/fix-undo-last-item-selection

Conversation

@fendermoon

Copy link
Copy Markdown

Summary

Fixes #1100.

Stack::pop() decreases the logical item count but keeps the row containing its
remote delta alive for cursor restoration. When the popped item was the last
one, transform_based_on_this_delta() returned early because self.is_empty()
checks that logical count. The subsequent cursor transform therefore used the
remote delta without rebasing it through the undo.

Check self.stack.is_empty() instead. This keeps the empty-storage guard while
allowing the retained row's delta to be transformed. It does not introduce a new
history or change the merge algorithm.

The regression starts with Hello world!, commits it, and then creates the
UndoManager. It deletes the selected ello, imports a peer's Hi and ii
insertions, and undoes the deletion. The text must be Hi Helloii world! and
the absolute selection positions delivered in on_pop metadata
(CursorWithPos.pos.pos) must be [4,8], not [4,10]. Resolving the retained
cursors through doc.get_cursor_pos(...).current.pos already returns [4,8]
before the fix; the regression distinguishes these two results.

The same test first runs a control with one unrelated map edit in the history.
The control passes without the fix; the last-item arm fails. Neither arm loads
a snapshot or calls clear() before editing. Both also check fresh snapshot
and peer readback of the document value and version vector.

Validation

Base: d9ddfba195f9363653ac1d83e744641dcb337f4e.
Prepared commit: 7db750e38d61fb83200d9f9665931e77bce5fad7.

  • Before the fix: the new test fails with older_item=false at the on_pop
    metadata assertion, actual [4,10], expected [4,8]. The older-item control,
    text/readback checks and cursor-resolution assertion pass first.
  • After the fix: cargo test --locked -p loro -p loro-internal --lib --tests -- --test-threads=1
    passes 1,050 tests; 12 existing tests are ignored. Native macOS, Rust 1.94.0,
    dev debug information disabled and incremental compilation disabled.
  • The focused test also passes after committing the patch.
  • WASM/browser tests, release-mode performance and fuzzing were not run.

Includes the repository-required patch changeset. No public API or storage
format changes are proposed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Selection expands after remote edits when undoing the last history item (Rust 1.16.0)

1 participant