Fix issues with adjacent diff hunks (#25367)

Cole Miller created

Closes #ISSUE

Release Notes:

- Fixed being unable to toggle diff hunks with the mouse in some cases

Change summary

crates/editor/src/editor.rs             |  13 +-
crates/editor/src/editor_tests.rs       | 157 +++++++++++++++++++++++++++
crates/editor/src/element.rs            |   2 
crates/multi_buffer/src/multi_buffer.rs |  60 ++++-----
4 files changed, 193 insertions(+), 39 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -12990,14 +12990,13 @@ impl Editor {
         })
     }
 
-    fn toggle_diff_hunks_in_ranges_narrow(
-        &mut self,
-        ranges: Vec<Range<Anchor>>,
-        cx: &mut Context<'_, Editor>,
-    ) {
+    fn toggle_single_diff_hunk(&mut self, range: Range<Anchor>, cx: &mut Context<Self>) {
         self.buffer.update(cx, |buffer, cx| {
-            let expand = !buffer.has_expanded_diff_hunks_in_ranges(&ranges, cx);
-            buffer.expand_or_collapse_diff_hunks_narrow(ranges, expand, cx);
+            let snapshot = buffer.snapshot(cx);
+            let excerpt_id = range.end.excerpt_id;
+            let point_range = range.to_point(&snapshot);
+            let expand = !buffer.single_hunk_is_expanded(range, cx);
+            buffer.expand_or_collapse_diff_hunks_inner([(point_range, excerpt_id)], expand, cx);
         })
     }
 

crates/editor/src/editor_tests.rs 🔗

@@ -14821,6 +14821,163 @@ async fn test_indent_guide_with_expanded_diff_hunks(cx: &mut TestAppContext) {
     );
 }
 
