Skip to content

Commit

Permalink
Fix bugs in expanding diff hunk (zed-industries#18885)
Browse files Browse the repository at this point in the history
Release Notes:

- Fixed an issue where diff hunks at the boundaries of multi buffer
excerpts could not be expanded
  • Loading branch information
maxbrunsfeld authored Oct 9, 2024
1 parent b0a9005 commit e351148
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 54 deletions.
54 changes: 54 additions & 0 deletions crates/editor/src/editor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11715,6 +11715,60 @@ async fn test_toggle_diff_expand_in_multi_buffer(cx: &mut gpui::TestAppContext)
);
}

#[gpui::test]
async fn test_expand_diff_hunk_at_excerpt_boundary(cx: &mut gpui::TestAppContext) {
init_test(cx, |_| {});

let base = "aaa\nbbb\nccc\nddd\neee\nfff\nggg\n";
let text = "aaa\nBBB\nBB2\nccc\nDDD\nEEE\nfff\nggg\n";

let buffer = cx.new_model(|cx| {
let mut buffer = Buffer::local(text.to_string(), cx);
buffer.set_diff_base(Some(base.into()), cx);
buffer
});

let multi_buffer = cx.new_model(|cx| {
let mut multibuffer = MultiBuffer::new(ReadWrite);
multibuffer.push_excerpts(
buffer.clone(),
[
ExcerptRange {
context: Point::new(0, 0)..Point::new(2, 0),
primary: None,
},
ExcerptRange {
context: Point::new(5, 0)..Point::new(7, 0),
primary: None,
},
],
cx,
);
multibuffer
});

let editor = cx.add_window(|cx| Editor::new(EditorMode::Full, multi_buffer, None, true, cx));
let mut cx = EditorTestContext::for_editor(editor, cx).await;
cx.run_until_parked();

cx.update_editor(|editor, cx| editor.expand_all_hunk_diffs(&Default::default(), cx));
cx.executor().run_until_parked();

cx.assert_diff_hunks(
"
aaa
- bbb
+ BBB
- ddd
- eee
+ EEE
fff
"
.unindent(),
);
}

