mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 03:14:56 +07:00
editor: Fix split diff spacer calculation for non-row-aligned patch groups (#53098)
Fixes a bug in the split diff spacer calculation when a patch group starts mid-row, sometimes causing extra spacers to be inserted. `spacer_blocks` already explicitly handles the case where `first_point` isn't at the start of `edit_for_first_point.old`, but the `while let Some(source_point) = source_points.next()` loop that follows implicitly assumes that `source_point` is at the start of `current_range`, which in turn seems to be based on the assumption that `current_range` starts at the beginning of a row. As it turns out, `current_range` isn't guaranteed to start at the beginning of a row, which can sometimes lead to incorrect spacer blocks being inserted. This addresses that by moving the existing `if edit_for_first_point.old.start < first_point` logic into the loop body as `if current_edit.old.start < current_boundary` in order to handle any non-row-aligned patch groups, not just the first one. Here's an example of how this bug could manifest: https://github.com/user-attachments/assets/1d3a5b4c-e4ad-4d87-804b-c4390d25f408 After: https://github.com/user-attachments/assets/b15acc62-33fe-4154-82e5-5cdf1806ffa7 Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed incorrect spacer blocks sometimes appearing in the split diff view when editing the file.
This commit is contained in:
parent
1376130066
commit
c079264372
2 changed files with 150 additions and 36 deletions
|
|
@ -1368,50 +1368,49 @@ impl BlockMap {
|
|||
|
||||
let mut delta = their_baseline.0 as i32 - our_baseline.0 as i32;
|
||||
|
||||
// If we started out in the middle of a hunk/group, work up to the end of that group to set up the main loop below.
|
||||
if edit_for_first_point.old.start < first_point {
|
||||
let mut current_boundary = first_point;
|
||||
let current_range = edit_for_first_point.new;
|
||||
while let Some(next_point) = source_points.peek().cloned() {
|
||||
let edit_for_next_point = excerpt.patch.edit_for_old_position(next_point);
|
||||
if edit_for_next_point.new.end > current_range.end {
|
||||
break;
|
||||
}
|
||||
source_points.next();
|
||||
current_boundary = next_point;
|
||||
}
|
||||
|
||||
let (new_delta, spacer) = determine_spacer(
|
||||
&mut our_wrapper,
|
||||
&mut companion_wrapper,
|
||||
current_boundary,
|
||||
current_range.end.min(excerpt.target_excerpt_range.end),
|
||||
delta,
|
||||
Bias::Left,
|
||||
);
|
||||
|
||||
delta = new_delta;
|
||||
if let Some((wrap_row, height)) = spacer {
|
||||
result.push((
|
||||
BlockPlacement::Above(wrap_row),
|
||||
Block::Spacer {
|
||||
id: SpacerId(self.next_block_id.fetch_add(1, SeqCst)),
|
||||
height,
|
||||
is_below: false,
|
||||
},
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
while let Some(source_point) = source_points.next() {
|
||||
let mut current_boundary = source_point;
|
||||
let current_range = excerpt.patch.edit_for_old_position(current_boundary).new;
|
||||
let current_edit = excerpt.patch.edit_for_old_position(current_boundary);
|
||||
let current_range = current_edit.new;
|
||||
|
||||
if current_boundary.column > 0 {
|
||||
debug_assert_eq!(current_boundary, excerpt.source_excerpt_range.end);
|
||||
break;
|
||||
}
|
||||
|
||||
if current_edit.old.start < current_boundary {
|
||||
while let Some(next_point) = source_points.peek().copied() {
|
||||
let edit_for_next_point = excerpt.patch.edit_for_old_position(next_point);
|
||||
if edit_for_next_point.new.end > current_range.end {
|
||||
break;
|
||||
}
|
||||
current_boundary = next_point;
|
||||
source_points.next();
|
||||
}
|
||||
|
||||
let (new_delta, spacer) = determine_spacer(
|
||||
&mut our_wrapper,
|
||||
&mut companion_wrapper,
|
||||
current_boundary,
|
||||
current_range.end.min(excerpt.target_excerpt_range.end),
|
||||
delta,
|
||||
Bias::Left,
|
||||
);
|
||||
|
||||
delta = new_delta;
|
||||
if let Some((wrap_row, height)) = spacer {
|
||||
result.push((
|
||||
BlockPlacement::Above(wrap_row),
|
||||
Block::Spacer {
|
||||
id: SpacerId(self.next_block_id.fetch_add(1, SeqCst)),
|
||||
height,
|
||||
is_below: false,
|
||||
},
|
||||
));
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
let (delta_at_start, mut spacer_at_start) = determine_spacer(
|
||||
&mut our_wrapper,
|
||||
&mut companion_wrapper,
|
||||
|
|
|
|||
|
|
@ -6062,6 +6062,121 @@ mod tests {
|
|||
cx.run_until_parked();
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_spacer_blocks_revert_after_temporary_edit(cx: &mut gpui::TestAppContext) {
|
||||
use rope::Point;
|
||||
use unindent::Unindent as _;
|
||||
|
||||
let (editor, mut cx) = init_test(cx, SoftWrap::EditorWidth, DiffViewStyle::Split).await;
|
||||
|
||||
let base_text = "
|
||||
aaa
|
||||
bbb
|
||||
"
|
||||
.unindent();
|
||||
let current_text = "
|
||||
aaa
|
||||
bbb
|
||||
ccc
|
||||
"
|
||||
.unindent();
|
||||
|
||||
let (buffer, diff) = buffer_with_diff(&base_text, ¤t_text, &mut cx);
|
||||
|
||||
editor.update(cx, |editor, cx| {
|
||||
let path = PathKey::for_buffer(&buffer, cx);
|
||||
editor.update_excerpts_for_path(
|
||||
path,
|
||||
buffer.clone(),
|
||||
vec![Point::new(0, 0)..buffer.read(cx).max_point()],
|
||||
0,
|
||||
diff.clone(),
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
assert_split_content(
|
||||
&editor,
|
||||
"
|
||||
§ <no file>
|
||||
§ -----
|
||||
aaa
|
||||
bbb
|
||||
ccc"
|
||||
.unindent(),
|
||||
"
|
||||
§ <no file>
|
||||
§ -----
|
||||
aaa
|
||||
bbb
|
||||
§ spacer"
|
||||
.unindent(),
|
||||
&mut cx,
|
||||
);
|
||||
|
||||
let buffer_snapshot = buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit([(Point::new(0, 3)..Point::new(0, 3), "\n")], None, cx);
|
||||
buffer.text_snapshot()
|
||||
});
|
||||
diff.update(cx, |diff, cx| {
|
||||
diff.recalculate_diff_sync(&buffer_snapshot, cx);
|
||||
});
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
assert_split_content(
|
||||
&editor,
|
||||
"
|
||||
§ <no file>
|
||||
§ -----
|
||||
aaa
|
||||
|
||||
bbb
|
||||
ccc"
|
||||
.unindent(),
|
||||
"
|
||||
§ <no file>
|
||||
§ -----
|
||||
aaa
|
||||
§ spacer
|
||||
bbb
|
||||
§ spacer"
|
||||
.unindent(),
|
||||
&mut cx,
|
||||
);
|
||||
|
||||
let buffer_snapshot = buffer.update(cx, |buffer, cx| {
|
||||
buffer.edit([(Point::new(0, 3)..Point::new(1, 0), "")], None, cx);
|
||||
buffer.text_snapshot()
|
||||
});
|
||||
diff.update(cx, |diff, cx| {
|
||||
diff.recalculate_diff_sync(&buffer_snapshot, cx);
|
||||
});
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
assert_split_content(
|
||||
&editor,
|
||||
"
|
||||
§ <no file>
|
||||
§ -----
|
||||
aaa
|
||||
bbb
|
||||
ccc"
|
||||
.unindent(),
|
||||
"
|
||||
§ <no file>
|
||||
§ -----
|
||||
aaa
|
||||
bbb
|
||||
§ spacer"
|
||||
.unindent(),
|
||||
&mut cx,
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_act_as_type(cx: &mut gpui::TestAppContext) {
|
||||
let (splittable_editor, cx) = init_test(cx, SoftWrap::None, DiffViewStyle::Split).await;
|
||||
|
|
|
|||
Loading…
Reference in a new issue