From d7129634eed30ec5a4140686e3f2885f77c39af0 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 9 Feb 2026 17:16:11 -0500 Subject: [PATCH] git: Fix missing excerpts panic with side-by-side diff (#48755) When we update excerpts, pull the changes to the `Companion` excerpt mappings into the multibuffer's update block, so that buffer subscriptions don't get the chance to run and observe an invalid state while attempting to snapshot the editor Release Notes: - N/A --------- Co-authored-by: cameron Co-authored-by: Jakub Konka --- crates/editor/src/split.rs | 262 ++++++++++++++++++------------------- 1 file changed, 131 insertions(+), 131 deletions(-) diff --git a/crates/editor/src/split.rs b/crates/editor/src/split.rs index 462e348aa7b5af8b64284b5ede370bc3a5792b57..c581d9b6af8fa8aeb0e566c64b0ed7ded916a508 100644 --- a/crates/editor/src/split.rs +++ b/crates/editor/src/split.rs @@ -573,7 +573,7 @@ impl SplittableEditor { }), ); - let mut lhs = LhsEditor { + let lhs = LhsEditor { editor: lhs_editor, multibuffer: lhs_multibuffer, was_last_focused: false, @@ -613,15 +613,23 @@ impl SplittableEditor { // stream this for (path, diff) in path_diffs { - for (lhs, rhs) in - lhs.update_path_excerpts_from_rhs(path, &self.rhs_multibuffer, diff.clone(), cx) - { - companion.add_excerpt_mapping(lhs, rhs); - } - companion.add_buffer_mapping( - diff.read(cx).base_text(cx).remote_id(), - diff.read(cx).buffer_id, - ); + 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( + path.clone(), + rhs_multibuffer, + lhs_multibuffer, + diff.clone(), + lhs_cx, + ) { + companion.add_excerpt_mapping(lhs, rhs); + } + companion.add_buffer_mapping( + diff.read(lhs_cx).base_text(lhs_cx).remote_id(), + diff.read(lhs_cx).buffer_id, + ); + }) + }); } let companion = cx.new(|_| companion); @@ -924,11 +932,7 @@ impl SplittableEditor { cx: &mut Context, ) -> (Vec>, bool) { let rhs_display_map = self.rhs_editor.read(cx).display_map.clone(); - let lhs_display_map = self - .lhs - .as_ref() - .map(|s| s.editor.read(cx).display_map.clone()); - + let lhs = &mut self.lhs; let (anchors, added_a_new_excerpt) = self.rhs_multibuffer.update(cx, |rhs_multibuffer, cx| { let (anchors, added_a_new_excerpt) = rhs_multibuffer.set_excerpts_for_path( @@ -945,22 +949,23 @@ impl SplittableEditor { { rhs_multibuffer.add_diff(diff.clone(), cx); } + + if let Some(lhs) = lhs { + lhs.multibuffer.update(cx, |lhs_multibuffer, lhs_cx| { + LhsEditor::sync_path_excerpts( + path, + rhs_multibuffer, + lhs_multibuffer, + diff, + &rhs_display_map, + lhs_cx, + ) + }) + } + (anchors, added_a_new_excerpt) }); - if let Some(lhs) = &mut self.lhs { - if let Some(lhs_display_map) = &lhs_display_map { - lhs.sync_path_excerpts( - path, - &self.rhs_multibuffer, - diff, - &rhs_display_map, - lhs_display_map, - cx, - ); - } - } - (anchors, added_a_new_excerpt) } @@ -971,55 +976,64 @@ impl SplittableEditor { direction: ExpandExcerptDirection, cx: &mut Context, ) { - let mut corresponding_paths = HashMap::default(); - self.rhs_multibuffer.update(cx, |multibuffer, cx| { - let snapshot = multibuffer.snapshot(cx); - if self.lhs.is_some() { - corresponding_paths = excerpt_ids + let rhs_display_map = self.rhs_editor.read(cx).display_map.clone(); + + let lhs = &mut self.lhs; + let lhs_multibuffer = lhs.as_ref().map(|l| l.multibuffer.clone()); + self.rhs_multibuffer.update(cx, |rhs_multibuffer, cx| { + if let Some(lhs_multibuffer) = lhs_multibuffer { + let snapshot = rhs_multibuffer.snapshot(cx); + let affected_paths: HashMap<_, _> = excerpt_ids .clone() .map(|excerpt_id| { - let path = multibuffer.path_for_excerpt(excerpt_id).unwrap(); + let path = rhs_multibuffer.path_for_excerpt(excerpt_id).unwrap(); let buffer = snapshot.buffer_for_excerpt(excerpt_id).unwrap(); - let diff = multibuffer.diff_for(buffer.remote_id()).unwrap(); + let diff = rhs_multibuffer.diff_for(buffer.remote_id()).unwrap(); (path, diff) }) - .collect::>(); - } - multibuffer.expand_excerpts(excerpt_ids.clone(), lines, direction, cx); - }); - - if let Some(lhs) = &mut self.lhs { - let rhs_display_map = self.rhs_editor.read(cx).display_map.clone(); - let lhs_display_map = lhs.editor.read(cx).display_map.clone(); - for (path, diff) in corresponding_paths { - lhs.sync_path_excerpts( - path, - &self.rhs_multibuffer, - diff, - &rhs_display_map, - &lhs_display_map, - cx, - ); - } - } + .collect(); + rhs_multibuffer.expand_excerpts(excerpt_ids.clone(), lines, direction, cx); + for (path, diff) in affected_paths { + lhs_multibuffer.update(cx, |lhs_multibuffer, lhs_cx| { + LhsEditor::sync_path_excerpts( + path, + rhs_multibuffer, + lhs_multibuffer, + diff, + &rhs_display_map, + lhs_cx, + ); + }); + } + } else { + rhs_multibuffer.expand_excerpts(excerpt_ids.clone(), lines, direction, cx); + }; + }); } pub fn remove_excerpts_for_path(&mut self, path: PathKey, cx: &mut Context) { - self.rhs_multibuffer.update(cx, |buffer, cx| { - buffer.remove_excerpts_for_path(path.clone(), cx) - }); + let rhs_display_map = self.rhs_editor.read(cx).display_map.clone(); + if let Some(lhs) = &self.lhs { - let rhs_display_map = self.rhs_editor.read(cx).display_map.clone(); - let lhs_display_map = lhs.editor.read(cx).display_map.clone(); - lhs.remove_mappings_for_path( - &path, - &self.rhs_multibuffer, - &rhs_display_map, - &lhs_display_map, - cx, - ); - lhs.multibuffer - .update(cx, |buffer, cx| buffer.remove_excerpts_for_path(path, cx)) + self.rhs_multibuffer.update(cx, |rhs_multibuffer, cx| { + lhs.multibuffer.update(cx, |lhs_multibuffer, cx| { + LhsEditor::remove_mappings_for_path( + &path, + rhs_multibuffer, + lhs_multibuffer, + &rhs_display_map, + cx, + ); + }); + rhs_multibuffer.remove_excerpts_for_path(path.clone(), cx); + }); + lhs.multibuffer.update(cx, |lhs_multibuffer, cx| { + lhs_multibuffer.remove_excerpts_for_path(path, cx); + }); + } else { + self.rhs_multibuffer.update(cx, |rhs_multibuffer, cx| { + rhs_multibuffer.remove_excerpts_for_path(path.clone(), cx); + }); } } } @@ -1786,32 +1800,29 @@ impl Render for SplittableEditor { impl LhsEditor { fn update_path_excerpts_from_rhs( - &mut self, path_key: PathKey, - rhs_multibuffer: &Entity, + rhs_multibuffer: &MultiBuffer, + lhs_multibuffer: &mut MultiBuffer, diff: Entity, - cx: &mut App, + lhs_cx: &mut Context, ) -> Vec<(ExcerptId, ExcerptId)> { - let rhs_multibuffer_ref = rhs_multibuffer.read(cx); let rhs_excerpt_ids: Vec = - rhs_multibuffer_ref.excerpts_for_path(&path_key).collect(); + rhs_multibuffer.excerpts_for_path(&path_key).collect(); - let Some(excerpt_id) = rhs_multibuffer_ref.excerpts_for_path(&path_key).next() else { - self.multibuffer.update(cx, |multibuffer, cx| { - multibuffer.remove_excerpts_for_path(path_key, cx); - }); + 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(); }; - let rhs_multibuffer_snapshot = rhs_multibuffer_ref.snapshot(cx); + let rhs_multibuffer_snapshot = rhs_multibuffer.snapshot(lhs_cx); let main_buffer = rhs_multibuffer_snapshot .buffer_for_excerpt(excerpt_id) .unwrap(); - let base_text_buffer = diff.read(cx).base_text_buffer(); - let diff_snapshot = diff.read(cx).snapshot(cx); - let base_text_buffer_snapshot = base_text_buffer.read(cx).snapshot(); - let new = rhs_multibuffer_ref - .excerpts_for_buffer(main_buffer.remote_id(), cx) + let base_text_buffer = diff.read(lhs_cx).base_text_buffer(); + let diff_snapshot = diff.read(lhs_cx).snapshot(lhs_cx); + let base_text_buffer_snapshot = base_text_buffer.read(lhs_cx).snapshot(); + let new = rhs_multibuffer + .excerpts_for_buffer(main_buffer.remote_id(), lhs_cx) .into_iter() .map(|(_, excerpt_range)| { let point_range_to_base_text_point_range = |range: Range| { @@ -1836,30 +1847,23 @@ impl LhsEditor { }) .collect(); - self.editor.update(cx, |editor, cx| { - editor.buffer().update(cx, |buffer, cx| { - let (ids, _) = buffer.update_path_excerpts( - path_key.clone(), - base_text_buffer.clone(), - &base_text_buffer_snapshot, - new, - cx, - ); - if !ids.is_empty() - && buffer - .diff_for(base_text_buffer.read(cx).remote_id()) - .is_none_or(|old_diff| old_diff.entity_id() != diff.entity_id()) - { - buffer.add_inverted_diff(diff, cx); - } - }) - }); + let (ids, _) = lhs_multibuffer.update_path_excerpts( + path_key.clone(), + base_text_buffer.clone(), + &base_text_buffer_snapshot, + new, + lhs_cx, + ); + if !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()) + { + lhs_multibuffer.add_inverted_diff(diff, lhs_cx); + } - let lhs_excerpt_ids: Vec = self - .multibuffer - .read(cx) - .excerpts_for_path(&path_key) - .collect(); + let lhs_excerpt_ids: Vec = + lhs_multibuffer.excerpts_for_path(&path_key).collect(); debug_assert_eq!(rhs_excerpt_ids.len(), lhs_excerpt_ids.len()); @@ -1867,30 +1871,34 @@ impl LhsEditor { } fn sync_path_excerpts( - &mut self, path_key: PathKey, - rhs_multibuffer: &Entity, + rhs_multibuffer: &MultiBuffer, + lhs_multibuffer: &mut MultiBuffer, diff: Entity, rhs_display_map: &Entity, - lhs_display_map: &Entity, - cx: &mut App, + lhs_cx: &mut Context, ) { - self.remove_mappings_for_path( + Self::remove_mappings_for_path( &path_key, rhs_multibuffer, + lhs_multibuffer, rhs_display_map, - lhs_display_map, - cx, + lhs_cx, ); - let mappings = - self.update_path_excerpts_from_rhs(path_key, rhs_multibuffer, diff.clone(), cx); + let mappings = Self::update_path_excerpts_from_rhs( + path_key, + rhs_multibuffer, + lhs_multibuffer, + diff.clone(), + lhs_cx, + ); - let lhs_buffer_id = diff.read(cx).base_text(cx).remote_id(); - let rhs_buffer_id = diff.read(cx).buffer_id; + 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(cx).companion().cloned() { - companion.update(cx, |c, _| { + 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); } @@ -1900,22 +1908,14 @@ impl LhsEditor { } fn remove_mappings_for_path( - &self, path_key: &PathKey, - rhs_multibuffer: &Entity, + rhs_multibuffer: &MultiBuffer, + lhs_multibuffer: &MultiBuffer, rhs_display_map: &Entity, - _lhs_display_map: &Entity, cx: &mut App, ) { - let rhs_excerpt_ids: Vec = rhs_multibuffer - .read(cx) - .excerpts_for_path(path_key) - .collect(); - let lhs_excerpt_ids: Vec = self - .multibuffer - .read(cx) - .excerpts_for_path(path_key) - .collect(); + let rhs_excerpt_ids: Vec = rhs_multibuffer.excerpts_for_path(path_key).collect(); + let lhs_excerpt_ids: Vec = lhs_multibuffer.excerpts_for_path(path_key).collect(); if let Some(companion) = rhs_display_map.read(cx).companion().cloned() { companion.update(cx, |c, _| {