From 8ab49c325f15943dbd7c2c3dd35ab71592014de3 Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Tue, 13 Jan 2026 00:58:31 +0100 Subject: [PATCH] editor: Fix regression with folds and relative counting (#46658) Closes #46516 Release Notes: - Fixed a regression with relative line numbering that would occur with folds present (preview only) --- 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(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index e7d0ef9f0faa88e888328d2028dea2b2b6457421..bf58203e2d86807623e76d784a791b9b4aaf6975 100644 --- a/crates/editor/src/display_map.rs +++ b/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 for DisplayRow { type Output = Self; diff --git a/crates/editor/src/display_map/fold_map.rs b/crates/editor/src/display_map/fold_map.rs index bb0d6885acc2afd95e97fe9121acd2d0580554f3..8010dd5187d374bfcbe9b7549f72509ad3257e5d 100644 --- a/crates/editor/src/display_map/fold_map.rs +++ b/crates/editor/src/display_map/fold_map.rs @@ -1168,7 +1168,7 @@ pub struct Fold { } #[derive(Clone, Debug, Eq, PartialEq)] -pub struct FoldRange(Range); +pub struct FoldRange(pub(crate) Range); impl Deref for FoldRange { type Target = Range; diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 5e5a39f07ea014e54152d823778ccb475f2531dc..1e91a8718139d08a95edc0d192fd25a7c75adbbf 100644 --- a/crates/editor/src/editor.rs +++ b/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::() 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, - relative_to: DisplayRow, + current_selection_head: DisplayRow, count_wrapped_lines: bool, ) -> HashMap { - 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() } } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 2d4e4f82a5feedb5f4c2dcb676363a2246db208a..d1e817ccf950a34fd6ed2503a609dcab61d57f22 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -3341,7 +3341,7 @@ impl EditorElement { rows: Range, buffer_rows: &[RowInfo], active_rows: &BTreeMap, - relative_line_base: Option, + current_selection_head: Option, 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::>(), + &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, |_| {});