Refactor & simplify merging logic #3

Merged
schmelczer merged 15 commits from asch/simplify into main 2025-06-22 15:07:04 +01:00
8 changed files with 157 additions and 175 deletions
Showing only changes of commit 5759b6bf66 - Show all commits

View file

@ -110,8 +110,8 @@ mod test {
}, // inside of "s|ample" because "text" got replaced by "sample" }, // inside of "s|ample" because "text" got replaced by "sample"
CursorPosition { CursorPosition {
id: 3, id: 3,
char_index: 42 char_index: 30
}, // before "cursor movements" }, // after "complex sample"
] ]
) )
); );

View file

@ -161,41 +161,47 @@ where
let original_length = operation.len() as i64; let original_length = operation.len() as i64;
let result = match side { let result = match side {
Side::Left => { Side::Left => {
let result = operation.merge_operations_with_context( let result = operation.merge_operations_with_context(order, &mut last_other_op);
order,
&mut last_other_op,
maybe_other_operation,
);
if let Some( if let ref op @ (OrderedOperation {
ref op @ (OrderedOperation { operation: Operation::Insert { .. },
operation: Operation::Insert { .. }, ..
.. }
} | OrderedOperation {
| OrderedOperation { operation: Operation::Equal { .. },
operation: Operation::Equal { .. }, ..
.. }) = result
}),
) = result
{ {
let mut shift = merged_length as i64 - seen_left_length as i64; println!(
if !matches!( "merrged_length: {}, seen_left_length: {}, op len {}, original_length \
op, {}",
OrderedOperation { merged_length,
operation: Operation::Equal { .. }, seen_left_length,
.. op.operation.len(),
} original_length
) { );
shift += op.operation.len() as i64 - original_length; let mut shift = merged_length as i64
} // - last_other_op
// .map(|op| op.operation.len() as i64)
// .unwrap_or(0) as i64
- seen_left_length as i64;
// if !matches!(
// op,
// OrderedOperation {
// operation: Operation::Equal { .. },
// ..
// }
// ) {
shift += op.operation.len() as i64 - original_length;
// }
while let Some(cursor) = left_cursors.next_if(|cursor| { while let Some(cursor) = left_cursors.next_if(|cursor| {
cursor.char_index <= seen_left_length + original_length as usize cursor.char_index <= seen_left_length + original_length as usize
}) { }) {
merged_cursors.push(cursor.with_index( println!("cursor {}", cursor.char_index);
(merged_length as i64).max(cursor.char_index as i64 + shift) merged_cursors.push(
as usize, cursor.with_index((cursor.char_index as i64 + shift) as usize),
)); );
} }
} }
@ -204,46 +210,53 @@ where
} }
maybe_left_op = left_iter.next(); maybe_left_op = left_iter.next();
last_left_op = result.clone(); last_left_op = Some(result.clone());
result result
} }
Side::Right => { Side::Right => {
let result = operation.merge_operations_with_context( let result = operation.merge_operations_with_context(order, &mut last_other_op);
order,
&mut last_other_op,
maybe_other_operation,
);
if let Some( if let ref op @ (OrderedOperation {
ref op @ (OrderedOperation { operation: Operation::Insert { .. },
operation: Operation::Insert { .. }, ..
.. }
} | OrderedOperation {
| OrderedOperation { operation: Operation::Equal { .. },
operation: Operation::Equal { .. }, ..
.. }) = result
}),
) = result
{ {
let mut shift = merged_length as i64 - seen_right_length as i64; println!(
if !matches!( "merrged_length: {}, seen_left_length: {}, op len {}, original_length \
op, {}",
OrderedOperation { merged_length,
operation: Operation::Equal { .. }, seen_left_length,
.. op.operation.len(),
} original_length
) { );
shift += op.operation.len() as i64 - original_length; let mut shift = merged_length as i64
} // - last_other_op
// .map(|op| op.operation.len() as i64)
// .unwrap_or(0) as i64
- seen_right_length as i64;
// if !matches!(
// op,
// OrderedOperation {
// operation: Operation::Equal { .. },
// ..
// }
// ) {
shift += op.operation.len() as i64 - original_length;
// }
while let Some(cursor) = right_cursors.next_if(|cursor| { while let Some(cursor) = right_cursors.next_if(|cursor| {
cursor.char_index <= seen_right_length + original_length as usize cursor.char_index <= seen_right_length + original_length as usize
}) { }) {
merged_cursors.push(cursor.with_index( println!("cursor {}", cursor.char_index);
(merged_length as i64).max(cursor.char_index as i64 + shift)
as usize, merged_cursors.push(
)); cursor.with_index((cursor.char_index as i64 + shift) as usize),
);
} }
} }
@ -252,7 +265,7 @@ where
} }
maybe_right_op = right_iter.next(); maybe_right_op = right_iter.next();
last_right_op = result.clone(); last_right_op = Some(result.clone());
result result
} }
@ -260,17 +273,15 @@ where
println!(" = {result:?}"); println!(" = {result:?}");
if let Some(operation) = result { if result.operation.len() == 0 {
if operation.operation.len() == 0 { continue;
continue;
}
if is_advancing_operation {
merged_length += operation.operation.len();
}
merged_operations.push(operation);
} }
if is_advancing_operation {
merged_length += result.operation.len();
}
merged_operations.push(result);
} }
for cursor in left_cursors.chain(right_cursors) { for cursor in left_cursors.chain(right_cursors) {

View file

@ -46,66 +46,45 @@ where
/// Creates an equal operation with the given index. /// Creates an equal operation with the given index.
/// This operation is used to indicate that the text at the given index /// This operation is used to indicate that the text at the given index
/// is unchanged. /// is unchanged.
pub fn create_equal(length: usize) -> Option<Self> { pub fn create_equal(length: usize) -> Self {
Some(Operation::Equal { Operation::Equal {
length, length,
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
text: None, text: None,
}) }
} }
pub fn create_equal_with_text(text: String) -> Option<Self> { pub fn create_equal_with_text(text: String) -> Self {
if text.is_empty() { Operation::Equal {
return None;
}
Some(Operation::Equal {
length: text.chars().count(), length: text.chars().count(),
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
text: Some(text), text: Some(text),
}) }
} }
/// Creates an insert operation with the given index and text. /// Creates an insert operation with the given index and text.
/// If the text is empty (meaning that the operation would be a no-op), pub fn create_insert(text: Vec<Token<T>>) -> Self { Operation::Insert { text } }
/// returns None.
pub fn create_insert(text: Vec<Token<T>>) -> Option<Self> {
if text.is_empty() {
return None;
}
Some(Operation::Insert { text })
}
/// Creates a delete operation with the given index and number of /// Creates a delete operation with the given index and number of
/// to-be-deleted characters. If the operation would delete 0 (meaning /// to-be-deleted characters.
/// that the operation would be a no-op), returns None. pub fn create_delete(deleted_character_count: usize) -> Self {
pub fn create_delete(deleted_character_count: usize) -> Option<Self> { Operation::Delete {
if deleted_character_count == 0 {
return None;
}
Some(Operation::Delete {
deleted_character_count, deleted_character_count,
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
deleted_text: None, deleted_text: None,
}) }
} }
pub fn create_delete_with_text(text: String) -> Option<Self> { pub fn create_delete_with_text(text: String) -> Self {
if text.is_empty() { Operation::Delete {
return None;
}
Some(Operation::Delete {
deleted_character_count: text.chars().count(), deleted_character_count: text.chars().count(),
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
deleted_text: Some(text), deleted_text: Some(text),
}) }
} }
/// Applies the operation to the given `StringBuilder`, returning the /// Applies the operation to the given `StringBuilder`, returning the
@ -181,12 +160,8 @@ where
self, self,
order: usize, order: usize,
previous_operation: &mut Option<OrderedOperation<T>>, previous_operation: &mut Option<OrderedOperation<T>>,
other_operation: Option<OrderedOperation<T>>, ) -> OrderedOperation<T> {
) -> Option<OrderedOperation<T>> { println!("mergin: {self} (order {order}) - previous: {previous_operation:?}");
println!(
"mergin: {self} (order {order}) - previous: {previous_operation:?} - other: \
{other_operation:?}"
);
let operation = self; let operation = self;
match (operation, previous_operation) { match (operation, previous_operation) {
@ -209,7 +184,10 @@ where
let trimmed_operation = Operation::create_insert(text[offset_in_tokens..].to_vec()); let trimmed_operation = Operation::create_insert(text[offset_in_tokens..].to_vec());
trimmed_operation.map(|operation| OrderedOperation { order, operation }) OrderedOperation {
order,
operation: trimmed_operation,
}
} }
( (
@ -248,10 +226,10 @@ where
#[cfg(not(debug_assertions))] #[cfg(not(debug_assertions))]
let updated_delete = Operation::create_delete(new_length); let updated_delete = Operation::create_delete(new_length);
updated_delete.map(|operation| OrderedOperation { OrderedOperation {
order: order + overlap, order: order + overlap,
operation, operation: updated_delete,
}) }
} }
( (
@ -268,19 +246,6 @@ where
}, },
), ),
) => { ) => {
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");
return Some(OrderedOperation {
order,
operation: Operation::create_equal(0).unwrap(),
});
}
}
let last_delete_end_index = last_delete.order + last_delete.operation.len(); let last_delete_end_index = last_delete.order + last_delete.operation.len();
let overlap = let overlap =
@ -299,30 +264,32 @@ where
#[cfg(not(debug_assertions))] #[cfg(not(debug_assertions))]
let updated_equal = Operation::create_equal((length as i64 - overlap) as usize); let updated_equal = Operation::create_equal((length as i64 - overlap) as usize);
updated_equal.map(|operation| OrderedOperation { OrderedOperation {
order: order + overlap as usize, order: order + overlap as usize,
operation, operation: updated_equal,
})
}
(operation @ Operation::Equal { .. }, _) => {
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(0)
} else {
Some(operation)
}
} else {
Some(operation)
} }
.map(|operation| OrderedOperation { order, operation })
} }
(operation, _) => Some(OrderedOperation { order, operation }), (
operation @ Operation::Equal { .. },
Some(
last_equal @ OrderedOperation {
operation: Operation::Equal { .. },
..
},
),
) => OrderedOperation {
order,
operation: if operation.len() == last_equal.operation.len()
&& order == last_equal.order
{
Operation::create_equal(0)
} else {
operation
},
},
(operation, _) => OrderedOperation { order, operation },
} }
} }
} }
@ -404,9 +371,8 @@ mod tests {
#[test] #[test]
fn test_apply_delete_with_create() { fn test_apply_delete_with_create() {
let builder = StringBuilder::new("hello world"); let builder = StringBuilder::new("hello world");
let delete_operation = let delete_operation = Operation::<()>::create_delete_with_text("hello ".to_owned());
Operation::<()>::create_delete_with_text("hello ".to_owned()).unwrap(); let retain_operation = Operation::<()>::create_equal(5);
let retain_operation = Operation::<()>::create_equal(5).unwrap();
let mut builder = delete_operation.apply(builder); let mut builder = delete_operation.apply(builder);
builder = retain_operation.apply(builder); builder = retain_operation.apply(builder);
@ -418,8 +384,8 @@ mod tests {
fn test_apply_insert() { fn test_apply_insert() {
let builder = StringBuilder::new("hello"); let builder = StringBuilder::new("hello");
let retain_operation = Operation::<()>::create_equal(5).unwrap(); let retain_operation = Operation::<()>::create_equal(5);
let insert_operation = Operation::create_insert(vec![" my friend".into()]).unwrap(); let insert_operation = Operation::create_insert(vec![" my friend".into()]);
let mut builder = retain_operation.apply(builder); let mut builder = retain_operation.apply(builder);
builder = insert_operation.apply(builder); builder = insert_operation.apply(builder);

View file

@ -11,31 +11,37 @@ where
{ {
let mut order = 0; // this is the start index of the operation on the original text let mut order = 0; // this is the start index of the operation on the original text
raw_operations.into_iter().filter_map(move |raw_operation| { raw_operations.into_iter().map(move |raw_operation| {
let length = raw_operation.original_text_length(); let length = raw_operation.original_text_length();
match raw_operation { match raw_operation {
RawOperation::Equal(..) => { RawOperation::Equal(..) => {
let op = if cfg!(debug_assertions) { let op = OrderedOperation {
Operation::create_equal_with_text(raw_operation.get_original_text()) order,
} else { operation: if cfg!(debug_assertions) {
Operation::create_equal(length) Operation::create_equal_with_text(raw_operation.get_original_text())
} } else {
.map(|operation| OrderedOperation { order, operation }); Operation::create_equal(length)
},
};
order += length; order += length;
op op
} }
RawOperation::Insert(tokens) => Operation::create_insert(tokens) RawOperation::Insert(tokens) => OrderedOperation {
.map(|operation| OrderedOperation { order, operation }), order,
operation: Operation::create_insert(tokens),
},
RawOperation::Delete(..) => { RawOperation::Delete(..) => {
let op = if cfg!(debug_assertions) { let op = OrderedOperation {
Operation::create_delete_with_text(raw_operation.get_original_text()) order,
} else { operation: if cfg!(debug_assertions) {
Operation::create_delete(length) Operation::create_delete_with_text(raw_operation.get_original_text())
} } else {
.map(|operation| OrderedOperation { order, operation }); Operation::create_delete(length)
},
};
order += length; order += length;

View file

@ -1,4 +1,3 @@
use core::ops::Range;
use std::iter::Iterator; use std::iter::Iterator;
/// A helper for building a string in-order based on an original string and a /// A helper for building a string in-order based on an original string and a

View file

@ -28,7 +28,7 @@ expected: long small
parent: long run of text where one barely has no changes but has cursors parent: long run of text where one barely has no changes but has cursors
left: long| run of tex|t where one barely has no |changes but has |cursors left: long| run of tex|t where one barely has no |changes but has |cursors
right: long run one barely has no changes cursors right: long run one barely has no changes cursors
expected: long| run| one barely has no |changes |cursors expected: long| ru|n one barely has no |changes |cursors
--- ---
parent: long text where the cursor has to be clamped after delete parent: long text where the cursor has to be clamped after delete

View file

@ -2,7 +2,7 @@
parent: original_1 original_2 original_3 parent: original_1 original_2 original_3
left: original_1 edit_1| original_3 left: original_1 edit_1| original_3
right: original_1 original_2| edit_2 right: original_1 original_2| edit_2
expected: original_1 edit_1|| edit_2 expected: original_1| edit_1| edit_2
--- ---
# Both replace the same token with the same value # Both replace the same token with the same value

View file

@ -43,7 +43,7 @@ expected: Send the |detailed |quarterly |detailed report to the |entire |team
parent: Ready, Set go parent: Ready, Set go
left: Ready! Set go| left: Ready! Set go|
right: Ready, Set, go!| right: Ready, Set, go!|
expected: Ready! Set, go!|| expected: Ready!| Set, go!|
--- ---
parent: "Total: $100" parent: "Total: $100"
@ -67,7 +67,7 @@ expected: market| placemarket|space
parent: A B C D parent: A B C D
left: A X B D| left: A X B D|
right: A B Y| right: A B Y|
expected: A X B Y|| expected: A X B| Y|
--- ---
parent: Please submit your assignment by Friday parent: Please submit your assignment by Friday