From 49ebf59d05b685c25681cf44cb87d04f4179d1b5 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 3 Nov 2025 18:48:31 -0500 Subject: [PATCH] wip turns out the test wasn't working Co-authored-by: cameron --- 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(-) diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index 8f6ae21b39a96e804b5e9b04333b169541caabfb..91e8f51adb4e0988419ef0165fad9d2a37f37f9f 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/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::>()); - // dbg!(block_map.transforms.borrow().iter().collect::>()); } #[gpui::test] diff --git a/crates/editor/src/display_map/filter_map.rs b/crates/editor/src/display_map/filter_map.rs index 8b1189455c8022e72cf89e6fd0165b93f5f4dd34..bc956920edb29cce9ceb027f0a4994aeefd61cc9 100644 --- a/crates/editor/src/display_map/filter_map.rs +++ b/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::(anchor..hunk.multi_buffer_range().start); - if !mode.should_remove(hunk.status().kind) { - expected_summary += self - .snapshot - .buffer_snapshot - .text_summary_for_range::(hunk.multi_buffer_range()); + .text_summary_for_range::( + 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::(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::(anchor..multi_buffer::Anchor::max()); + dbg!(self.snapshot.transforms.iter().collect::>()); + 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 = 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 = diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index cd68aea24675f501c62bc4a03d3c0bcd7900002c..53d5b78b802d29df1781445c059dd489fee00384 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/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, buffers: &mut Vec>, base_texts: &mut HashMap, 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, + boundary_rows: &HashSet, + has_diff: Option, + ) -> 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::>() + .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::(); @@ -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) + ) + ) + } } diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index a9121b9104400d88d5f22801db1bfebaeeb060d6..0ac826017ef20ce79430872ae0badae42833b2f9 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/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::>() + ); + dbg!( + hunks[0] + .multi_buffer_range() + .start + .bias_right(&snapshot) + .to_point(&snapshot) + ); +}