Merge #11574

11574: Small refactor text edit 2nd r=Veykril a=HansAuger

Some more changes to text_edit. Basic idea is to make `Indel` implement `PartialOrd` to take advantage of some sweet sweet iteration, most notably itertool's `merge`.



Co-authored-by: Moritz Vetter <mv@3yourmind.com>

3 files changed, 39 insertions, 16 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 64654c9961d..3f2ee2c5727 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1668,6 +1668,7 @@ checksum = "288cb548dbe72b652243ea797201f3d481a0609a967980fcc5b2315ea811560a"
 name = "text_edit"
 version = "0.0.0"
 dependencies = [
+ "itertools",
  "text-size",
 ]
 
diff --git a/crates/text_edit/Cargo.toml b/crates/text_edit/Cargo.toml
index e9c6bb33484..dae69e265d7 100644
--- a/crates/text_edit/Cargo.toml
+++ b/crates/text_edit/Cargo.toml
@@ -10,4 +10,5 @@ rust-version = "1.57"
 doctest = false
 
 [dependencies]
+itertools = "0.10.0"
 text-size = "1.0.0"
diff --git a/crates/text_edit/src/lib.rs b/crates/text_edit/src/lib.rs
index 19c96d34c57..1a6add05789 100644
--- a/crates/text_edit/src/lib.rs
+++ b/crates/text_edit/src/lib.rs
@@ -4,6 +4,8 @@
 //! so `TextEdit` is the ultimate representation of the work done by
 //! rust-analyzer.
 
+use itertools::Itertools;
+use std::cmp::max;
 pub use text_size::{TextRange, TextSize};
 
 /// `InsertDelete` -- a single "atomic" change to text
@@ -91,13 +93,15 @@ impl TextEdit {
         }
 
         let text_size = TextSize::of(&*text);
-        let mut total_len = text_size.clone();
+        let mut total_len = text_size;
+        let mut max_total_len = text_size.clone();
         for indel in &self.indels {
             total_len += TextSize::of(&indel.insert);
             total_len -= indel.delete.len();
+            max_total_len = max(max_total_len, total_len);
         }
 
-        if let Some(additional) = total_len.checked_sub(text_size.into()) {
+        if let Some(additional) = max_total_len.checked_sub(text_size.into()) {
             text.reserve(additional.into());
         }
 
@@ -109,16 +113,14 @@ impl TextEdit {
     }
 
     pub fn union(&mut self, other: TextEdit) -> Result<(), TextEdit> {
-        // FIXME: can be done without allocating intermediate vector
-        let mut all = self.iter().chain(other.iter()).collect::<Vec<_>>();
-        if !check_disjoint_and_sort(&mut all) {
+        let iter_merge =
+            self.iter().merge_by(other.iter(), |l, r| l.delete.start() <= r.delete.start());
+        if !check_disjoint(&mut iter_merge.clone()) {
             return Err(other);
         }
 
-        self.indels.extend(other.indels);
-        check_disjoint_and_sort(&mut self.indels);
         // Only dedup deletions and replacements, keep all insertions
-        self.indels.dedup_by(|a, b| a == b && !a.delete.is_empty());
+        self.indels = iter_merge.dedup_by(|a, b| a == b && !a.delete.is_empty()).cloned().collect();
         Ok(())
     }
 
@@ -188,14 +190,17 @@ impl TextEditBuilder {
 fn assert_disjoint_or_equal(indels: &mut [Indel]) {
     assert!(check_disjoint_and_sort(indels));
 }
-// FIXME: Remove the impl Bound here, it shouldn't be needed
-fn check_disjoint_and_sort(indels: &mut [impl std::borrow::Borrow<Indel>]) -> bool {
-    indels.sort_by_key(|indel| (indel.borrow().delete.start(), indel.borrow().delete.end()));
-    indels.iter().zip(indels.iter().skip(1)).all(|(l, r)| {
-        let l = l.borrow();
-        let r = r.borrow();
-        l.delete.end() <= r.delete.start() || l == r
-    })
+
+fn check_disjoint_and_sort(indels: &mut [Indel]) -> bool {
+    indels.sort_by_key(|indel| (indel.delete.start(), indel.delete.end()));
+    check_disjoint(&mut indels.iter())
+}
+
+fn check_disjoint<'a, I>(indels: &mut I) -> bool
+where
+    I: std::iter::Iterator<Item = &'a Indel> + Clone,
+{
+    indels.clone().zip(indels.skip(1)).all(|(l, r)| l.delete.end() <= r.delete.start() || l == r)
 }
 
 #[cfg(test)]
@@ -233,6 +238,22 @@ mod tests {
     }
 
     #[test]
+    fn test_union_with_duplicates() {
+        let mut builder1 = TextEditBuilder::default();
+        builder1.delete(range(7, 11));
+        builder1.delete(range(13, 17));
+
+        let mut builder2 = TextEditBuilder::default();
+        builder2.delete(range(1, 5));
+        builder2.delete(range(13, 17));
+
+        let mut edit1 = builder1.finish();
+        let edit2 = builder2.finish();
+        assert!(edit1.union(edit2).is_ok());
+        assert_eq!(edit1.indels.len(), 3);
+    }
+
+    #[test]
     fn test_union_panics() {
         let mut edit1 = TextEdit::delete(range(7, 11));
         let edit2 = TextEdit::delete(range(9, 13));