From c566eb7ef6bcd549409bc1179f414727975e00b2 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Tue, 27 Oct 2026 20:52:29 +0000 Subject: [PATCH] Make insert deduplication smarter --- src/operation_transformation/edited_text.rs | 8 +--- src/operation_transformation/operation.rs | 33 ++++++++++++-- .../find_longest_prefix_contained_within.rs | 45 +++++++++++-------- 3 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/operation_transformation/edited_text.rs b/src/operation_transformation/edited_text.rs index 0eda20a..4e77f79 100644 --- a/src/operation_transformation/edited_text.rs +++ b/src/operation_transformation/edited_text.rs @@ -161,13 +161,7 @@ where Side::Left => &mut last_right_op, Side::Right => &mut last_left_op, }; - let result = operation.merge_operations(previous_operation.as_ref()); - - if matches!(result, Operation::Insert { .. }) - && matches!(previous_operation, Some(Operation::Insert { .. })) - { - *previous_operation = None; - } + let result = operation.merge_operations_consuming_insert(previous_operation.as_mut()); match side { Side::Left => { diff --git a/src/operation_transformation/operation.rs b/src/operation_transformation/operation.rs index a6e2522..8f80cda 100644 --- a/src/operation_transformation/operation.rs +++ b/src/operation_transformation/operation.rs @@ -219,6 +219,34 @@ 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. + pub(crate) fn merge_operations_consuming_insert( + self, + previous_operation: Option<&mut Self>, + ) -> Self { + match (self, previous_operation) { + ( + Operation::Insert { order, text }, + Some(Operation::Insert { + text: previous_inserted_text, + .. + }), + ) => { + let overlap = find_longest_prefix_contained_within(previous_inserted_text, &text); + 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()) + } + } + } + /// Adjusts this operation based on `previous_operation` from the other side /// to avoid duplicating or conflicting changes #[allow(clippy::too_many_lines)] @@ -236,10 +264,9 @@ where // In case the current insert's prefix appears in the previously inserted text, // we can trim the current insert to only include the non-overlapping part. // This way, we don't end up duplicating text. - let offset_in_tokens = - find_longest_prefix_contained_within(previous_inserted_text, &text); + let overlap = find_longest_prefix_contained_within(previous_inserted_text, &text); - Operation::create_insert(order, text[offset_in_tokens..].to_vec()) + Operation::create_insert(order, text[overlap.len()..].to_vec()) } ( diff --git a/src/utils/find_longest_prefix_contained_within.rs b/src/utils/find_longest_prefix_contained_within.rs index 69516b4..feff917 100644 --- a/src/utils/find_longest_prefix_contained_within.rs +++ b/src/utils/find_longest_prefix_contained_within.rs @@ -1,9 +1,10 @@ -use std::fmt::Debug; +use std::{fmt::Debug, ops::Range}; use crate::Token; -/// Given two lists of tokens, returns `length` where the `old` list -/// somewhere within contains the `length` prefix of the `new` list +/// Returns the range in `old` containing the longest prefix of `new`. +/// Uses the first occurrence when that prefix appears more than once, +/// or `0..0` if there is no overlap. /// /// ## Example /// @@ -11,21 +12,21 @@ use crate::Token; /// old: [0, 1, 9, 0, 2, 5] /// new: [9, 0, 2, 5, 1] /// ``` -/// > results in a length of 4 +/// > results in a range of 2..6 /// /// /// ```not_rust /// old: [0, 1, 9, 0, 2, 5] /// new: [0, 2] /// ``` -/// > results in a length of 2 +/// > results in a range of 3..5 /// /// ```not_rust /// old: [0, 1, 9, 0, 2, 5] -/// new: [0, 4] +/// new: [0, 4] /// ``` -/// > results in a length of 1 -pub fn find_longest_prefix_contained_within(old: &[Token], new: &[Token]) -> usize +/// > results in a range of 0..1 +pub fn find_longest_prefix_contained_within(old: &[Token], new: &[Token]) -> Range where T: PartialEq + Clone + Debug, { @@ -33,12 +34,12 @@ where for len in (1..=max_possible).rev() { let prefix = &new[..len]; - if old.windows(len).any(|window| window == prefix) { - return len; + if let Some(start) = old.windows(len).position(|window| window == prefix) { + return start..start + len; } } - 0 + 0..0 } #[cfg(test)] @@ -51,7 +52,7 @@ mod tests { fn test_common_overlap() { assert_eq!( find_longest_prefix_contained_within(&["".into()], &["".into()]), - 1 + 0..1 ); assert_eq!( @@ -59,7 +60,7 @@ mod tests { &["a".into(), "b".into(), "c".into()], &["b".into(), "c".into(), "a".into()] ), - 2 + 1..3 ); assert_eq!( @@ -67,7 +68,7 @@ mod tests { &["a".into(), "b".into(), "c".into()], &["b".into(), "c".into()] ), - 2 + 1..3 ); assert_eq!( @@ -75,7 +76,7 @@ mod tests { &["a".into(), "b".into(), "c".into()], &["b".into()] ), - 1 + 1..2 ); assert_eq!( @@ -83,7 +84,7 @@ mod tests { &["a".into(), "b".into(), "c".into(), "b".into(), "a".into()], &["b".into(), "a".into()] ), - 2 + 3..5 ); assert_eq!( @@ -91,7 +92,15 @@ mod tests { &["a".into(), "a".into(), "a".into()], &["a".into(), "b".into(), "c".into()] ), - 1 + 0..1 + ); + + assert_eq!( + find_longest_prefix_contained_within( + &["a".into(), "b".into(), "a".into(), "b".into()], + &["a".into(), "b".into()] + ), + 0..2 ); assert_eq!( @@ -99,7 +108,7 @@ mod tests { &["a".into(), "b".into(), "c".into()], &["d".into(), "e".into(), "a".into()] ), - 0 + 0..0 ); } }