editor: Fix regression with folds and relative counting (#46658)

Finn Evers created

Closes #46516

Release Notes:

- Fixed a regression with relative line numbering that would occur with
folds present (preview only)

Change summary

crates/editor/src/display_map.rs          |   6 +
crates/editor/src/display_map/fold_map.rs |   2 
crates/editor/src/editor.rs               |  67 +++++++++----
crates/editor/src/element.rs              | 116 +++++++++++++++++++++++-
4 files changed, 158 insertions(+), 33 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -1612,6 +1612,12 @@ impl Sub for DisplayPoint {
 #[serde(transparent)]
 pub struct DisplayRow(pub u32);
 
+impl DisplayRow {
+    pub(crate) fn as_display_point(&self) -> DisplayPoint {
+        DisplayPoint::new(*self, 0)
+    }
+}
+
 impl Add<DisplayRow> for DisplayRow {
     type Output = Self;
 

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

@@ -1168,7 +1168,7 @@ pub struct Fold {
 }
 
 #[derive(Clone, Debug, Eq, PartialEq)]
-pub struct FoldRange(Range<Anchor>);
+pub struct FoldRange(pub(crate) Range<Anchor>);
 
 impl Deref for FoldRange {
     type Target = Range<Anchor>;

crates/editor/src/editor.rs 🔗

@@ -25583,31 +25583,37 @@ impl EditorSnapshot {
     /// This is positive if `base` is before `line`.
     fn relative_line_delta(
         &self,
-        base: DisplayRow,
-        line: DisplayRow,
+        current_selection_head: DisplayRow,
+        first_visible_row: DisplayRow,
         consider_wrapped_lines: bool,
     ) -> i64 {
-        let point = DisplayPoint::new(line, 0).to_point(self);
-        self.relative_line_delta_to_point(base, point, consider_wrapped_lines)
-    }
+        let current_selection_head = current_selection_head.as_display_point().to_point(self);
+        let first_visible_row = first_visible_row.as_display_point().to_point(self);
 
-    /// Returns the line delta from `base` to `point` in the multibuffer.
-    ///
-    /// This is positive if `base` is before `point`.
-    pub fn relative_line_delta_to_point(
-        &self,
-        base: DisplayRow,
-        point: Point,
-        consider_wrapped_lines: bool,
-    ) -> i64 {
-        let base_point = DisplayPoint::new(base, 0).to_point(self);
         if consider_wrapped_lines {
             let wrap_snapshot = self.wrap_snapshot();
-            let base_wrap_row = wrap_snapshot.make_wrap_point(base_point, Bias::Left).row();
-            let wrap_row = wrap_snapshot.make_wrap_point(point, Bias::Left).row();
+            let base_wrap_row = wrap_snapshot
+                .make_wrap_point(current_selection_head, Bias::Left)
+                .row();
+            let wrap_row = wrap_snapshot
+                .make_wrap_point(first_visible_row, Bias::Left)
+                .row();
             wrap_row.0 as i64 - base_wrap_row.0 as i64
         } else {
-            point.row as i64 - base_point.row as i64
+            let folds = if current_selection_head < first_visible_row {
+                self.folds_in_range(current_selection_head..first_visible_row)
+            } else {
+                self.folds_in_range(first_visible_row..current_selection_head)
+            };
+
+            let folded_lines = folds
+                .map(|fold| {
+                    let range = fold.range.0.to_point(self);
+                    range.end.row.saturating_sub(range.start.row)
+                })
+                .sum::<u32>() as i64;
+
+            first_visible_row.row as i64 - current_selection_head.row as i64 + folded_lines
         }
     }
 
@@ -25617,10 +25623,12 @@ impl EditorSnapshot {
     pub fn calculate_relative_line_numbers(
         &self,
         rows: &Range<DisplayRow>,
-        relative_to: DisplayRow,
+        current_selection_head: DisplayRow,
         count_wrapped_lines: bool,
     ) -> HashMap<DisplayRow, u32> {
-        let initial_offset = self.relative_line_delta(relative_to, rows.start, count_wrapped_lines);
+        let initial_offset =
+            self.relative_line_delta(current_selection_head, rows.start, count_wrapped_lines);
+        let current_selection_point = current_selection_head.as_display_point().to_point(self);
 
         self.row_infos(rows.start)
             .take(rows.len())
@@ -25631,10 +25639,23 @@ impl EditorSnapshot {
                     || (count_wrapped_lines && row_info.wrapped_buffer_row.is_some())
             })
             .enumerate()
-            .flat_map(|(i, (row, _row_info))| {
-                (row != relative_to)
-                    .then_some((row, (initial_offset + i as i64).unsigned_abs() as u32))
+            .filter(|(_, (row, row_info))| {
+                // We want to check here that
+                // - the row is not the current selection head to ensure the current
+                // line has absolute numbering
+                // - similarly, should the selection head live in a soft-wrapped line
+                // and we are not counting those, that the parent line keeps its
+                // absolute number
+                // - lastly, if we are in a deleted line, it is fine to number this
+                // relative with 0, as otherwise it would have no line number at all
+                (*row != current_selection_head
+                    && (count_wrapped_lines
+                        || row_info.buffer_row != Some(current_selection_point.row)))
+                    || row_info
+                        .diff_status
+                        .is_some_and(|status| status.is_deleted())
             })
+            .map(|(i, (row, _))| (row, (initial_offset + i as i64).unsigned_abs() as u32))
             .collect()
     }
 }

crates/editor/src/element.rs 🔗

@@ -3341,7 +3341,7 @@ impl EditorElement {
         rows: Range<DisplayRow>,
         buffer_rows: &[RowInfo],
         active_rows: &BTreeMap<DisplayRow, LineHighlightSpec>,
-        relative_line_base: Option<DisplayRow>,
+        current_selection_head: Option<DisplayRow>,
         snapshot: &EditorSnapshot,
         window: &mut Window,
         cx: &mut App,
@@ -3356,9 +3356,14 @@ impl EditorElement {
         let relative = self.editor.read(cx).relative_line_numbers(cx);
 
         let relative_line_numbers_enabled = relative.enabled();
-        let relative_rows = if relative_line_numbers_enabled && let Some(base) = relative_line_base
+        let relative_rows = if relative_line_numbers_enabled
+            && let Some(current_selection_head) = current_selection_head
         {
-            snapshot.calculate_relative_line_numbers(&rows, base, relative.wrapped())
+            snapshot.calculate_relative_line_numbers(
+                &rows,
+                current_selection_head,
+                relative.wrapped(),
+            )
         } else {
             Default::default()
         };
@@ -4770,12 +4775,13 @@ impl EditorElement {
             );
 
             let line_number = show_line_numbers.then(|| {
+                let start_display_row = start_point.to_display_point(snapshot).row();
                 let relative_number = relative_to
                     .filter(|_| relative_line_numbers != RelativeLineNumbers::Disabled)
                     .map(|base| {
-                        snapshot.relative_line_delta_to_point(
+                        snapshot.relative_line_delta(
                             base,
-                            start_point,
+                            start_display_row,
                             relative_line_numbers == RelativeLineNumbers::Wrapped,
                         )
                     });
@@ -9778,7 +9784,7 @@ impl Element for EditorElement {
                         );
 
                     // relative rows are based on newest selection, even outside the visible area
-                    let relative_row_base = self.editor.update(cx, |editor, cx| {
+                    let current_selection_head = self.editor.update(cx, |editor, cx| {
                         (editor.selections.count() != 0).then(|| {
                             let newest = editor
                                 .selections
@@ -9816,7 +9822,7 @@ impl Element for EditorElement {
                         start_row..end_row,
                         &row_infos,
                         &active_rows,
-                        relative_row_base,
+                        current_selection_head,
                         &snapshot,
                         window,
                         cx,
@@ -10151,7 +10157,7 @@ impl Element for EditorElement {
                             &text_hitbox,
                             &style,
                             relative,
-                            relative_row_base,
+                            current_selection_head,
                             window,
                             cx,
                         )
@@ -11965,7 +11971,7 @@ mod tests {
         editor_tests::{init_test, update_test_language_settings},
     };
     use gpui::{TestAppContext, VisualTestContext};
-    use language::language_settings;
+    use language::{Buffer, language_settings, tree_sitter_python};
     use log::info;
     use std::num::NonZeroU32;
     use util::test::sample_text;
@@ -12176,6 +12182,98 @@ mod tests {
         );
     }
 
+    #[gpui::test]
+    async fn test_layout_line_numbers_with_folded_lines(cx: &mut TestAppContext) {
+        init_test(cx, |_| {});
+
+        let python_lang = languages::language("python", tree_sitter_python::LANGUAGE.into());
+
+        let window = cx.add_window(|window, cx| {
+            let buffer = cx.new(|cx| {
+                Buffer::local(
+                    indoc::indoc! {"
+                        fn test() -> int {
+                            return 2;
+                        }
+
+                        fn another_test() -> int {
+                            # This is a very peculiar method that is hard to grasp.
+                            return 4;
+                        }
+                    "},
+                    cx,
+                )
+                .with_language(python_lang, cx)
+            });
+
+            let buffer = MultiBuffer::build_from_buffer(buffer, cx);
+            Editor::new(EditorMode::full(), buffer, None, window, cx)
+        });
+
+        let editor = window.root(cx).unwrap();
+        let style = editor.update(cx, |editor, cx| editor.style(cx).clone());
+        let line_height = window
+            .update(cx, |_, window, _| {
+                style.text.line_height_in_pixels(window.rem_size())
+            })
+            .unwrap();
+        let element = EditorElement::new(&editor, style);
+        let snapshot = window
+            .update(cx, |editor, window, cx| {
+                editor.fold_at(MultiBufferRow(0), window, cx);
+                editor.snapshot(window, cx)
+            })
+            .unwrap();
+
+        let layouts = cx
+            .update_window(*window, |_, window, cx| {
+                element.layout_line_numbers(
+                    None,
+                    GutterDimensions {
+                        left_padding: Pixels::ZERO,
+                        right_padding: Pixels::ZERO,
+                        width: px(30.0),
+                        margin: Pixels::ZERO,
+                        git_blame_entries_width: None,
+                    },
+                    line_height,
+                    gpui::Point::default(),
+                    DisplayRow(0)..DisplayRow(6),
+                    &(0..6)
+                        .map(|row| RowInfo {
+                            buffer_row: Some(row),
+                            ..Default::default()
+                        })
+                        .collect::<Vec<_>>(),
+                    &BTreeMap::default(),
+                    Some(DisplayRow(3)),
+                    &snapshot,
+                    window,
+                    cx,
+                )
+            })
+            .unwrap();
+        assert_eq!(layouts.len(), 6);
+
+        let relative_rows = window
+            .update(cx, |editor, window, cx| {
+                let snapshot = editor.snapshot(window, cx);
+                snapshot.calculate_relative_line_numbers(
+                    &(DisplayRow(0)..DisplayRow(6)),
+                    DisplayRow(3),
+                    false,
+                )
+            })
+            .unwrap();
+        assert_eq!(relative_rows[&DisplayRow(0)], 3);
+        assert_eq!(relative_rows[&DisplayRow(1)], 2);
+        assert_eq!(relative_rows[&DisplayRow(2)], 1);
+        // current line has no relative number
+        assert!(!relative_rows.contains_key(&DisplayRow(3)));
+        assert_eq!(relative_rows[&DisplayRow(4)], 1);
+        assert_eq!(relative_rows[&DisplayRow(5)], 2);
+    }
+
     #[gpui::test]
     fn test_layout_line_numbers_wrapping(cx: &mut TestAppContext) {
         init_test(cx, |_| {});