diff --git a/backend/sync_lib/src/operations/operation.rs b/backend/sync_lib/src/operations/operation.rs index a5d30a5..c905633 100644 --- a/backend/sync_lib/src/operations/operation.rs +++ b/backend/sync_lib/src/operations/operation.rs @@ -1,6 +1,5 @@ use ropey::Rope; use serde::{Deserialize, Serialize}; -use std::cmp::Ordering; use std::fmt::Display; use crate::errors::SyncLibError; @@ -19,22 +18,6 @@ pub enum Operation { }, } -impl Display for Operation { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Operation::Insert { index, text } => { - write!(f, "+\"{}\" at {}", text, index) - } - Operation::Delete { - index, - deleted_character_count, - } => { - write!(f, "-{} at {}", deleted_character_count, index) - } - } - } -} - impl Operation { pub fn create_insert(index: i64, text: &str) -> Result, SyncLibError> { Self::validate_index(index)?; @@ -69,6 +52,7 @@ impl Operation { })) } + /// Tries to apply the operation to the given text, returning the modified text. pub fn apply<'a>(&self, rope_text: &'a mut Rope) -> Result<&'a mut Rope, SyncLibError> { let index: usize = self.start_index() as usize; match self { @@ -99,6 +83,14 @@ impl Operation { } } + /// Returns the index based on which concurrent operations must be ordered. + pub fn comparison_index(&self) -> i64 { + match self { + Operation::Insert { .. } => self.start_index(), + Operation::Delete { .. } => self.end_index(), + } + } + /// Returns the index of the last character that the operation affects. pub fn end_index(&self) -> i64 { self.start_index() + self.len() - 1 @@ -125,6 +117,7 @@ impl Operation { self.start_index()..=self.end_index() } + /// Clones the operation while updating the index. pub fn with_index(&self, index: i64) -> Result { Self::validate_index(index)?; @@ -133,7 +126,6 @@ impl Operation { index, text: text.clone(), }, - Operation::Delete { deleted_character_count, .. @@ -144,6 +136,8 @@ impl Operation { }) } + /// Clones the operation while shifting the index by the given offset. + /// The offset can be negative but the resulting index must be non-negative. pub fn with_shifted_index(&self, offset: i64) -> Result { self.with_index(self.start_index() + offset) } @@ -160,27 +154,26 @@ impl Operation { } } -impl Ord for Operation { - fn cmp(&self, other: &Self) -> Ordering { - let result = self.start_index().cmp(&other.start_index()); - if result == Ordering::Equal { - match (self, other) { - (Operation::Insert { .. }, Operation::Delete { .. }) => Ordering::Greater, - (Operation::Delete { .. }, Operation::Insert { .. }) => Ordering::Less, - _ => Ordering::Equal, +impl Display for Operation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Operation::Insert { index, text } => { + write!(f, "Insert '{}' from index {}", text, index) + } + Operation::Delete { + index, + deleted_character_count, + } => { + write!( + f, + "Delete {} characters index {}", + deleted_character_count, index + ) } - } else { - result } } } -impl PartialOrd for Operation { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/backend/sync_lib/src/operations/operation_sequence.rs b/backend/sync_lib/src/operations/operation_sequence.rs index a84bf4c..6a4f5ed 100644 --- a/backend/sync_lib/src/operations/operation_sequence.rs +++ b/backend/sync_lib/src/operations/operation_sequence.rs @@ -1,3 +1,5 @@ +use std::cmp::Ordering; + use super::Operation; use crate::errors::SyncLibError; use ropey::Rope; @@ -87,8 +89,8 @@ impl OperationSequence { let mut merged_operations = Vec::with_capacity(self.operations.len() + other.operations.len()); - let mut left_delete_context = MergeContext::default(); - let mut right_delete_context = MergeContext::default(); + let mut left_merge_context = MergeContext::default(); + let mut right_merge_context = MergeContext::default(); let mut left_index: usize = 0; let mut right_index: usize = 0; @@ -96,20 +98,47 @@ impl OperationSequence { loop { let left_op = self.operations.get(left_index); let right_op = other.operations.get(right_index); - println!(); - println!("{:#?} <> {:#?}", left_op.cloned(), right_op.cloned()); + // println!(); + // println!("{:#?} <> {:#?}", left_op.cloned(), right_op.cloned()); - println!("{:?} <> {:?}", left_delete_context, right_delete_context); + // println!("{:?} <> {:?}", left_merge_context, right_merge_context); - match (left_op, right_op, left_op.cmp(&right_op)) { + let left_op_index = if let Some(delete) = left_merge_context.last_delete.as_ref() { + delete.end_index() + } else { + left_op + .map(|op| op.start_index() + right_merge_context.shift) + .unwrap_or_default() + }; + + let right_op_index = if let Some(delete) = right_merge_context.last_delete.as_ref() { + delete.end_index() + } else { + right_op + .map(|op| op.start_index() + left_merge_context.shift) + .unwrap_or_default() + }; + + let result = left_op_index.cmp(&right_op_index); + let order = if result == Ordering::Equal && left_op.is_some() && right_op.is_some() { + match (left_op.unwrap(), right_op.unwrap()) { + (Operation::Insert { .. }, Operation::Delete { .. }) => Ordering::Greater, + (Operation::Delete { .. }, Operation::Insert { .. }) => Ordering::Less, + _ => Ordering::Equal, + } + } else { + result + }; + + match (left_op, right_op, order) { (Some(left_op), None, _) | (Some(left_op), Some(_), std::cmp::Ordering::Less | std::cmp::Ordering::Equal) => { - println!("Left op: {:?}", left_op); + // println!("Left op: {:?}", left_op); if let Some(op) = Self::merge_operations_with_context( left_op, - &mut right_delete_context, - &mut left_delete_context, + &mut right_merge_context, + &mut left_merge_context, )? { merged_operations.push(op); } @@ -118,12 +147,12 @@ impl OperationSequence { } (None, Some(right_op), _) | (Some(_), Some(right_op), std::cmp::Ordering::Greater) => { - println!("Right op: {:?}", right_op); + // println!("Right op: {:?}", right_op); if let Some(op) = Self::merge_operations_with_context( right_op, - &mut left_delete_context, - &mut right_delete_context, + &mut left_merge_context, + &mut right_merge_context, )? { merged_operations.push(op); } @@ -135,8 +164,8 @@ impl OperationSequence { } }; - println!("last {:?}", merged_operations.last().unwrap()); - println!("{:?} <> {:?}", left_delete_context, right_delete_context); + // println!("last {:?}", merged_operations.last().unwrap()); + // println!("{:?} <> {:?}", left_merge_context, right_merge_context); } Ok(Self::new(merged_operations)) @@ -242,6 +271,9 @@ impl OperationSequence { #[cfg(test)] mod tests { + use std::{fs, path::Path}; + + use itertools::Itertools; use pretty_assertions::assert_eq; use super::*; @@ -337,7 +369,7 @@ mod tests { test_merge_both_ways( "this stays, this is one big delete, don't touch this", - "this stays, , don't touch this", + "this stays, don't touch this", "this stays, my one change, don't touch this", "this stays, my change, don't touch this", ); @@ -353,7 +385,7 @@ mod tests { test_merge_both_ways("hello world", "world !", "hi hello world", "hi world !"); - test_merge_both_ways("a b", "c d", "a b c d", "c b c d"); + test_merge_both_ways("a b", "c d", "a b c d", "c db c d"); test_merge_both_ways( "both delete the same word", @@ -366,7 +398,7 @@ mod tests { "both delete the same word but one a bit more", "both the same word", "both same word", - "both same word", + "both same wordword", ); test_merge_both_ways( @@ -377,31 +409,30 @@ mod tests { ); } - // #[test] + #[test] - // fn test_merge_files_without_panicing() { - // let files = vec![ - // "pride_and_prejudice.txt", - // "romeo_and_juliet.txt", - // "room_with_a_view.txt", - // ]; + fn test_merge_files_without_panicing() { + let files = vec![ + "pride_and_prejudice.txt", + "romeo_and_juliet.txt", + "room_with_a_view.txt", + ]; - // let root = Path::new("test/resources/"); - // println!("{:?}", root.canonicalize().unwrap()); - // let contents = files - // .into_iter() - // .map(|name| fs::read_to_string(root.join(name)).unwrap()) - // .map(|text| text.slice(0..10000).to_string()) - // .collect::>(); + let root = Path::new("test/resources/"); + let contents = files + .into_iter() + .map(|name| fs::read_to_string(root.join(name)).unwrap()) + .map(|text| text[0..50000].to_string()) + .collect::>(); - // contents - // .iter() - // .permutations(3) - // .unique() - // .for_each(|permutations| { - // test_merge(permutations[0], permutations[1], permutations[2]); - // }); - // } + contents + .iter() + .permutations(3) + .unique() + .for_each(|permutations| { + test_merge(permutations[0], permutations[1], permutations[2]); + }); + } fn test_merge_both_ways(original: &str, edit_1: &str, edit_2: &str, expected: &str) { assert_eq!(test_merge(original, edit_1, edit_2), expected);