From 2c4737999f43e012914be545693adfec6085d664 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Thu, 14 Nov 2024 22:30:16 +0000 Subject: [PATCH] more working --- backend/sync_lib/src/operations/operation.rs | 56 +++++++++---------- .../src/operations/operation_sequence.rs | 45 +++++++-------- 2 files changed, 48 insertions(+), 53 deletions(-) diff --git a/backend/sync_lib/src/operations/operation.rs b/backend/sync_lib/src/operations/operation.rs index a9084b9..e9217d5 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 similar::{Change, ChangeTag}; use std::cmp::Ordering; use std::fmt::Display; @@ -36,7 +35,7 @@ impl Display for Operation { } impl Operation { - pub fn create(tag: ChangeTag, index: i64, text: &str) -> Result { + pub fn create_insert(index: i64, text: &str) -> Result, SyncLibError> { if index < 0 { return Err(SyncLibError::NegativeOperationIndexError(format!( "Index {} is negative", @@ -44,29 +43,17 @@ impl Operation { ))); } - Ok(match tag { - ChangeTag::Insert => Operation::Insert { - index, - text: text.to_string(), - }, - ChangeTag::Delete => Operation::Delete { - index, - deleted_character_count: text.chars().count() as i64, - }, - _ => { - return Err(SyncLibError::OperationConversionError(format!( - "Cannot convert editing operation because {:?}", - tag - ))) - } - }) + if text.is_empty() { + return Ok(None); + } + + Ok(Some(Operation::Insert { + index, + text: text.to_string(), + })) } - pub fn create_insert(index: i64, text: &str) -> Result { - Self::create(ChangeTag::Insert, index, text) - } - - pub fn create_delete(index: i64, length: i64) -> Result { + pub fn create_delete(index: i64, length: i64) -> Result, SyncLibError> { if index < 0 { return Err(SyncLibError::NegativeOperationIndexError(format!( "Index {} is negative", @@ -81,16 +68,20 @@ impl Operation { ))); } - Ok(Operation::Delete { + if length == 0 { + return Ok(None); + } + + Ok(Some(Operation::Delete { index, deleted_character_count: length, - }) + })) } 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 { - Operation::Insert { text, .. } => rope_text.try_insert(index, &text).map_err(|err| { + Operation::Insert { text, .. } => rope_text.try_insert(index, text).map_err(|err| { SyncLibError::OperationApplicationError(format!("Failed to insert text: {}", err)) }), Operation::Delete { @@ -133,6 +124,10 @@ impl Operation { } } + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + /// Returns the range of indices of characters that the operation affects, inclusive. pub fn range(&self) -> std::ops::RangeInclusive { self.start_index()..=self.end_index() @@ -144,6 +139,7 @@ impl Operation { index, text: text.clone(), }, + Operation::Delete { deleted_character_count, .. @@ -177,7 +173,7 @@ impl Ord for Operation { impl PartialOrd for Operation { fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(&other)) + Some(self.cmp(other)) } } @@ -187,8 +183,6 @@ mod tests { #[test] fn test_creation_errors() { - insta::assert_debug_snapshot!(Operation::create(ChangeTag::Insert, -1, "hi")); - insta::assert_debug_snapshot!(Operation::create(ChangeTag::Equal, 0, "hi")); insta::assert_debug_snapshot!(Operation::create_insert(-1, "hi")); insta::assert_debug_snapshot!(Operation::create_delete(0, -1)); insta::assert_debug_snapshot!(Operation::create_delete(-1, -1)); @@ -212,7 +206,7 @@ mod tests { #[test] fn test_apply_delete_with_create() -> Result<(), SyncLibError> { let mut rope = Rope::from_str("hello world"); - let operation = Operation::create(ChangeTag::Delete, 6, "world")?; + let operation = Operation::create_delete(5, 6)?.unwrap(); operation.apply(&mut rope)?; @@ -239,7 +233,7 @@ mod tests { #[test] fn test_apply_insert_with_create() -> Result<(), SyncLibError> { let mut rope = Rope::from_str("hello"); - let operation = Operation::create(ChangeTag::Insert, 5, " my friend")?; + let operation = Operation::create_insert(5, " my friend")?.unwrap(); operation.apply(&mut rope)?; diff --git a/backend/sync_lib/src/operations/operation_sequence.rs b/backend/sync_lib/src/operations/operation_sequence.rs index a1ee9ed..7da472e 100644 --- a/backend/sync_lib/src/operations/operation_sequence.rs +++ b/backend/sync_lib/src/operations/operation_sequence.rs @@ -1,11 +1,9 @@ -use super::{operation, Operation}; +use super::Operation; use crate::errors::SyncLibError; -use log::info; use ropey::Rope; -use serde::{de, Deserialize, Serialize}; -use similar::utils::diff_graphemes; +use serde::{Deserialize, Serialize}; +use similar::Algorithm; use similar::{utils::TextDiffRemapper, ChangeTag, TextDiff}; -use similar::{Algorithm, DiffableStrRef}; #[derive(Debug, Clone, Default)] struct MergeContext { @@ -43,7 +41,7 @@ impl OperationSequence { if diff_ratio > diff_ratio_threshold { return Err(SyncLibError::DiffTooLarge { diff_ratio, - diff_ratio_limit: diff_ratio_threshold as f32, + diff_ratio_limit: diff_ratio_threshold, }); } @@ -56,16 +54,18 @@ impl OperationSequence { .map(|(tag, text)| match tag { ChangeTag::Equal => { index += text.chars().count(); - None + Ok(None) } ChangeTag::Insert => { - let result = Some(Operation::create(tag, index as i64, text)); + let result = Operation::create_insert(index as i64, text); index += text.chars().count(); result } - ChangeTag::Delete => Some(Operation::create(tag, index as i64, text)), + ChangeTag::Delete => { + Operation::create_delete(index as i64, text.chars().count() as i64) + } }) - .flat_map(Option::into_iter) + .flat_map(|result| result.transpose().into_iter()) .collect::, SyncLibError>>() .map(Self::new) } @@ -93,7 +93,7 @@ impl OperationSequence { loop { let left_op = self.operations.get(left_index); let right_op = other.operations.get(right_index); - println!(""); + println!(); println!("{:#?} <> {:#?}", left_op.cloned(), right_op.cloned()); println!("{:?} <> {:?}", left_delete_context, right_delete_context); @@ -161,12 +161,12 @@ impl OperationSequence { if last_delete .range() - .contains(&(&operation.start_index() + affecting_context.shift)) + .contains(&(operation.start_index() + affecting_context.shift)) { - affecting_context.last_delete = Some(Operation::create_delete( + affecting_context.last_delete = Operation::create_delete( last_delete.start_index() + operation.len(), 0.max(last_delete.len() - operation.len()), - )?); + )?; Some(operation.with_index(last_delete.start_index())) } else { @@ -188,15 +188,15 @@ impl OperationSequence { { affecting_context.shift -= shifted_operation.start_index() - last_delete.start_index(); - affecting_context.last_delete = Some(Operation::create_delete( - shifted_operation.end_index() + 1, - last_delete.end_index() - shifted_operation.end_index(), - )?); + affecting_context.last_delete = Operation::create_delete( + shifted_operation.start_index(), + last_delete.len() - shifted_operation.len(), + )?; None } else if last_delete .range() - .contains(&shifted_operation.start_index()) + .contains(&(operation.start_index() + affecting_context.shift)) { let overlap = last_delete.end_index() - (operation.start_index() + affecting_context.shift) @@ -204,10 +204,10 @@ impl OperationSequence { affecting_context.last_delete = None; affecting_context.shift -= last_delete.len() - overlap; - let operation = Some(Operation::create_delete( + let operation = Operation::create_delete( last_delete.start_index(), operation.len() - overlap, - )?); + )?; Self::replace_delete_in_produced_context(produced_context, operation.clone()); operation } else { @@ -246,7 +246,6 @@ impl OperationSequence { #[cfg(test)] mod tests { - use crate::operations::test; use super::*; @@ -296,6 +295,8 @@ mod tests { #[test] fn test_merges() { + test_merge_both_ways("a b c d e", "a e", "a c e", "a e"); + test_merge_both_ways( "hello world", "hi, world",