From f0a35bfa31ceeb310fb81afc414a8179ae22b9dc Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Tue, 22 Sep 2026 21:25:58 +0100 Subject: [PATCH] Ensure reconcile(p, a, a) == a --- src/operation_transformation.rs | 3 + .../utils/elongate_operations.rs | 114 ++++++++++-------- src/utils/myers_diff.rs | 39 ++++-- tests/idempotence.rs | 81 +++++++++++++ 4 files changed, 179 insertions(+), 58 deletions(-) create mode 100644 tests/idempotence.rs diff --git a/src/operation_transformation.rs b/src/operation_transformation.rs index bbaf73e..c549b6c 100644 --- a/src/operation_transformation.rs +++ b/src/operation_transformation.rs @@ -14,6 +14,9 @@ use crate::{Tokenizer, types::text_with_cursors::TextWithCursors}; /// return a document containing all changes from both `left` /// and `right`. /// +/// If both updated texts are identical, the resulting text is exactly that +/// text, regardless of the original document. +/// /// If a span has been inserted in either the `left` or `right` /// versions, it will be present in the return value. If both sides /// insert the same span with a common prefix, that prefix will only diff --git a/src/operation_transformation/utils/elongate_operations.rs b/src/operation_transformation/utils/elongate_operations.rs index fd388be..2096bef 100644 --- a/src/operation_transformation/utils/elongate_operations.rs +++ b/src/operation_transformation/utils/elongate_operations.rs @@ -1,4 +1,3 @@ -use core::iter; use std::fmt::Debug; use crate::raw_operation::RawOperation; @@ -11,55 +10,40 @@ where I: IntoIterator>, T: PartialEq + Clone + Debug, { - // This might look bad, but this makes sense. The inserts and deltes can be - // interleaved, such as: IDIDID and we need to turn this into IIIDDD. - // So we need to keep track of both the last insert and delete operations, not - // just the last one. - let mut maybe_previous_insert: Option> = None; - let mut maybe_previous_delete: Option> = None; + let mut inserts: Vec> = Vec::new(); + let mut deletes: Vec> = Vec::new(); + let mut result = Vec::new(); - // We don't elongate `equals` as they're needed to maintain cursor positions - // when merging against deletes. - let mut result: Vec> = raw_operations - .into_iter() - .flat_map(|next| match next { - RawOperation::Insert(..) => match maybe_previous_insert.take() { - Some(prev) if prev.is_right_joinable() && next.is_left_joinable() => { - maybe_previous_insert = Some(prev.join(next)); - Box::new(iter::empty()) as Box>> - } - prev => { - maybe_previous_insert = Some(next); - Box::new(prev.into_iter()) - } - }, - RawOperation::Delete(..) => match maybe_previous_delete.take() { - Some(prev) if prev.is_right_joinable() && next.is_left_joinable() => { - maybe_previous_delete = Some(prev.join(next)); - Box::new(iter::empty()) as Box>> - } - prev => { - maybe_previous_delete = Some(next); - Box::new(prev.into_iter()) - } - }, - RawOperation::Equal(..) => Box::new( - maybe_previous_delete - .take() - .into_iter() - .chain(maybe_previous_insert.take()) - .chain(iter::once(next)), - ) as Box>>, - }) - .collect(); + // Emit all deletions before insertions within each changed span, even + // when tokens cannot be joined. Otherwise an insertion followed by a + // deletion at the same offset can separate identical insertions during + // merging and prevent their deduplication. + for next in raw_operations { + let pending = match next { + RawOperation::Insert(..) => &mut inserts, + RawOperation::Delete(..) => &mut deletes, + RawOperation::Equal(..) => { + result.append(&mut deletes); + result.append(&mut inserts); - if let Some(prev) = maybe_previous_delete { - result.push(prev); + // Keep retains separate for cursor positioning + result.push(next); + + continue; + } + }; + + match pending.pop() { + Some(prev) if prev.is_right_joinable() && next.is_left_joinable() => { + pending.push(prev.join(next)); + } + Some(prev) => pending.extend([prev, next]), + None => pending.push(next), + } } - if let Some(prev) = maybe_previous_insert { - result.push(prev); - } + result.append(&mut deletes); + result.append(&mut inserts); result } @@ -115,7 +99,7 @@ mod tests { #[test] fn merges_interleaved_insert_delete_sequences() { - // Pattern IDID -> II DD + // Pattern IDID -> DD II let ops = vec![ins(&["i1"]), del(&["d1"]), ins(&["i2"]), del(&["d2"])]; let result = elongate_operations(ops); @@ -123,4 +107,40 @@ mod tests { assert!(matches!(result[0], RawOperation::Delete(_))); assert!(matches!(result[1], RawOperation::Insert(_))); } + + #[test] + fn orders_non_joinable_edits_before_each_retain() { + let insert1 = ins_custom("a", false, false); + let insert2 = ins_custom("\n", false, false); + let delete1 = RawOperation::Delete(vec![Token::new( + "b".to_owned(), + "b".to_owned(), + false, + false, + )]); + let delete2 = del(&["c"]); + let retain = RawOperation::Equal(vec!["d".into()]); + let ops = vec![ + insert1.clone(), + delete1.clone(), + insert2.clone(), + delete2.clone(), + retain.clone(), + insert1.clone(), + delete1.clone(), + ]; + + assert_eq!( + elongate_operations(ops), + vec![ + delete1.clone(), + delete2, + insert1.clone(), + insert2, + retain, + delete1, + insert1, + ], + ); + } } diff --git a/src/utils/myers_diff.rs b/src/utils/myers_diff.rs index f04df56..0480b3b 100644 --- a/src/utils/myers_diff.rs +++ b/src/utils/myers_diff.rs @@ -306,10 +306,10 @@ fn conquer( // Check for common prefix let prefix_len = common_prefix_len(old, old_range.clone(), new, new_range.clone()); if prefix_len > 0 { - result.extend( - old[old_range.start..old_range.start + prefix_len] - .iter() - .map(|token| RawOperation::Equal(vec![token.clone()])), + emit_matches( + &old[old_range.start..old_range.start + prefix_len], + &new[new_range.start..new_range.start + prefix_len], + result, ); } old_range.start += prefix_len; @@ -317,9 +317,10 @@ fn conquer( // Check for common suffix let suffix_len = common_suffix_len(old, old_range.clone(), new, new_range.clone()); - let suffix_start = old_range.end - suffix_len; - old_range.end -= suffix_len; - new_range.end -= suffix_len; + let old_suffix_start = old_range.end - suffix_len; + let new_suffix_start = new_range.end - suffix_len; + old_range.end = old_suffix_start; + new_range.end = new_suffix_start; if old_range.is_empty() && new_range.is_empty() { // do nothing @@ -377,14 +378,30 @@ fn conquer( } if suffix_len > 0 { - result.extend( - old[suffix_start..suffix_start + suffix_len] - .iter() - .map(|token| RawOperation::Equal(vec![token.clone()])), + emit_matches( + &old[old_suffix_start..old_suffix_start + suffix_len], + &new[new_suffix_start..new_suffix_start + suffix_len], + result, ); } } +/// Normalized matches guide alignment, but only identical original text can +/// be retained. Emit replacements for matches with different spellings. +fn emit_matches(old: &[Token], new: &[Token], result: &mut Vec>) +where + T: PartialEq + Clone + Debug, +{ + for (old_token, new_token) in old.iter().zip(new) { + if old_token.original() == new_token.original() { + result.push(RawOperation::Equal(vec![old_token.clone()])); + } else { + result.push(RawOperation::Delete(vec![old_token.clone()])); + result.push(RawOperation::Insert(vec![new_token.clone()])); + } + } +} + #[cfg(test)] mod tests { use insta::assert_debug_snapshot; diff --git a/tests/idempotence.rs b/tests/idempotence.rs new file mode 100644 index 0000000..ca154f7 --- /dev/null +++ b/tests/idempotence.rs @@ -0,0 +1,81 @@ +use reconcile_text::{BuiltinTokenizer, Token, reconcile}; +use test_case::{test_case, test_matrix}; + +#[test_matrix([ + BuiltinTokenizer::Character, + BuiltinTokenizer::Line, + BuiltinTokenizer::Markdown, + BuiltinTokenizer::Word +])] +fn identical_edits_preserve_text(tokenizer: BuiltinTokenizer) { + let texts = [ + "", + "a", + "b\n", + "\r\n\r\n", + "# Héllo\n\n- 世界\n", + "a a a\nb b b\n", + ]; + for parent in texts { + for updated in texts { + let input = updated.into(); + assert_eq!( + reconcile(parent, &input, &input, &*tokenizer) + .apply() + .text(), + updated, + "tokenizer={tokenizer:?}, parent={parent:?}, updated={updated:?}", + ); + } + } +} + +#[test_matrix([ + BuiltinTokenizer::Character, + BuiltinTokenizer::Line, + BuiltinTokenizer::Markdown, + BuiltinTokenizer::Word +])] +fn identical_short_edits_preserve_text(tokenizer: BuiltinTokenizer) { + let mut texts = vec![String::new()]; + let mut level = texts.clone(); + for _ in 0..3 { + level = level + .iter() + .flat_map(|text| ['a', 'b', ' ', '\n'].map(|ch| format!("{text}{ch}"))) + .collect(); + texts.extend(level.iter().cloned()); + } + + for parent in &texts { + for updated in &texts { + let input = updated.into(); + assert_eq!( + reconcile(parent, &input, &input, &*tokenizer) + .apply() + .text(), + *updated, + "tokenizer={tokenizer:?}, parent={parent:?}, updated={updated:?}", + ); + } + } +} + +#[test_case("A", "a"; "lowercase")] +#[test_case("a", "A"; "uppercase")] +#[test_case("Hello WORLD", "HELLO world")] +#[test_case("X A", "x a\n")] +#[test_case("A", "new a"; "normalized_suffix")] +#[test_case("a old B", "A new b"; "normalized_prefix_and_suffix")] +fn identical_edits_preserve_original_spelling(parent: &str, updated: &str) { + let tokenizer = |text: &str| { + text.chars() + .map(|ch| Token::new(ch.to_ascii_lowercase(), ch.to_string(), false, false)) + .collect() + }; + let input = updated.into(); + assert_eq!( + reconcile(parent, &input, &input, &tokenizer).apply().text(), + updated, + ); +}