editor: Fix active line number regressions with relative counting (#45741)

Finn Evers created

Follow-up to https://github.com/zed-industries/zed/pull/45164 which
caused the active line number to always be `0` instead of the actual
current line number. No release notes since its only on nightly

Release Notes:

- N/A

Change summary

crates/editor/src/editor.rs       | 64 ++++++++++++++----------------
crates/editor/src/editor_tests.rs |  5 +
crates/editor/src/element.rs      | 70 ++++++++++++++++----------------
3 files changed, 68 insertions(+), 71 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -25298,36 +25298,34 @@ impl EditorSnapshot {
     /// Returns the line delta from `base` to `line` in the multibuffer, ignoring wrapped lines.
     ///
     /// This is positive if `base` is before `line`.
-    fn relative_line_delta(&self, base: DisplayRow, line: DisplayRow) -> i64 {
-        let point = DisplayPoint::new(line, 0).to_point(self);
-        self.relative_line_delta_to_point(base, point)
-    }
-
-    /// Returns the line delta from `base` to `point` in the multibuffer, ignoring wrapped lines.
-    ///
-    /// This is positive if `base` is before `point`.
-    pub fn relative_line_delta_to_point(&self, base: DisplayRow, point: Point) -> i64 {
-        let base_point = DisplayPoint::new(base, 0).to_point(self);
-        point.row as i64 - base_point.row as i64
-    }
-
-    /// Returns the line delta from `base` to `line` in the multibuffer, counting wrapped lines.
-    ///
-    /// This is positive if `base` is before `line`.
-    fn relative_wrapped_line_delta(&self, base: DisplayRow, line: DisplayRow) -> i64 {
+    fn relative_line_delta(
+        &self,
+        base: DisplayRow,
+        line: DisplayRow,
+        consider_wrapped_lines: bool,
+    ) -> i64 {
         let point = DisplayPoint::new(line, 0).to_point(self);
-        self.relative_wrapped_line_delta_to_point(base, point)
+        self.relative_line_delta_to_point(base, point, consider_wrapped_lines)
     }
 
-    /// Returns the line delta from `base` to `point` in the multibuffer, counting wrapped lines.
+    /// Returns the line delta from `base` to `point` in the multibuffer.
     ///
     /// This is positive if `base` is before `point`.
-    pub fn relative_wrapped_line_delta_to_point(&self, base: DisplayRow, point: Point) -> i64 {
+    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);
-        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();
-        wrap_row.0 as i64 - base_wrap_row.0 as i64
+        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();
+            wrap_row.0 as i64 - base_wrap_row.0 as i64
+        } else {
+            point.row as i64 - base_point.row as i64
+        }
     }
 
     /// Returns the unsigned relative line number to display for each row in `rows`.
@@ -25339,23 +25337,21 @@ impl EditorSnapshot {
         relative_to: DisplayRow,
         count_wrapped_lines: bool,
     ) -> HashMap<DisplayRow, u32> {
-        let initial_offset = if count_wrapped_lines {
-            self.relative_wrapped_line_delta(relative_to, rows.start)
-        } else {
-            self.relative_line_delta(relative_to, rows.start)
-        };
-        let display_row_infos = self
-            .row_infos(rows.start)
+        let initial_offset = self.relative_line_delta(relative_to, rows.start, count_wrapped_lines);
+
+        self.row_infos(rows.start)
             .take(rows.len())
             .enumerate()
-            .map(|(i, row_info)| (DisplayRow(rows.start.0 + i as u32), row_info));
-        display_row_infos
+            .map(|(i, row_info)| (DisplayRow(rows.start.0 + i as u32), row_info))
             .filter(|(_row, row_info)| {
                 row_info.buffer_row.is_some()
                     || (count_wrapped_lines && row_info.wrapped_buffer_row.is_some())
             })
             .enumerate()
-            .map(|(i, (row, _row_info))| (row, (initial_offset + i as i64).unsigned_abs() as u32))
+            .flat_map(|(i, (row, _row_info))| {
+                (row != relative_to)
+                    .then_some((row, (initial_offset + i as i64).unsigned_abs() as u32))
+            })
             .collect()
     }
 }

crates/editor/src/editor_tests.rs 🔗

@@ -28725,7 +28725,7 @@ fn test_relative_line_numbers(cx: &mut TestAppContext) {
             assert_eq!(
                 relative_number,
                 snapshot
-                    .relative_line_delta(display_row, base_display_row)
+                    .relative_line_delta(display_row, base_display_row, false)
                     .unsigned_abs() as u32,
             );
         }
