From 2eb4d6b7ebe43acd30ef998c635c4bb900261a74 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 13 Mar 2025 12:56:54 -0400 Subject: [PATCH] Fix being unable to put a cursor after trailing deletion hunks (#26621) 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 --- crates/multi_buffer/src/multi_buffer.rs | 50 +++++--- crates/multi_buffer/src/multi_buffer_tests.rs | 110 ++++++++++++++++-- 2 files changed, 131 insertions(+), 29 deletions(-) diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 78ad43b90240816d44f4dd6c2cc4338e21ae50b0..0c219465437b6073c09199d36180162700db03b9 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -2007,22 +2007,17 @@ impl MultiBuffer { cx: &App, ) -> Option<(Entity, Point, ExcerptId)> { let snapshot = self.read(cx); - let point = point.to_point(&snapshot); - let mut cursor = snapshot.cursor::(); - 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()[®ion.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::(); 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())); diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index 8b10b787da251126fcefa93fbd9b3d2b27bbf604..af604cf93fb652449baa11883210fcaaa1ca91e6 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/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::new(2, 0)]) + .collect::>(), + 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::new(2, 0)]) + .collect::>(), + 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, @@ -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) {