Revert oversimplifications

examples/website/src/index.ts

@@ -102,10 +102,10 @@ function updateMergedText(): void {
   const selection =
     results.cursors?.length === 2
       ? {
-        start: results.cursors[0].position,
+          start: results.cursors[0].position,
-        end: results.cursors[1].position,
+          end: results.cursors[1].position,
-        side: leftCursors ? 'left' : 'right',
+          side: leftCursors ? 'left' : 'right',
-      }
+        }
       : undefined;
 
   const fragment = document.createDocumentFragment();
@@ -127,10 +127,7 @@ function updateMergedText(): void {
     }
 
     fragment.appendChild(
-      createSelectionOverlay(
+      createSelectionOverlay(selection.side === 'left', selection.start !== selection.end)
-        selection.side === 'left',
-        selection.start !== selection.end
-      )
     );
   }
 

reconcile-js/.gitignore

@@ -1 +1,2 @@
+README.md
 LICENSE

src/operation_transformation/diff_error.rs

@@ -17,6 +17,13 @@ pub enum DiffError {
         available: usize,
     },
 
+    /// A diff character count was too large to represent as usize
+    #[error("Invalid diff: character count {value} cannot be represented as usize")]
+    LengthOverflow {
+        /// The absolute character count from the diff
+        value: u64,
+    },
+
     /// A character count was too large to represent as i64
     #[error("Integer overflow: value {value} cannot be represented as i64")]
     IntegerOverflow {

src/operation_transformation/edited_text.rs

@@ -505,11 +505,8 @@ where
     /// # Errors
     ///
     /// Returns `DiffError::LengthExceedsOriginal` if the diff references a
-    /// range that exceeds the original text length.
+    /// range that exceeds the original text length, or `DiffError::LengthOverflow`
-    ///
+    /// if a character count cannot be represented as usize.
-    /// # Panics
-    ///
-    /// Panics if a character count cannot be represented as usize.
     pub fn from_diff(
         original_text: &'a str,
         diff: Vec<NumberOrText>,
@@ -522,8 +519,11 @@ where
         for item in diff {
             match item {
                 NumberOrText::Number(length) => {
-                    let count =
+                    let count = usize::try_from(length.unsigned_abs()).map_err(|_| {
-                        usize::try_from(length.unsigned_abs()).expect("length must fit in usize");
+                        DiffError::LengthOverflow {
+                            value: length.unsigned_abs(),
+                        }
+                    })?;
                     let available = text_length - order;
                     if count > available {
                         return Err(DiffError::LengthExceedsOriginal {

src/operation_transformation/operation.rs

@@ -202,25 +202,25 @@ where
     /// Applies the operation to the given `StringBuilder`, returning the
     /// modified `StringBuilder`.
     ///
-    /// When compiled in debug mode, panics if a delete operation is attempted
+    /// When compiled in debug mode, panics if a retain or delete operation is
-    /// on a range of text that does not match the text to be deleted.
+    /// out of order, out of bounds, or does not match its expected text.
     pub fn apply<'a>(&self, mut builder: StringBuilder<'a>) -> StringBuilder<'a> {
         match self {
             Operation::Equal {
                 #[cfg(debug_assertions)]
                 text,
+                #[cfg(debug_assertions)]
+                order,
                 length,
                 ..
             } => {
                 #[cfg(debug_assertions)]
-                debug_assert!(
+                {
-                    text.as_ref()
+                    builder.assert_range(*order, *length);
-                        .is_none_or(|text| builder.get_slice_from_remaining(self.len()) == *text),
+                    if let Some(text) = text {
-                    "Text (`{}`) which is supposed to be equal does not match the text in the \
+                        builder.assert_text(*length, text);
-                     range: `{}`",
+                    }
-                    text.as_ref().unwrap_or(&String::new()),
+                }
-                    builder.get_slice_from_remaining(self.len())
-                );
 
                 builder.retain(*length);
             }
@@ -230,18 +230,18 @@ where
             Operation::Delete {
                 #[cfg(debug_assertions)]
                 deleted_text,
+                #[cfg(debug_assertions)]
+                order,
                 deleted_character_count,
                 ..
             } => {
                 #[cfg(debug_assertions)]
-                debug_assert!(
+                {
-                    deleted_text
+                    builder.assert_range(*order, *deleted_character_count);
-                        .as_ref()
+                    if let Some(text) = deleted_text {
-                        .is_none_or(|text| builder.get_slice_from_remaining(self.len()) == *text),
+                        builder.assert_text(*deleted_character_count, text);
-                    "Text to-be-deleted `{}` does not match the text in the range: `{}`",
+                    }
-                    deleted_text.as_ref().unwrap_or(&String::new()),
+                }
-                    builder.get_slice_from_remaining(self.len())
-                );
 
                 builder.delete(*deleted_character_count);
             }

src/utils/string_builder.rs

@@ -8,7 +8,7 @@ pub struct StringBuilder<'a> {
     buffer: String,
 
     #[cfg(debug_assertions)]
-    remaining: String,
+    position: usize,
 }
 
 impl fmt::Debug for StringBuilder<'_> {
@@ -17,7 +17,7 @@ impl fmt::Debug for StringBuilder<'_> {
         debug_struct.field("buffer", &self.buffer);
 
         #[cfg(debug_assertions)]
-        debug_struct.field("remaining", &self.remaining);
+        debug_struct.field("remaining", &self.original.as_str());
 
         debug_struct.finish_non_exhaustive()
     }
@@ -30,7 +30,7 @@ impl StringBuilder<'_> {
             buffer: String::with_capacity(original.len()),
 
             #[cfg(debug_assertions)]
-            remaining: original.to_owned(),
+            position: 0,
         }
     }
 
@@ -50,7 +50,7 @@ impl StringBuilder<'_> {
 
         #[cfg(debug_assertions)]
         {
-            self.remaining = self.remaining.chars().skip(length).collect();
+            self.position += length;
         }
     }
 
@@ -60,7 +60,7 @@ impl StringBuilder<'_> {
 
         #[cfg(debug_assertions)]
         {
-            self.remaining = self.remaining.chars().skip(length).collect();
+            self.position += length;
         }
     }
 
@@ -70,16 +70,24 @@ impl StringBuilder<'_> {
         std::mem::take(&mut self.buffer)
     }
 
-    /// Get a slice of the remaining original string. The slice starts from
+    /// Check the original-text position and range without copying the text.
-    /// where the next delete/retain operation would start and is of length
-    /// `length`.
     #[cfg(debug_assertions)]
-    pub fn get_slice_from_remaining(&self, length: usize) -> String {
+    pub fn assert_range(&self, order: usize, length: usize) {
-        let result = self.remaining.chars().take(length).collect::<String>();
+        debug_assert_eq!(self.position, order, "Operation out of order");
+        debug_assert!(
+            length == 0 || self.original.clone().nth(length - 1).is_some(),
+            "Range out of bounds"
+        );
+    }
 
-        debug_assert_eq!(result.chars().count(), length, "Range out of bounds");
+    /// Check the next original characters without allocating a substring.
-
+    #[cfg(debug_assertions)]
-        result
+    pub fn assert_text(&self, length: usize, text: &str) {
+        debug_assert!(
+            self.original.clone().take(length).eq(text.chars()),
+            "Text `{text}` does not match the next {length} characters of `{}`",
+            self.original.as_str()
+        );
     }
 }
 
@@ -132,21 +140,6 @@ mod tests {
         assert_eq!(builder.take(), "こんに世界, ちは");
     }
 
-    #[cfg(debug_assertions)]
-    #[test]
-    fn test_get_slice() {
-        let original = "abcdef";
-        let builder = StringBuilder::new(original);
-
-        // Test getting a slice of the original string
-        assert_eq!(builder.get_slice_from_remaining(3), "abc");
-
-        // Test getting a slice that includes both buffer and remaining original
-        let mut builder = StringBuilder::new(original);
-        builder.retain(2); // "ab" in buffer
-        assert_eq!(builder.get_slice_from_remaining(2), "cd");
-    }
-
     #[test]
     fn test_retain_all() {
         let original = "Hello, world!";

tests/example_document.rs

@@ -40,10 +40,9 @@ impl ExampleDocument {
     /// If the result string does not match the expected string, the program
     /// will panic.
     pub fn assert_eq(&self, result: &EditedText<'_, String>) {
-        let result_str = ExampleDocument::text_with_cursors_to_string(result);
         assert_eq!(
-            self.expected, result_str,
+            Self::string_to_text_with_cursors(&self.expected),
-            "Left (expected) isn't equal to right (actual). Actual: ```\n{result_str}```",
+            result.apply()
         );
     }
 
@@ -62,30 +61,6 @@ impl ExampleDocument {
         );
     }
 
-    fn text_with_cursors_to_string(document: &EditedText<'_, String>) -> String {
-        let merged = document.apply();
-        let mut result = merged.text();
-        for (i, cursor) in merged.cursors().iter().enumerate() {
-            assert!(
-                cursor.char_index() <= result.len(), // equals in case of insert at the end
-                "Cursor index out of bounds: {} > {} when testing for '{}.'",
-                cursor.char_index(),
-                result.len(),
-                result
-            );
-
-            result.insert(
-                result
-                    .char_indices()
-                    .nth(cursor.char_index() + i)
-                    .map_or_else(|| result.len(), |(byte_index, _)| byte_index), /* find the utf8 char index of the insert
-                                                                                  * in byte index */
-                '|',
-            );
-        }
-        result
-    }
-
     fn string_to_text_with_cursors(text: &str) -> TextWithCursors {
         let cursors = Self::parse_cursors(text);
         let text = text.replace('|', "");

tests/wasm.rs

@@ -1,6 +1,6 @@
 #![cfg(feature = "wasm")]
 
-use reconcile_text::{BuiltinTokenizer, CursorPosition, TextWithCursors, wasm::*};
+use reconcile_text::{BuiltinTokenizer, CursorPosition, EditedText, TextWithCursors, wasm::*};
 use wasm_bindgen_test::*;
 
 #[wasm_bindgen_test(unsupported = test)]
@@ -55,6 +55,16 @@ fn test_merge_binary() {
     );
 }
 
+#[wasm_bindgen_test(unsupported = test)]
+fn test_from_diff_rejects_oversized_lengths() {
+    for length in [1_i64 << 32, -(1_i64 << 40), i64::MAX, i64::MIN] {
+        let error = EditedText::from_diff("abc", vec![length.into()], &*BuiltinTokenizer::Word)
+            .unwrap_err();
+
+        assert!(error.to_string().starts_with("Invalid diff:"));
+    }
+}
+
 #[wasm_bindgen_test] // JsValue isn't supported outside of wasm
 fn test_diff() {
     let parent = "hello ";