Fix panic folding in multi-buffers (#21511)

Conrad Irwin created

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

Change summary

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(-)

Detailed changes

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<Point>) -> Range<Point> {
+        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;
         }

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)

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<Self>) {
+        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<Self>) {
+        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))
             {

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)
     }
 }

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 {

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();

crates/vim/src/command.rs 🔗

@@ -136,7 +136,7 @@ pub fn register(editor: &mut Editor, cx: &mut ViewContext<Vim>) {
         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()))
     }
 }
 

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());

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 {

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;