Fix `folds_in_range` and add a test for it

Antonio Scandurra created

With the current ordering, a linear scan is required in order to
determine which folds intersect the given range.

Change summary

zed/src/editor/buffer_view.rs          | 40 +++++------------
zed/src/editor/display_map/fold_map.rs | 64 +++++++++++++++++++--------
zed/src/editor/display_map/mod.rs      |  6 ++
3 files changed, 61 insertions(+), 49 deletions(-)

Detailed changes

zed/src/editor/buffer_view.rs 🔗

@@ -863,9 +863,8 @@ impl BufferView {
 
             // Cut the text from the selected rows and paste it at the start of the previous line.
             if display_rows.start != 0 {
-                let selection_row_start =
-                    Point::new(buffer_rows.start, 0).to_offset(buffer).unwrap();
-                let selection_row_end = Point::new(
+                let start = Point::new(buffer_rows.start, 0).to_offset(buffer).unwrap();
+                let end = Point::new(
                     buffer_rows.end - 1,
                     buffer.line_len(buffer_rows.end - 1).unwrap(),
                 )
@@ -878,14 +877,10 @@ impl BufferView {
                     .unwrap();
 
                 let mut text = String::new();
-                text.extend(
-                    buffer
-                        .text_for_range(selection_row_start..selection_row_end)
-                        .unwrap(),
-                );
+                text.extend(buffer.text_for_range(start..end).unwrap());
                 text.push('\n');
                 edits.push((prev_row_start..prev_row_start, text));
-                edits.push((selection_row_start - 1..selection_row_end, String::new()));
+                edits.push((start - 1..end, String::new()));
 
                 let row_delta = buffer_rows.start
                     - prev_row_display_start
@@ -900,11 +895,8 @@ impl BufferView {
                 }
 
                 // Move folds up.
-                old_folds.push(selection_row_start..selection_row_end);
-                for fold in map
-                    .folds_in_range(selection_row_start..selection_row_end, app)
-                    .unwrap()
-                {
+                old_folds.push(start..end);
+                for fold in map.folds_in_range(start..end, app).unwrap() {
                     let mut start = fold.start.to_point(buffer).unwrap();
                     let mut end = fold.end.to_point(buffer).unwrap();
                     start.row -= row_delta;
@@ -962,9 +954,8 @@ impl BufferView {
 
             // Cut the text from the selected rows and paste it at the end of the next line.
             if display_rows.end <= map.max_point(app).row() {
-                let selection_row_start =
-                    Point::new(buffer_rows.start, 0).to_offset(buffer).unwrap();
-                let selection_row_end = Point::new(
+                let start = Point::new(buffer_rows.start, 0).to_offset(buffer).unwrap();
+                let end = Point::new(
                     buffer_rows.end - 1,
                     buffer.line_len(buffer_rows.end - 1).unwrap(),
                 )
@@ -981,12 +972,8 @@ impl BufferView {
 
                 let mut text = String::new();
                 text.push('\n');
-                text.extend(
-                    buffer
-                        .text_for_range(selection_row_start..selection_row_end)
-                        .unwrap(),
-                );
-                edits.push((selection_row_start..selection_row_end + 1, String::new()));
+                text.extend(buffer.text_for_range(start..end).unwrap());
+                edits.push((start..end + 1, String::new()));
                 edits.push((next_row_end..next_row_end, text));
 
                 let row_delta = next_row_display_end
@@ -1003,11 +990,8 @@ impl BufferView {
                 }
 
                 // Move folds down.
-                old_folds.push(selection_row_start..selection_row_end);
-                for fold in map
-                    .folds_in_range(selection_row_start..selection_row_end, app)
-                    .unwrap()
-                {
+                old_folds.push(start..end);
+                for fold in map.folds_in_range(start..end, app).unwrap() {
                     let mut start = fold.start.to_point(buffer).unwrap();
                     let mut end = fold.end.to_point(buffer).unwrap();
                     start.row += row_delta;

zed/src/editor/display_map/fold_map.rs 🔗

@@ -89,31 +89,20 @@ impl FoldMap {
         DisplayPoint(self.transforms.summary().display.rightmost_point)
     }
 
-    pub fn folds_in_range<T>(&self, range: Range<T>, app: &AppContext) -> Result<&[Range<Anchor>]>
+    pub fn folds_in_range<'a, T>(
+        &'a self,
+        range: Range<T>,
+        app: &'a AppContext,
+    ) -> Result<impl Iterator<Item = &'a Range<Anchor>>>
     where
         T: ToOffset,
     {
         let buffer = self.buffer.read(app);
         let range = buffer.anchor_before(range.start)?..buffer.anchor_before(range.end)?;
-        let mut start_ix = find_insertion_index(&self.folds, |probe| probe.cmp(&range, buffer))?;
-        let mut end_ix = start_ix;
-
-        for fold in self.folds[..start_ix].iter().rev() {
-            if fold.end.cmp(&range.start, buffer)? == Ordering::Greater {
-                start_ix -= 1;
-            } else {
-                break;
-            }
-        }
-        for fold in &self.folds[end_ix..] {
-            if range.end.cmp(&fold.start, buffer)? == Ordering::Greater {
-                end_ix += 1;
-            } else {
-                break;
-            }
-        }
-
-        Ok(&self.folds[start_ix..end_ix])
+        Ok(self.folds.iter().filter(move |fold| {
+            range.start.cmp(&fold.end, buffer).unwrap() == Ordering::Less
+                && range.end.cmp(&fold.start, buffer).unwrap() == Ordering::Greater
+        }))
     }
 
     pub fn fold<T: ToOffset>(
@@ -500,6 +489,7 @@ impl<'a> Dimension<'a, TransformSummary> for usize {
 mod tests {
     use super::*;
     use crate::test::sample_text;
+    use buffer::ToPoint;
     use gpui::App;
 
     #[test]
@@ -645,6 +635,40 @@ mod tests {
         });
     }
 
+    #[test]
+    fn test_folds_in_range() {
+        App::test((), |app| {
+            let buffer = app.add_model(|ctx| Buffer::new(0, sample_text(5, 6), ctx));
+            let mut map = FoldMap::new(buffer.clone(), app.as_ref());
+            let buffer = buffer.read(app);
+
+            map.fold(
+                vec![
+                    Point::new(0, 2)..Point::new(2, 2),
+                    Point::new(0, 4)..Point::new(1, 0),
+                    Point::new(1, 2)..Point::new(3, 2),
+                    Point::new(3, 1)..Point::new(4, 1),
+                ],
+                app.as_ref(),
+            )
+            .unwrap();
+            let fold_ranges = map
+                .folds_in_range(Point::new(1, 0)..Point::new(1, 3), app.as_ref())
+                .unwrap()
+                .map(|fold| {
+                    fold.start.to_point(buffer).unwrap()..fold.end.to_point(buffer).unwrap()
+                })
+                .collect::<Vec<_>>();
+            assert_eq!(
+                fold_ranges,
+                vec![
+                    Point::new(0, 2)..Point::new(2, 2),
+                    Point::new(1, 2)..Point::new(3, 2)
+                ]
+            );
+        });
+    }
+
     #[test]
     fn test_random_folds() {
         use crate::editor::ToPoint;

zed/src/editor/display_map/mod.rs 🔗

@@ -34,7 +34,11 @@ impl DisplayMap {
         }
     }
 
-    pub fn folds_in_range<T>(&self, range: Range<T>, app: &AppContext) -> Result<&[Range<Anchor>]>
+    pub fn folds_in_range<'a, T>(
+        &'a self,
+        range: Range<T>,
+        app: &'a AppContext,
+    ) -> Result<impl Iterator<Item = &'a Range<Anchor>>>
     where
         T: ToOffset,
     {