From d58b474669ee826a7247bc112e504334fe4693d8 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sun, 22 Jun 2025 09:24:36 +0100 Subject: [PATCH] DOn't use start_index in EditedText --- src/operation_transformation/edited_text.rs | 48 +++++++-------- src/operation_transformation/merge_context.rs | 4 +- src/operation_transformation/operation.rs | 60 ++++++++++++------- .../ordered_operation.rs | 2 +- 4 files changed, 61 insertions(+), 53 deletions(-) diff --git a/src/operation_transformation/edited_text.rs b/src/operation_transformation/edited_text.rs index 4548a41..56254a0 100644 --- a/src/operation_transformation/edited_text.rs +++ b/src/operation_transformation/edited_text.rs @@ -120,18 +120,18 @@ where let mut merged_length: usize = 0; loop { - let (side, OrderedOperation { operation, order }) = + let (side, OrderedOperation { operation, order }, maybe_other_operation) = match (maybe_left_op.clone(), maybe_right_op.clone()) { (Some(left_op), Some(right_op)) => { if left_op < right_op { - (Side::Left, left_op) + (Side::Left, left_op, maybe_right_op.clone()) } else { - (Side::Right, right_op) + (Side::Right, right_op, maybe_left_op.clone()) } } - (Some(left_op), None) => (Side::Left, left_op), - (None, Some(right_op)) => (Side::Right, right_op), + (Some(left_op), None) => (Side::Left, left_op, None), + (None, Some(right_op)) => (Side::Right, right_op, None), (None, None) => break, }; @@ -145,24 +145,25 @@ where let result = match side { Side::Left => { let result = operation.merge_operations_with_context( + order, &mut right_merge_context, &mut left_merge_context, + maybe_other_operation, ); if let Some(ref op @ (Operation::Insert { .. } | Operation::Equal { .. })) = result { - println!("merged_length {merged_length}, idx {}", op.start_index()); - // assert_eq!(op.start_index() as i64, merged_length as i64); - let shift = op.start_index() as i64 - seen_left_length as i64 - + op.len() as i64 - - original_length; + let mut shift = merged_length as i64 - seen_left_length as i64; + if !matches!(op, Operation::Equal { .. }) { + shift += op.len() as i64 - original_length; + } while let Some(cursor) = left_cursors.next_if(|cursor| { cursor.char_index <= seen_left_length + original_length as usize }) { merged_cursors.push(cursor.with_index( - (op.start_index() as i64).max(cursor.char_index as i64 + shift) + (merged_length as i64).max(cursor.char_index as i64 + shift) as usize, )); } @@ -178,25 +179,25 @@ where } Side::Right => { let result = operation.merge_operations_with_context( + order, &mut left_merge_context, &mut right_merge_context, + maybe_other_operation, ); if let Some(ref op @ (Operation::Insert { .. } | Operation::Equal { .. })) = result { - println!("merged_length {merged_length}, idx {}", op.start_index()); - // assert_eq!(op.start_index() as i64, merged_length as i64); - - let shift = op.start_index() as i64 - seen_right_length as i64 - + op.len() as i64 - - original_length; + let mut shift = merged_length as i64 - seen_right_length as i64; + if !matches!(op, Operation::Equal { .. }) { + shift += op.len() as i64 - original_length; + } while let Some(cursor) = right_cursors.next_if(|cursor| { cursor.char_index <= seen_right_length + original_length as usize }) { merged_cursors.push(cursor.with_index( - (op.start_index() as i64).max(cursor.char_index as i64 + shift) + (merged_length as i64).max(cursor.char_index as i64 + shift) as usize, )); } @@ -215,15 +216,8 @@ where println!(" = {result:?}"); if let Some(operation) = result { - let last_operation = merged_operations.last(); - if let Some(last_operation) = last_operation { - if matches!(last_operation.operation, Operation::Equal { .. }) - && matches!(operation, Operation::Equal { .. }) - && last_operation.operation.eq(&operation) - { - println!("skipping equal"); - continue; - } + if operation.len() == 0 { + continue; } if is_advancing_operation { diff --git a/src/operation_transformation/merge_context.rs b/src/operation_transformation/merge_context.rs index e397226..b40d2f3 100644 --- a/src/operation_transformation/merge_context.rs +++ b/src/operation_transformation/merge_context.rs @@ -59,13 +59,13 @@ where .. }, ) => { - if threshold_index + self.shift > op.end_index() as i64 { + if threshold_index + self.shift >= op.end_index() as i64 { self.shift -= *deleted_character_count as i64; self.last_operation = None; } } Some(op @ Operation::Insert { .. }) | Some(op @ Operation::Equal { .. }) => { - if threshold_index + self.shift - op.len() as i64 > op.end_index() as i64 { + if threshold_index + self.shift - op.len() as i64 >= op.end_index() as i64 { self.last_operation = None; } } diff --git a/src/operation_transformation/operation.rs b/src/operation_transformation/operation.rs index 862a5b6..73722aa 100644 --- a/src/operation_transformation/operation.rs +++ b/src/operation_transformation/operation.rs @@ -6,11 +6,12 @@ use serde::{Deserialize, Serialize}; use super::merge_context::MergeContext; use crate::{ + Token, + operation_transformation::ordered_operation::OrderedOperation, utils::{ find_longest_prefix_contained_within::find_longest_prefix_contained_within, string_builder::StringBuilder, }, - Token, }; /// Represents a change that can be applied on a `StringBuilder`. @@ -84,10 +85,6 @@ where /// This operation is used to indicate that the text at the given index /// is unchanged. pub fn create_equal(index: usize, length: usize) -> Option { - if length == 0 { - return None; - } - Some(Operation::Equal { index, length, @@ -213,18 +210,12 @@ where } } - /// Returns the index of the last character that the operation affects. - pub fn end_index(&self) -> usize { - debug_assert!( - self.len() > 0, - " len() must be greater than 0 because operations must be non-empty" - ); - self.start_index() + self.len() - 1 - } + /// Returns the first index after the last character that the operation + /// affects. + pub fn end_index(&self) -> usize { self.start_index() + self.len() } /// Returns the range of indices of characters that the operation affects. - #[allow(clippy::range_plus_one)] - pub fn range(&self) -> Range { self.start_index()..self.end_index() + 1 } + pub fn range(&self) -> Range { self.start_index()..self.end_index() } /// Returns the number of affected characters. It is always greater than 0 /// because empty operations cannot be created. @@ -294,8 +285,10 @@ where #[allow(clippy::too_many_lines)] pub fn merge_operations_with_context( self, + order: usize, affecting_context: &mut MergeContext, produced_context: &mut MergeContext, + other_operation: Option>, ) -> Option> { affecting_context.consume_last_operation_if_it_is_too_behind(self.start_index() as i64); let operation = self.with_shifted_index(affecting_context.shift); @@ -362,7 +355,7 @@ where let moved_operation = operation.with_index(last_delete.start_index()); affecting_context.replace_last_operation(Operation::create_delete( - moved_operation.end_index() + 1, + moved_operation.end_index(), (last_delete.len() as i64 - difference) as usize, )); affecting_context.shift -= difference; @@ -415,19 +408,19 @@ where ); let overlap = (length as i64) - .min(last_delete.end_index() as i64 - operation.start_index() as i64 + 1); + .min(last_delete.end_index() as i64 - operation.start_index() as i64); #[cfg(debug_assertions)] let result = text.as_ref().map_or_else( || { Operation::create_equal( - operation.end_index().min(last_delete.end_index()), + operation.end_index().min(last_delete.end_index()) - 1, (length as i64 - overlap) as usize, ) }, |text| { Operation::create_equal_with_text( - operation.end_index().min(last_delete.end_index()), + operation.end_index().min(last_delete.end_index()) - 1, text.chars().skip(overlap as usize).collect::(), ) }, @@ -441,7 +434,26 @@ where result } - (operation @ Operation::Equal { .. }, _) => Some(operation), + + (operation @ Operation::Equal { .. }, _) => { + let result = if let Some(other_operation) = other_operation { + if matches!(other_operation.operation, Operation::Equal { .. }) + && operation.len() == other_operation.operation.len() + && order == other_operation.order + { + println!(" !!!equal to next"); + Operation::create_equal(operation.start_index(), 0) + } else { + Some(operation) + } + } else { + Some(operation) + }; + + produced_context.consume_and_replace_last_operation(result.clone()); + + result + } } } } @@ -531,9 +543,11 @@ mod tests { #[test] #[should_panic(expected = "Shifted index must be non-negative")] fn test_shifting_error() { - insta::assert_debug_snapshot!(Operation::create_insert(1, vec!["hi".into()]) - .unwrap() - .with_shifted_index(-2)); + insta::assert_debug_snapshot!( + Operation::create_insert(1, vec!["hi".into()]) + .unwrap() + .with_shifted_index(-2) + ); } #[test] diff --git a/src/operation_transformation/ordered_operation.rs b/src/operation_transformation/ordered_operation.rs index c9c0b67..8878c45 100644 --- a/src/operation_transformation/ordered_operation.rs +++ b/src/operation_transformation/ordered_operation.rs @@ -1,7 +1,7 @@ #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use crate::{operation_transformation::Operation, Token}; +use crate::{Token, operation_transformation::Operation}; #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[derive(Debug, Clone, PartialEq)]