From 57ea23d161c99d7f3995c276cc77d33a51af17cf Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Sat, 27 Dec 2025 17:26:08 +0100 Subject: [PATCH] editor: Fix active line number regressions with relative counting (#45741) 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 --- 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(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index d985a4d269f2eaeb3fa6056192095b7913b579b6..44693466d7ad3303c36ddb032d7fbba862df547a 100644 --- a/crates/editor/src/editor.rs +++ b/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 { - 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() } } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index dc990d69f218dad083c888f75819cadae3f22cb1..155d027ee051d7ae198e4dd647defb8ea4afa05a 100644 --- a/crates/editor/src/editor_tests.rs +++ b/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, ); } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index ae8dd527122f19bdf7566e4dba3a504e6d19261d..e9d7c52b6aea4e2db5f7a0f8a192a7a524108244 100644 --- a/crates/editor/src/element.rs +++ b/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::(&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); }