From e8221e03c7024d67e9fdab9d561eabce074fcc8c Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Tue, 27 Oct 2026 21:59:51 +0000 Subject: [PATCH] Fix deduplication of consecutive insertions --- src/operation_transformation/edited_text.rs | 23 +++++++++++-- src/operation_transformation/operation.rs | 38 ++++++++++++++++++--- tests/idempotence.rs | 23 +++++++++++++ 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/operation_transformation/edited_text.rs b/src/operation_transformation/edited_text.rs index 059836f..3bfed80 100644 --- a/src/operation_transformation/edited_text.rs +++ b/src/operation_transformation/edited_text.rs @@ -95,6 +95,25 @@ where } } + fn remember_operation(previous: &mut Option>, operation: &Operation) { + // Consecutive insertions can all contribute to a later overlap match. + // Preserve their unconsumed tokens even when the latest result is empty. + if let ( + Some(Operation::Insert { + order: previous_order, + text: previous_text, + }), + Operation::Insert { order, text }, + ) = (previous.as_mut(), operation) + && previous_order == order + { + previous_text.extend_from_slice(text); + return; + } + + *previous = Some(operation.clone()); + } + /// Merge two `EditedText` instances. The two instances must be derived /// from the same original text. The operations are merged using the /// principles of Operational Transformation. The cursors are updated @@ -209,7 +228,7 @@ where } maybe_left_op = left_iter.next(); - last_left_op = Some(result.clone()); + Self::remember_operation(&mut last_left_op, &result); } Side::Right => { if is_advancing_operation { @@ -217,7 +236,7 @@ where } maybe_right_op = right_iter.next(); - last_right_op = Some(result.clone()); + Self::remember_operation(&mut last_right_op, &result); } } diff --git a/src/operation_transformation/operation.rs b/src/operation_transformation/operation.rs index 8f80cda..2c4b7ee 100644 --- a/src/operation_transformation/operation.rs +++ b/src/operation_transformation/operation.rs @@ -42,6 +42,13 @@ where }, } +/// Character bounds of a matched prefix within the cached insertion. +#[derive(Debug)] +pub(crate) struct InsertionOverlap { + pub offset_from_end: usize, + pub length: usize, +} + impl Operation where T: PartialEq + Clone + Debug, @@ -221,10 +228,12 @@ where /// Merges operations while consuming the matched part of a cached insertion. /// Keeps its suffix for later matches until new text is emitted after it. + /// Also returns the matched prefix's position within the cached insertion, + /// for positioning cursors in deduplicated text. pub(crate) fn merge_operations_consuming_insert( self, previous_operation: Option<&mut Self>, - ) -> Self { + ) -> (Self, Option) { match (self, previous_operation) { ( Operation::Insert { order, text }, @@ -234,16 +243,35 @@ where }), ) => { let overlap = find_longest_prefix_contained_within(previous_inserted_text, &text); + let insertion_overlap = (!overlap.is_empty()).then(|| { + let length = previous_inserted_text[overlap.clone()] + .iter() + .map(Token::get_original_length) + .sum::(); + let trailing_length = previous_inserted_text[overlap.end..] + .iter() + .map(Token::get_original_length) + .sum::(); + + InsertionOverlap { + offset_from_end: length + trailing_length, + length, + } + }); if overlap.len() == text.len() { previous_inserted_text.drain(..overlap.end); } else { previous_inserted_text.clear(); } - Operation::create_insert(order, text[overlap.len()..].to_vec()) - } - (operation, previous_operation) => { - operation.merge_operations(previous_operation.as_deref()) + ( + Operation::create_insert(order, text[overlap.len()..].to_vec()), + insertion_overlap, + ) } + (operation, previous_operation) => ( + operation.merge_operations(previous_operation.as_deref()), + None, + ), } } diff --git a/tests/idempotence.rs b/tests/idempotence.rs index ca154f7..916977d 100644 --- a/tests/idempotence.rs +++ b/tests/idempotence.rs @@ -79,3 +79,26 @@ fn identical_edits_preserve_original_spelling(parent: &str, updated: &str) { updated, ); } + +#[test_case("a", "\n\n", "a\n\n", "\n\n"; "deleted_prefix")] +#[test_case("a b", "\n\n", "b\n\n", "\n\n"; "partially_deleted_prefix")] +fn consecutive_markdown_insertions_preserve_text_in_both_orders( + parent: &str, + left: &str, + right: &str, + expected: &str, +) { + for (left, right) in [(left, right), (right, left)] { + assert_eq!( + reconcile( + parent, + &left.into(), + &right.into(), + &*BuiltinTokenizer::Markdown + ) + .apply() + .text(), + expected, + ); + } +}