Merge pull request #2287 from zed-industries/fix-fold-range-finding

Mikayla Maki created

Fix code folds with wraps

Change summary

crates/editor/src/display_map.rs          |  86 ++++++++++++++------
crates/editor/src/display_map/fold_map.rs |  20 ++-
crates/editor/src/display_map/tab_map.rs  |   2 
crates/editor/src/editor.rs               |  95 ++++++++--------------
crates/editor/src/element.rs              | 103 +++++++++++++-----------
crates/editor/src/multi_buffer.rs         |   6 +
6 files changed, 168 insertions(+), 144 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -4,7 +4,7 @@ mod tab_map;
 mod wrap_map;
 
 use crate::{Anchor, AnchorRangeExt, MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint};
-use block_map::{BlockMap, BlockPoint};
+pub use block_map::{BlockMap, BlockPoint};
 use collections::{HashMap, HashSet};
 use fold_map::FoldMap;
 use gpui::{
@@ -24,6 +24,8 @@ pub use block_map::{
     BlockDisposition, BlockId, BlockProperties, BlockStyle, RenderBlock, TransformBlock,
 };
 
+use self::tab_map::TabSnapshot;
+
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum FoldStatus {
     Folded,
@@ -544,11 +546,8 @@ impl DisplaySnapshot {
         self.folds_snapshot.intersects_fold(offset)
     }
 
-    pub fn is_line_folded(&self, display_row: u32) -> bool {
-        let block_point = BlockPoint(Point::new(display_row, 0));
-        let wrap_point = self.blocks_snapshot.to_wrap_point(block_point);
-        let tab_point = self.wraps_snapshot.to_tab_point(wrap_point);
-        self.folds_snapshot.is_line_folded(tab_point.row())
+    pub fn is_line_folded(&self, buffer_row: u32) -> bool {
+        self.folds_snapshot.is_line_folded(buffer_row)
     }
 
     pub fn is_block_line(&self, display_row: u32) -> bool {
@@ -594,6 +593,32 @@ impl DisplaySnapshot {
         (indent, is_blank)
     }
 
+    pub fn line_indent_for_buffer_row(&self, buffer_row: u32) -> (u32, bool) {
+        let (buffer, range) = self
+            .buffer_snapshot
+            .buffer_line_for_row(buffer_row)
+            .unwrap();
+        let chars = buffer.chars_at(Point::new(range.start.row, 0));
+
+        let mut is_blank = false;
+        let indent_size = TabSnapshot::expand_tabs(
+            chars.take_while(|c| {
+                if *c == ' ' || *c == '\t' {
+                    true
+                } else {
+                    if *c == '\n' {
+                        is_blank = true;
+                    }
+                    false
+                }
+            }),
+            buffer.line_len(buffer_row) as usize, // Never collapse
+            self.tabs_snapshot.tab_size,
+        );
+
+        (indent_size as u32, is_blank)
+    }
+
     pub fn line_len(&self, row: u32) -> u32 {
         self.blocks_snapshot.line_len(row)
     }
@@ -602,49 +627,54 @@ impl DisplaySnapshot {
         self.blocks_snapshot.longest_row()
     }
 
-    pub fn fold_for_line(self: &Self, display_row: u32) -> Option<FoldStatus> {
-        if self.is_foldable(display_row) {
-            Some(FoldStatus::Foldable)
-        } else if self.is_line_folded(display_row) {
+    pub fn fold_for_line(self: &Self, buffer_row: u32) -> Option<FoldStatus> {
+        if self.is_line_folded(buffer_row) {
             Some(FoldStatus::Folded)
+        } else if self.is_foldable(buffer_row) {
+            Some(FoldStatus::Foldable)
         } else {
             None
         }
     }
 
-    pub fn is_foldable(self: &Self, row: u32) -> bool {
-        let max_point = self.max_point();
-        if row >= max_point.row() {
+    pub fn is_foldable(self: &Self, buffer_row: u32) -> bool {
+        let max_row = self.buffer_snapshot.max_buffer_row();
+        if buffer_row >= max_row {
             return false;
         }
 
-        let (start_indent, is_blank) = self.line_indent(row);
+        let (indent_size, is_blank) = self.line_indent_for_buffer_row(buffer_row);
         if is_blank {
             return false;
         }
 
-        for display_row in next_rows(row, self) {
-            let (indent, is_blank) = self.line_indent(display_row);
-            if !is_blank {
-                return indent > start_indent;
+        for next_row in (buffer_row + 1)..=max_row {
+            let (next_indent_size, next_line_is_blank) = self.line_indent_for_buffer_row(next_row);
+            if next_indent_size > indent_size {
+                return true;
+            } else if !next_line_is_blank {
+                break;
             }
         }
 
-        return false;
+        false
     }
 
-    pub fn foldable_range(self: &Self, row: u32) -> Option<Range<DisplayPoint>> {
-        let start = DisplayPoint::new(row, self.line_len(row));
-
-        if self.is_foldable(row) && !self.is_line_folded(start.row()) {
-            let (start_indent, _) = self.line_indent(row);
-            let max_point = self.max_point();
+    pub fn foldable_range(self: &Self, buffer_row: u32) -> Option<Range<Point>> {
+        let start = Point::new(buffer_row, self.buffer_snapshot.line_len(buffer_row));
+        if self.is_foldable(start.row) && !self.is_line_folded(start.row) {
+            let (start_indent, _) = self.line_indent_for_buffer_row(buffer_row);
+            let max_point = self.buffer_snapshot.max_point();
             let mut end = None;
 
-            for row in next_rows(row, self) {
-                let (indent, is_blank) = self.line_indent(row);
+            for row in (buffer_row + 1)..=max_point.row {
+                let (indent, is_blank) = self.line_indent_for_buffer_row(row);
                 if !is_blank && indent <= start_indent {
-                    end = Some(DisplayPoint::new(row - 1, self.line_len(row - 1)));
+                    let prev_row = row - 1;
+                    end = Some(Point::new(
+                        prev_row,
+                        self.buffer_snapshot.line_len(prev_row),
+                    ));
                     break;
                 }
             }

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

@@ -639,14 +639,14 @@ impl FoldSnapshot {
         cursor.item().map_or(false, |t| t.output_text.is_some())
     }
 
-    pub fn is_line_folded(&self, output_row: u32) -> bool {
-        let mut cursor = self.transforms.cursor::<FoldPoint>();
-        cursor.seek(&FoldPoint::new(output_row, 0), Bias::Right, &());
+    pub fn is_line_folded(&self, buffer_row: u32) -> bool {
+        let mut cursor = self.transforms.cursor::<Point>();
+        cursor.seek(&Point::new(buffer_row, 0), Bias::Right, &());
         while let Some(transform) = cursor.item() {
             if transform.output_text.is_some() {
                 return true;
             }
-            if cursor.end(&()).row() == output_row {
+            if cursor.end(&()).row == buffer_row {
                 cursor.next(&())
             } else {
                 break;
@@ -1214,6 +1214,7 @@ pub type FoldEdit = Edit<FoldOffset>;
 mod tests {
     use super::*;
     use crate::{MultiBuffer, ToPoint};
+    use collections::HashSet;
     use rand::prelude::*;
     use settings::Settings;
     use std::{cmp::Reverse, env, mem, sync::Arc};
@@ -1593,10 +1594,13 @@ mod tests {
                 fold_row += 1;
             }
 
-            for fold_range in map.merged_fold_ranges() {
-                let fold_point =
-                    snapshot.to_fold_point(fold_range.start.to_point(&buffer_snapshot), Right);
-                assert!(snapshot.is_line_folded(fold_point.row()));
+            let fold_start_rows = map
+                .merged_fold_ranges()
+                .iter()
+                .map(|range| range.start.to_point(&buffer_snapshot).row)
+                .collect::<HashSet<_>>();
+            for row in fold_start_rows {
+                assert!(snapshot.is_line_folded(row));
             }
 
             for _ in 0..5 {

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

@@ -262,7 +262,7 @@ impl TabSnapshot {
             .to_buffer_point(&self.fold_snapshot)
     }
 
-    fn expand_tabs(
+    pub fn expand_tabs(
         chars: impl Iterator<Item = char>,
         column: usize,
         tab_size: NonZeroU32,

crates/editor/src/editor.rs 🔗

@@ -163,12 +163,12 @@ pub struct ToggleComments {
 
 #[derive(Clone, Default, Deserialize, PartialEq)]
 pub struct FoldAt {
-    pub display_row: u32,
+    pub buffer_row: u32,
 }
 
 #[derive(Clone, Default, Deserialize, PartialEq)]
 pub struct UnfoldAt {
-    pub display_row: u32,
+    pub buffer_row: u32,
 }
 
 #[derive(Clone, Default, Deserialize, PartialEq)]
@@ -2705,31 +2705,26 @@ impl Editor {
 
     pub fn render_fold_indicators(
         &self,
-        fold_data: Option<Vec<(u32, FoldStatus)>>,
-        active_rows: &BTreeMap<u32, bool>,
+        fold_data: Vec<Option<(FoldStatus, u32, bool)>>,
         style: &EditorStyle,
         gutter_hovered: bool,
         line_height: f32,
         gutter_margin: f32,
         cx: &mut RenderContext<Self>,
-    ) -> Option<Vec<(u32, ElementBox)>> {
+    ) -> Vec<Option<ElementBox>> {
         enum FoldIndicators {}
 
         let style = style.folds.clone();
 
-        fold_data.map(|fold_data| {
-            fold_data
-                .iter()
-                .copied()
-                .filter_map(|(fold_location, fold_status)| {
-                    (gutter_hovered
-                        || fold_status == FoldStatus::Folded
-                        || !*active_rows.get(&fold_location).unwrap_or(&true))
-                    .then(|| {
-                        (
-                            fold_location,
+        fold_data
+            .iter()
+            .enumerate()
+            .map(|(ix, fold_data)| {
+                fold_data
+                    .map(|(fold_status, buffer_row, active)| {
+                        (active || gutter_hovered || fold_status == FoldStatus::Folded).then(|| {
                             MouseEventHandler::<FoldIndicators>::new(
-                                fold_location as usize,
+                                ix as usize,
                                 cx,
                                 |mouse_state, _| -> ElementBox {
                                     Svg::new(match fold_status {
@@ -2760,21 +2755,17 @@ impl Editor {
                             .on_click(MouseButton::Left, {
                                 move |_, cx| {
                                     cx.dispatch_any_action(match fold_status {
-                                        FoldStatus::Folded => Box::new(UnfoldAt {
-                                            display_row: fold_location,
-                                        }),
-                                        FoldStatus::Foldable => Box::new(FoldAt {
-                                            display_row: fold_location,
-                                        }),
+                                        FoldStatus::Folded => Box::new(UnfoldAt { buffer_row }),
+                                        FoldStatus::Foldable => Box::new(FoldAt { buffer_row }),
                                     });
                                 }
                             })
-                            .boxed(),
-                        )
+                            .boxed()
+                        })
                     })
-                })
-                .collect()
-        })
+                    .flatten()
+            })
+            .collect()
     }
 
     pub fn context_menu_visible(&self) -> bool {
@@ -5759,18 +5750,16 @@ impl Editor {
 
         let selections = self.selections.all::<Point>(cx);
         for selection in selections {
-            let range = selection.display_range(&display_map).sorted();
-            let buffer_start_row = range.start.to_point(&display_map).row;
+            let range = selection.range().sorted();
+            let buffer_start_row = range.start.row;
 
-            for row in (0..=range.end.row()).rev() {
-                let fold_range = display_map.foldable_range(row).map(|range| {
-                    range.start.to_point(&display_map)..range.end.to_point(&display_map)
-                });
+            for row in (0..=range.end.row).rev() {
+                let fold_range = display_map.foldable_range(row);
 
                 if let Some(fold_range) = fold_range {
                     if fold_range.end.row >= buffer_start_row {
                         fold_ranges.push(fold_range);
-                        if row <= range.start.row() {
+                        if row <= range.start.row {
                             break;
                         }
                     }
@@ -5782,19 +5771,15 @@ impl Editor {
     }
 
     pub fn fold_at(&mut self, fold_at: &FoldAt, cx: &mut ViewContext<Self>) {
-        let display_row = fold_at.display_row;
-
+        let buffer_row = fold_at.buffer_row;
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
 
-        if let Some(fold_range) = display_map.foldable_range(display_row) {
+        if let Some(fold_range) = display_map.foldable_range(buffer_row) {
             let autoscroll = self
                 .selections
                 .all::<Point>(cx)
                 .iter()
-                .any(|selection| fold_range.overlaps(&selection.display_range(&display_map)));
-
-            let fold_range =
-                fold_range.start.to_point(&display_map)..fold_range.end.to_point(&display_map);
+                .any(|selection| fold_range.overlaps(&selection.range()));
 
             self.fold_ranges(std::iter::once(fold_range), autoscroll, cx);
         }
@@ -5822,25 +5807,19 @@ impl Editor {
     pub fn unfold_at(&mut self, unfold_at: &UnfoldAt, cx: &mut ViewContext<Self>) {
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
 
-        let intersection_range = DisplayPoint::new(unfold_at.display_row, 0)
-            ..DisplayPoint::new(
-                unfold_at.display_row,
-                display_map.line_len(unfold_at.display_row),
+        let intersection_range = Point::new(unfold_at.buffer_row, 0)
+            ..Point::new(
+                unfold_at.buffer_row,
+                display_map.buffer_snapshot.line_len(unfold_at.buffer_row),
             );
 
-        let autoscroll =
-            self.selections.all::<Point>(cx).iter().any(|selection| {
-                intersection_range.overlaps(&selection.display_range(&display_map))
-            });
-
-        let display_point = DisplayPoint::new(unfold_at.display_row, 0).to_point(&display_map);
-
-        let mut point_range = display_point..display_point;
-
-        point_range.start.column = 0;
-        point_range.end.column = display_map.buffer_snapshot.line_len(point_range.end.row);
+        let autoscroll = self
+            .selections
+            .all::<Point>(cx)
+            .iter()
+            .any(|selection| selection.range().overlaps(&intersection_range));
 
-        self.unfold_ranges(std::iter::once(point_range), true, autoscroll, cx)
+        self.unfold_ranges(std::iter::once(intersection_range), true, autoscroll, cx)
     }
 
     pub fn fold_selected_ranges(&mut self, _: &FoldSelectedRanges, cx: &mut ViewContext<Self>) {

crates/editor/src/element.rs 🔗

@@ -574,6 +574,20 @@ impl EditorElement {
             }
         }
 
+        for (ix, fold_indicator) in layout.fold_indicators.iter_mut().enumerate() {
+            if let Some(indicator) = fold_indicator.as_mut() {
+                let mut x = bounds.width() - layout.gutter_padding;
+                let mut y = ix as f32 * line_height - scroll_top;
+
+                x += ((layout.gutter_padding + layout.gutter_margin) - indicator.size().x()) / 2.;
+                y += (line_height - indicator.size().y()) / 2.;
+
+                let indicator_origin = bounds.origin() + vec2f(x, y);
+
+                indicator.paint(indicator_origin, visible_bounds, cx);
+            }
+        }
+
         if let Some((row, indicator)) = layout.code_actions_indicator.as_mut() {
             let mut x = 0.;
             let mut y = *row as f32 * line_height - scroll_top;
@@ -581,19 +595,6 @@ impl EditorElement {
             y += (line_height - indicator.size().y()) / 2.;
             indicator.paint(bounds.origin() + vec2f(x, y), visible_bounds, cx);
         }
-
-        layout.fold_indicators.as_mut().map(|fold_indicators| {
-            for (line, fold_indicator) in fold_indicators.iter_mut() {
-                let mut x = bounds.width() - layout.gutter_padding;
-                let mut y = *line as f32 * line_height - scroll_top;
-
-                x += ((layout.gutter_padding + layout.gutter_margin) - fold_indicator.size().x())
-                    / 2.;
-                y += (line_height - fold_indicator.size().y()) / 2.;
-
-                fold_indicator.paint(bounds.origin() + vec2f(x, y), visible_bounds, cx);
-            }
-        });
     }
 
     fn paint_diff_hunks(bounds: RectF, layout: &mut LayoutState, cx: &mut PaintContext) {
@@ -739,10 +740,15 @@ impl EditorElement {
                 });
 
                 let display_row = range.start.row();
+
+                let buffer_row = DisplayPoint::new(display_row, 0)
+                    .to_point(&layout.position_map.snapshot.display_snapshot)
+                    .row;
+
                 cx.scene.push_mouse_region(
                     MouseRegion::new::<FoldMarkers>(self.view.id(), *id as usize, bound)
                         .on_click(MouseButton::Left, move |_, cx| {
-                            cx.dispatch_action(UnfoldAt { display_row })
+                            cx.dispatch_action(UnfoldAt { buffer_row })
                         })
                         .with_notify_on_hover(true)
                         .with_notify_on_click(true),
@@ -1181,24 +1187,6 @@ impl EditorElement {
             .width()
     }
 
-    fn get_fold_indicators(
-        &self,
-        is_singleton: bool,
-        display_rows: Range<u32>,
-        snapshot: &EditorSnapshot,
-    ) -> Option<Vec<(u32, FoldStatus)>> {
-        is_singleton.then(|| {
-            display_rows
-                .into_iter()
-                .filter_map(|display_row| {
-                    snapshot
-                        .fold_for_line(display_row)
-                        .map(|fold_status| (display_row, fold_status))
-                })
-                .collect()
-        })
-    }
-
     //Folds contained in a hunk are ignored apart from shrinking visual size
     //If a fold contains any hunks then that fold line is marked as modified
     fn layout_git_gutters(
@@ -1226,12 +1214,17 @@ impl EditorElement {
         &self,
         rows: Range<u32>,
         active_rows: &BTreeMap<u32, bool>,
+        is_singleton: bool,
         snapshot: &EditorSnapshot,
         cx: &LayoutContext,
-    ) -> Vec<Option<text_layout::Line>> {
+    ) -> (
+        Vec<Option<text_layout::Line>>,
+        Vec<Option<(FoldStatus, BufferRow, bool)>>,
+    ) {
         let style = &self.style;
         let include_line_numbers = snapshot.mode == EditorMode::Full;
         let mut line_number_layouts = Vec::with_capacity(rows.len());
+        let mut fold_statuses = Vec::with_capacity(rows.len());
         let mut line_number = String::new();
         for (ix, row) in snapshot
             .buffer_rows(rows.start)
@@ -1239,10 +1232,10 @@ impl EditorElement {
             .enumerate()
         {
             let display_row = rows.start + ix as u32;
-            let color = if active_rows.contains_key(&display_row) {
-                style.line_number_active
+            let (active, color) = if active_rows.contains_key(&display_row) {
+                (true, style.line_number_active)
             } else {
-                style.line_number
+                (false, style.line_number)
             };
             if let Some(buffer_row) = row {
                 if include_line_numbers {
@@ -1260,13 +1253,23 @@ impl EditorElement {
                             },
                         )],
                     )));
+                    fold_statuses.push(
+                        is_singleton
+                            .then(|| {
+                                snapshot
+                                    .fold_for_line(buffer_row)
+                                    .map(|fold_status| (fold_status, buffer_row, active))
+                            })
+                            .flatten(),
+                    )
                 }
             } else {
+                fold_statuses.push(None);
                 line_number_layouts.push(None);
             }
         }
 
-        line_number_layouts
+        (line_number_layouts, fold_statuses)
     }
 
     fn layout_lines(
@@ -1797,13 +1800,16 @@ impl Element for EditorElement {
             })
             .collect();
 
-        let line_number_layouts =
-            self.layout_line_numbers(start_row..end_row, &active_rows, &snapshot, cx);
+        let (line_number_layouts, fold_statuses) = self.layout_line_numbers(
+            start_row..end_row,
+            &active_rows,
+            is_singleton,
+            &snapshot,
+            cx,
+        );
 
         let display_hunks = self.layout_git_gutters(start_row..end_row, &snapshot);
 
-        let folds = self.get_fold_indicators(is_singleton, start_row..end_row, &snapshot);
-
         let scrollbar_row_range = scroll_position.y()..(scroll_position.y() + height_in_lines);
 
         let mut max_visible_line_width = 0.0;
@@ -1896,8 +1902,7 @@ impl Element for EditorElement {
             mode = view.mode;
 
             view.render_fold_indicators(
-                folds,
-                &active_rows,
+                fold_statuses,
                 &style,
                 view.gutter_hovered,
                 line_height,
@@ -1929,8 +1934,8 @@ impl Element for EditorElement {
             );
         }
 
-        fold_indicators.as_mut().map(|fold_indicators| {
-            for (_, indicator) in fold_indicators.iter_mut() {
+        for fold_indicator in fold_indicators.iter_mut() {
+            if let Some(indicator) = fold_indicator.as_mut() {
                 indicator.layout(
                     SizeConstraint::strict_along(
                         Axis::Vertical,
@@ -1939,7 +1944,7 @@ impl Element for EditorElement {
                     cx,
                 );
             }
-        });
+        }
 
         if let Some((_, hover_popovers)) = hover.as_mut() {
             for hover_popover in hover_popovers.iter_mut() {
@@ -2123,7 +2128,7 @@ pub struct LayoutState {
     context_menu: Option<(DisplayPoint, ElementBox)>,
     code_actions_indicator: Option<(u32, ElementBox)>,
     hover_popovers: Option<(DisplayPoint, Vec<ElementBox>)>,
-    fold_indicators: Option<Vec<(u32, ElementBox)>>,
+    fold_indicators: Vec<Option<ElementBox>>,
 }
 
 pub struct PositionMap {
@@ -2524,7 +2529,9 @@ mod tests {
             let snapshot = editor.snapshot(cx);
             let mut presenter = cx.build_presenter(window_id, 30., Default::default());
             let layout_cx = presenter.build_layout_context(Vector2F::zero(), false, cx);
-            element.layout_line_numbers(0..6, &Default::default(), &snapshot, &layout_cx)
+            element
+                .layout_line_numbers(0..6, &Default::default(), false, &snapshot, &layout_cx)
+                .0
         });
         assert_eq!(layouts.len(), 6);
     }

crates/editor/src/multi_buffer.rs 🔗

@@ -2194,7 +2194,11 @@ impl MultiBufferSnapshot {
 
     pub fn buffer_line_for_row(&self, row: u32) -> Option<(&BufferSnapshot, Range<Point>)> {
         let mut cursor = self.excerpts.cursor::<Point>();
-        cursor.seek(&Point::new(row, 0), Bias::Right, &());
+        let point = Point::new(row, 0);
+        cursor.seek(&point, Bias::Right, &());
+        if cursor.item().is_none() && *cursor.start() == point {
+            cursor.prev(&());
+        }
         if let Some(excerpt) = cursor.item() {
             let overshoot = row - cursor.start().row;
             let excerpt_start = excerpt.range.context.start.to_point(&excerpt.buffer);