WIP: Track down bugs with longest_row on wrap map

Nathan Sobo and Max Brunsfeld created

Co-Authored-By: Max Brunsfeld <maxbrunsfeld@gmail.com>

Change summary

crates/buffer/src/rope.rs                  |  36 +++++
crates/editor/src/display_map/block_map.rs |   9 +
crates/editor/src/display_map/tab_map.rs   |  54 ++++++---
crates/editor/src/display_map/wrap_map.rs  | 141 +++++++++++++++++++++++
crates/editor/src/test.rs                  |   1 
5 files changed, 216 insertions(+), 25 deletions(-)

Detailed changes

crates/buffer/src/rope.rs 🔗

@@ -767,7 +767,7 @@ mod tests {
     use super::*;
     use crate::random_char_iter::RandomCharIter;
     use rand::prelude::*;
-    use std::env;
+    use std::{cmp::Ordering, env};
     use Bias::{Left, Right};
 
     #[test]
@@ -814,7 +814,7 @@ mod tests {
     }
 
     #[gpui::test(iterations = 100)]
-    fn test_random(mut rng: StdRng) {
+    fn test_random_rope(mut rng: StdRng) {
         let operations = env::var("OPERATIONS")
             .map(|i| i.parse().expect("invalid `OPERATIONS` variable"))
             .unwrap_or(10);
@@ -898,6 +898,38 @@ mod tests {
                     TextSummary::from(&expected[start_ix..end_ix])
                 );
             }
+
+            let mut expected_longest_rows = Vec::new();
+            let mut longest_line_len = -1_isize;
+            for (row, line) in expected.split('\n').enumerate() {
+                let row = row as u32;
+                assert_eq!(
+                    actual.line_len(row),
+                    line.len() as u32,
+                    "invalid line len for row {}",
+                    row
+                );
+
+                let line_char_count = line.chars().count() as isize;
+                match line_char_count.cmp(&longest_line_len) {
+                    Ordering::Less => {}
+                    Ordering::Equal => expected_longest_rows.push(row),
+                    Ordering::Greater => {
+                        longest_line_len = line_char_count;
+                        expected_longest_rows.clear();
+                        expected_longest_rows.push(row);
+                    }
+                }
+            }
+
+            let longest_row = actual.summary().longest_row;
+            assert!(
+                expected_longest_rows.contains(&longest_row),
+                "incorrect longest row {}. expected {:?} with length {}",
+                longest_row,
+                expected_longest_rows,
+                longest_line_len,
+            );
         }
     }
 

crates/editor/src/display_map/block_map.rs 🔗

@@ -1330,23 +1330,26 @@ mod tests {
                     row
                 );
 
-                match (line.len() as isize).cmp(&longest_line_len) {
+                let line_char_count = line.chars().count() as isize;
+                match line_char_count.cmp(&longest_line_len) {
                     Ordering::Less => {}
                     Ordering::Equal => expected_longest_rows.push(row),
                     Ordering::Greater => {
-                        longest_line_len = line.len() as isize;
+                        longest_line_len = line_char_count;
                         expected_longest_rows.clear();
                         expected_longest_rows.push(row);
                     }
                 }
             }
 
