History

buffer_diff: Fix panic when staging hunks with stale buffer snapshot (#51641)

When the buffer is edited after the diff is computed but before staging, anchor positions shift while diff_base_byte_range values don't. If the primary (HEAD) hunk extends past the unstaged (index) hunk, an edit in the extension region causes the overshoot calculation to produce an index_end that exceeds index_text.len(), panicking in rope::Cursor::suffix. Fix by clamping index_end to index_text.len(). This is safe because the computed index text is an optimistic approximation — the real staging happens at the filesystem level via git add/git reset. Closes ZED-5R2 Release Notes: - Fixed a source of panics when staging diff hunks

Lukas Wirth committed 8 months ago

Commit ebd80d71 parent a93c66e1 file · +55 −0Browse files

Showing the changes to crates/buffer_diff/src/buffer_diff.rs (1 of 1 file). Show all

Unified · Split

crates/buffer_diff/src/buffer_diff.rs+55−0
@@ -843,6 +843,16 @@ impl BufferDiffInner<Entity<language::Buffer>> { · 842 unmodified lines
843843 .end
844844 .saturating_sub(prev_unstaged_hunk_buffer_end);
845845 let index_end = prev_unstaged_hunk_base_text_end + end_overshoot;
846+
847+ // Clamp to the index text bounds. The overshoot mapping assumes that
848+ // text between unstaged hunks is identical in the buffer and index.
849+ // When the buffer has been edited since the diff was computed, anchor
850+ // positions shift while diff_base_byte_range values don't, which can
851+ // cause index_end to exceed index_text.len().
852+ // See `test_stage_all_with_stale_buffer` which would hit an assert
853+ // without these min calls
854+ let index_end = index_end.min(index_text.len());
855+ let index_start = index_start.min(index_end);
846856 let index_byte_range = index_start..index_end;
847857
848858 let replacement_text = match new_status {
@@ -2678,6 +2688,51 @@ mod tests { · 1829 unmodified lines
26782688 });
26792689 }
26802690
2691+ #[gpui::test]
2692+ async fn test_stage_all_with_stale_buffer(cx: &mut TestAppContext) {
2693+ // Regression test for ZED-5R2: when the buffer is edited after the diff is
2694+ // computed but before staging, anchor positions shift while diff_base_byte_range
2695+ // values don't. If the primary (HEAD) hunk extends past the unstaged (index)
2696+ // hunk, an edit in the extension region shifts the primary hunk end without
2697+ // shifting the unstaged hunk end. The overshoot calculation then produces an
2698+ // index_end that exceeds index_text.len().
2699+ //
2700+ // Setup:
2701+ // HEAD: "aaa\nbbb\nccc\n" (primary hunk covers lines 1-2)
2702+ // Index: "aaa\nbbb\nCCC\n" (unstaged hunk covers line 1 only)
2703+ // Buffer: "aaa\nBBB\nCCC\n" (both lines differ from HEAD)
2704+ //
2705+ // The primary hunk spans buffer offsets 4..12, but the unstaged hunk only
2706+ // spans 4..8. The pending hunk extends 4 bytes past the unstaged hunk.
2707+ // An edit at offset 9 (inside "CCC") shifts the primary hunk end from 12
2708+ // to 13 but leaves the unstaged hunk end at 8, making index_end = 13 > 12.
2709+ let head_text = "aaa\nbbb\nccc\n";
2710+ let index_text = "aaa\nbbb\nCCC\n";
2711+ let buffer_text = "aaa\nBBB\nCCC\n";
2712+
2713+ let mut buffer = Buffer::new(
2714+ ReplicaId::LOCAL,
2715+ BufferId::new(1).unwrap(),
2716+ buffer_text.to_string(),
2717+ );
2718+
2719+ let unstaged_diff = cx.new(|cx| BufferDiff::new_with_base_text(index_text, &buffer, cx));
2720+ let uncommitted_diff = cx.new(|cx| {
2721+ let mut diff = BufferDiff::new_with_base_text(head_text, &buffer, cx);
2722+ diff.set_secondary_diff(unstaged_diff);
2723+ diff
2724+ });
2725+
2726+ // Edit the buffer in the region between the unstaged hunk end (offset 8)
2727+ // and the primary hunk end (offset 12). This shifts the primary hunk end
2728+ // but not the unstaged hunk end.
2729+ buffer.edit([(9..9, "Z")]);
2730+
2731+ uncommitted_diff.update(cx, |diff, cx| {
2732+ diff.stage_or_unstage_all_hunks(true, &buffer, true, cx);
2733+ });
2734+ }
2735+
26812736 #[gpui::test]
26822737 async fn test_toggling_stage_and_unstage_same_hunk(cx: &mut TestAppContext) {
26832738 let head_text = "