From 196fd65601af0b22cba3f6f0fbbda821b0403427 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 3 Dec 2024 23:01:32 -0800 Subject: [PATCH] Fix panic folding in multi-buffers (#21511) Closes #19054 Rename `max_buffer_row()` to `widest_line_number()` to (hopefully) prevent people assuming it means the same as `max_point().row`. Release Notes: - Fixed a panic when folding in a multibuffer --- crates/editor/src/display_map.rs | 13 ++++++------- crates/editor/src/display_map/inlay_map.rs | 2 +- crates/editor/src/editor.rs | 14 +++++++++++--- crates/editor/src/element.rs | 8 +------- crates/editor/src/movement.rs | 6 +++--- crates/multi_buffer/src/multi_buffer.rs | 19 +++++++++++-------- crates/vim/src/command.rs | 12 +++++++----- crates/vim/src/motion.rs | 2 +- crates/vim/src/normal/yank.rs | 4 ++-- crates/vim/src/object.rs | 6 +++--- 10 files changed, 46 insertions(+), 40 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index b95c9312c5..a75c2ce9fa 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -684,8 +684,8 @@ impl DisplaySnapshot { .map(|row| row.map(MultiBufferRow)) } - pub fn max_buffer_row(&self) -> MultiBufferRow { - self.buffer_snapshot.max_buffer_row() + pub fn widest_line_number(&self) -> u32 { + self.buffer_snapshot.widest_line_number() } pub fn prev_line_boundary(&self, mut point: MultiBufferPoint) -> (Point, DisplayPoint) { @@ -726,11 +726,10 @@ impl DisplaySnapshot { // used by line_mode selections and tries to match vim behavior pub fn expand_to_line(&self, range: Range) -> Range { + let max_row = self.buffer_snapshot.max_row().0; let new_start = if range.start.row == 0 { MultiBufferPoint::new(0, 0) - } else if range.start.row == self.max_buffer_row().0 - || (range.end.column > 0 && range.end.row == self.max_buffer_row().0) - { + } else if range.start.row == max_row || (range.end.column > 0 && range.end.row == max_row) { MultiBufferPoint::new( range.start.row - 1, self.buffer_snapshot @@ -742,7 +741,7 @@ impl DisplaySnapshot { let new_end = if range.end.column == 0 { range.end - } else if range.end.row < self.max_buffer_row().0 { + } else if range.end.row < max_row { self.buffer_snapshot .clip_point(MultiBufferPoint::new(range.end.row + 1, 0), Bias::Left) } else { @@ -1127,7 +1126,7 @@ impl DisplaySnapshot { } pub fn starts_indent(&self, buffer_row: MultiBufferRow) -> bool { - let max_row = self.buffer_snapshot.max_buffer_row(); + let max_row = self.buffer_snapshot.max_row(); if buffer_row >= max_row { return false; } diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 673b9383bc..4598a5c015 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1019,7 +1019,7 @@ impl InlaySnapshot { let inlay_point = InlayPoint::new(row, 0); cursor.seek(&inlay_point, Bias::Left, &()); - let max_buffer_row = MultiBufferRow(self.buffer.max_point().row); + let max_buffer_row = self.buffer.max_row(); let mut buffer_point = cursor.start().1; let buffer_row = if row == 0 { MultiBufferRow(0) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 3901ba47aa..ea03d027c4 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6502,7 +6502,7 @@ impl Editor { let mut revert_changes = HashMap::default(); let multi_buffer_snapshot = self.buffer.read(cx).snapshot(cx); for hunk in hunks_for_rows( - Some(MultiBufferRow(0)..multi_buffer_snapshot.max_buffer_row()).into_iter(), + Some(MultiBufferRow(0)..multi_buffer_snapshot.max_row()).into_iter(), &multi_buffer_snapshot, ) { Self::prepare_revert_change(&mut revert_changes, self.buffer(), &hunk, cx); @@ -11051,10 +11051,14 @@ impl Editor { } fn fold_at_level(&mut self, fold_at: &FoldAtLevel, cx: &mut ViewContext) { + if !self.buffer.read(cx).is_singleton() { + return; + } + let fold_at_level = fold_at.level; let snapshot = self.buffer.read(cx).snapshot(cx); let mut to_fold = Vec::new(); - let mut stack = vec![(0, snapshot.max_buffer_row().0, 1)]; + let mut stack = vec![(0, snapshot.max_row().0, 1)]; while let Some((mut start_row, end_row, current_level)) = stack.pop() { while start_row < end_row { @@ -11083,10 +11087,14 @@ impl Editor { } pub fn fold_all(&mut self, _: &actions::FoldAll, cx: &mut ViewContext) { + if !self.buffer.read(cx).is_singleton() { + return; + } + let mut fold_ranges = Vec::new(); let snapshot = self.buffer.read(cx).snapshot(cx); - for row in 0..snapshot.max_buffer_row().0 { + for row in 0..snapshot.max_row().0 { if let Some(foldable_range) = self.snapshot(cx).crease_for_buffer_row(MultiBufferRow(row)) { diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index a82820f265..2bb40c4602 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -4141,13 +4141,7 @@ impl EditorElement { } fn max_line_number_width(&self, snapshot: &EditorSnapshot, cx: &WindowContext) -> Pixels { - let digit_count = snapshot - .max_buffer_row() - .next_row() - .as_f32() - .log10() - .floor() as usize - + 1; + let digit_count = (snapshot.widest_line_number() as f32).log10().floor() as usize + 1; self.column_pixels(digit_count, cx) } } diff --git a/crates/editor/src/movement.rs b/crates/editor/src/movement.rs index 8189dd2947..8fbf0d16f1 100644 --- a/crates/editor/src/movement.rs +++ b/crates/editor/src/movement.rs @@ -2,7 +2,7 @@ //! in editor given a given motion (e.g. it handles converting a "move left" command into coordinates in editor). It is exposed mostly for use by vim crate. use super::{Bias, DisplayPoint, DisplaySnapshot, SelectionGoal, ToDisplayPoint}; -use crate::{scroll::ScrollAnchor, CharKind, DisplayRow, EditorStyle, RowExt, ToOffset, ToPoint}; +use crate::{scroll::ScrollAnchor, CharKind, DisplayRow, EditorStyle, ToOffset, ToPoint}; use gpui::{Pixels, WindowTextSystem}; use language::Point; use multi_buffer::{MultiBufferRow, MultiBufferSnapshot}; @@ -382,12 +382,12 @@ pub fn end_of_paragraph( mut count: usize, ) -> DisplayPoint { let point = display_point.to_point(map); - if point.row == map.max_buffer_row().0 { + if point.row == map.buffer_snapshot.max_row().0 { return map.max_point(); } let mut found_non_blank_line = false; - for row in point.row..map.max_buffer_row().next_row().0 { + for row in point.row..=map.buffer_snapshot.max_row().0 { let blank = map.buffer_snapshot.is_line_blank(MultiBufferRow(row)); if found_non_blank_line && blank { if count <= 1 { diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 461498d00d..d52d65bca2 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -281,8 +281,7 @@ pub struct ExcerptSummary { excerpt_id: ExcerptId, /// The location of the last [`Excerpt`] being summarized excerpt_locator: Locator, - /// The maximum row of the [`Excerpt`]s being summarized - max_buffer_row: MultiBufferRow, + widest_line_number: u32, text: TextSummary, } @@ -2556,8 +2555,8 @@ impl MultiBufferSnapshot { self.excerpts.summary().text.len == 0 } - pub fn max_buffer_row(&self) -> MultiBufferRow { - self.excerpts.summary().max_buffer_row + pub fn widest_line_number(&self) -> u32 { + self.excerpts.summary().widest_line_number + 1 } pub fn clip_offset(&self, offset: usize, bias: Bias) -> usize { @@ -3026,6 +3025,10 @@ impl MultiBufferSnapshot { self.text_summary().lines } + pub fn max_row(&self) -> MultiBufferRow { + MultiBufferRow(self.text_summary().lines.row) + } + pub fn text_summary(&self) -> TextSummary { self.excerpts.summary().text.clone() } @@ -4824,7 +4827,7 @@ impl sum_tree::Item for Excerpt { ExcerptSummary { excerpt_id: self.id, excerpt_locator: self.locator.clone(), - max_buffer_row: MultiBufferRow(self.max_buffer_row), + widest_line_number: self.max_buffer_row, text, } } @@ -4869,7 +4872,7 @@ impl sum_tree::Summary for ExcerptSummary { debug_assert!(summary.excerpt_locator > self.excerpt_locator); self.excerpt_locator = summary.excerpt_locator.clone(); self.text.add_summary(&summary.text, &()); - self.max_buffer_row = cmp::max(self.max_buffer_row, summary.max_buffer_row); + self.widest_line_number = cmp::max(self.widest_line_number, summary.widest_line_number); } } @@ -6383,8 +6386,8 @@ mod tests { } assert_eq!( - snapshot.max_buffer_row().0, - expected_buffer_rows.into_iter().flatten().max().unwrap() + snapshot.widest_line_number(), + expected_buffer_rows.into_iter().flatten().max().unwrap() + 1 ); let mut excerpt_starts = excerpt_starts.into_iter(); diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index 5a958da012..68aefc8cd7 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -136,7 +136,7 @@ pub fn register(editor: &mut Editor, cx: &mut ViewContext) { vim.update_editor(cx, |vim, editor, cx| { let snapshot = editor.snapshot(cx); if let Ok(range) = action.range.buffer_range(vim, editor, cx) { - let end = if range.end < snapshot.max_buffer_row() { + let end = if range.end < snapshot.buffer_snapshot.max_row() { Point::new(range.end.0 + 1, 0) } else { snapshot.buffer_snapshot.max_point() @@ -436,9 +436,11 @@ impl Position { .row .saturating_add_signed(*offset) } - Position::LastLine { offset } => { - snapshot.max_buffer_row().0.saturating_add_signed(*offset) - } + Position::LastLine { offset } => snapshot + .buffer_snapshot + .max_row() + .0 + .saturating_add_signed(*offset), Position::CurrentLine { offset } => editor .selections .newest_anchor() @@ -448,7 +450,7 @@ impl Position { .saturating_add_signed(*offset), }; - Ok(MultiBufferRow(target).min(snapshot.max_buffer_row())) + Ok(MultiBufferRow(target).min(snapshot.buffer_snapshot.max_row())) } } diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 08cf219722..0e236861b6 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -1866,7 +1866,7 @@ fn end_of_document( let new_row = if let Some(line) = line { (line - 1) as u32 } else { - map.max_buffer_row().0 + map.buffer_snapshot.max_row().0 }; let new_point = Point::new(new_row, point.column()); diff --git a/crates/vim/src/normal/yank.rs b/crates/vim/src/normal/yank.rs index 763f1a3d16..85c6531f6a 100644 --- a/crates/vim/src/normal/yank.rs +++ b/crates/vim/src/normal/yank.rs @@ -154,9 +154,9 @@ impl Vim { // contains a newline (so that delete works as expected). We undo that change // here. let is_last_line = linewise - && end.row == buffer.max_buffer_row().0 + && end.row == buffer.max_row().0 && buffer.max_point().column > 0 - && start.row < buffer.max_buffer_row().0 + && start.row < buffer.max_row().0 && start == Point::new(start.row, buffer.line_len(MultiBufferRow(start.row))); if is_last_line { diff --git a/crates/vim/src/object.rs b/crates/vim/src/object.rs index b6f164cdb1..c63cb0e843 100644 --- a/crates/vim/src/object.rs +++ b/crates/vim/src/object.rs @@ -724,7 +724,7 @@ fn indent( // Loop forwards until we find a non-blank line with less indent let mut end_row = row; - let max_rows = map.max_buffer_row().0; + let max_rows = map.buffer_snapshot.max_row().0; for next_row in (row + 1)..=max_rows { let indent = map.line_indent_for_buffer_row(MultiBufferRow(next_row)); if indent.is_line_empty() { @@ -958,13 +958,13 @@ pub fn start_of_paragraph(map: &DisplaySnapshot, display_point: DisplayPoint) -> /// The trailing newline is excluded from the paragraph. pub fn end_of_paragraph(map: &DisplaySnapshot, display_point: DisplayPoint) -> DisplayPoint { let point = display_point.to_point(map); - if point.row == map.max_buffer_row().0 { + if point.row == map.buffer_snapshot.max_row().0 { return map.max_point(); } let is_current_line_blank = map.buffer_snapshot.is_line_blank(MultiBufferRow(point.row)); - for row in point.row + 1..map.max_buffer_row().0 + 1 { + for row in point.row + 1..map.buffer_snapshot.max_row().0 + 1 { let blank = map.buffer_snapshot.is_line_blank(MultiBufferRow(row)); if blank != is_current_line_blank { let previous_row = row - 1;