@@ -28735,6 +28735,7 @@ fn test_relative_line_numbers(cx: &mut TestAppContext) {
             .into_iter()
             .enumerate()
             .map(|(i, row)| (DisplayRow(row), i.abs_diff(wrapped_base_row) as u32))
+            .filter(|(row, _)| *row != base_display_row)
             .collect_vec();
         let actual_relative_numbers = snapshot
             .calculate_relative_line_numbers(
@@ -28751,7 +28752,7 @@ fn test_relative_line_numbers(cx: &mut TestAppContext) {
             assert_eq!(
                 relative_number,
                 snapshot
-                    .relative_wrapped_line_delta(display_row, base_display_row)
+                    .relative_line_delta(display_row, base_display_row, true)
                     .unsigned_abs() as u32,
             );
         }

crates/editor/src/element.rs 🔗

@@ -4611,15 +4611,15 @@ impl EditorElement {
             );
 
             let line_number = show_line_numbers.then(|| {
-                let relative_number = relative_to.and_then(|base| match relative_line_numbers {
-                    RelativeLineNumbers::Disabled => None,
-                    RelativeLineNumbers::Enabled => {
-                        Some(snapshot.relative_line_delta_to_point(base, start_point))
-                    }
-                    RelativeLineNumbers::Wrapped => {
-                        Some(snapshot.relative_wrapped_line_delta_to_point(base, start_point))
-                    }
-                });
+                let relative_number = relative_to
+                    .filter(|_| relative_line_numbers != RelativeLineNumbers::Disabled)
+                    .map(|base| {
+                        snapshot.relative_line_delta_to_point(
+                            base,
+                            start_point,
+                            relative_line_numbers == RelativeLineNumbers::Wrapped,
+                        )
+                    });
                 let number = relative_number
                     .filter(|&delta| delta != 0)
                     .map(|delta| delta.unsigned_abs() as u32)
@@ -9055,14 +9055,8 @@ impl Element for EditorElement {
                     let em_advance = window.text_system().em_advance(font_id, font_size).unwrap();
                     let glyph_grid_cell = size(em_advance, line_height);
 
-                    let gutter_dimensions = snapshot
-                        .gutter_dimensions(
-                            font_id,
-                            font_size,
-                            style,
-                            window,
-                            cx,
-                        );
+                    let gutter_dimensions =
+                        snapshot.gutter_dimensions(font_id, font_size, style, window, cx);
                     let text_width = bounds.size.width - gutter_dimensions.width;
 
                     let settings = EditorSettings::get_global(cx);
@@ -9276,10 +9270,10 @@ impl Element for EditorElement {
                         };
 
                         let background_color = match diff_status.kind {
-                            DiffHunkStatusKind::Added =>
-                                cx.theme().colors().version_control_added,
-                            DiffHunkStatusKind::Deleted =>
-                                cx.theme().colors().version_control_deleted,
+                            DiffHunkStatusKind::Added => cx.theme().colors().version_control_added,
+                            DiffHunkStatusKind::Deleted => {
+                                cx.theme().colors().version_control_deleted
+                            }
                             DiffHunkStatusKind::Modified => {
                                 debug_panic!("modified diff status for row info");
                                 continue;
@@ -9423,25 +9417,26 @@ 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| {
-                        if editor.selections.count()==0 {
-                            return None;
-                        }
+                    let relative_row_base = self.editor.update(cx, |editor, cx| {
+                        (editor.selections.count() != 0).then(|| {
                             let newest = editor
                                 .selections
                                 .newest::<Point>(&editor.display_snapshot(cx));
-                            Some(SelectionLayout::new(
+
+                            SelectionLayout::new(
                                 newest,
                                 editor.selections.line_mode(),
                                 editor.cursor_offset_on_selection,
                                 editor.cursor_shape,
-                                &snapshot.display_snapshot,
+                                &snapshot,
                                 true,
                                 true,
                                 None,
                             )
-                            .head.row())
-                        });
+                            .head
+                            .row()
+                        })
+                    });
 
                     let mut breakpoint_rows = self.editor.update(cx, |editor, cx| {
                         editor.active_breakpoints(start_row..end_row, window, cx)
@@ -9601,9 +9596,10 @@ impl Element for EditorElement {
                                 cx,
                             );
                         } else {
-                            debug_panic!(
-                                "skipping recursive prepaint at max depth. renderer widths may be stale."
-                            );
+                            debug_panic!(concat!(
+                                "skipping recursive prepaint at max depth. ",
+                                "renderer widths may be stale."
+                            ));
                         }
                     }
 
@@ -9715,9 +9711,10 @@ impl Element for EditorElement {
                                 cx,
                             );
                         } else {
-                            debug_panic!(
-                                "skipping recursive prepaint at max depth. block layout may be stale."
-                            );
+                            debug_panic!(concat!(
+                                "skipping recursive prepaint at max depth. ",
+                                "block layout may be stale."
+                            ));
                         }
                     }
 
@@ -11723,6 +11720,7 @@ mod tests {
         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);
 
@@ -11869,6 +11867,7 @@ mod tests {
         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);
 
@@ -11924,6 +11923,7 @@ mod tests {
         assert_eq!(relative_rows[&DisplayRow(1)], 2);
         assert_eq!(relative_rows[&DisplayRow(2)], 1);
         // current line, even if deleted, 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);
     }