#[gpui::test]
async fn test_edits_around_expanded_insertion_hunks(
executor: BackgroundExecutor,
Expand Down
84 changes: 31 additions & 53 deletions crates/editor/src/hunk_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,19 +209,20 @@ impl Editor {

retain
});
for remaining_hunk in hunks_to_toggle {
let remaining_hunk_point_range =
Point::new(remaining_hunk.row_range.start.0, 0)
..Point::new(remaining_hunk.row_range.end.0, 0);
for hunk in hunks_to_toggle {
let remaining_hunk_point_range = Point::new(hunk.row_range.start.0, 0)
..Point::new(hunk.row_range.end.0, 0);
let hunk_start = snapshot
.buffer_snapshot
.anchor_before(remaining_hunk_point_range.start);
let hunk_end = snapshot
.buffer_snapshot
.anchor_in_excerpt(hunk_start.excerpt_id, hunk.buffer_range.end)
.unwrap();
hunks_to_expand.push(HoveredHunk {
status: hunk_status(&remaining_hunk),
multi_buffer_range: snapshot
.buffer_snapshot
.anchor_before(remaining_hunk_point_range.start)
..snapshot
.buffer_snapshot
.anchor_after(remaining_hunk_point_range.end),
diff_base_byte_range: remaining_hunk.diff_base_byte_range.clone(),
status: hunk_status(&hunk),
multi_buffer_range: hunk_start..hunk_end,
diff_base_byte_range: hunk.diff_base_byte_range.clone(),
});
}

Expand All @@ -246,33 +247,22 @@ impl Editor {
hunk: &HoveredHunk,
cx: &mut ViewContext<'_, Editor>,
) -> Option<()> {
let multi_buffer_snapshot = self.buffer().read(cx).snapshot(cx);
let buffer = self.buffer.clone();
let multi_buffer_snapshot = buffer.read(cx).snapshot(cx);
let hunk_range = hunk.multi_buffer_range.clone();
let hunk_point_range = hunk_range.to_point(&multi_buffer_snapshot);

let buffer = self.buffer().clone();
let snapshot = self.snapshot(cx);
let (diff_base_buffer, deleted_text_lines) = buffer.update(cx, |buffer, cx| {
let hunk = buffer_diff_hunk(&snapshot.buffer_snapshot, hunk_point_range.clone())?;
let mut buffer_ranges = buffer.range_to_buffer_ranges(hunk_point_range, cx);
if buffer_ranges.len() == 1 {
let (buffer, _, _) = buffer_ranges.pop()?;
let diff_base_buffer = diff_base_buffer
.or_else(|| self.current_diff_base_buffer(&buffer, cx))
.or_else(|| create_diff_base_buffer(&buffer, cx))?;
let buffer = buffer.read(cx);
let deleted_text_lines = buffer.diff_base().map(|diff_base| {
let diff_start_row = diff_base
.offset_to_point(hunk.diff_base_byte_range.start)
.row;
let diff_end_row = diff_base.offset_to_point(hunk.diff_base_byte_range.end).row;

diff_end_row - diff_start_row
})?;
Some((diff_base_buffer, deleted_text_lines))
} else {
None
}
let buffer = buffer.buffer(hunk_range.start.buffer_id?)?;
let diff_base_buffer = diff_base_buffer
.or_else(|| self.current_diff_base_buffer(&buffer, cx))
.or_else(|| create_diff_base_buffer(&buffer, cx))?;
let deleted_text_lines = buffer.read(cx).diff_base().map(|diff_base| {
let diff_start_row = diff_base
.offset_to_point(hunk.diff_base_byte_range.start)
.row;
let diff_end_row = diff_base.offset_to_point(hunk.diff_base_byte_range.end).row;
diff_end_row - diff_start_row
})?;
Some((diff_base_buffer, deleted_text_lines))
})?;

let block_insert_index = match self.expanded_hunks.hunks.binary_search_by(|probe| {
Expand Down Expand Up @@ -1128,21 +1118,6 @@ fn editor_with_deleted_text(
(editor_height, editor)
}

fn buffer_diff_hunk(
buffer_snapshot: &MultiBufferSnapshot,
row_range: Range<Point>,
) -> Option<MultiBufferDiffHunk> {
let mut hunks = buffer_snapshot.git_diff_hunks_in_range(
MultiBufferRow(row_range.start.row)..MultiBufferRow(row_range.end.row),
);
let hunk = hunks.next()?;
let second_hunk = hunks.next();
if second_hunk.is_none() {
return Some(hunk);
}
None
}

impl DisplayDiffHunk {
pub fn start_display_row(&self) -> DisplayRow {
match self {
Expand Down Expand Up @@ -1209,7 +1184,10 @@ pub fn diff_hunk_to_display(
let hunk_end_point = Point::new(hunk_end_row.0, 0);

let multi_buffer_start = snapshot.buffer_snapshot.anchor_before(hunk_start_point);
let multi_buffer_end = snapshot.buffer_snapshot.anchor_after(hunk_end_point);
let multi_buffer_end = snapshot
.buffer_snapshot
.anchor_in_excerpt(multi_buffer_start.excerpt_id, hunk.buffer_range.end)
.unwrap();
let end = hunk_end_point.to_display_point(snapshot).row();

DisplayDiffHunk::Unfolded {
Expand Down
1 change: 0 additions & 1 deletion crates/language/src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,6 @@ impl Buffer {
this.non_text_state_update_count += 1;
if let Some(BufferDiffBase::PastBufferVersion { rope, .. }) = &mut this.diff_base {
*rope = diff_base_rope;
cx.emit(BufferEvent::DiffBaseChanged);
}
cx.emit(BufferEvent::DiffUpdated);
})
Expand Down

0 comments on commit e351148

Please sign in to comment.