Fix bugs in code folding

Mikayla Maki created 2 years ago

Change summary

crates/editor/src/display_map.rs    | 27 ++++++++++++++++
crates/editor/src/editor.rs         | 52 ++++++++++++++++++------------
crates/editor/src/editor_tests.rs   |  5 ++
crates/editor/src/items.rs          |  2 
crates/editor/src/multi_buffer.rs   |  4 --
crates/search/src/project_search.rs |  2 
crates/util/src/util.rs             | 17 ++++++++++
7 files changed, 82 insertions(+), 27 deletions(-)

Detailed changes

crates/editor/src/display_map.rs

@@ -12,6 +12,7 @@ use gpui::{
     Entity, ModelContext, ModelHandle,
 };
 use language::{OffsetUtf16, Point, Subscription as BufferSubscription};
+use serde::{Deserialize, Serialize};
 use settings::Settings;
 use std::{any::TypeId, fmt::Debug, num::NonZeroU32, ops::Range, sync::Arc};
 use sum_tree::{Bias, TreeMap};
@@ -660,6 +661,32 @@ impl DisplaySnapshot {
 #[derive(Copy, Clone, Default, Eq, Ord, PartialOrd, PartialEq)]
 pub struct DisplayPoint(BlockPoint);
 
+#[derive(Copy, Clone, Default, Eq, Ord, PartialOrd, PartialEq, Deserialize, Serialize)]
+#[repr(transparent)]
+pub struct DisplayRow(pub u32);
+
+impl DisplayRow {
+    pub fn new(display_row: u32) -> Self {
+        DisplayRow(display_row)
+    }
+
+    pub fn to_span(self, display_map: &DisplaySnapshot) -> Range<DisplayPoint> {
+        let row_start = DisplayPoint::new(self.0, 0);
+        let row_end = DisplayPoint::new(
+            self.0,
+            display_map.buffer_snapshot.line_len(row_start.row()),
+        );
+
+        row_start..row_end
+    }
+}
+
+impl From<DisplayPoint> for DisplayRow {
+    fn from(value: DisplayPoint) -> Self {
+        DisplayRow(value.row())
+    }
+}
+
 impl Debug for DisplayPoint {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.write_fmt(format_args!(

crates/editor/src/editor.rs

@@ -85,7 +85,7 @@ use std::{
 };
 pub use sum_tree::Bias;
 use theme::{DiagnosticStyle, Theme};
-use util::{post_inc, RangeExt, ResultExt, TryFutureExt};
+use util::{post_inc, MapRangeEndsExt, RangeExt, ResultExt, TryFutureExt};
 use workspace::{ItemNavHistory, ViewId, Workspace, WorkspaceId};
 
 use crate::git::diff_hunk_to_display;
@@ -163,12 +163,12 @@ pub struct ToggleComments {
 
 #[derive(Clone, Default, Deserialize, PartialEq)]
 pub struct FoldAt {
-    pub display_row: u32,
+    pub display_row: DisplayRow,
 }
 
 #[derive(Clone, Default, Deserialize, PartialEq)]
 pub struct UnfoldAt {
-    pub display_row: u32,
+    pub display_row: DisplayRow,
 }
 
 actions!(
@@ -2716,10 +2716,10 @@ impl Editor {
                     move |_, cx| {
                         cx.dispatch_any_action(match fold_status {
                             FoldStatus::Folded => Box::new(UnfoldAt {
-                                display_row: fold_location,
+                                display_row: DisplayRow::new(fold_location),
                             }),
                             FoldStatus::Foldable => Box::new(FoldAt {
-                                display_row: fold_location,
+                                display_row: DisplayRow::new(fold_location),
                             }),
                         });
                     }
@@ -3381,13 +3381,13 @@ impl Editor {
         }
 
         self.transact(cx, |this, cx| {
-            this.unfold_ranges(unfold_ranges, true, cx);
+            this.unfold_ranges(unfold_ranges, true, true, cx);
             this.buffer.update(cx, |buffer, cx| {
                 for (range, text) in edits {
                     buffer.edit([(range, text)], None, cx);
                 }
             });
-            this.fold_ranges(refold_ranges, cx);
+            this.fold_ranges(refold_ranges, true, cx);
             this.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 s.select(new_selections);
             })
@@ -3472,13 +3472,13 @@ impl Editor {
         }
 
         self.transact(cx, |this, cx| {
-            this.unfold_ranges(unfold_ranges, true, cx);
+            this.unfold_ranges(unfold_ranges, true, true, cx);
             this.buffer.update(cx, |buffer, cx| {
                 for (range, text) in edits {
                     buffer.edit([(range, text)], None, cx);
                 }
             });
-            this.fold_ranges(refold_ranges, cx);
+            this.fold_ranges(refold_ranges, true, cx);
             this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(new_selections));
         });
     }
@@ -4306,7 +4306,7 @@ impl Editor {
                 to_unfold.push(selection.start..selection.end);
             }
         }
-        self.unfold_ranges(to_unfold, true, cx);
+        self.unfold_ranges(to_unfold, true, true, cx);
         self.change_selections(Some(Autoscroll::fit()), cx, |s| {
             s.select_ranges(new_selection_ranges);
         });
@@ -4455,7 +4455,7 @@ impl Editor {
                 }
 
                 if let Some(next_selected_range) = next_selected_range {
-                    self.unfold_ranges([next_selected_range.clone()], false, cx);
+                    self.unfold_ranges([next_selected_range.clone()], false, true, cx);
                     self.change_selections(Some(Autoscroll::newest()), cx, |s| {
                         if action.replace_newest {
                             s.delete(s.newest_anchor().id);
@@ -4488,7 +4488,7 @@ impl Editor {
                     wordwise: true,
                     done: false,
                 };
-                self.unfold_ranges([selection.start..selection.end], false, cx);
+                self.unfold_ranges([selection.start..selection.end], false, true, cx);
                 self.change_selections(Some(Autoscroll::newest()), cx, |s| {
                     s.select(selections);
                 });
@@ -5726,7 +5726,7 @@ impl Editor {
             }
         }
 
-        self.fold_ranges(fold_ranges, cx);
+        self.fold_ranges(fold_ranges, true, cx);
     }
 
     pub fn fold_at(&mut self, fold_at: &FoldAt, cx: &mut ViewContext<Self>) {
@@ -5734,8 +5734,8 @@ impl Editor {
 
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
 
-        if let Some(fold_range) = display_map.foldable_range_for_line(display_row) {
-            self.fold_ranges(std::iter::once(fold_range), cx);
+        if let Some(fold_range) = display_map.foldable_range_for_line(display_row.0) {
+            self.fold_ranges(std::iter::once(fold_range), true, cx);
         }
     }
 
@@ -5754,31 +5754,38 @@ impl Editor {
                 start..end
             })
             .collect::<Vec<_>>();
-        self.unfold_ranges(ranges, true, cx);
+        self.unfold_ranges(ranges, true, true, cx);
     }
 
     pub fn unfold_at(&mut self, fold_at: &UnfoldAt, cx: &mut ViewContext<Self>) {
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
-        let unfold_range = display_map.buffer_snapshot.row_span(fold_at.display_row);
 
-        self.unfold_ranges(std::iter::once(unfold_range), true, cx)
+        let display_range = fold_at
+            .display_row
+            .to_span(&display_map)
+            .map_endpoints(|endpoint| endpoint.to_point(&display_map));
+
+        self.unfold_ranges(std::iter::once(display_range), true, true, cx)
     }
 
     pub fn fold_selected_ranges(&mut self, _: &FoldSelectedRanges, cx: &mut ViewContext<Self>) {
         let selections = self.selections.all::<Point>(cx);
         let ranges = selections.into_iter().map(|s| s.start..s.end);
-        self.fold_ranges(ranges, cx);
+        self.fold_ranges(ranges, true, cx);
     }
 
     pub fn fold_ranges<T: ToOffset>(
         &mut self,
         ranges: impl IntoIterator<Item = Range<T>>,
+        auto_scroll: bool,
         cx: &mut ViewContext<Self>,
     ) {
         let mut ranges = ranges.into_iter().peekable();
         if ranges.peek().is_some() {
             self.display_map.update(cx, |map, cx| map.fold(ranges, cx));
-            self.request_autoscroll(Autoscroll::fit(), cx);
+            if auto_scroll {
+                self.request_autoscroll(Autoscroll::fit(), cx);
+            }
             cx.notify();
         }
     }
@@ -5787,13 +5794,16 @@ impl Editor {
         &mut self,
         ranges: impl IntoIterator<Item = Range<T>>,
         inclusive: bool,
+        auto_scroll: bool,
         cx: &mut ViewContext<Self>,
     ) {
         let mut ranges = ranges.into_iter().peekable();
         if ranges.peek().is_some() {
             self.display_map
                 .update(cx, |map, cx| map.unfold(ranges, inclusive, cx));
-            self.request_autoscroll(Autoscroll::fit(), cx);
+            if auto_scroll {
+                self.request_autoscroll(Autoscroll::fit(), cx);
+            }
             cx.notify();
         }
     }

crates/editor/src/editor_tests.rs

@@ -447,6 +447,7 @@ fn test_clone(cx: &mut gpui::MutableAppContext) {
                 Point::new(1, 0)..Point::new(2, 0),
                 Point::new(3, 0)..Point::new(4, 0),
             ],
+            true,
             cx,
         );
     });
@@ -807,6 +808,7 @@ fn test_move_cursor_multibyte(cx: &mut gpui::MutableAppContext) {
                 Point::new(1, 2)..Point::new(1, 4),
                 Point::new(2, 4)..Point::new(2, 8),
             ],
+            true,
             cx,
         );
         assert_eq!(view.display_text(cx), "ⓐⓑ…ⓔ\nab…e\nαβ…ε\n");
@@ -2119,6 +2121,7 @@ fn test_move_line_up_down(cx: &mut gpui::MutableAppContext) {
                 Point::new(2, 3)..Point::new(4, 1),
                 Point::new(7, 0)..Point::new(8, 4),
             ],
+            true,
             cx,
         );
         view.change_selections(None, cx, |s| {
@@ -2586,6 +2589,7 @@ fn test_split_selection_into_lines(cx: &mut gpui::MutableAppContext) {
                 Point::new(2, 3)..Point::new(4, 1),
                 Point::new(7, 0)..Point::new(8, 4),
             ],
+            true,
             cx,
         );
         view.change_selections(None, cx, |s| {
@@ -2983,6 +2987,7 @@ async fn test_select_larger_smaller_syntax_node(cx: &mut gpui::TestAppContext) {
                 Point::new(0, 21)..Point::new(0, 24),
                 Point::new(3, 20)..Point::new(3, 22),
             ],
+            true,
             cx,
         );
         view.select_larger_syntax_node(&SelectLargerSyntaxNode, cx);

crates/editor/src/items.rs

@@ -886,7 +886,7 @@ impl SearchableItem for Editor {
         matches: Vec<Range<Anchor>>,
         cx: &mut ViewContext<Self>,
     ) {
-        self.unfold_ranges([matches[index].clone()], false, cx);
+        self.unfold_ranges([matches[index].clone()], false, true, cx);
         self.change_selections(Some(Autoscroll::fit()), cx, |s| {
             s.select_ranges([matches[index].clone()])
         });

crates/editor/src/multi_buffer.rs

@@ -1916,10 +1916,6 @@ impl MultiBufferSnapshot {
         }
     }
 
-    pub fn row_span(&self, display_row: u32) -> Range<Point> {
-        Point::new(display_row, 0)..Point::new(display_row, self.line_len(display_row))
-    }
-
     pub fn buffer_rows(&self, start_row: u32) -> MultiBufferRows {
         let mut result = MultiBufferRows {
             buffer_row_range: 0..0,

crates/search/src/project_search.rs

@@ -538,7 +538,7 @@ impl ProjectSearchView {
 
             let range_to_select = match_ranges[new_index].clone();
             self.results_editor.update(cx, |editor, cx| {
-                editor.unfold_ranges([range_to_select.clone()], false, cx);
+                editor.unfold_ranges([range_to_select.clone()], false, true, cx);
                 editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                     s.select_ranges([range_to_select])
                 });

crates/util/src/util.rs

@@ -145,6 +145,23 @@ where
     }
 }
 
+pub trait MapRangeEndsExt<R> {
+    fn map_endpoints<T, F>(self, f: F) -> Range<T>
+    where
+        Self: Sized,
+        F: Fn(R) -> T;
+}
+
+impl<R> MapRangeEndsExt<R> for Range<R> {
+    fn map_endpoints<T, F>(self, f: F) -> Range<T>
+    where
+        Self: Sized,
+        F: Fn(R) -> T,
+    {
+        f(self.start)..f(self.end)
+    }
+}
+
 pub struct LogErrorFuture<F>(F, log::Level);
 
 impl<F, T> Future for LogErrorFuture<F>