+            log::info!("getting longest row >>>>>>>>>>>>>>>>>>>>>>>>");
             let longest_row = blocks_snapshot.longest_row();
             assert!(
                 expected_longest_rows.contains(&longest_row),
-                "incorrect longest row {}. expected {:?}",
+                "incorrect longest row {}. expected {:?} with length {}",
                 longest_row,
                 expected_longest_rows,
+                longest_line_len,
             );
 
             for row in 0..=blocks_snapshot.wrap_snapshot.max_point().row() {

crates/editor/src/display_map/tab_map.rs 🔗

@@ -110,34 +110,31 @@ impl Snapshot {
             .text_summary_for_range(input_start..input_end);
 
         let mut first_line_chars = 0;
-        let mut first_line_bytes = 0;
+        let line_end = if range.start.row() == range.end.row() {
+            range.end
+        } else {
+            self.max_point()
+        };
         for c in self
-            .chunks(range.start..self.max_point(), false)
+            .chunks(range.start..line_end, false)
             .flat_map(|chunk| chunk.text.chars())
         {
-            if c == '\n'
-                || (range.start.row() == range.end.row() && first_line_bytes == range.end.column())
-            {
+            if c == '\n' {
                 break;
             }
             first_line_chars += 1;
-            first_line_bytes += c.len_utf8() as u32;
         }
 
         let mut last_line_chars = 0;
-        let mut last_line_bytes = 0;
-        for c in self
-            .chunks(
-                TabPoint::new(range.end.row(), 0).max(range.start)..self.max_point(),
-                false,
-            )
-            .flat_map(|chunk| chunk.text.chars())
-        {
-            if last_line_bytes == range.end.column() {
-                break;
+        if range.start.row() == range.end.row() {
+            last_line_chars = first_line_chars;
+        } else {
+            for _ in self
+                .chunks(TabPoint::new(range.end.row(), 0)..self.max_point(), false)
+                .flat_map(|chunk| chunk.text.chars())
+            {
+                last_line_chars += 1;
             }
-            last_line_chars += 1;
-            last_line_bytes += c.len_utf8() as u32;
         }
 
         TextSummary {
@@ -427,6 +424,12 @@ impl<'a> Iterator for Chunks<'a> {
 
 #[cfg(test)]
 mod tests {
+    use buffer::RandomCharIter;
+    use language::Buffer;
+    use rand::{prelude::StdRng, Rng};
+
+    use crate::display_map::fold_map::FoldMap;
+
     use super::*;
 
     #[test]
@@ -435,4 +438,19 @@ mod tests {
         assert_eq!(Snapshot::expand_tabs("\t".chars(), 1, 4), 4);
         assert_eq!(Snapshot::expand_tabs("\ta".chars(), 2, 4), 5);
     }
+
+    #[gpui::test(iterations = 100)]
+    fn test_text_summary_for_range(cx: &mut gpui::MutableAppContext, mut rng: StdRng) {
+        let tab_size = rng.gen_range(1..=4);
+        let buffer = cx.add_model(|cx| {
+            let len = rng.gen_range(0..30);
+            let text = RandomCharIter::new(&mut rng).take(len).collect::<String>();
+            Buffer::new(0, text, cx)
+        });
+        let (_, folds_snapshot) = FoldMap::new(buffer.clone(), cx);
+        let (_, tabs_snapshot) = TabMap::new(folds_snapshot.clone(), tab_size);
+
+        println!("{:?}", tabs_snapshot.text());
+        // TODO: Test text_summary_for_range with random ranges
+    }
 }

crates/editor/src/display_map/wrap_map.rs 🔗

@@ -299,11 +299,20 @@ impl Snapshot {
     }
 
     fn interpolate(&mut self, new_tab_snapshot: TabSnapshot, tab_edits: &[TabEdit]) -> Patch {
+        log::info!("INTERPOLATING");
+
+        log::info!("updating transforms... old transforms are:");
+        for transform in self.transforms.items(&()) {
+            log::info!("  - i {:?}", transform.summary.input);
+            log::info!("    o {:?}", transform.summary.output);
+        }
+
         let mut new_transforms;
         if tab_edits.is_empty() {
             new_transforms = self.transforms.clone();
         } else {
             let mut old_cursor = self.transforms.cursor::<TabPoint>();
+
             let mut tab_edits_iter = tab_edits.iter().peekable();
             new_transforms = old_cursor.slice(
                 &tab_edits_iter.peek().unwrap().old_lines.start,
@@ -311,12 +320,26 @@ impl Snapshot {
                 &(),
             );
 
+            log::info!("sliced, new_transforms are:");
+            for transform in new_transforms.items(&()) {
+                log::info!("  - i {:?}", transform.summary.input);
+                log::info!("    o {:?}", transform.summary.output);
+            }
+
             while let Some(edit) = tab_edits_iter.next() {
+                log::info!("processing edit {:?}", edit);
+
                 if edit.new_lines.start > TabPoint::from(new_transforms.summary().input.lines) {
                     let summary = new_tab_snapshot.text_summary_for_range(
                         TabPoint::from(new_transforms.summary().input.lines)..edit.new_lines.start,
                     );
+                    log::info!("pushing prefix before edit: {:?}", summary);
                     new_transforms.push_or_extend(Transform::isomorphic(summary));
+                    log::info!("new transforms are now:");
+                    for transform in new_transforms.items(&()) {
+                        log::info!("  - i {:?}", transform.summary.input);
+                        log::info!("    o {:?}", transform.summary.output);
+                    }
                 }
 
                 if !edit.new_lines.is_empty() {
@@ -325,6 +348,12 @@ impl Snapshot {
                     ));
                 }
 
+                log::info!("pushed summary within edit new range; new transforms now:");
+                for transform in new_transforms.items(&()) {
+                    log::info!("  - i {:?}", transform.summary.input);
+                    log::info!("    o {:?}", transform.summary.output);
+                }
+
                 old_cursor.seek_forward(&edit.old_lines.end, Bias::Right, &());
                 if let Some(next_edit) = tab_edits_iter.peek() {
                     if next_edit.old_lines.start > old_cursor.end(&()) {
@@ -334,21 +363,46 @@ impl Snapshot {
                                 .text_summary_for_range(edit.old_lines.end..old_cursor.end(&()));
                             new_transforms.push_or_extend(Transform::isomorphic(summary));
                         }
+
+                        log::info!("pushed transform suffix after edit; new transforms now:");
+                        for transform in new_transforms.items(&()) {
+                            log::info!("  - i {:?}", transform.summary.input);
+                            log::info!("    o {:?}", transform.summary.output);
+                        }
+
                         old_cursor.next(&());
                         new_transforms.push_tree(
                             old_cursor.slice(&next_edit.old_lines.start, Bias::Right, &()),
                             &(),
                         );
+
+                        log::info!("pushed tree suffix after edit; new transforms now:");
+                        for transform in new_transforms.items(&()) {
+                            log::info!("  - i {:?}", transform.summary.input);
+                            log::info!("    o {:?}", transform.summary.output);
+                        }
                     }
                 } else {
+                    log::info!("no more edits");
                     if old_cursor.end(&()) > edit.old_lines.end {
                         let summary = self
                             .tab_snapshot
                             .text_summary_for_range(edit.old_lines.end..old_cursor.end(&()));
                         new_transforms.push_or_extend(Transform::isomorphic(summary));
+
+                        log::info!("pushed transform suffix after edit; new transforms now:");
+                        for transform in new_transforms.items(&()) {
+                            log::info!("  - i {:?}", transform.summary.input);
+                            log::info!("    o {:?}", transform.summary.output);
+                        }
                     }
                     old_cursor.next(&());
                     new_transforms.push_tree(old_cursor.suffix(&()), &());
+                    log::info!("pushed suffix:");
+                    for transform in new_transforms.items(&()) {
+                        log::info!("  - i {:?}", transform.summary.input);
+                        log::info!("    o {:?}", transform.summary.output);
+                    }
                 }
             }
         }
@@ -378,6 +432,12 @@ impl Snapshot {
             new_rows: Range<u32>,
         }
 
+        log::info!("updating transforms... old transforms are:");
+        for transform in self.transforms.items(&()) {
+            log::info!("  - i {:?}", transform.summary.input);
+            log::info!("    o {:?}", transform.summary.output);
+        }
+
         let mut tab_edits_iter = tab_edits.into_iter().peekable();
         let mut row_edits = Vec::new();
         while let Some(edit) = tab_edits_iter.next() {
@@ -399,6 +459,11 @@ impl Snapshot {
             row_edits.push(row_edit);
         }
 
+        log::info!("row edits are:");
+        for edit in &row_edits {
+            log::info!("  {:?}", edit);
+        }
+
         let mut new_transforms;
         if row_edits.is_empty() {
             new_transforms = self.transforms.clone();
@@ -412,6 +477,12 @@ impl Snapshot {
                 &(),
             );
 
+            log::info!("sliced a prefix:");
+            for transform in new_transforms.items(&()) {
+                log::info!("  - i {:?}", transform.summary.input);
+                log::info!("    o {:?}", transform.summary.output);
+            }
+
             while let Some(edit) = row_edits.next() {
                 if edit.new_rows.start > new_transforms.summary().input.lines.row {
                     let summary = new_tab_snapshot.text_summary_for_range(
@@ -465,9 +536,15 @@ impl Snapshot {
                 }
 
                 let mut edit_transforms = edit_transforms.into_iter();
+                log::info!("extending tree with edit transforms");
                 if let Some(transform) = edit_transforms.next() {
+                    log::info!(
+                        "push or extend with first transform: {:?}",
+                        transform.summary.output
+                    );
                     new_transforms.push_or_extend(transform);
                 }
+                log::info!("extending with remaining transforms");
                 new_transforms.extend(edit_transforms, &());
 
                 old_cursor.seek_forward(&TabPoint::new(edit.old_rows.end, 0), Bias::Right, &());
@@ -859,10 +936,17 @@ impl sum_tree::Item for Transform {
 }
 
 fn push_isomorphic(transforms: &mut Vec<Transform>, summary: TextSummary) {
+    log::info!("push_isomorphic: {:?}", summary);
     if let Some(last_transform) = transforms.last_mut() {
         if last_transform.is_isomorphic() {
             last_transform.summary.input += &summary;
             last_transform.summary.output += &summary;
+
+            log::info!(
+                "  extended previous isomorphic: {:?}",
+                last_transform.summary.output
+            );
+
             return;
         }
     }
@@ -879,15 +963,28 @@ impl SumTreeExt for SumTree<Transform> {
         self.update_last(
             |last_transform| {
                 if last_transform.is_isomorphic() && transform.as_ref().unwrap().is_isomorphic() {
+                    // log::info!("extending last transform in tree");
+                    // log::info!(
+                    //     "  extending with: {:?}",
+                    //     transform.as_ref().map(|t| t.summary.output.clone()),
+                    // );
+                    // log::info!("  last transform was: {:?}", last_transform.summary.output);
+
                     let transform = transform.take().unwrap();
                     last_transform.summary.input += &transform.summary.input;
                     last_transform.summary.output += &transform.summary.output;
+
+                    // log::info!(
+                    //     "  last transform is now {:?}",
+                    //     last_transform.summary.output,
+                    // )
                 }
             },
             &(),
         );
 
         if let Some(transform) = transform {
+            log::info!("!!!!!!!!!!! push transform: {:?}", transform.summary.output,);
             self.push(transform, &());
         }
     }
@@ -1011,7 +1108,7 @@ mod tests {
         let unwrapped_text = tabs_snapshot.text();
         let expected_text = wrap_text(&unwrapped_text, wrap_width, &mut line_wrapper);
 
-        let (wrap_map, initial_snapshot) =
+        let (wrap_map, _) =
             cx.update(|cx| WrapMap::new(tabs_snapshot.clone(), font_id, font_size, wrap_width, cx));
         let (_observer, notifications) = Observer::new(&wrap_map, &mut cx);
 
@@ -1019,6 +1116,11 @@ mod tests {
             notifications.recv().await.unwrap();
         }
 
+        let (initial_snapshot, _) = wrap_map.update(&mut cx, |map, cx| {
+            assert!(!map.is_rewrapping());
+            map.sync(tabs_snapshot.clone(), Vec::new(), cx)
+        });
+
         let actual_text = initial_snapshot.text();
         assert_eq!(
             actual_text, expected_text,
@@ -1029,6 +1131,8 @@ mod tests {
 
         let mut edits = Vec::new();
         for _i in 0..operations {
+            log::info!("{} ==============================================", _i);
+
             match rng.gen_range(0..=100) {
                 0..=19 => {
                     wrap_width = if rng.gen_bool(0.2) {
@@ -1088,15 +1192,48 @@ mod tests {
                 let (mut wrapped_snapshot, wrap_edits) =
                     wrap_map.update(&mut cx, |map, cx| map.sync(tabs_snapshot, Vec::new(), cx));
                 let actual_text = wrapped_snapshot.text();
+                let actual_longest_row = wrapped_snapshot.longest_row();
                 log::info!("Wrapping finished: {:?}", actual_text);
                 wrapped_snapshot.check_invariants();
                 wrapped_snapshot.verify_chunks(&mut rng);
-                edits.push((wrapped_snapshot, wrap_edits));
+                edits.push((wrapped_snapshot.clone(), wrap_edits));
                 assert_eq!(
                     actual_text, expected_text,
                     "unwrapped text is: {:?}",
                     unwrapped_text
                 );
+
+                let mut summary = TextSummary::default();
+                for (ix, item) in wrapped_snapshot
+                    .transforms
+                    .items(&())
+                    .into_iter()
+                    .enumerate()
+                {
+                    summary += &item.summary.output;
+                    log::info!("{} summary: {:?}", ix, item.summary.output,);
+                }
+
+                let mut expected_longest_rows = Vec::new();
+                let mut longest_line_len = -1;
+                for (row, line) in expected_text.split('\n').enumerate() {
+                    let line_char_count = line.chars().count() as isize;
+                    if line_char_count > longest_line_len {
+                        expected_longest_rows.clear();
+                        longest_line_len = line_char_count;
+                    }
+                    if line_char_count >= longest_line_len {
+                        expected_longest_rows.push(row as u32);
+                    }
+                }
+
+                assert!(
+                    expected_longest_rows.contains(&actual_longest_row),
+                    "incorrect longest row {}. expected {:?} with length {}",
+                    actual_longest_row,
+                    expected_longest_rows,
+                    longest_line_len,
+                )
             }
         }
 

crates/editor/src/test.rs 🔗

@@ -5,6 +5,7 @@ use std::marker::PhantomData;
 #[cfg(test)]
 #[ctor::ctor]
 fn init_logger() {
+    // std::env::set_var("RUST_LOG", "info");
     env_logger::init();
 }