Fix being unable to put a cursor after trailing deletion hunks (#26621)

Cole Miller and Max created

Closes #26541

Release Notes:

- Fixed a bug that prevented putting the cursor after a deletion hunk at
the end of a file, in the absence of trailing newlines

---------

Co-authored-by: Max <max@zed.dev>

Change summary

crates/multi_buffer/src/multi_buffer.rs       |  50 +++++---
crates/multi_buffer/src/multi_buffer_tests.rs | 110 +++++++++++++++++++-
2 files changed, 131 insertions(+), 29 deletions(-)

Detailed changes

crates/multi_buffer/src/multi_buffer.rs 🔗

@@ -2007,22 +2007,17 @@ impl MultiBuffer {
         cx: &App,
     ) -> Option<(Entity<Buffer>, Point, ExcerptId)> {
         let snapshot = self.read(cx);
-        let point = point.to_point(&snapshot);
-        let mut cursor = snapshot.cursor::<Point>();
-        cursor.seek(&point);
-
-        cursor.region().and_then(|region| {
-            if !region.is_main_buffer {
-                return None;
-            }
-
-            let overshoot = point - region.range.start;
-            let buffer_point = region.buffer_range.start + overshoot;
-            let buffer = self.buffers.borrow()[&region.buffer.remote_id()]
+        let (buffer, point, is_main_buffer) =
+            snapshot.point_to_buffer_point(point.to_point(&snapshot))?;
+        Some((
+            self.buffers
+                .borrow()
+                .get(&buffer.remote_id())?
                 .buffer
-                .clone();
-            Some((buffer, buffer_point, region.excerpt.id))
-        })
+                .clone(),
+            point,
+            is_main_buffer,
+        ))
     }
 
     pub fn buffer_point_to_anchor(
@@ -4176,22 +4171,36 @@ impl MultiBufferSnapshot {
         let region = cursor.region()?;
         let overshoot = offset - region.range.start;
         let buffer_offset = region.buffer_range.start + overshoot;
-        if buffer_offset > region.buffer.len() {
+        if buffer_offset == region.buffer.len() + 1
+            && region.has_trailing_newline
+            && !region.is_main_buffer
+        {
+            return Some((&cursor.excerpt()?.buffer, cursor.main_buffer_position()?));
+        } else if buffer_offset > region.buffer.len() {
             return None;
         }
         Some((region.buffer, buffer_offset))
     }
 
-    pub fn point_to_buffer_point(&self, point: Point) -> Option<(&BufferSnapshot, Point, bool)> {
+    pub fn point_to_buffer_point(
+        &self,
+        point: Point,
+    ) -> Option<(&BufferSnapshot, Point, ExcerptId)> {
         let mut cursor = self.cursor::<Point>();
         cursor.seek(&point);
         let region = cursor.region()?;
         let overshoot = point - region.range.start;
         let buffer_point = region.buffer_range.start + overshoot;
-        if buffer_point > region.buffer.max_point() {
+        let excerpt = cursor.excerpt()?;
+        if buffer_point == region.buffer.max_point() + Point::new(1, 0)
+            && region.has_trailing_newline
+            && !region.is_main_buffer
+        {
+            return Some((&excerpt.buffer, cursor.main_buffer_position()?, excerpt.id));
+        } else if buffer_point > region.buffer.max_point() {
             return None;
         }
-        Some((region.buffer, buffer_point, region.is_main_buffer))
+        Some((region.buffer, buffer_point, excerpt.id))
     }
 
     pub fn suggested_indents(
@@ -4733,6 +4742,9 @@ impl MultiBufferSnapshot {
                         .buffer
                         .text_summary_for_range(region.buffer_range.start.key..buffer_point),
                 );
+                if point == region.range.end.key && region.has_trailing_newline {
+                    position.add_assign(&D::from_text_summary(&TextSummary::newline()));
+                }
                 return Some(position);
             } else {
                 return Some(D::from_text_summary(&self.text_summary()));

crates/multi_buffer/src/multi_buffer_tests.rs 🔗

@@ -3121,6 +3121,100 @@ fn test_summaries_for_anchors(cx: &mut TestAppContext) {
     assert_eq!(point_2, Point::new(3, 0));
 }
 
+#[gpui::test]
+fn test_trailing_deletion_without_newline(cx: &mut TestAppContext) {
+    let base_text_1 = "one\ntwo".to_owned();
+    let text_1 = "one\n".to_owned();
+
+    let buffer_1 = cx.new(|cx| Buffer::local(text_1, cx));
+    let diff_1 = cx.new(|cx| BufferDiff::new_with_base_text(&base_text_1, &buffer_1, cx));
+    cx.run_until_parked();
+
+    let multibuffer = cx.new(|cx| {
+        let mut multibuffer = MultiBuffer::singleton(buffer_1.clone(), cx);
+        multibuffer.add_diff(diff_1.clone(), cx);
+        multibuffer.expand_diff_hunks(vec![Anchor::min()..Anchor::max()], cx);
+        multibuffer
+    });
+
+    let (mut snapshot, mut subscription) = multibuffer.update(cx, |multibuffer, cx| {
+        (multibuffer.snapshot(cx), multibuffer.subscribe())
+    });
+
+    assert_new_snapshot(
+        &multibuffer,
+        &mut snapshot,
+        &mut subscription,
+        cx,
+        indoc!(
+            "
+              one
+            - two
+            "
+        ),
+    );
+
+    assert_eq!(snapshot.max_point(), Point::new(2, 0));
+    assert_eq!(snapshot.len(), 8);
+
+    assert_eq!(
+        snapshot
+            .dimensions_from_points::<Point>([Point::new(2, 0)])
+            .collect::<Vec<_>>(),
+        vec![Point::new(2, 0)]
+    );
+
+    let (_, translated_offset) = snapshot.point_to_buffer_offset(Point::new(2, 0)).unwrap();
+    assert_eq!(translated_offset, "one\n".len());
+    let (_, translated_point, _) = snapshot.point_to_buffer_point(Point::new(2, 0)).unwrap();
+    assert_eq!(translated_point, Point::new(1, 0));
+
+    // The same, for an excerpt that's not at the end of the multibuffer.
+
+    let text_2 = "foo\n".to_owned();
+    let buffer_2 = cx.new(|cx| Buffer::local(&text_2, cx));
+    multibuffer.update(cx, |multibuffer, cx| {
+        multibuffer.push_excerpts(
+            buffer_2.clone(),
+            [ExcerptRange {
+                context: Point::new(0, 0)..Point::new(1, 0),
+                primary: None,
+            }],
+            cx,
+        );
+    });
+
+    assert_new_snapshot(
+        &multibuffer,
+        &mut snapshot,
+        &mut subscription,
+        cx,
+        indoc!(
+            "
+              one
+            - two
+
+              foo
+            "
+        ),
+    );
+
+    assert_eq!(
+        snapshot
+            .dimensions_from_points::<Point>([Point::new(2, 0)])
+            .collect::<Vec<_>>(),
+        vec![Point::new(2, 0)]
+    );
+
+    let buffer_1_id = buffer_1.read_with(cx, |buffer_1, _| buffer_1.remote_id());
+    let (buffer, translated_offset) = snapshot.point_to_buffer_offset(Point::new(2, 0)).unwrap();
+    assert_eq!(buffer.remote_id(), buffer_1_id);
+    assert_eq!(translated_offset, "one\n".len());
+    let (buffer, translated_point, _) = snapshot.point_to_buffer_point(Point::new(2, 0)).unwrap();
+    assert_eq!(buffer.remote_id(), buffer_1_id);
+    assert_eq!(translated_point, Point::new(1, 0));
+}
+
 fn format_diff(
     text: &str,
     row_infos: &Vec<RowInfo>,
@@ -3379,16 +3473,12 @@ fn assert_position_translation(snapshot: &MultiBufferSnapshot) {
         }
     }
 
-    let point = snapshot.max_point();
-    let Some((buffer, offset)) = snapshot.point_to_buffer_offset(point) else {
-        return;
-    };
-    assert!(offset <= buffer.len(),);
-
-    let Some((buffer, point, _)) = snapshot.point_to_buffer_point(point) else {
-        return;
-    };
-    assert!(point <= buffer.max_point(),);
+    if let Some((buffer, offset)) = snapshot.point_to_buffer_offset(snapshot.max_point()) {
+        assert!(offset <= buffer.len());
+    }
+    if let Some((buffer, point, _)) = snapshot.point_to_buffer_point(snapshot.max_point()) {
+        assert!(point <= buffer.max_point());
+    }
 }
 
 fn assert_line_indents(snapshot: &MultiBufferSnapshot) {