From ff6a1518442a51d5af2c1ce6c8317848f0d1e094 Mon Sep 17 00:00:00 2001 From: "zed-zippy[bot]" <234243425+zed-zippy[bot]@users.noreply.github.com> Date: Tue, 17 Feb 2026 22:27:25 +0000 Subject: [PATCH] git: Fix panic in split diff when edits cause excerpts to merge (#49122) (cherry-pick to preview) (#49413) Cherry-pick of #49122 to preview ---- Our strategy for keeping excerpts in sync between the two sides of the split diff is generally - mutate the excerpts on the RHS - pull out the new excerpts for the affected path from the RHS - translate those ranges to the LHS - update the LHS excerpts with those ranges But this can break down when the translated ranges overlap on the LHS despite the original ranges not overlapping on the RHS. This can happen, for example, when a large deletion in a file crosses several excerpt boundaries. It's rare because normally excerpt boundaries in the project diff and branch diff are adjusted to keep entire diff hunks in view, but we defer this for dirty files, so it's not hard to trigger it when using the agent or editing an individual file while the diff is open in another tab. Release Notes: - Fixed a panic that could occur when editing files while the project diff or branch diff was open. --------- Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com> Co-authored-by: Cole Miller Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com> --- crates/editor/src/split.rs | 192 +++++++++++++++++++----- crates/multi_buffer/src/multi_buffer.rs | 31 +++- crates/multi_buffer/src/path_key.rs | 24 ++- 3 files changed, 203 insertions(+), 44 deletions(-) diff --git a/crates/editor/src/split.rs b/crates/editor/src/split.rs index bc6ab567d36336a97fd39f31e5123edaeebf189d..b37a95d3f4239d95a359c9e5f0c35cf5e4013a7b 100644 --- a/crates/editor/src/split.rs +++ b/crates/editor/src/split.rs @@ -674,21 +674,34 @@ impl SplittableEditor { // stream this for (path, diff) in path_diffs { self.rhs_multibuffer.update(cx, |rhs_multibuffer, cx| { - lhs.multibuffer.update(cx, |lhs_multibuffer, lhs_cx| { - for (lhs, rhs) in LhsEditor::update_path_excerpts_from_rhs( + let sync_result = lhs.multibuffer.update(cx, |lhs_multibuffer, lhs_cx| { + LhsEditor::update_path_excerpts_from_rhs( path.clone(), rhs_multibuffer, lhs_multibuffer, diff.clone(), lhs_cx, - ) { - companion.add_excerpt_mapping(lhs, rhs); + ) + }); + + if let Some((lhs_excerpt_ids, rhs_merge_groups)) = sync_result { + let mut final_rhs_ids = Vec::with_capacity(lhs_excerpt_ids.len()); + for group in rhs_merge_groups { + if group.len() == 1 { + final_rhs_ids.push(group[0]); + } else { + let merged_id = rhs_multibuffer.merge_excerpts(&group, cx); + final_rhs_ids.push(merged_id); + } } - companion.add_buffer_mapping( - diff.read(lhs_cx).base_text(lhs_cx).remote_id(), - diff.read(lhs_cx).buffer_id, - ); - }) + + for (rhs_id, lhs_id) in final_rhs_ids.iter().zip(lhs_excerpt_ids.iter()) { + companion.add_excerpt_mapping(*lhs_id, *rhs_id); + } + let lhs_buffer_id = diff.read(cx).base_text(cx).remote_id(); + let rhs_buffer_id = diff.read(cx).buffer_id; + companion.add_buffer_mapping(lhs_buffer_id, rhs_buffer_id); + } }); } @@ -1940,18 +1953,52 @@ fn mutate_excerpts_for_paths( let result = mutate(rhs_multibuffer, cx); if let Some(lhs) = lhs { + let mut sync_results = Vec::new(); + let mut diffs_for_mapping = Vec::new(); + for ((path, diff), old_rhs_ids) in paths_with_diffs.into_iter().zip(old_rhs_ids) { - lhs.multibuffer.update(cx, |lhs_multibuffer, lhs_cx| { + let sync_result = lhs.multibuffer.update(cx, |lhs_multibuffer, lhs_cx| { LhsEditor::sync_path_excerpts( path, old_rhs_ids, rhs_multibuffer, lhs_multibuffer, - diff, + diff.clone(), rhs_display_map, lhs_cx, - ); + ) }); + if let Some(sync_result) = sync_result { + sync_results.push(sync_result); + diffs_for_mapping.push(diff); + } + } + + for ((lhs_excerpt_ids, rhs_merge_groups), diff) in + sync_results.into_iter().zip(diffs_for_mapping.into_iter()) + { + let mut final_rhs_ids = Vec::with_capacity(lhs_excerpt_ids.len()); + for group in rhs_merge_groups { + if group.len() == 1 { + final_rhs_ids.push(group[0]); + } else { + let merged_id = rhs_multibuffer.merge_excerpts(&group, cx); + final_rhs_ids.push(merged_id); + } + } + + debug_assert_eq!(final_rhs_ids.len(), lhs_excerpt_ids.len()); + + if let Some(companion) = rhs_display_map.read(cx).companion().cloned() { + let lhs_buffer_id = diff.read(cx).base_text(cx).remote_id(); + let rhs_buffer_id = diff.read(cx).buffer_id; + companion.update(cx, |c, _| { + for (rhs_id, lhs_id) in final_rhs_ids.iter().zip(lhs_excerpt_ids.iter()) { + c.add_excerpt_mapping(*lhs_id, *rhs_id); + } + c.add_buffer_mapping(lhs_buffer_id, rhs_buffer_id); + }); + } } } @@ -1965,13 +2012,13 @@ impl LhsEditor { lhs_multibuffer: &mut MultiBuffer, diff: Entity, lhs_cx: &mut Context, - ) -> Vec<(ExcerptId, ExcerptId)> { + ) -> Option<(Vec, Vec>)> { let rhs_excerpt_ids: Vec = rhs_multibuffer.excerpts_for_path(&path_key).collect(); let Some(excerpt_id) = rhs_multibuffer.excerpts_for_path(&path_key).next() else { lhs_multibuffer.remove_excerpts_for_path(path_key, lhs_cx); - return Vec::new(); + return None; }; let rhs_multibuffer_snapshot = rhs_multibuffer.snapshot(lhs_cx); @@ -2007,14 +2054,14 @@ impl LhsEditor { }) .collect(); - let (ids, _) = lhs_multibuffer.update_path_excerpts( - path_key.clone(), + let lhs_result = lhs_multibuffer.update_path_excerpts( + path_key, base_text_buffer.clone(), &base_text_buffer_snapshot, new, lhs_cx, ); - if !ids.is_empty() + if !lhs_result.excerpt_ids.is_empty() && lhs_multibuffer .diff_for(base_text_buffer.read(lhs_cx).remote_id()) .is_none_or(|old_diff| old_diff.entity_id() != diff.entity_id()) @@ -2022,12 +2069,32 @@ impl LhsEditor { lhs_multibuffer.add_inverted_diff(diff, lhs_cx); } - let lhs_excerpt_ids: Vec = - lhs_multibuffer.excerpts_for_path(&path_key).collect(); + let rhs_merge_groups: Vec> = { + let mut groups = Vec::new(); + let mut current_group = Vec::new(); + let mut last_id = None; + + for (i, &lhs_id) in lhs_result.excerpt_ids.iter().enumerate() { + if last_id == Some(lhs_id) { + current_group.push(rhs_excerpt_ids[i]); + } else { + if !current_group.is_empty() { + groups.push(current_group); + } + current_group = vec![rhs_excerpt_ids[i]]; + last_id = Some(lhs_id); + } + } + if !current_group.is_empty() { + groups.push(current_group); + } + groups + }; - debug_assert_eq!(rhs_excerpt_ids.len(), lhs_excerpt_ids.len()); + let deduplicated_lhs_ids: Vec = + lhs_result.excerpt_ids.iter().dedup().copied().collect(); - lhs_excerpt_ids.into_iter().zip(rhs_excerpt_ids).collect() + Some((deduplicated_lhs_ids, rhs_merge_groups)) } fn sync_path_excerpts( @@ -2038,7 +2105,7 @@ impl LhsEditor { diff: Entity, rhs_display_map: &Entity, lhs_cx: &mut Context, - ) { + ) -> Option<(Vec, Vec>)> { let old_lhs_excerpt_ids: Vec = lhs_multibuffer.excerpts_for_path(&path_key).collect(); @@ -2048,25 +2115,13 @@ impl LhsEditor { }); } - let mappings = Self::update_path_excerpts_from_rhs( + Self::update_path_excerpts_from_rhs( path_key, rhs_multibuffer, lhs_multibuffer, - diff.clone(), + diff, lhs_cx, - ); - - let lhs_buffer_id = diff.read(lhs_cx).base_text(lhs_cx).remote_id(); - let rhs_buffer_id = diff.read(lhs_cx).buffer_id; - - if let Some(companion) = rhs_display_map.read(lhs_cx).companion().cloned() { - companion.update(lhs_cx, |c, _| { - for (lhs, rhs) in mappings { - c.add_excerpt_mapping(lhs, rhs); - } - c.add_buffer_mapping(lhs_buffer_id, rhs_buffer_id); - }); - } + ) } } @@ -5455,6 +5510,69 @@ mod tests { ); } + #[gpui::test] + async fn test_edit_spanning_excerpt_boundaries_then_resplit(cx: &mut gpui::TestAppContext) { + use rope::Point; + use unindent::Unindent as _; + + let (editor, mut cx) = init_test(cx, SoftWrap::None, DiffViewStyle::Split).await; + + let base_text = " + aaa + bbb + ccc + ddd + eee + fff + ggg + hhh + iii + jjj + kkk + lll + " + .unindent(); + let current_text = base_text.clone(); + + 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.set_excerpts_for_path( + path, + buffer.clone(), + vec![ + Point::new(0, 0)..Point::new(3, 3), + Point::new(5, 0)..Point::new(8, 3), + Point::new(10, 0)..Point::new(11, 3), + ], + 0, + diff.clone(), + cx, + ); + }); + + cx.run_until_parked(); + + buffer.update(cx, |buffer, cx| { + buffer.edit([(Point::new(1, 0)..Point::new(10, 0), "")], None, cx); + }); + + cx.run_until_parked(); + + editor.update_in(cx, |splittable_editor, window, cx| { + splittable_editor.unsplit(window, cx); + }); + + cx.run_until_parked(); + + editor.update_in(cx, |splittable_editor, window, cx| { + splittable_editor.split(window, cx); + }); + + cx.run_until_parked(); + } + #[gpui::test] async fn test_range_folds_removed_on_split(cx: &mut gpui::TestAppContext) { use rope::Point; diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 303fdfad30378214c084fdd285693e2f14b67a25..cd40c20499c3400feaeb041d52ddf12ede8a62ca 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -57,7 +57,7 @@ use theme::SyntaxTheme; use util::post_inc; use ztracing::instrument; -pub use self::path_key::PathKey; +pub use self::path_key::{PathExcerptInsertResult, PathKey}; pub static EXCERPT_CONTEXT_LINES: OnceLock u32> = OnceLock::new(); @@ -2164,6 +2164,35 @@ impl MultiBuffer { None } + pub fn merge_excerpts( + &mut self, + excerpt_ids: &[ExcerptId], + cx: &mut Context, + ) -> ExcerptId { + debug_assert!(!excerpt_ids.is_empty()); + if excerpt_ids.len() == 1 { + return excerpt_ids[0]; + } + + let snapshot = self.snapshot(cx); + + let first_range = snapshot + .context_range_for_excerpt(excerpt_ids[0]) + .expect("first excerpt must exist"); + let last_range = snapshot + .context_range_for_excerpt(*excerpt_ids.last().unwrap()) + .expect("last excerpt must exist"); + + let union_range = first_range.start..last_range.end; + + drop(snapshot); + + self.resize_excerpt(excerpt_ids[0], union_range, cx); + self.remove_excerpts(excerpt_ids[1..].iter().copied(), cx); + + excerpt_ids[0] + } + pub fn remove_excerpts( &mut self, excerpt_ids: impl IntoIterator, diff --git a/crates/multi_buffer/src/path_key.rs b/crates/multi_buffer/src/path_key.rs index 8c20f211f61990b3775d231dc37a80352d7b9b98..e829ee00794bf773e30cf61f98f3013f51396f8b 100644 --- a/crates/multi_buffer/src/path_key.rs +++ b/crates/multi_buffer/src/path_key.rs @@ -13,6 +13,12 @@ use crate::{ Anchor, ExcerptId, ExcerptRange, ExpandExcerptDirection, MultiBuffer, build_excerpt_ranges, }; +#[derive(Debug, Clone)] +pub struct PathExcerptInsertResult { + pub excerpt_ids: Vec, + pub added_new_excerpt: bool, +} + #[derive(PartialEq, Eq, Ord, PartialOrd, Clone, Hash, Debug)] pub struct PathKey { // Used by the derived PartialOrd & Ord @@ -268,12 +274,15 @@ impl MultiBuffer { counts: Vec, cx: &mut Context, ) -> (Vec>, bool) { - let (excerpt_ids, added_a_new_excerpt) = - self.update_path_excerpts(path, buffer, buffer_snapshot, new, cx); + let insert_result = self.update_path_excerpts(path, buffer, buffer_snapshot, new, cx); let mut result = Vec::new(); let mut ranges = ranges.into_iter(); - for (excerpt_id, range_count) in excerpt_ids.into_iter().zip(counts.into_iter()) { + for (excerpt_id, range_count) in insert_result + .excerpt_ids + .into_iter() + .zip(counts.into_iter()) + { for range in ranges.by_ref().take(range_count) { let range = Anchor::range_in_buffer( excerpt_id, @@ -283,7 +292,7 @@ impl MultiBuffer { result.push(range) } } - (result, added_a_new_excerpt) + (result, insert_result.added_new_excerpt) } pub fn update_path_excerpts( @@ -293,7 +302,7 @@ impl MultiBuffer { buffer_snapshot: &BufferSnapshot, new: Vec>, cx: &mut Context, - ) -> (Vec, bool) { + ) -> PathExcerptInsertResult { let mut insert_after = self .excerpts_by_path .range(..path.clone()) @@ -443,6 +452,9 @@ impl MultiBuffer { self.excerpts_by_path.insert(path, excerpt_ids); } - (excerpt_ids, added_a_new_excerpt) + PathExcerptInsertResult { + excerpt_ids, + added_new_excerpt: added_a_new_excerpt, + } } }