Ensure reconcile(p, a, a) == a

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

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<Item = RawOperation<T>>,
     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<RawOperation<T>> = None;
-    let mut maybe_previous_delete: Option<RawOperation<T>> = None;
+    let mut inserts: Vec<RawOperation<T>> = Vec::new();
+    let mut deletes: Vec<RawOperation<T>> = 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<RawOperation<T>> = 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<dyn Iterator<Item = RawOperation<T>>>
-                }
-                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<dyn Iterator<Item = RawOperation<T>>>
-                }
-                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<dyn Iterator<Item = RawOperation<T>>>,
-        })
-        .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,
+            ],
+        );
+    }
 }

src/utils/myers_diff.rs

@@ -306,10 +306,10 @@ fn conquer<T>(
     // 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<T>(
 
     // 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<T>(
     }
 
     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<T>(old: &[Token<T>], new: &[Token<T>], result: &mut Vec<RawOperation<T>>)
+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;

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,
+    );
+}