wip turns out the test wasn't working

Cole Miller and cameron created

Co-authored-by: cameron <cameron.studdstreet@gmail.com>

Change summary

crates/editor/src/display_map/block_map.rs    |  11 -
crates/editor/src/display_map/filter_map.rs   | 101 ++++++++++++++------
crates/multi_buffer/src/multi_buffer.rs       |  78 ++++++++++++++-
crates/multi_buffer/src/multi_buffer_tests.rs |  54 +++++++++++
4 files changed, 197 insertions(+), 47 deletions(-)

Detailed changes

crates/editor/src/display_map/block_map.rs 🔗

@@ -571,7 +571,6 @@ impl BlockMap {
         let mut edits = edits.into_iter().peekable();
 
         while let Some(edit) = edits.next() {
-            dbg!(&edit);
             let mut old_start = WrapRow(edit.old.start);
             let mut new_start = WrapRow(edit.new.start);
 
@@ -621,7 +620,7 @@ impl BlockMap {
                     // replacement.
                     debug_assert!(transform.summary.input_rows > 0);
                     old_start.0 -= transform_rows_before_edit;
-                    new_start.0 -= dbg!(transform_rows_before_edit);
+                    new_start.0 -= transform_rows_before_edit;
                 }
             }
 
@@ -677,17 +676,14 @@ impl BlockMap {
             if true {
                 // FIXME persistent cursors/iterators for row boundaries and diff hunks
                 let mut current_wrap_row = new_start.0;
-                dbg!("---------", new_end.0);
                 loop {
-                    if dbg!(current_wrap_row) > new_end.0 {
-                        dbg!();
+                    if current_wrap_row > new_end.0 {
                         break;
                     }
 
                     let Some(next_row_boundary) =
                         wrap_snapshot.next_row_boundary(WrapPoint::new(current_wrap_row, 0))
                     else {
-                        dbg!();
                         break;
                     };
 
@@ -2345,9 +2341,6 @@ mod tests {
                 (6..7, BlockId::ExcerptBoundary(excerpt_ids[2])), // path, header
             ]
         );
-
-        dbg!(wrap_snapshot.row_infos(0).collect::<Vec<_>>());
-        // dbg!(block_map.transforms.borrow().iter().collect::<Vec<_>>());
     }
 
     #[gpui::test]

crates/editor/src/display_map/filter_map.rs 🔗

@@ -1,7 +1,7 @@
 use std::{cmp, ops::Range};
 
 use buffer_diff::{DiffHunkStatus, DiffHunkStatusKind};
-use multi_buffer::{AnchorRangeExt as _, MultiBufferSnapshot};
+use multi_buffer::{AnchorRangeExt as _, MultiBufferSnapshot, ToPoint as _};
 use rope::{Point, TextSummary};
 use schemars::transform;
 use sum_tree::{Dimensions, SumTree};
@@ -223,16 +223,37 @@ impl FilterMap {
         let mut expected_summary = TextSummary::default();
         let mut anchor = multi_buffer::Anchor::min();
         for hunk in self.snapshot.buffer_snapshot.diff_hunks() {
+            dbg!(&hunk);
+
             expected_summary += self
                 .snapshot
                 .buffer_snapshot
-                .text_summary_for_range::<TextSummary, _>(anchor..hunk.multi_buffer_range().start);
-            if !mode.should_remove(hunk.status().kind) {
-                expected_summary += self
-                    .snapshot
-                    .buffer_snapshot
-                    .text_summary_for_range::<TextSummary, _>(hunk.multi_buffer_range());
+                .text_summary_for_range::<TextSummary, _>(
+                    anchor
+                        ..hunk
+                            .multi_buffer_range()
+                            .start
+                            .bias_left(&self.snapshot.buffer_snapshot),
+                );
+            let (deletion_summary, addition_summary) =
+                text_summaries_for_diff_hunk(&hunk, &self.snapshot.buffer_snapshot);
+
+            dbg!(deletion_summary.len, addition_summary.len,);
+
+            match mode {
+                FilterMode::RemoveDeletions => {
+                    expected_summary += addition_summary;
+                }
+                FilterMode::RemoveInsertions => {
+                    expected_summary += deletion_summary;
+                }
             }
+            // if !mode.should_remove(hunk.status().kind) {
+            //     expected_summary += self
+            //         .snapshot
+            //         .buffer_snapshot
+            //         .text_summary_for_range::<TextSummary, _>(hunk.multi_buffer_range());
+            // }
             anchor = hunk.multi_buffer_range().end;
         }
         expected_summary += self
@@ -240,6 +261,8 @@ impl FilterMap {
             .buffer_snapshot
             .text_summary_for_range::<TextSummary, _>(anchor..multi_buffer::Anchor::max());
 
+        dbg!(self.snapshot.transforms.iter().collect::<Vec<_>>());
+
         pretty_assertions::assert_eq!(
             self.snapshot.transforms.summary().output.0,
             expected_summary,
@@ -276,8 +299,6 @@ impl FilterMap {
             );
         };
 
-        dbg!(&buffer_edits);
-
         let mut new_transforms: SumTree<Transform> = SumTree::new(());
         let mut transform_cursor = self
             .snapshot
@@ -328,11 +349,6 @@ impl FilterMap {
         while let Some(buffer_edit) = buffer_edits.next() {
             debug_assert!(transform_cursor.start().0 <= buffer_edit.old.end);
 
-            dbg!(
-                transform_cursor.start(),
-                buffer_edit.old.end,
-                transform_cursor.end()
-            );
             // Reuse any old transforms that strictly precede the start of the edit.
             log::info!(
                 "input len before append is {}",
@@ -343,11 +359,6 @@ impl FilterMap {
                 "input len after append is {}",
                 new_transforms.summary().input.0.len
             );
-            dbg!(
-                transform_cursor.start(),
-                buffer_edit.old.end,
-                transform_cursor.end()
-            );
 
             let mut edit_old_start = transform_cursor.start().1;
             let mut edit_new_start = FilterOffset(new_transforms.summary().output.0.len);
@@ -399,14 +410,31 @@ impl FilterMap {
                     &mut new_transforms,
                     buffer_snapshot.text_summary_for_range(prefix_range),
                 );
-                let hunk_summary = buffer_snapshot.text_summary_for_range(hunk_range);
-                if (mode == FilterMode::RemoveDeletions)
-                    == (hunk.status().kind == DiffHunkStatusKind::Deleted)
-                {
-                    push_filter(&mut new_transforms, hunk_summary);
-                } else {
-                    push_isomorphic(&mut new_transforms, hunk_summary);
+
+                let (deletion_summary, addition_summary) =
+                    text_summaries_for_diff_hunk(&hunk, &buffer_snapshot);
+
+                // let hunk_summary = buffer_snapshot.text_summary_for_range(hunk_range);
+                // let start_of_hunk_line = ...;
+                // let switch_mode_line = ...;
+                // let end_of_hunk_line = ...;
+                match mode {
+                    FilterMode::RemoveDeletions => {
+                        push_filter(&mut new_transforms, deletion_summary);
+                        push_isomorphic(&mut new_transforms, addition_summary);
+                    }
+                    FilterMode::RemoveInsertions => {
+                        push_isomorphic(&mut new_transforms, deletion_summary);
+                        push_filter(&mut new_transforms, addition_summary);
+                    }
                 }
+                // if (mode == FilterMode::RemoveDeletions)
+                //     == (hunk.status().kind == DiffHunkStatusKind::Deleted)
+                // {
+                //     push_filter(&mut new_transforms, hunk_summary);
+                // } else {
+                //     push_isomorphic(&mut new_transforms, hunk_summary);
+                // }
             }
 
             // Push any non-hunk content after the last hunk.
@@ -453,8 +481,6 @@ impl FilterMap {
             }) {
                 let suffix_start = new_transforms.summary().input.0.len;
                 let suffix_len = transform_cursor.end().0 - buffer_edit.old.end;
-                dbg!(suffix_start, suffix_start + suffix_len);
-                dbg!(buffer_snapshot.text());
                 let summary = buffer_snapshot.text_summary_for_range(
                     suffix_start..std::cmp::min(suffix_start + suffix_len, buffer_snapshot.len()),
                 );
@@ -492,6 +518,23 @@ impl FilterMap {
     }
 }
 
+fn text_summaries_for_diff_hunk(
+    hunk: &multi_buffer::MultiBufferDiffHunk,
+    buffer_snapshot: &MultiBufferSnapshot,
+) -> (TextSummary, TextSummary) {
+    // TODO does it make sense to do this in terms of points?
+    let start_of_hunk = Point::new(hunk.row_range.start.0, 0);
+    let switch_point = hunk
+        .multi_buffer_range()
+        .start
+        .bias_right(&buffer_snapshot)
+        .to_point(&buffer_snapshot);
+    let end_of_hunk = Point::new(hunk.row_range.end.0, 0);
+    let deletion_summary = buffer_snapshot.text_summary_for_range(start_of_hunk..switch_point);
+    let addition_summary = buffer_snapshot.text_summary_for_range(switch_point..end_of_hunk);
+    (deletion_summary, addition_summary)
+}
+
 impl FilterSnapshot {
     #[cfg(any(test, feature = "test-support"))]
     fn text(&self) -> String {
@@ -683,7 +726,7 @@ mod tests {
                 &mut buffers,
                 &mut base_texts,
                 &mut needs_diff_calculation,
-                rng.clone(),
+                &mut rng,
                 cx,
             );
             let buffer_snapshot =

crates/multi_buffer/src/multi_buffer.rs 🔗

@@ -126,6 +126,13 @@ pub enum Event {
     BufferDiffChanged,
 }
 
+// ex1
+// line4 of buffer1
+// ex2
+// line17 of buffer2
+// 
+// in multibuffer coordinates, Point::new(1, 0)
+
 /// A diff hunk, representing a range of consequent lines in a multibuffer.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct MultiBufferDiffHunk {
@@ -7174,26 +7181,66 @@ pub mod debug {
     }
 }
 
+// FIXME figure out whether this should have a different API or something
 #[cfg(feature = "test-support")]
 pub fn randomly_mutate_multibuffer_with_diffs(
     multibuffer: Entity<MultiBuffer>,
     buffers: &mut Vec<Entity<Buffer>>,
     base_texts: &mut HashMap<BufferId, String>,
     needs_diff_calculation: &mut bool,
-    mut rng: rand::rngs::StdRng,
+    rng: &mut rand::rngs::StdRng,
     cx: &mut gpui::TestAppContext,
 ) {
     use rand::Rng as _;
     use rand::seq::IndexedRandom as _;
 
+    fn format_diff(
+        text: &str,
+        row_infos: &Vec<RowInfo>,
+        boundary_rows: &HashSet<MultiBufferRow>,
+        has_diff: Option<bool>,
+    ) -> String {
+        let has_diff =
+            has_diff.unwrap_or_else(|| row_infos.iter().any(|info| info.diff_status.is_some()));
+        text.split('\n')
+            .enumerate()
+            .zip(row_infos)
+            .map(|((ix, line), info)| {
+                let marker = match info.diff_status.map(|status| status.kind) {
+                    Some(DiffHunkStatusKind::Added) => "+ ",
+                    Some(DiffHunkStatusKind::Deleted) => "- ",
+                    Some(DiffHunkStatusKind::Modified) => unreachable!(),
+                    None => {
+                        if has_diff && !line.is_empty() {
+                            "  "
+                        } else {
+                            ""
+                        }
+                    }
+                };
+                let boundary_row = if boundary_rows.contains(&MultiBufferRow(ix as u32)) {
+                    if has_diff {
+                        "  ----------\n"
+                    } else {
+                        "---------\n"
+                    }
+                } else {
+                    ""
+                };
+                format!("{boundary_row}{marker}{line}")
+            })
+            .collect::<Vec<_>>()
+            .join("\n")
+    }
+
     let excerpt_ids = multibuffer.read_with(cx, |multibuffer, _| multibuffer.excerpt_ids());
     match rng.random_range(0..100) {
         0..=14 if !buffers.is_empty() => {
-            let buffer = buffers.choose(&mut rng).unwrap();
+            let buffer = buffers.choose(rng).unwrap();
             buffer.update(cx, |buf, cx| {
                 let edit_count = rng.random_range(1..5);
                 log::info!("editing buffer");
-                buf.randomly_edit(&mut rng, edit_count, cx);
+                buf.randomly_edit(rng, edit_count, cx);
                 *needs_diff_calculation = true;
             });
         }
@@ -7202,7 +7249,7 @@ pub fn randomly_mutate_multibuffer_with_diffs(
                 let ids = multibuffer.excerpt_ids();
                 let mut excerpts = HashSet::default();
                 for _ in 0..rng.random_range(0..ids.len()) {
-                    excerpts.extend(ids.choose(&mut rng).copied());
+                    excerpts.extend(ids.choose(rng).copied());
                 }
 
                 let line_count = rng.random_range(0..5);
@@ -7219,7 +7266,7 @@ pub fn randomly_mutate_multibuffer_with_diffs(
         20..=29 if !excerpt_ids.is_empty() => {
             let mut ids_to_remove = vec![];
             for _ in 0..rng.random_range(1..=3) {
-                let Some(id) = excerpt_ids.choose(&mut rng) else {
+                let Some(id) = excerpt_ids.choose(rng) else {
                     break;
                 };
                 ids_to_remove.push(*id);
@@ -7239,7 +7286,7 @@ pub fn randomly_mutate_multibuffer_with_diffs(
                         .diff_for(snapshot.remote_id())
                         .unwrap()
                         .update(cx, |diff, cx| {
-                            log::info!("recalculating diff for buffer {:?}", snapshot.remote_id(),);
+                            log::info!("recalculating diff for buffer {:?}", snapshot.remote_id());
                             diff.recalculate_diff_sync(snapshot.text, cx);
                         });
                 }
@@ -7248,7 +7295,7 @@ pub fn randomly_mutate_multibuffer_with_diffs(
         }
         _ => {
             let buffer_handle = if buffers.is_empty() || rng.random_bool(0.4) {
-                let mut base_text = util::RandomCharIter::new(&mut rng)
+                let mut base_text = util::RandomCharIter::new(&mut *rng)
                     .take(256)
                     .collect::<String>();
 
@@ -7261,12 +7308,12 @@ pub fn randomly_mutate_multibuffer_with_diffs(
                 buffers.push(buffer);
                 buffers.last().unwrap()
             } else {
-                buffers.choose(&mut rng).unwrap()
+                buffers.choose(rng).unwrap()
             };
 
             let prev_excerpt_id = multibuffer
                 .read_with(cx, |multibuffer, cx| multibuffer.excerpt_ids())
-                .choose(&mut rng)
+                .choose(rng)
                 .copied()
                 .unwrap_or(ExcerptId::max());
 
@@ -7310,4 +7357,17 @@ pub fn randomly_mutate_multibuffer_with_diffs(
             });
         }
     }
+
+    {
+        let snapshot = multibuffer.read_with(cx, |multibuffer, cx| multibuffer.snapshot(cx));
+        log::info!(
+            "multibuffer contents:\n{}",
+            format_diff(
+                &snapshot.text(),
+                &snapshot.row_infos(MultiBufferRow(0)).collect(),
+                &Default::default(),
+                Some(true)
+            )
+        )
+    }
 }

crates/multi_buffer/src/multi_buffer_tests.rs 🔗

@@ -3962,3 +3962,57 @@ fn test_random_chunk_bitmaps_with_diffs(cx: &mut App, mut rng: StdRng) {
         }
     }
 }
+
+// FIXME
+#[gpui::test]
+async fn test_diff_hunk_row_ranges(cx: &mut TestAppContext) {
+    let text = indoc!(
+        "
+        ONE
+        "
+    );
+    let base_text = indoc!(
+        "
+        one
+        "
+    );
+
+    let buffer = cx.new(|cx| Buffer::local(text, cx));
+    let diff = cx.new(|cx| BufferDiff::new_with_base_text(base_text, &buffer, cx));
+    cx.run_until_parked();
+
+    let multibuffer = cx.new(|cx| {
+        let mut multibuffer = MultiBuffer::singleton(buffer.clone(), cx);
+        multibuffer.set_all_diff_hunks_expanded(cx);
+        multibuffer.add_diff(diff.clone(), 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
+             + ONE
+             "
+        ),
+    );
+    let hunks = dbg!(
+        snapshot
+            .diff_hunks_in_range(Anchor::min()..Anchor::max())
+            .collect::<Vec<_>>()
+    );
+    dbg!(
+        hunks[0]
+            .multi_buffer_range()
+            .start
+            .bias_right(&snapshot)
+            .to_point(&snapshot)
+    );
+}