From 0823cacb7a7a7adce66a6452d4f5df832e1734fd Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 20 Feb 2026 16:49:35 +0100 Subject: [PATCH] multi_buffer: Fix outdated anchors resolving incorrectly in diff hunks in `resolve_summary_for_anchor` (#49719) Closes https://github.com/zed-industries/zed/issues/48441 Release Notes: - Fixed panics with selection handling in expanded diff hunk --- crates/editor/src/selections_collection.rs | 9 ++- crates/multi_buffer/src/multi_buffer.rs | 94 +++++++++++++++++++--- crates/vim/src/normal/change.rs | 33 +++++++- 3 files changed, 123 insertions(+), 13 deletions(-) diff --git a/crates/editor/src/selections_collection.rs b/crates/editor/src/selections_collection.rs index 8c55a47882c2c9b99e2fdebde7fed3246c3493e8..01f67be06d5effed50a6a83a3574d3403dfa90f3 100644 --- a/crates/editor/src/selections_collection.rs +++ b/crates/editor/src/selections_collection.rs @@ -1197,7 +1197,14 @@ fn resolve_selections_point<'a>( selections.map(move |s| { let start = summaries.next().unwrap(); let end = summaries.next().unwrap(); - assert!(start <= end, "start: {:?}, end: {:?}", start, end); + assert!( + start <= end, + "anchors: start: {:?}, end: {:?}; resolved to: start: {:?}, end: {:?}", + s.start, + s.end, + start, + end + ); Selection { id: s.id, start, diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 15d7b9f3610eaf9e9063c7da95e915c73f95a341..c1764ff25034da6fa6c6218e0de33f6bd63d24bb 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -5347,8 +5347,8 @@ impl MultiBufferSnapshot { } } + let excerpt_start_position = ExcerptDimension(start); if self.diff_transforms.is_empty() { - let excerpt_start_position = ExcerptDimension(start); if let Some(excerpt) = item { if excerpt.id != excerpt_id && excerpt_id != ExcerptId::max() { return excerpt_start_position.0; @@ -5382,10 +5382,9 @@ impl MultiBufferSnapshot { .cursor::, OutputDimension>>(()); diff_transforms_cursor.next(); - let excerpt_start_position = ExcerptDimension(start); if let Some(excerpt) = item { if excerpt.id != excerpt_id && excerpt_id != ExcerptId::max() { - return self.resolve_summary_for_anchor( + return self.resolve_summary_for_min_or_max_anchor( &Anchor::min(), excerpt_start_position, &mut diff_transforms_cursor, @@ -5413,10 +5412,15 @@ impl MultiBufferSnapshot { if diff_transforms_cursor.start().0 < position { diff_transforms_cursor.seek_forward(&position, Bias::Left); } - self.resolve_summary_for_anchor(&anchor, position, &mut diff_transforms_cursor) + self.resolve_summary_for_anchor( + &anchor, + position, + &mut diff_transforms_cursor, + &excerpt.buffer, + ) } else { diff_transforms_cursor.seek_forward(&excerpt_start_position, Bias::Left); - self.resolve_summary_for_anchor( + self.resolve_summary_for_min_or_max_anchor( &Anchor::max(), excerpt_start_position, &mut diff_transforms_cursor, @@ -5425,6 +5429,9 @@ impl MultiBufferSnapshot { } } + /// Maps an anchor's excerpt-space position to its output-space position by + /// walking the diff transforms. The cursor is shared across consecutive + /// calls, so it may already be partway through the transform list. fn resolve_summary_for_anchor( &self, anchor: &Anchor, @@ -5433,24 +5440,29 @@ impl MultiBufferSnapshot { DiffTransform, Dimensions, OutputDimension>, >, + excerpt_buffer: &text::BufferSnapshot, ) -> MBD where MBD: MultiBufferDimension + Ord + Sub + AddAssign<::Output>, { loop { let transform_end_position = diff_transforms.end().0; - let at_transform_end = - transform_end_position == excerpt_position && diff_transforms.item().is_some(); - if at_transform_end && anchor.text_anchor.bias == Bias::Right { + let item = diff_transforms.item(); + let at_transform_end = transform_end_position == excerpt_position && item.is_some(); + + // A right-biased anchor at a transform boundary belongs to the + // *next* transform, so advance past the current one. + if anchor.text_anchor.bias == Bias::Right && at_transform_end { diff_transforms.next(); continue; } let mut position = diff_transforms.start().1; - match diff_transforms.item() { + match item { Some(DiffTransform::DeletedHunk { buffer_id, base_text_byte_range, + hunk_info, .. }) => { if let Some(diff_base_anchor) = &anchor.diff_base_anchor @@ -5458,6 +5470,8 @@ impl MultiBufferSnapshot { self.diffs.get(buffer_id).map(|diff| diff.base_text()) && diff_base_anchor.is_valid(&base_text) { + // The anchor carries a diff-base position — resolve it + // to a location inside the deleted hunk. let base_text_offset = diff_base_anchor.to_offset(base_text); if base_text_offset >= base_text_byte_range.start && base_text_offset <= base_text_byte_range.end @@ -5468,12 +5482,34 @@ impl MultiBufferSnapshot { ); position.0.add_text_dim(&position_in_hunk); } else if at_transform_end { + // diff_base offset falls outside this hunk's range; + // advance to see if the next transform is a better fit. diff_transforms.next(); continue; } + } else if at_transform_end + && anchor + .text_anchor + .cmp(&hunk_info.hunk_start_anchor, excerpt_buffer) + .is_gt() + { + // The anchor has no (valid) diff-base position, so it + // belongs in the buffer content, not in the deleted + // hunk. However, after an edit deletes the text between + // the hunk boundary and this anchor, both resolve to + // the same excerpt_position—landing us here on the + // DeletedHunk left behind by the shared cursor. Use the + // CRDT ordering to detect that the anchor is strictly + // *past* the hunk boundary and skip to the following + // BufferContent. + diff_transforms.next(); + continue; } } _ => { + // On a BufferContent (or no transform). If the anchor + // carries a diff_base_anchor it needs a DeletedHunk, so + // advance to find one. if at_transform_end && anchor.diff_base_anchor.is_some() { diff_transforms.next(); continue; @@ -5487,6 +5523,41 @@ impl MultiBufferSnapshot { } } + /// Like `resolve_summary_for_anchor` but optimized for min/max anchors. + fn resolve_summary_for_min_or_max_anchor( + &self, + anchor: &Anchor, + excerpt_position: ExcerptDimension, + diff_transforms: &mut Cursor< + DiffTransform, + Dimensions, OutputDimension>, + >, + ) -> MBD + where + MBD: MultiBufferDimension + Ord + Sub + AddAssign<::Output>, + { + loop { + let transform_end_position = diff_transforms.end().0; + let item = diff_transforms.item(); + let at_transform_end = transform_end_position == excerpt_position && item.is_some(); + + // A right-biased anchor at a transform boundary belongs to the + // *next* transform, so advance past the current one. + if anchor.text_anchor.bias == Bias::Right && at_transform_end { + diff_transforms.next(); + continue; + } + + let mut position = diff_transforms.start().1; + if let Some(DiffTransform::BufferContent { .. }) | None = item { + let overshoot = excerpt_position - diff_transforms.start().0; + position += overshoot; + } + + return position.0; + } + } + fn excerpt_offset_for_anchor(&self, anchor: &Anchor) -> ExcerptOffset { let mut cursor = self .excerpts @@ -5557,7 +5628,7 @@ impl MultiBufferSnapshot { let excerpt_start_position = ExcerptDimension(MBD::from_summary(&cursor.start().text)); if let Some(excerpt) = cursor.item() { if excerpt.id != excerpt_id && excerpt_id != ExcerptId::max() { - let position = self.resolve_summary_for_anchor( + let position = self.resolve_summary_for_min_or_max_anchor( &Anchor::min(), excerpt_start_position, &mut diff_transforms_cursor, @@ -5595,11 +5666,12 @@ impl MultiBufferSnapshot { anchor, position, &mut diff_transforms_cursor, + &excerpt.buffer, )); } } else { diff_transforms_cursor.seek_forward(&excerpt_start_position, Bias::Left); - let position = self.resolve_summary_for_anchor( + let position = self.resolve_summary_for_min_or_max_anchor( &Anchor::max(), excerpt_start_position, &mut diff_transforms_cursor, diff --git a/crates/vim/src/normal/change.rs b/crates/vim/src/normal/change.rs index ceb0c1d51b3e77a5dd4a2f7397b052905a7f2406..8872f23b7c2f3f1e2f57a01cca12b77d4e8f28c5 100644 --- a/crates/vim/src/normal/change.rs +++ b/crates/vim/src/normal/change.rs @@ -205,7 +205,8 @@ fn expand_changed_word_selection( mod test { use indoc::indoc; - use crate::test::NeovimBackedTestContext; + use crate::state::Mode; + use crate::test::{NeovimBackedTestContext, VimTestContext}; #[gpui::test] async fn test_change_h(cx: &mut gpui::TestAppContext) { @@ -703,4 +704,34 @@ mod test { .assert_matches(); } } + + #[gpui::test] + async fn test_change_with_selection_spanning_expanded_diff_hunk(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + + let diff_base = indoc! {" + fn main() { + println!(\"old\"); + } + "}; + + cx.set_state( + indoc! {" + fn main() { + ˇprintln!(\"new\"); + } + "}, + Mode::Normal, + ); + cx.set_head_text(diff_base); + cx.update_editor(|editor, window, cx| { + editor.expand_all_diff_hunks(&editor::actions::ExpandAllDiffHunks, window, cx); + }); + + // Enter visual mode and move up so the selection spans from the + // insertion (current line) into the deletion (diff base line). + // Then press `c` which in visual mode dispatches `vim::Substitute`, + // performing the change operation across the insertion/deletion boundary. + cx.simulate_keystrokes("v k c"); + } }