Fix divergence bug in undo/redo

As part of #1405, we changed the way we performed undo and redo to
support combining transactions that were not temporally adjacent for
IME purposes.

We introduced a bug with that release that caused divergence
when performing undo: the bug was caused by only changing the visibility
of fragments whose insertion id was contained in the undo operation. However,
an undo operation also affects deletions which we were mistakenly not
considering. Randomized tests caught this but I guess we didn't run enough
of them.
This commit is contained in:
Antonio Scandurra 2022-08-17 11:26:05 +02:00
parent da805b3d13
commit ab236a6008
3 changed files with 3 additions and 14 deletions

View file

@ -39,7 +39,6 @@ pub fn serialize_operation(operation: &Operation) -> proto::Operation {
local_timestamp: undo.id.value, local_timestamp: undo.id.value,
lamport_timestamp: lamport_timestamp.value, lamport_timestamp: lamport_timestamp.value,
version: serialize_version(&undo.version), version: serialize_version(&undo.version),
transaction_version: serialize_version(&undo.transaction_version),
counts: undo counts: undo
.counts .counts
.iter() .iter()
@ -199,7 +198,6 @@ pub fn deserialize_operation(message: proto::Operation) -> Result<Operation> {
) )
}) })
.collect(), .collect(),
transaction_version: deserialize_version(undo.transaction_version),
}, },
}), }),
proto::operation::Variant::UpdateSelections(message) => { proto::operation::Variant::UpdateSelections(message) => {

View file

@ -901,8 +901,7 @@ message Operation {
uint32 local_timestamp = 2; uint32 local_timestamp = 2;
uint32 lamport_timestamp = 3; uint32 lamport_timestamp = 3;
repeated VectorClockEntry version = 4; repeated VectorClockEntry version = 4;
repeated VectorClockEntry transaction_version = 6; repeated UndoCount counts = 5;
repeated UndoCount counts = 7;
} }
message UpdateSelections { message UpdateSelections {

View file

@ -512,7 +512,6 @@ pub struct EditOperation {
pub struct UndoOperation { pub struct UndoOperation {
pub id: clock::Local, pub id: clock::Local,
pub counts: HashMap<clock::Local, u32>, pub counts: HashMap<clock::Local, u32>,
pub transaction_version: clock::Global,
pub version: clock::Global, pub version: clock::Global,
} }
@ -1109,14 +1108,8 @@ impl Buffer {
let mut fragment = fragment.clone(); let mut fragment = fragment.clone();
let fragment_was_visible = fragment.visible; let fragment_was_visible = fragment.visible;
if fragment.was_visible(&undo.transaction_version, &self.undo_map) fragment.visible = fragment.is_visible(&self.undo_map);
|| undo fragment.max_undos.observe(undo.id);
.counts
.contains_key(&fragment.insertion_timestamp.local())
{
fragment.visible = fragment.is_visible(&self.undo_map);
fragment.max_undos.observe(undo.id);
}
let old_start = old_fragments.start().1; let old_start = old_fragments.start().1;
let new_start = new_fragments.summary().text.visible; let new_start = new_fragments.summary().text.visible;
@ -1297,7 +1290,6 @@ impl Buffer {
id: self.local_clock.tick(), id: self.local_clock.tick(),
version: self.version(), version: self.version(),
counts, counts,
transaction_version: transaction.start,
}; };
self.apply_undo(&undo)?; self.apply_undo(&undo)?;
let operation = Operation::Undo { let operation = Operation::Undo {