DOn't use start_index in EditedText

This commit is contained in:
Andras Schmelczer 2025-06-22 09:24:36 +01:00
parent 2089cbc7aa
commit d58b474669
No known key found for this signature in database
GPG key ID: FC8F2C3D3D1A718C
4 changed files with 61 additions and 53 deletions

View file

@ -120,18 +120,18 @@ where
let mut merged_length: usize = 0;
loop {
let (side, OrderedOperation { operation, order }) =
let (side, OrderedOperation { operation, order }, maybe_other_operation) =
match (maybe_left_op.clone(), maybe_right_op.clone()) {
(Some(left_op), Some(right_op)) => {
if left_op < right_op {
(Side::Left, left_op)
(Side::Left, left_op, maybe_right_op.clone())
} else {
(Side::Right, right_op)
(Side::Right, right_op, maybe_left_op.clone())
}
}
(Some(left_op), None) => (Side::Left, left_op),
(None, Some(right_op)) => (Side::Right, right_op),
(Some(left_op), None) => (Side::Left, left_op, None),
(None, Some(right_op)) => (Side::Right, right_op, None),
(None, None) => break,
};
@ -145,24 +145,25 @@ where
let result = match side {
Side::Left => {
let result = operation.merge_operations_with_context(
order,
&mut right_merge_context,
&mut left_merge_context,
maybe_other_operation,
);
if let Some(ref op @ (Operation::Insert { .. } | Operation::Equal { .. })) =
result
{
println!("merged_length {merged_length}, idx {}", op.start_index());
// assert_eq!(op.start_index() as i64, merged_length as i64);
let shift = op.start_index() as i64 - seen_left_length as i64
+ op.len() as i64
- original_length;
let mut shift = merged_length as i64 - seen_left_length as i64;
if !matches!(op, Operation::Equal { .. }) {
shift += op.len() as i64 - original_length;
}
while let Some(cursor) = left_cursors.next_if(|cursor| {
cursor.char_index <= seen_left_length + original_length as usize
}) {
merged_cursors.push(cursor.with_index(
(op.start_index() as i64).max(cursor.char_index as i64 + shift)
(merged_length as i64).max(cursor.char_index as i64 + shift)
as usize,
));
}
@ -178,25 +179,25 @@ where
}
Side::Right => {
let result = operation.merge_operations_with_context(
order,
&mut left_merge_context,
&mut right_merge_context,
maybe_other_operation,
);
if let Some(ref op @ (Operation::Insert { .. } | Operation::Equal { .. })) =
result
{
println!("merged_length {merged_length}, idx {}", op.start_index());
// assert_eq!(op.start_index() as i64, merged_length as i64);
let shift = op.start_index() as i64 - seen_right_length as i64
+ op.len() as i64
- original_length;
let mut shift = merged_length as i64 - seen_right_length as i64;
if !matches!(op, Operation::Equal { .. }) {
shift += op.len() as i64 - original_length;
}
while let Some(cursor) = right_cursors.next_if(|cursor| {
cursor.char_index <= seen_right_length + original_length as usize
}) {
merged_cursors.push(cursor.with_index(
(op.start_index() as i64).max(cursor.char_index as i64 + shift)
(merged_length as i64).max(cursor.char_index as i64 + shift)
as usize,
));
}
@ -215,15 +216,8 @@ where
println!(" = {result:?}");
if let Some(operation) = result {
let last_operation = merged_operations.last();
if let Some(last_operation) = last_operation {
if matches!(last_operation.operation, Operation::Equal { .. })
&& matches!(operation, Operation::Equal { .. })
&& last_operation.operation.eq(&operation)
{
println!("skipping equal");
continue;
}
if operation.len() == 0 {
continue;
}
if is_advancing_operation {

View file

@ -59,13 +59,13 @@ where
..
},
) => {
if threshold_index + self.shift > op.end_index() as i64 {
if threshold_index + self.shift >= op.end_index() as i64 {
self.shift -= *deleted_character_count as i64;
self.last_operation = None;
}
}
Some(op @ Operation::Insert { .. }) | Some(op @ Operation::Equal { .. }) => {
if threshold_index + self.shift - op.len() as i64 > op.end_index() as i64 {
if threshold_index + self.shift - op.len() as i64 >= op.end_index() as i64 {
self.last_operation = None;
}
}

View file

@ -6,11 +6,12 @@ use serde::{Deserialize, Serialize};
use super::merge_context::MergeContext;
use crate::{
Token,
operation_transformation::ordered_operation::OrderedOperation,
utils::{
find_longest_prefix_contained_within::find_longest_prefix_contained_within,
string_builder::StringBuilder,
},
Token,
};
/// Represents a change that can be applied on a `StringBuilder`.
@ -84,10 +85,6 @@ where
/// This operation is used to indicate that the text at the given index
/// is unchanged.
pub fn create_equal(index: usize, length: usize) -> Option<Self> {
if length == 0 {
return None;
}
Some(Operation::Equal {
index,
length,
@ -213,18 +210,12 @@ where
}
}
/// Returns the index of the last character that the operation affects.
pub fn end_index(&self) -> usize {
debug_assert!(
self.len() > 0,
" len() must be greater than 0 because operations must be non-empty"
);
self.start_index() + self.len() - 1
}
/// Returns the first index after the last character that the operation
/// affects.
pub fn end_index(&self) -> usize { self.start_index() + self.len() }
/// Returns the range of indices of characters that the operation affects.
#[allow(clippy::range_plus_one)]
pub fn range(&self) -> Range<usize> { self.start_index()..self.end_index() + 1 }
pub fn range(&self) -> Range<usize> { self.start_index()..self.end_index() }
/// Returns the number of affected characters. It is always greater than 0
/// because empty operations cannot be created.
@ -294,8 +285,10 @@ where
#[allow(clippy::too_many_lines)]
pub fn merge_operations_with_context(
self,
order: usize,
affecting_context: &mut MergeContext<T>,
produced_context: &mut MergeContext<T>,
other_operation: Option<OrderedOperation<T>>,
) -> Option<Operation<T>> {
affecting_context.consume_last_operation_if_it_is_too_behind(self.start_index() as i64);
let operation = self.with_shifted_index(affecting_context.shift);
@ -362,7 +355,7 @@ where
let moved_operation = operation.with_index(last_delete.start_index());
affecting_context.replace_last_operation(Operation::create_delete(
moved_operation.end_index() + 1,
moved_operation.end_index(),
(last_delete.len() as i64 - difference) as usize,
));
affecting_context.shift -= difference;
@ -415,19 +408,19 @@ where
);
let overlap = (length as i64)
.min(last_delete.end_index() as i64 - operation.start_index() as i64 + 1);
.min(last_delete.end_index() as i64 - operation.start_index() as i64);
#[cfg(debug_assertions)]
let result = text.as_ref().map_or_else(
|| {
Operation::create_equal(
operation.end_index().min(last_delete.end_index()),
operation.end_index().min(last_delete.end_index()) - 1,
(length as i64 - overlap) as usize,
)
},
|text| {
Operation::create_equal_with_text(
operation.end_index().min(last_delete.end_index()),
operation.end_index().min(last_delete.end_index()) - 1,
text.chars().skip(overlap as usize).collect::<String>(),
)
},
@ -441,7 +434,26 @@ where
result
}
(operation @ Operation::Equal { .. }, _) => Some(operation),
(operation @ Operation::Equal { .. }, _) => {
let result = 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(operation.start_index(), 0)
} else {
Some(operation)
}
} else {
Some(operation)
};
produced_context.consume_and_replace_last_operation(result.clone());
result
}
}
}
}
@ -531,9 +543,11 @@ mod tests {
#[test]
#[should_panic(expected = "Shifted index must be non-negative")]
fn test_shifting_error() {
insta::assert_debug_snapshot!(Operation::create_insert(1, vec!["hi".into()])
.unwrap()
.with_shifted_index(-2));
insta::assert_debug_snapshot!(
Operation::create_insert(1, vec!["hi".into()])
.unwrap()
.with_shifted_index(-2)
);
}
#[test]

View file

@ -1,7 +1,7 @@
#[cfg(feature = "serde")]
use serde::{Deserialize, Serialize};
use crate::{operation_transformation::Operation, Token};
use crate::{Token, operation_transformation::Operation};
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[derive(Debug, Clone, PartialEq)]