+#[gpui::test]
+async fn test_adjacent_diff_hunks(executor: BackgroundExecutor, cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+    let mut cx = EditorTestContext::new(cx).await;
+
+    let diff_base = r#"
+        a
+        b
+        c
+        "#
+    .unindent();
+
+    cx.set_state(
+        &r#"
+        ˇA
+        b
+        C
+        "#
+        .unindent(),
+    );
+    cx.set_diff_base(&diff_base);
+    cx.update_editor(|editor, window, cx| {
+        editor.expand_all_diff_hunks(&ExpandAllHunkDiffs, window, cx);
+    });
+    executor.run_until_parked();
+
+    let both_hunks_expanded = r#"
+        - a
+        + ˇA
+          b
+        - c
+        + C
+        "#
+    .unindent();
+
+    cx.assert_state_with_diff(both_hunks_expanded.clone());
+
+    let hunk_ranges = cx.update_editor(|editor, window, cx| {
+        let snapshot = editor.snapshot(window, cx);
+        let hunks = editor
+            .diff_hunks_in_ranges(&[Anchor::min()..Anchor::max()], &snapshot.buffer_snapshot)
+            .collect::<Vec<_>>();
+        let excerpt_id = editor.buffer.read(cx).excerpt_ids()[0];
+        let buffer_id = hunks[0].buffer_id;
+        hunks
+            .into_iter()
+            .map(|hunk| Anchor::range_in_buffer(excerpt_id, buffer_id, hunk.buffer_range.clone()))
+            .collect::<Vec<_>>()
+    });
+    assert_eq!(hunk_ranges.len(), 2);
+
+    cx.update_editor(|editor, _, cx| {
+        editor.toggle_single_diff_hunk(hunk_ranges[0].clone(), cx);
+    });
+    executor.run_until_parked();
+
+    let second_hunk_expanded = r#"
+          ˇA
+          b
+        - c
+        + C
+        "#
+    .unindent();
+
+    cx.assert_state_with_diff(second_hunk_expanded);
+
+    cx.update_editor(|editor, _, cx| {
+        editor.toggle_single_diff_hunk(hunk_ranges[0].clone(), cx);
+    });
+    executor.run_until_parked();
+
+    cx.assert_state_with_diff(both_hunks_expanded.clone());
+
+    cx.update_editor(|editor, _, cx| {
+        editor.toggle_single_diff_hunk(hunk_ranges[1].clone(), cx);
+    });
+    executor.run_until_parked();
+
+    let first_hunk_expanded = r#"
+        - a
+        + ˇA
+          b
+          C
+        "#
+    .unindent();
+
+    cx.assert_state_with_diff(first_hunk_expanded);
+
+    cx.update_editor(|editor, _, cx| {
+        editor.toggle_single_diff_hunk(hunk_ranges[1].clone(), cx);
+    });
+    executor.run_until_parked();
+
+    cx.assert_state_with_diff(both_hunks_expanded);
+
+    cx.set_state(
+        &r#"
+        ˇA
+        b
+        "#
+        .unindent(),
+    );
+    cx.run_until_parked();
+
+    // TODO this cursor position seems bad
+    cx.assert_state_with_diff(
+        r#"
+        - ˇa
+        + A
+          b
+        "#
+        .unindent(),
+    );
+
+    cx.update_editor(|editor, window, cx| {
+        editor.expand_all_diff_hunks(&ExpandAllHunkDiffs, window, cx);
+    });
+
+    cx.assert_state_with_diff(
+        r#"
+            - ˇa
+            + A
+              b
+            - c
+            "#
+        .unindent(),
+    );
+
+    let hunk_ranges = cx.update_editor(|editor, window, cx| {
+        let snapshot = editor.snapshot(window, cx);
+        let hunks = editor
+            .diff_hunks_in_ranges(&[Anchor::min()..Anchor::max()], &snapshot.buffer_snapshot)
+            .collect::<Vec<_>>();
+        let excerpt_id = editor.buffer.read(cx).excerpt_ids()[0];
+        let buffer_id = hunks[0].buffer_id;
+        hunks
+            .into_iter()
+            .map(|hunk| Anchor::range_in_buffer(excerpt_id, buffer_id, hunk.buffer_range.clone()))
+            .collect::<Vec<_>>()
+    });
+    assert_eq!(hunk_ranges.len(), 2);
+
+    cx.update_editor(|editor, _, cx| {
+        editor.toggle_single_diff_hunk(hunk_ranges[1].clone(), cx);
+    });
+    executor.run_until_parked();
+
+    cx.assert_state_with_diff(
+        r#"
+        - ˇa
+        + A
+          b
+        "#
+        .unindent(),
+    );
+}
+
 #[gpui::test]
 fn test_crease_insertion_and_rendering(cx: &mut TestAppContext) {
     init_test(cx, |_| {});

crates/editor/src/element.rs 🔗

@@ -547,7 +547,7 @@ impl EditorElement {
         let mut modifiers = event.modifiers;
 
         if let Some(hovered_hunk) = hovered_hunk {
-            editor.toggle_diff_hunks_in_ranges_narrow(vec![hovered_hunk], cx);
+            editor.toggle_single_diff_hunk(hovered_hunk, cx);
             cx.notify();
             return;
         } else if gutter_hitbox.is_hovered(window) {

crates/multi_buffer/src/multi_buffer.rs 🔗

@@ -2388,6 +2388,23 @@ impl MultiBuffer {
             .is_some()
     }
 
+    pub fn single_hunk_is_expanded(&self, range: Range<Anchor>, cx: &App) -> bool {
+        let snapshot = self.read(cx);
+        let mut cursor = snapshot.diff_transforms.cursor::<usize>(&());
+        let offset_range = range.to_offset(&snapshot);
+        cursor.seek(&offset_range.start, Bias::Right, &());
+        while let Some(item) = cursor.item() {
+            if *cursor.start() >= offset_range.end && *cursor.start() > offset_range.start {
+                break;
+            }
+            if item.hunk_info().is_some() {
+                return true;
+            }
+            cursor.next(&());
+        }
+        false
+    }
+
     pub fn has_expanded_diff_hunks_in_ranges(&self, ranges: &[Range<Anchor>], cx: &App) -> bool {
         let snapshot = self.read(cx);
         let mut cursor = snapshot.diff_transforms.cursor::<usize>(&());
@@ -2411,9 +2428,9 @@ impl MultiBuffer {
         false
     }
 
-    fn expand_or_collapse_diff_hunks_internal(
+    pub fn expand_or_collapse_diff_hunks_inner(
         &mut self,
-        ranges: impl Iterator<Item = (Range<Point>, ExcerptId)>,
+        ranges: impl IntoIterator<Item = (Range<Point>, ExcerptId)>,
         expand: bool,
         cx: &mut Context<Self>,
     ) {
@@ -2459,22 +2476,6 @@ impl MultiBuffer {
         });
     }
 
-    pub fn expand_or_collapse_diff_hunks_narrow(
-        &mut self,
-        ranges: Vec<Range<Anchor>>,
-        expand: bool,
-        cx: &mut Context<Self>,
-    ) {
-        let snapshot = self.snapshot.borrow().clone();
-        self.expand_or_collapse_diff_hunks_internal(
-            ranges
-                .iter()
-                .map(move |range| (range.to_point(&snapshot), range.end.excerpt_id)),
-            expand,
-            cx,
-        );
-    }
-
     pub fn expand_or_collapse_diff_hunks(
         &mut self,
         ranges: Vec<Range<Anchor>>,
@@ -2482,19 +2483,16 @@ impl MultiBuffer {
         cx: &mut Context<Self>,
     ) {
         let snapshot = self.snapshot.borrow().clone();
-        self.expand_or_collapse_diff_hunks_internal(
-            ranges.iter().map(move |range| {
-                let end_excerpt_id = range.end.excerpt_id;
-                let range = range.to_point(&snapshot);
-                let mut peek_end = range.end;
-                if range.end.row < snapshot.max_row().0 {
-                    peek_end = Point::new(range.end.row + 1, 0);
-                };
-                (range.start..peek_end, end_excerpt_id)
-            }),
-            expand,
-            cx,
-        );
+        let ranges = ranges.iter().map(move |range| {
+            let end_excerpt_id = range.end.excerpt_id;
+            let range = range.to_point(&snapshot);
+            let mut peek_end = range.end;
+            if range.end.row < snapshot.max_row().0 {
+                peek_end = Point::new(range.end.row + 1, 0);
+            };
+            (range.start..peek_end, end_excerpt_id)
+        });
+        self.expand_or_collapse_diff_hunks_inner(ranges, expand, cx);
     }
 
     pub fn resize_excerpt(