diff --git a/backend/reconcile/src/operation_transformation/edited_text.rs b/backend/reconcile/src/operation_transformation/edited_text.rs index 9e44c99..da9ed33 100644 --- a/backend/reconcile/src/operation_transformation/edited_text.rs +++ b/backend/reconcile/src/operation_transformation/edited_text.rs @@ -272,7 +272,7 @@ mod tests { #[test] fn test_calculate_operations_with_insert() { let original = "hello world! ..."; - let left = "Hello world! I'm Andras."; + let left = "hello world! I'm Andras."; let right = "Hello world! How are you?"; let expected = "Hello world! I'm Andras.How are you?"; diff --git a/backend/reconcile/src/operation_transformation/merge_context.rs b/backend/reconcile/src/operation_transformation/merge_context.rs index a08d02d..6e77a03 100644 --- a/backend/reconcile/src/operation_transformation/merge_context.rs +++ b/backend/reconcile/src/operation_transformation/merge_context.rs @@ -5,7 +5,7 @@ pub struct MergeContext where T: PartialEq + Clone, { - pub last_delete: Option>, + last_operation: Option>, pub shift: i64, } @@ -15,7 +15,7 @@ where { fn default() -> Self { MergeContext { - last_delete: None, + last_operation: None, shift: 0, } } @@ -25,28 +25,49 @@ impl MergeContext where T: PartialEq + Clone, { - /// Replace the last delete operation (if there was one) with a new one while - /// applying it to the shift. - pub fn replace_delete(&mut self, delete: Option>) { - if let Some(produced_last_delete) = self.last_delete.take() { - self.shift -= produced_last_delete.len() as i64; - } - - self.last_delete = delete; + pub fn last_operation(&self) -> Option<&Operation> { + self.last_operation.as_ref() } - /// Remove the last delete operation (if there was one) in case it is behind the - /// threshold operation. - pub fn consume_delete_if_behind_operation(&mut self, threshold_operation: &Operation) { - match self.last_delete.as_ref() { - Some(last_delete) - if threshold_operation.start_index() as i64 + self.shift - > last_delete.end_index() as i64 => + /// Replace the last delete operation (if there was one) with a new one while + /// applying it to the shift. + pub fn consume_and_replace_last_operation(&mut self, operation: Option>) { + if let Some(Operation::Delete { + deleted_character_count, + .. + }) = self.last_operation.take() + { + self.shift -= deleted_character_count as i64; + } + + self.last_operation = operation; + } + + pub fn replace_last_operation(&mut self, operation: Option>) { + self.last_operation = operation; + } + + /// Remove the last operation (if there was one) in case it is behind the + /// threshold operation. This changes the shift in case the last operation was + /// a delete. + pub fn consume_last_operation_if_it_is_too_behind( + &mut self, + threshold_operation: &Operation, + ) { + if let Some(last_operation) = self.last_operation.as_ref() { + if threshold_operation.start_index() as i64 + self.shift + > last_operation.end_index() as i64 { - self.shift -= last_delete.len() as i64; - self.last_delete = None; + if let Operation::Delete { + deleted_character_count, + .. + } = last_operation + { + self.shift -= *deleted_character_count as i64; + } + + self.last_operation = None; } - _ => {} } } } diff --git a/backend/reconcile/src/operation_transformation/operation.rs b/backend/reconcile/src/operation_transformation/operation.rs index a3751b0..8dc3725 100644 --- a/backend/reconcile/src/operation_transformation/operation.rs +++ b/backend/reconcile/src/operation_transformation/operation.rs @@ -204,22 +204,31 @@ where affecting_context: &mut MergeContext, produced_context: &mut MergeContext, ) -> Option> { - affecting_context.consume_delete_if_behind_operation(&self); + affecting_context.consume_last_operation_if_it_is_too_behind(&self); let operation = self.with_shifted_index(affecting_context.shift); - match (operation, affecting_context.last_delete.clone()) { + match (operation, affecting_context.last_operation().clone()) { (operation @ Operation::Insert { .. }, None) => { produced_context.shift += operation.len() as i64; Some(operation) } - (operation @ Operation::Delete { .. }, None) => { - produced_context.replace_delete(Some(operation.clone())); + (operation, Some(last_insert @ Operation::Insert { .. })) => { + produced_context.shift += operation.len() as i64; Some(operation) } - (operation @ Operation::Insert { .. }, Some(last_delete)) => { + // We can never delete inside an insert + (operation @ Operation::Delete { .. }, None) => { + produced_context.consume_and_replace_last_operation(Some(operation.clone())); + Some(operation) + } + + ( + operation @ Operation::Insert { .. }, + Some(last_delete @ Operation::Delete { .. }), + ) => { produced_context.shift += operation.len() as i64; debug_assert!( @@ -231,16 +240,19 @@ where let moved_operation = operation.with_index(last_delete.start_index()); - affecting_context.last_delete = Operation::create_delete( + affecting_context.replace_last_operation(Operation::create_delete( moved_operation.end_index() + 1, (last_delete.len() as i64 - difference) as usize, - ); + )); affecting_context.shift -= difference; Some(moved_operation) } - (operation @ Operation::Delete { .. }, Some(last_delete)) => { + ( + operation @ Operation::Delete { .. }, + Some(last_delete @ Operation::Delete { .. }), + ) => { debug_assert!( last_delete.range().contains(&operation.start_index()), "There is a last delete ({last_delete}) but the operation ({operation}) is not contained in it" @@ -253,13 +265,13 @@ where 0.max(operation.end_index() as i64 - last_delete.end_index() as i64) as usize, ); - affecting_context.shift -= difference; - affecting_context.last_delete = Operation::create_delete( + affecting_context.replace_last_operation(Operation::create_delete( last_delete.start_index(), 0.max(last_delete.end_index() as i64 - operation.end_index() as i64) as usize, - ); + )); + affecting_context.shift -= difference; - produced_context.replace_delete(updated_delete.clone()); + produced_context.consume_and_replace_last_operation(updated_delete.clone()); updated_delete }