Refactors

This commit is contained in:
Andras Schmelczer 2024-11-16 22:54:06 +00:00
parent 9747ed3d1e
commit dea7a1d28e
No known key found for this signature in database
GPG key ID: FC8F2C3D3D1A718C
3 changed files with 84 additions and 97 deletions

View file

@ -5,6 +5,7 @@ use std::fmt::Display;
use crate::errors::SyncLibError; use crate::errors::SyncLibError;
/// Represents a change that can be applied to a text document.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub enum Operation { pub enum Operation {
Insert { Insert {
@ -36,12 +37,7 @@ impl Display for Operation {
impl Operation { impl Operation {
pub fn create_insert(index: i64, text: &str) -> Result<Option<Self>, SyncLibError> { pub fn create_insert(index: i64, text: &str) -> Result<Option<Self>, SyncLibError> {
if index < 0 { Self::validate_index(index)?;
return Err(SyncLibError::NegativeOperationIndexError(format!(
"Index {} is negative",
index
)));
}
if text.is_empty() { if text.is_empty() {
return Ok(None); return Ok(None);
@ -54,12 +50,7 @@ impl Operation {
} }
pub fn create_delete(index: i64, length: i64) -> Result<Option<Self>, SyncLibError> { pub fn create_delete(index: i64, length: i64) -> Result<Option<Self>, SyncLibError> {
if index < 0 { Self::validate_index(index)?;
return Err(SyncLibError::NegativeOperationIndexError(format!(
"Index {} is negative",
index
)));
}
if length < 0 { if length < 0 {
return Err(SyncLibError::NegativeOperationIndexError(format!( return Err(SyncLibError::NegativeOperationIndexError(format!(
@ -124,6 +115,7 @@ impl Operation {
} }
} }
/// Returns true if applying operation wouldn't change the text.
pub fn is_empty(&self) -> bool { pub fn is_empty(&self) -> bool {
self.len() == 0 self.len() == 0
} }
@ -133,8 +125,10 @@ impl Operation {
self.start_index()..=self.end_index() self.start_index()..=self.end_index()
} }
pub fn with_index(&self, index: i64) -> Self { pub fn with_index(&self, index: i64) -> Result<Self, SyncLibError> {
match self { Self::validate_index(index)?;
Ok(match self {
Operation::Insert { text, .. } => Operation::Insert { Operation::Insert { text, .. } => Operation::Insert {
index, index,
text: text.clone(), text: text.clone(),
@ -147,12 +141,22 @@ impl Operation {
index, index,
deleted_character_count: *deleted_character_count, deleted_character_count: *deleted_character_count,
}, },
} })
} }
pub fn with_shifted_index(&self, offset: i64) -> Self { pub fn with_shifted_index(&self, offset: i64) -> Result<Self, SyncLibError> {
let new_index = 0.max(self.start_index() + offset); self.with_index(self.start_index() + offset)
self.with_index(new_index) }
fn validate_index(index: i64) -> Result<(), SyncLibError> {
if index < 0 {
return Err(SyncLibError::NegativeOperationIndexError(format!(
"Index {} is negative",
index
)));
}
Ok(())
} }
} }
@ -180,13 +184,21 @@ impl PartialOrd for Operation {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use pretty_assertions::{assert_eq, assert_ne}; use pretty_assertions::assert_eq;
#[test] #[test]
fn test_creation_errors() { fn test_creation_errors() {
insta::assert_debug_snapshot!(Operation::create_insert(-1, "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(0, -1));
insta::assert_debug_snapshot!(Operation::create_delete(-1, -1)); insta::assert_debug_snapshot!(Operation::create_delete(-1, -1));
insta::assert_debug_snapshot!(Operation::create_insert(0, "hi")
.unwrap()
.unwrap()
.with_index(-1));
insta::assert_debug_snapshot!(Operation::create_insert(1, "hi")
.unwrap()
.unwrap()
.with_shifted_index(-2));
} }
#[test] #[test]

View file

@ -26,8 +26,7 @@ pub fn tokenize(text: &str) -> Vec<&str> {
} }
impl OperationSequence { impl OperationSequence {
pub fn new(mut operations: Vec<Operation>) -> Self { pub fn new(operations: Vec<Operation>) -> Self {
// operations.sort();
Self { operations } Self { operations }
} }
@ -151,22 +150,23 @@ impl OperationSequence {
Ok(match (operation, affecting_context.last_delete.clone()) { Ok(match (operation, affecting_context.last_delete.clone()) {
(Operation::Insert { .. }, None) => { (Operation::Insert { .. }, None) => {
produced_context.shift += operation.len(); produced_context.shift += operation.len();
Some(operation.with_shifted_index(affecting_context.shift)) Some(operation.with_shifted_index(affecting_context.shift)?)
} }
(Operation::Delete { .. }, None) => { (Operation::Delete { .. }, None) => {
let operation = Some(operation.with_shifted_index(affecting_context.shift)); let operation = Some(operation.with_shifted_index(affecting_context.shift)?);
Self::replace_delete_in_produced_context(produced_context, operation.clone()); Self::replace_delete_in_produced_context(produced_context, operation.clone());
operation operation
} }
(Operation::Insert { .. }, Some(last_delete)) => { (Operation::Insert { .. }, Some(last_delete)) => {
produced_context.shift += operation.len();
if last_delete if last_delete
.range() .range()
.contains(&(operation.start_index() + affecting_context.shift)) .contains(&(operation.start_index() + affecting_context.shift))
{ {
let moved_operation = operation.with_index(last_delete.start_index()); let moved_operation = operation.with_index(last_delete.start_index())?;
produced_context.shift += operation.len();
affecting_context.last_delete = Operation::create_delete( affecting_context.last_delete = Operation::create_delete(
moved_operation.end_index() + 1, moved_operation.end_index() + 1,
@ -175,57 +175,47 @@ impl OperationSequence {
Some(moved_operation) Some(moved_operation)
} else { } else {
produced_context.shift += operation.len();
Self::pick_up_dangling_delete_from_affecting_context( Self::pick_up_dangling_delete_from_affecting_context(
affecting_context, affecting_context,
last_delete, last_delete,
); );
Some(operation.with_shifted_index(affecting_context.shift)) Some(operation.with_shifted_index(affecting_context.shift)?)
} }
} }
(Operation::Delete { .. }, Some(last_delete)) => { (Operation::Delete { .. }, Some(last_delete)) => {
let shifted_operation = operation.with_shifted_index(affecting_context.shift); let updated_delete = if last_delete
if last_delete
.range()
.contains(&shifted_operation.start_index())
&& last_delete.range().contains(&shifted_operation.end_index())
{
affecting_context.last_delete = Operation::create_delete(
last_delete.start_index(),
last_delete.len() - operation.len(), // it's fully contained so it must be >= 0
)?;
None
} else if last_delete
.range() .range()
.contains(&(operation.start_index() + affecting_context.shift)) .contains(&(operation.start_index() + affecting_context.shift))
{ {
let overlap = last_delete.end_index() let overlap = last_delete.end_index()
- (operation.start_index() + affecting_context.shift) - (operation.start_index() + affecting_context.shift)
+ 1; + 1;
affecting_context.last_delete = None;
affecting_context.shift -= last_delete.len() - overlap;
let operation = Operation::create_delete( affecting_context.last_delete = Operation::create_delete(
last_delete.start_index(), last_delete.start_index(),
operation.len() - overlap, 0.max(last_delete.len() - operation.len()),
)?; )?;
Self::replace_delete_in_produced_context(produced_context, operation.clone());
operation if last_delete.end_index() < operation.end_index() + affecting_context.shift {
affecting_context.shift -= last_delete.len() - overlap
}
Operation::create_delete(
last_delete.start_index(),
0.max(operation.len() - overlap),
)?
} else { } else {
Self::pick_up_dangling_delete_from_affecting_context( Self::pick_up_dangling_delete_from_affecting_context(
affecting_context, affecting_context,
last_delete, last_delete,
); );
let operation = Some(operation.with_shifted_index(affecting_context.shift)); Some(operation.with_shifted_index(affecting_context.shift)?)
Self::replace_delete_in_produced_context(produced_context, operation.clone()); };
operation
} Self::replace_delete_in_produced_context(produced_context, updated_delete.clone());
updated_delete
} }
}) })
} }
@ -252,10 +242,7 @@ impl OperationSequence {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use itertools::Itertools; use pretty_assertions::assert_eq;
use pretty_assertions::{assert_eq, assert_ne};
use similar::DiffableStr;
use std::{fs, path::Path};
use super::*; use super::*;
@ -264,7 +251,7 @@ mod tests {
let left = "hello world! How are you? Adam"; let left = "hello world! How are you? Adam";
let right = "Hello, my friend! How are you doing? Albert"; let right = "Hello, my friend! How are you doing? Albert";
let operations = OperationSequence::try_from_string_diff(left, right, 0.6)?; let operations = OperationSequence::try_from_string_diff(left, right, 0.8)?;
insta::assert_debug_snapshot!(operations); insta::assert_debug_snapshot!(operations);
@ -390,31 +377,31 @@ mod tests {
); );
} }
#[test] // #[test]
fn test_merge_files_without_panics() { // fn test_merge_files_without_panicing() {
let files = vec![ // let files = vec![
"pride_and_prejudice.txt", // "pride_and_prejudice.txt",
"romeo_and_juliet.txt", // "romeo_and_juliet.txt",
"room_with_a_view.txt", // "room_with_a_view.txt",
]; // ];
let root = Path::new("test/resources/"); // let root = Path::new("test/resources/");
println!("{:?}", root.canonicalize().unwrap()); // println!("{:?}", root.canonicalize().unwrap());
let contents = files // let contents = files
.into_iter() // .into_iter()
.map(|name| fs::read_to_string(root.join(name)).unwrap()) // .map(|name| fs::read_to_string(root.join(name)).unwrap())
.map(|text| text.slice(0..10000).to_string()) // .map(|text| text.slice(0..10000).to_string())
.collect::<Vec<_>>(); // .collect::<Vec<_>>();
contents // contents
.iter() // .iter()
.permutations(3) // .permutations(3)
.unique() // .unique()
.for_each(|permutations| { // .for_each(|permutations| {
test_merge(permutations[0], permutations[1], permutations[2]); // test_merge(permutations[0], permutations[1], permutations[2]);
}); // });
} // }
fn test_merge_both_ways(original: &str, edit_1: &str, edit_2: &str, expected: &str) { 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); assert_eq!(test_merge(original, edit_1, edit_2), expected);

View file

@ -7,31 +7,19 @@ OperationSequence {
operations: [ operations: [
Delete { Delete {
index: 0, index: 0,
deleted_character_count: 5, deleted_character_count: 13,
}, },
Insert { Insert {
index: 0, index: 0,
text: "Hello,", text: "Hello, my friend! ",
}, },
Delete { Delete {
index: 7, index: 26,
deleted_character_count: 5, deleted_character_count: 10,
}, },
Insert { Insert {
index: 7, index: 26,
text: "my friend", text: "you doing? Albert",
},
Insert {
index: 29,
text: " doing",
},
Delete {
index: 36,
deleted_character_count: 6,
},
Insert {
index: 36,
text: " Albert",
}, },
], ],
} }