diff --git a/crates/buffer_diff/src/buffer_diff.rs b/crates/buffer_diff/src/buffer_diff.rs index c34cd0e49e13e6bb16ca8a76672649e8d3772763..04e5743224c9d6c05db5ce840ed3330d8c6723bf 100644 --- a/crates/buffer_diff/src/buffer_diff.rs +++ b/crates/buffer_diff/src/buffer_diff.rs @@ -248,7 +248,7 @@ impl BufferDiffSnapshot { self.inner.buffer_version() } - pub fn original_buffer_snapshot(&self) -> &text::BufferSnapshot { + fn original_buffer_snapshot(&self) -> &text::BufferSnapshot { &self.inner.buffer_snapshot } diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index a0b7f0d28d167419b5ae22eff6297541ef797327..df2c20bbc79c335a69561288c7197ae08e64ea3b 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -4595,7 +4595,7 @@ mod tests { [ExcerptRange::new(text::Anchor::MIN..text::Anchor::MAX)], cx, ); - mb.add_inverted_diff(diff.clone(), cx); + mb.add_inverted_diff(diff.clone(), rhs_buffer.clone(), cx); mb }); let rhs_multibuffer = cx.new(|cx| { diff --git a/crates/editor/src/split.rs b/crates/editor/src/split.rs index 8b9d2e20bb489eb9c8866af617ea6d0d16a5733f..ea2e9180fb94a57a017f629ec2bcb566aa14db36 100644 --- a/crates/editor/src/split.rs +++ b/crates/editor/src/split.rs @@ -2069,7 +2069,10 @@ impl LhsEditor { .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 main_buffer_entity = rhs_multibuffer + .buffer(main_buffer.remote_id()) + .expect("main buffer should exist in rhs_multibuffer"); + lhs_multibuffer.add_inverted_diff(diff, main_buffer_entity, lhs_cx); } let rhs_merge_groups: Vec> = { diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index 559ae80077f829eea6c95ee6023849d142c2ee0f..5cf1989431ff71abc97528e1b7f66a3c6c8ea209 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -2395,6 +2395,161 @@ mod tests { assert_eq!(contents, "ours\n"); } + #[gpui::test(iterations = 50)] + async fn test_split_diff_conflict_path_transition_with_dirty_buffer_invalid_anchor_panics( + cx: &mut TestAppContext, + ) { + init_test(cx); + + cx.update(|cx| { + cx.update_global::(|store, cx| { + store.update_user_settings(cx, |settings| { + settings.editor.diff_view_style = Some(DiffViewStyle::Split); + }); + }); + }); + + let build_conflict_text: fn(usize) -> String = |tag: usize| { + let mut lines = (0..80) + .map(|line_index| format!("line {line_index}")) + .collect::>(); + for offset in [5usize, 20, 37, 61] { + lines[offset] = format!("base-{tag}-line-{offset}"); + } + format!("{}\n", lines.join("\n")) + }; + let initial_conflict_text = build_conflict_text(0); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ + ".git": {}, + "helper.txt": "same\n", + "conflict.txt": initial_conflict_text, + }), + ) + .await; + fs.with_git_state(path!("/project/.git").as_ref(), true, |state| { + state + .refs + .insert("MERGE_HEAD".into(), "conflict-head".into()); + }) + .unwrap(); + fs.set_status_for_repo( + path!("/project/.git").as_ref(), + &[( + "conflict.txt", + FileStatus::Unmerged(UnmergedStatus { + first_head: UnmergedStatusCode::Updated, + second_head: UnmergedStatusCode::Updated, + }), + )], + ); + fs.set_merge_base_content_for_repo( + path!("/project/.git").as_ref(), + &[ + ("conflict.txt", build_conflict_text(1)), + ("helper.txt", "same\n".to_string()), + ], + ); + + let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + let _project_diff = cx + .update(|window, cx| { + ProjectDiff::new_with_default_branch(project.clone(), workspace, window, cx) + }) + .await + .unwrap(); + cx.run_until_parked(); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/project/conflict.txt"), cx) + }) + .await + .unwrap(); + buffer.update(cx, |buffer, cx| buffer.edit([(0..0, "dirty\n")], None, cx)); + assert!(buffer.read_with(cx, |buffer, _| buffer.is_dirty())); + cx.run_until_parked(); + + cx.update(|window, cx| { + let fs = fs.clone(); + window + .spawn(cx, async move |cx| { + cx.background_executor().simulate_random_delay().await; + fs.with_git_state(path!("/project/.git").as_ref(), true, |state| { + state.refs.insert("HEAD".into(), "head-1".into()); + state.refs.remove("MERGE_HEAD"); + }) + .unwrap(); + fs.set_status_for_repo( + path!("/project/.git").as_ref(), + &[ + ( + "conflict.txt", + FileStatus::Tracked(TrackedStatus { + index_status: git::status::StatusCode::Modified, + worktree_status: git::status::StatusCode::Modified, + }), + ), + ( + "helper.txt", + FileStatus::Tracked(TrackedStatus { + index_status: git::status::StatusCode::Modified, + worktree_status: git::status::StatusCode::Modified, + }), + ), + ], + ); + // FakeFs assigns deterministic OIDs by entry position; flipping order churns + // conflict diff identity without reaching into ProjectDiff internals. + fs.set_merge_base_content_for_repo( + path!("/project/.git").as_ref(), + &[ + ("helper.txt", "helper-base\n".to_string()), + ("conflict.txt", build_conflict_text(2)), + ], + ); + }) + .detach(); + }); + + cx.update(|window, cx| { + let buffer = buffer.clone(); + window + .spawn(cx, async move |cx| { + cx.background_executor().simulate_random_delay().await; + for edit_index in 0..10 { + if edit_index > 0 { + cx.background_executor().simulate_random_delay().await; + } + buffer.update(cx, |buffer, cx| { + let len = buffer.len(); + if edit_index % 2 == 0 { + buffer.edit( + [(0..0, format!("status-burst-head-{edit_index}\n"))], + None, + cx, + ); + } else { + buffer.edit( + [(len..len, format!("status-burst-tail-{edit_index}\n"))], + None, + cx, + ); + } + }); + } + }) + .detach(); + }); + + cx.run_until_parked(); + } + #[gpui::test] async fn test_new_hunk_in_modified_file(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 15d7b9f3610eaf9e9063c7da95e915c73f95a341..95b90ccc297fb0f70c15429b8c3ec097bf4c0927 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -522,7 +522,7 @@ struct BufferState { struct DiffState { diff: Entity, - is_inverted: bool, + main_buffer: Option>, _subscription: gpui::Subscription, } @@ -530,7 +530,7 @@ impl DiffState { fn snapshot(&self, cx: &App) -> DiffStateSnapshot { DiffStateSnapshot { diff: self.diff.read(cx).snapshot(cx), - is_inverted: self.is_inverted, + main_buffer: self.main_buffer.as_ref().map(|b| b.read(cx).snapshot()), } } } @@ -538,7 +538,7 @@ impl DiffState { #[derive(Clone)] struct DiffStateSnapshot { diff: BufferDiffSnapshot, - is_inverted: bool, + main_buffer: Option, } impl std::ops::Deref for DiffStateSnapshot { @@ -573,32 +573,47 @@ impl DiffState { _ => {} }), diff, - is_inverted: false, + main_buffer: None, } } - fn new_inverted(diff: Entity, cx: &mut Context) -> Self { + fn new_inverted( + diff: Entity, + main_buffer: Entity, + cx: &mut Context, + ) -> Self { + let weak_main_buffer = main_buffer.downgrade(); DiffState { _subscription: cx.subscribe(&diff, { - move |this, diff, event, cx| match event { - BufferDiffEvent::DiffChanged(DiffChanged { - changed_range: _, - base_text_changed_range, - extended_range: _, - }) => { - if let Some(base_text_changed_range) = base_text_changed_range.clone() { - this.inverted_buffer_diff_changed(diff, base_text_changed_range, cx) + move |this, diff, event, cx| { + let Some(main_buffer) = weak_main_buffer.upgrade() else { + return; + }; + match event { + BufferDiffEvent::DiffChanged(DiffChanged { + changed_range: _, + base_text_changed_range, + extended_range: _, + }) => { + if let Some(base_text_changed_range) = base_text_changed_range.clone() { + this.inverted_buffer_diff_changed( + diff, + main_buffer, + base_text_changed_range, + cx, + ) + } + cx.emit(Event::BufferDiffChanged); } - cx.emit(Event::BufferDiffChanged); - } - BufferDiffEvent::LanguageChanged => { - this.inverted_buffer_diff_language_changed(diff, cx) + BufferDiffEvent::LanguageChanged => { + this.inverted_buffer_diff_language_changed(diff, main_buffer, cx) + } + _ => {} } - _ => {} } }), diff, - is_inverted: true, + main_buffer: Some(main_buffer), } } } @@ -2297,7 +2312,10 @@ impl MultiBuffer { // Recalculate has_inverted_diff after removing diffs if !removed_buffer_ids.is_empty() { - snapshot.has_inverted_diff = snapshot.diffs.iter().any(|(_, diff)| diff.is_inverted); + snapshot.has_inverted_diff = snapshot + .diffs + .iter() + .any(|(_, diff)| diff.main_buffer.is_some()); } if changed_trailing_excerpt { @@ -2399,7 +2417,7 @@ impl MultiBuffer { let buffer_id = diff.buffer_id; let diff = DiffStateSnapshot { diff: diff.snapshot(cx), - is_inverted: false, + main_buffer: None, }; self.snapshot.get_mut().diffs.insert(buffer_id, diff); } @@ -2407,13 +2425,15 @@ impl MultiBuffer { fn inverted_buffer_diff_language_changed( &mut self, diff: Entity, + main_buffer: Entity, cx: &mut Context, ) { let base_text_buffer_id = diff.read(cx).base_text_buffer().read(cx).remote_id(); + let main_buffer_snapshot = main_buffer.read(cx).snapshot(); let diff = diff.read(cx); let diff = DiffStateSnapshot { diff: diff.snapshot(cx), - is_inverted: true, + main_buffer: Some(main_buffer_snapshot), }; self.snapshot .get_mut() @@ -2437,7 +2457,7 @@ impl MultiBuffer { }; let new_diff = DiffStateSnapshot { diff: diff.snapshot(cx), - is_inverted: false, + main_buffer: None, }; let mut snapshot = self.snapshot.get_mut(); let base_text_changed = snapshot @@ -2468,20 +2488,22 @@ impl MultiBuffer { fn inverted_buffer_diff_changed( &mut self, diff: Entity, + main_buffer: Entity, diff_change_range: Range, cx: &mut Context, ) { self.sync_mut(cx); - let diff = diff.read(cx); - let base_text_buffer_id = diff.base_text_buffer().read(cx).remote_id(); + let base_text_buffer_id = diff.read(cx).base_text_buffer().read(cx).remote_id(); let Some(buffer_state) = self.buffers.get(&base_text_buffer_id) else { return; }; + let main_buffer_snapshot = main_buffer.read(cx).snapshot(); + let diff = diff.read(cx); let new_diff = DiffStateSnapshot { diff: diff.snapshot(cx), - is_inverted: true, + main_buffer: Some(main_buffer_snapshot), }; let mut snapshot = self.snapshot.get_mut(); snapshot @@ -2673,14 +2695,21 @@ impl MultiBuffer { self.diffs.insert(buffer_id, DiffState::new(diff, cx)); } - pub fn add_inverted_diff(&mut self, diff: Entity, cx: &mut Context) { + pub fn add_inverted_diff( + &mut self, + diff: Entity, + main_buffer: Entity, + cx: &mut Context, + ) { let snapshot = diff.read(cx).base_text(cx); let base_text_buffer_id = snapshot.remote_id(); let diff_change_range = 0..snapshot.len(); self.snapshot.get_mut().has_inverted_diff = true; - self.inverted_buffer_diff_changed(diff.clone(), diff_change_range, cx); - self.diffs - .insert(base_text_buffer_id, DiffState::new_inverted(diff, cx)); + self.inverted_buffer_diff_changed(diff.clone(), main_buffer.clone(), diff_change_range, cx); + self.diffs.insert( + base_text_buffer_id, + DiffState::new_inverted(diff, main_buffer, cx), + ); } pub fn diff_for(&self, buffer_id: BufferId) -> Option> { @@ -3092,7 +3121,7 @@ impl MultiBuffer { // Those base text ranges are usize, so make sure if the base text changed // we also update the diff snapshot so that we don't use stale offsets if buffer_diff.get(id).is_none_or(|existing_diff| { - if !existing_diff.is_inverted { + if existing_diff.main_buffer.is_none() { return false; } let base_text = diff.diff.read(cx).base_text_buffer().read(cx); @@ -3297,7 +3326,7 @@ impl MultiBuffer { .. }) => excerpts.item().is_some_and(|excerpt| { if let Some(diff) = snapshot.diffs.get(&excerpt.buffer_id) - && diff.is_inverted + && diff.main_buffer.is_some() { return true; } @@ -3409,10 +3438,10 @@ impl MultiBuffer { excerpt_buffer_start + edit.new.end.saturating_sub(excerpt_start); let edit_buffer_end = edit_buffer_end.min(excerpt_buffer_end); - if diff.is_inverted { + if let Some(main_buffer) = &diff.main_buffer { for hunk in diff.hunks_intersecting_base_text_range( edit_buffer_start..edit_buffer_end, - diff.original_buffer_snapshot(), + main_buffer, ) { did_expand_hunks = true; let hunk_buffer_range = hunk.diff_base_byte_range.clone(); @@ -3966,15 +3995,12 @@ impl MultiBufferSnapshot { let query_range = range.start.to_point(self)..range.end.to_point(self); self.lift_buffer_metadata(query_range.clone(), move |buffer, buffer_range| { let diff = self.diffs.get(&buffer.remote_id())?; - let iter = if diff.is_inverted { + let iter = if let Some(main_buffer) = &diff.main_buffer { let buffer_start = buffer.point_to_offset(buffer_range.start); let buffer_end = buffer.point_to_offset(buffer_range.end); itertools::Either::Left( - diff.hunks_intersecting_base_text_range( - buffer_start..buffer_end, - diff.original_buffer_snapshot(), - ) - .map(move |hunk| (hunk, buffer, true)), + diff.hunks_intersecting_base_text_range(buffer_start..buffer_end, main_buffer) + .map(move |hunk| (hunk, buffer, true)), ) } else { let buffer_start = buffer.anchor_before(buffer_range.start); @@ -4552,11 +4578,10 @@ impl MultiBufferSnapshot { .to_offset(&excerpt.buffer); if let Some(diff) = self.diffs.get(&excerpt.buffer_id) { - if diff.is_inverted { - for hunk in diff.hunks_intersecting_base_text_range_rev( - excerpt_start..excerpt_end, - diff.original_buffer_snapshot(), - ) { + if let Some(main_buffer) = &diff.main_buffer { + for hunk in diff + .hunks_intersecting_base_text_range_rev(excerpt_start..excerpt_end, main_buffer) + { if hunk.diff_base_byte_range.end >= current_position { continue; } @@ -4590,11 +4615,11 @@ impl MultiBufferSnapshot { let Some(diff) = self.diffs.get(&excerpt.buffer_id) else { continue; }; - if diff.is_inverted { + if let Some(main_buffer) = &diff.main_buffer { let Some(hunk) = diff .hunks_intersecting_base_text_range_rev( excerpt.range.context.to_offset(&excerpt.buffer), - diff.original_buffer_snapshot(), + main_buffer, ) .next() else { diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index 455bef14ec8c07532be2c91da4e4927ef9be2ebd..480b17e472c02457db9535e30886d9b21753ba19 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/crates/multi_buffer/src/multi_buffer_tests.rs @@ -528,7 +528,7 @@ async fn test_inverted_diff_hunks_in_range(cx: &mut TestAppContext) { }); multibuffer.update(cx, |multibuffer, cx| { - multibuffer.add_inverted_diff(diff, cx); + multibuffer.add_inverted_diff(diff, buffer.clone(), cx); }); assert_new_snapshot( @@ -2336,7 +2336,7 @@ async fn test_diff_hunks_with_multiple_excerpts(cx: &mut TestAppContext) { struct ReferenceMultibuffer { excerpts: Vec, diffs: HashMap>, - inverted_diffs: HashMap>, + inverted_diffs: HashMap, Entity)>, } #[derive(Debug)] @@ -2483,13 +2483,14 @@ impl ReferenceMultibuffer { let buffer_id = buffer.remote_id(); let buffer_range = excerpt.range.to_offset(buffer); - if let Some(diff) = self.inverted_diffs.get(&buffer_id) { + if let Some((diff, main_buffer)) = self.inverted_diffs.get(&buffer_id) { let diff_snapshot = diff.read(cx).snapshot(cx); + let main_buffer_snapshot = main_buffer.read(cx).snapshot(); let mut offset = buffer_range.start; for hunk in diff_snapshot.hunks_intersecting_base_text_range( buffer_range.clone(), - diff_snapshot.original_buffer_snapshot(), + &main_buffer_snapshot.text, ) { let mut hunk_base_range = hunk.diff_base_byte_range.clone(); @@ -2788,9 +2789,15 @@ impl ReferenceMultibuffer { self.diffs.insert(buffer_id, diff); } - fn add_inverted_diff(&mut self, diff: Entity, cx: &App) { + fn add_inverted_diff( + &mut self, + diff: Entity, + main_buffer: Entity, + cx: &App, + ) { let base_text_buffer_id = diff.read(cx).base_text(cx).remote_id(); - self.inverted_diffs.insert(base_text_buffer_id, diff); + self.inverted_diffs + .insert(base_text_buffer_id, (diff, main_buffer)); } } @@ -3162,9 +3169,9 @@ async fn test_random_multibuffer(cx: &mut TestAppContext, mut rng: StdRng) { excerpt_buffer.read_with(cx, |buffer, _| buffer.remote_id()); multibuffer.update(cx, |multibuffer, cx| { if multibuffer.diff_for(excerpt_buffer_id).is_none() { - if inverted_main_buffer.is_some() { - reference.add_inverted_diff(diff.clone(), cx); - multibuffer.add_inverted_diff(diff, cx); + if let Some(main_buffer) = inverted_main_buffer { + reference.add_inverted_diff(diff.clone(), main_buffer.clone(), cx); + multibuffer.add_inverted_diff(diff, main_buffer, cx); } else { reference.add_diff(diff.clone(), cx); multibuffer.add_diff(diff, cx); @@ -3938,7 +3945,7 @@ async fn test_singleton_with_inverted_diff(cx: &mut TestAppContext) { let multibuffer = cx.new(|cx| { let mut multibuffer = MultiBuffer::singleton(base_text_buffer.clone(), cx); multibuffer.set_all_diff_hunks_expanded(cx); - multibuffer.add_inverted_diff(diff.clone(), cx); + multibuffer.add_inverted_diff(diff.clone(), buffer.clone(), cx); multibuffer }); @@ -4089,7 +4096,7 @@ async fn test_inverted_diff_base_text_change(cx: &mut TestAppContext) { let multibuffer = cx.new(|cx| { let mut multibuffer = MultiBuffer::singleton(base_text_buffer.clone(), cx); multibuffer.set_all_diff_hunks_expanded(cx); - multibuffer.add_inverted_diff(diff.clone(), cx); + multibuffer.add_inverted_diff(diff.clone(), buffer.clone(), cx); multibuffer }); @@ -4134,6 +4141,62 @@ async fn test_inverted_diff_base_text_change(cx: &mut TestAppContext) { .collect(); } +#[gpui::test] +async fn test_inverted_diff_secondary_version_mismatch(cx: &mut TestAppContext) { + let base_text = "one\ntwo\nthree\nfour\nfive\n"; + let index_text = "one\nTWO\nthree\nfour\nfive\n"; + let buffer_text = "one\nTWO\nthree\nFOUR\nfive\n"; + + let buffer = cx.new(|cx| Buffer::local(buffer_text, cx)); + + let unstaged_diff = cx + .new(|cx| BufferDiff::new_with_base_text(index_text, &buffer.read(cx).text_snapshot(), cx)); + cx.run_until_parked(); + + let uncommitted_diff = cx.new(|cx| { + let mut diff = + BufferDiff::new_with_base_text(base_text, &buffer.read(cx).text_snapshot(), cx); + diff.set_secondary_diff(unstaged_diff.clone()); + diff + }); + cx.run_until_parked(); + + buffer.update(cx, |buffer, cx| { + buffer.edit([(0..0, "ZERO\n")], None, cx); + }); + + let update = unstaged_diff + .update(cx, |diff, cx| { + diff.update_diff( + buffer.read(cx).text_snapshot(), + Some(index_text.into()), + None, + None, + cx, + ) + }) + .await; + unstaged_diff + .update(cx, |diff, cx| { + diff.set_snapshot(update, &buffer.read(cx).text_snapshot(), cx) + }) + .await; + + let base_text_buffer = uncommitted_diff.read_with(cx, |diff, _| diff.base_text_buffer()); + + let multibuffer = cx.new(|cx| { + let mut multibuffer = MultiBuffer::singleton(base_text_buffer.clone(), cx); + multibuffer.set_all_diff_hunks_expanded(cx); + multibuffer.add_inverted_diff(uncommitted_diff.clone(), buffer.clone(), cx); + multibuffer + }); + + let _hunks: Vec<_> = multibuffer + .read_with(cx, |multibuffer, cx| multibuffer.snapshot(cx)) + .diff_hunks() + .collect(); +} + #[track_caller] fn assert_excerpts_match( multibuffer: &Entity,