Refactored apart the forward and the backwards iterator for diff hunks

Mikayla Maki created

Change summary

crates/collab/src/tests/integration_tests.rs  |  18 +-
crates/editor/src/editor.rs                   | 124 ++++++++++++--------
crates/editor/src/element.rs                  |   4 
crates/editor/src/multi_buffer.rs             |  85 +++++++++++--
crates/editor/src/test/editor_test_context.rs |   4 
crates/git/src/diff.rs                        |  47 +++++--
crates/language/src/buffer.rs                 |  14 +
crates/project/src/worktree.rs                |   2 
crates/text/src/text.rs                       |   8 +
9 files changed, 209 insertions(+), 97 deletions(-)

Detailed changes

crates/collab/src/tests/integration_tests.rs 🔗

@@ -2437,7 +2437,7 @@ async fn test_git_diff_base_change(
     buffer_local_a.read_with(cx_a, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(diff_base.as_ref()));
         git::diff::assert_hunks(
-            buffer.snapshot().git_diff_hunks_in_row_range(0..4, false),
+            buffer.snapshot().git_diff_hunks_in_row_range(0..4),
             &buffer,
             &diff_base,
             &[(1..2, "", "two\n")],
@@ -2457,7 +2457,7 @@ async fn test_git_diff_base_change(
     buffer_remote_a.read_with(cx_b, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(diff_base.as_ref()));
         git::diff::assert_hunks(
-            buffer.snapshot().git_diff_hunks_in_row_range(0..4, false),
+            buffer.snapshot().git_diff_hunks_in_row_range(0..4),
             &buffer,
             &diff_base,
             &[(1..2, "", "two\n")],
@@ -2481,7 +2481,7 @@ async fn test_git_diff_base_change(
         assert_eq!(buffer.diff_base(), Some(new_diff_base.as_ref()));
 
         git::diff::assert_hunks(
-            buffer.snapshot().git_diff_hunks_in_row_range(0..4, false),
+            buffer.snapshot().git_diff_hunks_in_row_range(0..4),
             &buffer,
             &diff_base,
             &[(2..3, "", "three\n")],
@@ -2492,7 +2492,7 @@ async fn test_git_diff_base_change(
     buffer_remote_a.read_with(cx_b, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(new_diff_base.as_ref()));
         git::diff::assert_hunks(
-            buffer.snapshot().git_diff_hunks_in_row_range(0..4, false),
+            buffer.snapshot().git_diff_hunks_in_row_range(0..4),
             &buffer,
             &diff_base,
             &[(2..3, "", "three\n")],
@@ -2535,7 +2535,7 @@ async fn test_git_diff_base_change(
     buffer_local_b.read_with(cx_a, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(diff_base.as_ref()));
         git::diff::assert_hunks(
-            buffer.snapshot().git_diff_hunks_in_row_range(0..4, false),
+            buffer.snapshot().git_diff_hunks_in_row_range(0..4),
             &buffer,
             &diff_base,
             &[(1..2, "", "two\n")],
@@ -2555,7 +2555,7 @@ async fn test_git_diff_base_change(
     buffer_remote_b.read_with(cx_b, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(diff_base.as_ref()));
         git::diff::assert_hunks(
-            buffer.snapshot().git_diff_hunks_in_row_range(0..4, false),
+            buffer.snapshot().git_diff_hunks_in_row_range(0..4),
             &buffer,
             &diff_base,
             &[(1..2, "", "two\n")],
@@ -2583,12 +2583,12 @@ async fn test_git_diff_base_change(
             "{:?}",
             buffer
                 .snapshot()
-                .git_diff_hunks_in_row_range(0..4, false)
+                .git_diff_hunks_in_row_range(0..4)
                 .collect::<Vec<_>>()
         );
 
         git::diff::assert_hunks(
-            buffer.snapshot().git_diff_hunks_in_row_range(0..4, false),
+            buffer.snapshot().git_diff_hunks_in_row_range(0..4),
             &buffer,
             &diff_base,
             &[(2..3, "", "three\n")],
@@ -2599,7 +2599,7 @@ async fn test_git_diff_base_change(
     buffer_remote_b.read_with(cx_b, |buffer, _| {
         assert_eq!(buffer.diff_base(), Some(new_diff_base.as_ref()));
         git::diff::assert_hunks(
-            buffer.snapshot().git_diff_hunks_in_row_range(0..4, false),
+            buffer.snapshot().git_diff_hunks_in_row_range(0..4),
             &buffer,
             &diff_base,
             &[(2..3, "", "three\n")],

crates/editor/src/editor.rs 🔗

@@ -20,6 +20,7 @@ mod editor_tests;
 #[cfg(any(test, feature = "test-support"))]
 pub mod test;
 
+use ::git::diff::DiffHunk;
 use aho_corasick::AhoCorasick;
 use anyhow::{anyhow, Result};
 use blink_manager::BlinkManager;
@@ -527,7 +528,7 @@ pub struct EditorSnapshot {
 impl EditorSnapshot {
     fn has_scrollbar_info(&self) -> bool {
         self.buffer_snapshot
-            .git_diff_hunks_in_range(0..self.max_point().row(), false)
+            .git_diff_hunks_in_range(0..self.max_point().row())
             .next()
             .is_some()
     }
@@ -5569,68 +5570,91 @@ impl Editor {
     }
 
     fn go_to_hunk(&mut self, _: &GoToHunk, cx: &mut ViewContext<Self>) {
-        self.go_to_hunk_impl(Direction::Next, cx)
-    }
+        let snapshot = self
+            .display_map
+            .update(cx, |display_map, cx| display_map.snapshot(cx));
+        let selection = self.selections.newest::<Point>(cx);
 
-    fn go_to_prev_hunk(&mut self, _: &GoToPrevHunk, cx: &mut ViewContext<Self>) {
-        self.go_to_hunk_impl(Direction::Prev, cx)
+        if !self.seek_in_direction(
+            &snapshot,
+            selection.head(),
+            false,
+            snapshot
+                .buffer_snapshot
+                .git_diff_hunks_in_range((selection.head().row + 1)..u32::MAX),
+            cx,
+        ) {
+            let wrapped_point = Point::zero();
+            self.seek_in_direction(
+                &snapshot,
+                wrapped_point,
+                true,
+                snapshot
+                    .buffer_snapshot
+                    .git_diff_hunks_in_range((wrapped_point.row + 1)..u32::MAX),
+                cx,
+            );
+        }
     }
 
-    pub fn go_to_hunk_impl(&mut self, direction: Direction, cx: &mut ViewContext<Self>) {
+    fn go_to_prev_hunk(&mut self, _: &GoToPrevHunk, cx: &mut ViewContext<Self>) {
         let snapshot = self
             .display_map
             .update(cx, |display_map, cx| display_map.snapshot(cx));
         let selection = self.selections.newest::<Point>(cx);
 
-        fn seek_in_direction(
-            this: &mut Editor,
-            snapshot: &DisplaySnapshot,
-            initial_point: Point,
-            is_wrapped: bool,
-            direction: Direction,
-            cx: &mut ViewContext<Editor>,
-        ) -> bool {
-            let hunks = if direction == Direction::Next {
-                snapshot
-                    .buffer_snapshot
-                    .git_diff_hunks_in_range(initial_point.row..u32::MAX, false)
-            } else {
+        if !self.seek_in_direction(
+            &snapshot,
+            selection.head(),
+            false,
+            snapshot
+                .buffer_snapshot
+                .git_diff_hunks_in_range_rev(0..selection.head().row),
+            cx,
+        ) {
+            let wrapped_point = snapshot.buffer_snapshot.max_point();
+            self.seek_in_direction(
+                &snapshot,
+                wrapped_point,
+                true,
                 snapshot
                     .buffer_snapshot
-                    .git_diff_hunks_in_range(0..initial_point.row, true)
-            };
-
-            let display_point = initial_point.to_display_point(snapshot);
-            let mut hunks = hunks
-                .map(|hunk| diff_hunk_to_display(hunk, &snapshot))
-                .skip_while(|hunk| {
-                    if is_wrapped {
-                        false
-                    } else {
-                        hunk.contains_display_row(display_point.row())
-                    }
-                })
-                .dedup();
+                    .git_diff_hunks_in_range_rev(0..wrapped_point.row),
+                cx,
+            );
+        }
+    }
 
-            if let Some(hunk) = hunks.next() {
-                this.change_selections(Some(Autoscroll::fit()), cx, |s| {
-                    let row = hunk.start_display_row();
-                    let point = DisplayPoint::new(row, 0);
-                    s.select_display_ranges([point..point]);
-                });
+    fn seek_in_direction(
+        &mut self,
+        snapshot: &DisplaySnapshot,
+        initial_point: Point,
+        is_wrapped: bool,
+        hunks: impl Iterator<Item = DiffHunk<u32>>,
+        cx: &mut ViewContext<Editor>,
+    ) -> bool {
+        let display_point = initial_point.to_display_point(snapshot);
+        let mut hunks = hunks
+            .map(|hunk| diff_hunk_to_display(hunk, &snapshot))
+            .skip_while(|hunk| {
+                if is_wrapped {
+                    false
+                } else {
+                    hunk.contains_display_row(display_point.row())
+                }
+            })
+            .dedup();
 
-                true
-            } else {
-                false
-            }
-        }
+        if let Some(hunk) = hunks.next() {
+            self.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                let row = hunk.start_display_row();
+                let point = DisplayPoint::new(row, 0);
+                s.select_display_ranges([point..point]);
+            });
 
-        if !seek_in_direction(self, &snapshot, selection.head(), false, direction, cx) {
-            let wrapped_point = match direction {
-                Direction::Next => Point::zero(),
-                Direction::Prev => snapshot.buffer_snapshot.max_point(),
-            };
-            seek_in_direction(self, &snapshot, wrapped_point, true, direction, cx);
+            true
+        } else {
+            false
         }
     }
 

crates/editor/src/element.rs 🔗

@@ -1057,7 +1057,7 @@ impl EditorElement {
                 .position_map
                 .snapshot
                 .buffer_snapshot
-                .git_diff_hunks_in_range(0..(max_row.floor() as u32), false)
+                .git_diff_hunks_in_range(0..(max_row.floor() as u32))
             {
                 let start_display = Point::new(hunk.buffer_range.start, 0)
                     .to_display_point(&layout.position_map.snapshot.display_snapshot);
@@ -1274,7 +1274,7 @@ impl EditorElement {
             .row;
 
         buffer_snapshot
-            .git_diff_hunks_in_range(buffer_start_row..buffer_end_row, false)
+            .git_diff_hunks_in_range(buffer_start_row..buffer_end_row)
             .map(|hunk| diff_hunk_to_display(hunk, snapshot))
             .dedup()
             .collect()

crates/editor/src/multi_buffer.rs 🔗

@@ -2841,20 +2841,15 @@ impl MultiBufferSnapshot {
             })
     }
 
-    pub fn git_diff_hunks_in_range<'a>(
+    pub fn git_diff_hunks_in_range_rev<'a>(
         &'a self,
         row_range: Range<u32>,
-        reversed: bool,
     ) -> impl 'a + Iterator<Item = DiffHunk<u32>> {
         let mut cursor = self.excerpts.cursor::<Point>();
 
-        if reversed {
-            cursor.seek(&Point::new(row_range.end, 0), Bias::Left, &());
-            if cursor.item().is_none() {
-                cursor.prev(&());
-            }
-        } else {
-            cursor.seek(&Point::new(row_range.start, 0), Bias::Right, &());
+        cursor.seek(&Point::new(row_range.end, 0), Bias::Left, &());
+        if cursor.item().is_none() {
+            cursor.prev(&());
         }
 
         std::iter::from_fn(move || {
@@ -2884,7 +2879,7 @@ impl MultiBufferSnapshot {
 
             let buffer_hunks = excerpt
                 .buffer
-                .git_diff_hunks_intersecting_range(buffer_start..buffer_end, reversed)
+                .git_diff_hunks_intersecting_range_rev(buffer_start..buffer_end)
                 .filter_map(move |hunk| {
                     let start = multibuffer_start.row
                         + hunk
@@ -2904,12 +2899,70 @@ impl MultiBufferSnapshot {
                     })
                 });
 
-            if reversed {
-                cursor.prev(&());
-            } else {
-                cursor.next(&());
+            cursor.prev(&());
+
+            Some(buffer_hunks)
+        })
+        .flatten()
+    }
+
+    pub fn git_diff_hunks_in_range<'a>(
+        &'a self,
+        row_range: Range<u32>,
+    ) -> impl 'a + Iterator<Item = DiffHunk<u32>> {
+        let mut cursor = self.excerpts.cursor::<Point>();
+
+        cursor.seek(&Point::new(row_range.start, 0), Bias::Right, &());
+
+        std::iter::from_fn(move || {
+            let excerpt = cursor.item()?;
+            let multibuffer_start = *cursor.start();
+            let multibuffer_end = multibuffer_start + excerpt.text_summary.lines;
+            if multibuffer_start.row >= row_range.end {
+                return None;
             }
 
+            let mut buffer_start = excerpt.range.context.start;
+            let mut buffer_end = excerpt.range.context.end;
+            let excerpt_start_point = buffer_start.to_point(&excerpt.buffer);
+            let excerpt_end_point = excerpt_start_point + excerpt.text_summary.lines;
+
+            if row_range.start > multibuffer_start.row {
+                let buffer_start_point =
+                    excerpt_start_point + Point::new(row_range.start - multibuffer_start.row, 0);
+                buffer_start = excerpt.buffer.anchor_before(buffer_start_point);
+            }
+
+            if row_range.end < multibuffer_end.row {
+                let buffer_end_point =
+                    excerpt_start_point + Point::new(row_range.end - multibuffer_start.row, 0);
+                buffer_end = excerpt.buffer.anchor_before(buffer_end_point);
+            }
+
+            let buffer_hunks = excerpt
+                .buffer
+                .git_diff_hunks_intersecting_range(buffer_start..buffer_end)
+                .filter_map(move |hunk| {
+                    let start = multibuffer_start.row
+                        + hunk
+                            .buffer_range
+                            .start
+                            .saturating_sub(excerpt_start_point.row);
+                    let end = multibuffer_start.row
+                        + hunk
+                            .buffer_range
+                            .end
+                            .min(excerpt_end_point.row + 1)
+                            .saturating_sub(excerpt_start_point.row);
+
+                    Some(DiffHunk {
+                        buffer_range: start..end,
+                        diff_base_byte_range: hunk.diff_base_byte_range.clone(),
+                    })
+                });
+
+            cursor.next(&());
+
             Some(buffer_hunks)
         })
         .flatten()
@@ -4647,7 +4700,7 @@ mod tests {
 
         assert_eq!(
             snapshot
-                .git_diff_hunks_in_range(0..12, false)
+                .git_diff_hunks_in_range(0..12)
                 .map(|hunk| (hunk.status(), hunk.buffer_range))
                 .collect::<Vec<_>>(),
             &expected,
@@ -4655,7 +4708,7 @@ mod tests {
 
         assert_eq!(
             snapshot
-                .git_diff_hunks_in_range(0..12, true)
+                .git_diff_hunks_in_range_rev(0..12)
                 .map(|hunk| (hunk.status(), hunk.buffer_range))
                 .collect::<Vec<_>>(),
             expected

crates/editor/src/test/editor_test_context.rs 🔗

@@ -204,6 +204,7 @@ impl<'a> EditorTestContext<'a> {
         self.assert_selections(expected_selections, marked_text.to_string())
     }
 
+    #[track_caller]
     pub fn assert_editor_background_highlights<Tag: 'static>(&mut self, marked_text: &str) {
         let expected_ranges = self.ranges(marked_text);
         let actual_ranges: Vec<Range<usize>> = self.update_editor(|editor, cx| {
@@ -220,6 +221,7 @@ impl<'a> EditorTestContext<'a> {
         assert_set_eq!(actual_ranges, expected_ranges);
     }
 
+    #[track_caller]
     pub fn assert_editor_text_highlights<Tag: ?Sized + 'static>(&mut self, marked_text: &str) {
         let expected_ranges = self.ranges(marked_text);
         let snapshot = self.update_editor(|editor, cx| editor.snapshot(cx));
@@ -233,12 +235,14 @@ impl<'a> EditorTestContext<'a> {
         assert_set_eq!(actual_ranges, expected_ranges);
     }
 
+    #[track_caller]
     pub fn assert_editor_selections(&mut self, expected_selections: Vec<Range<usize>>) {
         let expected_marked_text =
             generate_marked_text(&self.buffer_text(), &expected_selections, true);
         self.assert_selections(expected_selections, expected_marked_text)
     }
 
+    #[track_caller]
     fn assert_selections(
         &mut self,
         expected_selections: Vec<Range<usize>>,

crates/git/src/diff.rs 🔗

@@ -1,6 +1,6 @@
 use std::{iter, ops::Range};
 use sum_tree::SumTree;
-use text::{Anchor, BufferSnapshot, Point};
+use text::{Anchor, BufferSnapshot, OffsetRangeExt, Point};
 
 pub use git2 as libgit;
 use libgit::{DiffLineType as GitDiffLineType, DiffOptions as GitOptions, Patch as GitPatch};
@@ -75,18 +75,17 @@ impl BufferDiff {
         &'a self,
         range: Range<u32>,
         buffer: &'a BufferSnapshot,
-        reversed: bool,
     ) -> impl 'a + Iterator<Item = DiffHunk<u32>> {
         let start = buffer.anchor_before(Point::new(range.start, 0));
         let end = buffer.anchor_after(Point::new(range.end, 0));
-        self.hunks_intersecting_range(start..end, buffer, reversed)
+
+        self.hunks_intersecting_range(start..end, buffer)
     }
 
     pub fn hunks_intersecting_range<'a>(
         &'a self,
         range: Range<Anchor>,
         buffer: &'a BufferSnapshot,
-        reversed: bool,
     ) -> impl 'a + Iterator<Item = DiffHunk<u32>> {
         let mut cursor = self.tree.filter::<_, DiffHunkSummary>(move |summary| {
             let before_start = summary.buffer_range.end.cmp(&range.start, buffer).is_lt();
@@ -95,12 +94,7 @@ impl BufferDiff {
         });
 
         let anchor_iter = std::iter::from_fn(move || {
-            if reversed {
-                cursor.prev(buffer);
-            } else {
-                cursor.next(buffer);
-            }
-
+            cursor.next(buffer);
             cursor.item()
         })
         .flat_map(move |hunk| {
@@ -129,6 +123,35 @@ impl BufferDiff {
         })
     }
 
+    pub fn hunks_intersecting_range_rev<'a>(
+        &'a self,
+        range: Range<Anchor>,
+        buffer: &'a BufferSnapshot,
+    ) -> impl 'a + Iterator<Item = DiffHunk<u32>> {
+        let mut cursor = self.tree.filter::<_, DiffHunkSummary>(move |summary| {
+            let before_start = summary.buffer_range.end.cmp(&range.start, buffer).is_lt();
+            let after_end = summary.buffer_range.start.cmp(&range.end, buffer).is_gt();
+            !before_start && !after_end
+        });
+
+        std::iter::from_fn(move || {
+            cursor.prev(buffer);
+
+            let hunk = cursor.item()?;
+            let range = hunk.buffer_range.to_point(buffer);
+            let end_row = if range.end.column > 0 {
+                range.end.row + 1
+            } else {
+                range.end.row
+            };
+
+            Some(DiffHunk {
+                buffer_range: range.start.row..end_row,
+                diff_base_byte_range: hunk.diff_base_byte_range.clone(),
+            })
+        })
+    }
+
     pub fn clear(&mut self, buffer: &text::BufferSnapshot) {
         self.last_buffer_version = Some(buffer.version().clone());
         self.tree = SumTree::new();
@@ -163,7 +186,7 @@ impl BufferDiff {
     fn hunks<'a>(&'a self, text: &'a BufferSnapshot) -> impl 'a + Iterator<Item = DiffHunk<u32>> {
         let start = text.anchor_before(Point::new(0, 0));
         let end = text.anchor_after(Point::new(u32::MAX, u32::MAX));
-        self.hunks_intersecting_range(start..end, text, false)
+        self.hunks_intersecting_range(start..end, text)
     }
 
     fn diff<'a>(head: &'a str, current: &'a str) -> Option<GitPatch<'a>> {
@@ -379,7 +402,7 @@ mod tests {
         assert_eq!(diff.hunks(&buffer).count(), 8);
 
         assert_hunks(
-            diff.hunks_in_row_range(7..12, &buffer, false),
+            diff.hunks_in_row_range(7..12, &buffer),
             &buffer,
             &diff_base,
             &[

crates/language/src/buffer.rs 🔗

@@ -2509,18 +2509,22 @@ impl BufferSnapshot {
     pub fn git_diff_hunks_in_row_range<'a>(
         &'a self,
         range: Range<u32>,
-        reversed: bool,
     ) -> impl 'a + Iterator<Item = git::diff::DiffHunk<u32>> {
-        self.git_diff.hunks_in_row_range(range, self, reversed)
+        self.git_diff.hunks_in_row_range(range, self)
     }
 
     pub fn git_diff_hunks_intersecting_range<'a>(
         &'a self,
         range: Range<Anchor>,
-        reversed: bool,
     ) -> impl 'a + Iterator<Item = git::diff::DiffHunk<u32>> {
-        self.git_diff
-            .hunks_intersecting_range(range, self, reversed)
+        self.git_diff.hunks_intersecting_range(range, self)
+    }
+
+    pub fn git_diff_hunks_intersecting_range_rev<'a>(
+        &'a self,
+        range: Range<Anchor>,
+    ) -> impl 'a + Iterator<Item = git::diff::DiffHunk<u32>> {
+        self.git_diff.hunks_intersecting_range_rev(range, self)
     }
 
     pub fn diagnostics_in_range<'a, T, O>(

crates/project/src/worktree.rs 🔗

@@ -4779,7 +4779,7 @@ mod tests {
                     Some(GitFileStatus::Added)
                 );
             });
-            dbg!("RENAMING");
+
             std::fs::rename(
                 root_path.join("projects/project1"),
                 root_path.join("projects/project2"),

crates/text/src/text.rs 🔗

@@ -1785,10 +1785,14 @@ impl BufferSnapshot {
         A: 'a + IntoIterator<Item = &'a Anchor>,
     {
         let anchors = anchors.into_iter();
-        self.summaries_for_anchors_with_payload::<D, _, ()>(anchors.map(|a| (a, ()))).map(|d| d.0)
+        self.summaries_for_anchors_with_payload::<D, _, ()>(anchors.map(|a| (a, ())))
+            .map(|d| d.0)
     }
 
-    pub fn summaries_for_anchors_with_payload<'a, D, A, T>(&'a self, anchors: A) -> impl 'a + Iterator<Item = (D, T)>
+    pub fn summaries_for_anchors_with_payload<'a, D, A, T>(
+        &'a self,
+        anchors: A,
+    ) -> impl 'a + Iterator<Item = (D, T)>
     where
         D: 'a + TextDimension,
         A: 'a + IntoIterator<Item = (&'a Anchor, T)>,