From 697c2ba71fa3d1d87e917b75bcee605a9447c4b2 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 May 2025 13:34:39 -0700 Subject: [PATCH] Enable merge conflict parsing for currently-unmerged files (#31549) Previously, we only enabled merge conflict parsing for files that were unmerged at the last time a change was detected to the repo's merge heads. Now we enable the parsing for these files *and* any files that are currently unmerged. The old strategy meant that conflicts produced via `git stash pop` would not be parsed. Release Notes: - Fixed parsing of merge conflicts when the conflict was produced by a `git stash pop` --- crates/git_ui/src/git_panel.rs | 8 +- crates/git_ui/src/project_diff.rs | 4 +- crates/project/src/git_store.rs | 21 ++-- crates/project/src/git_store/conflict_set.rs | 106 ++++++++++++++++++- 4 files changed, 124 insertions(+), 15 deletions(-) diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index c7b15c011e..f49c5a576a 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -199,7 +199,9 @@ impl GitHeaderEntry { let this = &self.header; let status = status_entry.status; match this { - Section::Conflict => repo.has_conflict(&status_entry.repo_path), + Section::Conflict => { + repo.had_conflict_on_last_merge_head_change(&status_entry.repo_path) + } Section::Tracked => !status.is_created(), Section::New => status.is_created(), } @@ -2345,7 +2347,7 @@ impl GitPanel { let repo = repo.read(cx); for entry in repo.cached_status() { - let is_conflict = repo.has_conflict(&entry.repo_path); + let is_conflict = repo.had_conflict_on_last_merge_head_change(&entry.repo_path); let is_new = entry.status.is_created(); let staging = entry.status.staging(); @@ -2516,7 +2518,7 @@ impl GitPanel { continue; }; self.entry_count += 1; - if repo.has_conflict(&status_entry.repo_path) { + if repo.had_conflict_on_last_merge_head_change(&status_entry.repo_path) { self.conflicted_count += 1; if self.entry_staging(status_entry).has_staged() { self.conflicted_staged_count += 1; diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index a8c4c78649..5e06b7bc66 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -219,7 +219,7 @@ impl ProjectDiff { }; let repo = git_repo.read(cx); - let namespace = if repo.has_conflict(&entry.repo_path) { + let namespace = if repo.had_conflict_on_last_merge_head_change(&entry.repo_path) { CONFLICT_NAMESPACE } else if entry.status.is_created() { NEW_NAMESPACE @@ -372,7 +372,7 @@ impl ProjectDiff { }; let namespace = if GitPanelSettings::get_global(cx).sort_by_path { TRACKED_NAMESPACE - } else if repo.has_conflict(&entry.repo_path) { + } else if repo.had_conflict_on_last_merge_head_change(&entry.repo_path) { CONFLICT_NAMESPACE } else if entry.status.is_created() { NEW_NAMESPACE diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 344c00b63f..0be12c30cc 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -778,11 +778,7 @@ impl GitStore { let is_unmerged = self .repository_and_path_for_buffer_id(buffer_id, cx) .map_or(false, |(repo, path)| { - repo.read(cx) - .snapshot - .merge - .conflicted_paths - .contains(&path) + repo.read(cx).snapshot.has_conflict(&path) }); let git_store = cx.weak_entity(); let buffer_git_state = self @@ -1145,7 +1141,7 @@ impl GitStore { cx: &mut Context, ) { let id = repo.read(cx).id; - let merge_conflicts = repo.read(cx).snapshot.merge.conflicted_paths.clone(); + let repo_snapshot = repo.read(cx).snapshot.clone(); for (buffer_id, diff) in self.diffs.iter() { if let Some((buffer_repo, repo_path)) = self.repository_and_path_for_buffer_id(*buffer_id, cx) @@ -1155,7 +1151,7 @@ impl GitStore { if let Some(conflict_set) = &diff.conflict_set { let conflict_status_changed = conflict_set.update(cx, |conflict_set, cx| { - let has_conflict = merge_conflicts.contains(&repo_path); + let has_conflict = repo_snapshot.has_conflict(&repo_path); conflict_set.set_has_conflict(has_conflict, cx) })?; if conflict_status_changed { @@ -2668,8 +2664,17 @@ impl RepositorySnapshot { .ok() } + pub fn had_conflict_on_last_merge_head_change(&self, repo_path: &RepoPath) -> bool { + self.merge.conflicted_paths.contains(&repo_path) + } + pub fn has_conflict(&self, repo_path: &RepoPath) -> bool { - self.merge.conflicted_paths.contains(repo_path) + let had_conflict_on_last_merge_head_change = + self.merge.conflicted_paths.contains(&repo_path); + let has_conflict_currently = self + .status_for_path(&repo_path) + .map_or(false, |entry| entry.status.is_conflicted()); + had_conflict_on_last_merge_head_change || has_conflict_currently } /// This is the name that will be displayed in the repository selector for this repository. diff --git a/crates/project/src/git_store/conflict_set.rs b/crates/project/src/git_store/conflict_set.rs index 0c4ecb5a88..de447c5c6e 100644 --- a/crates/project/src/git_store/conflict_set.rs +++ b/crates/project/src/git_store/conflict_set.rs @@ -254,7 +254,7 @@ impl EventEmitter for ConflictSet {} #[cfg(test)] mod tests { - use std::sync::mpsc; + use std::{path::Path, sync::mpsc}; use crate::{Project, project_settings::ProjectSettings}; @@ -265,7 +265,7 @@ mod tests { use language::language_settings::AllLanguageSettings; use serde_json::json; use settings::Settings as _; - use text::{Buffer, BufferId, ToOffset as _}; + use text::{Buffer, BufferId, Point, ToOffset as _}; use unindent::Unindent as _; use util::path; use worktree::WorktreeSettings; @@ -558,4 +558,106 @@ mod tests { assert_eq!(update.old_range, 0..1); assert_eq!(update.new_range, 0..0); } + + #[gpui::test] + async fn test_conflict_updates_without_merge_head( + executor: BackgroundExecutor, + cx: &mut TestAppContext, + ) { + zlog::init_test(); + cx.update(|cx| { + settings::init(cx); + WorktreeSettings::register(cx); + ProjectSettings::register(cx); + AllLanguageSettings::register(cx); + }); + + let initial_text = " + zero + <<<<<<< HEAD + one + ======= + two + >>>>>>> Stashed Changes + three + " + .unindent(); + + let fs = FakeFs::new(executor); + fs.insert_tree( + path!("/project"), + json!({ + ".git": {}, + "a.txt": initial_text, + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await; + let (git_store, buffer) = project.update(cx, |project, cx| { + ( + project.git_store().clone(), + project.open_local_buffer(path!("/project/a.txt"), cx), + ) + }); + + cx.run_until_parked(); + fs.with_git_state(path!("/project/.git").as_ref(), true, |state| { + state.unmerged_paths.insert( + "a.txt".into(), + UnmergedStatus { + first_head: UnmergedStatusCode::Updated, + second_head: UnmergedStatusCode::Updated, + }, + ) + }) + .unwrap(); + + let buffer = buffer.await.unwrap(); + + // Open the conflict set for a file that currently has conflicts. + let conflict_set = git_store.update(cx, |git_store, cx| { + git_store.open_conflict_set(buffer.clone(), cx) + }); + + cx.run_until_parked(); + conflict_set.update(cx, |conflict_set, cx| { + let conflict_range = conflict_set.snapshot().conflicts[0] + .range + .to_point(buffer.read(cx)); + assert_eq!(conflict_range, Point::new(1, 0)..Point::new(6, 0)); + }); + + // Simulate the conflict being removed by e.g. staging the file. + fs.with_git_state(path!("/project/.git").as_ref(), true, |state| { + state.unmerged_paths.remove(Path::new("a.txt")) + }) + .unwrap(); + + cx.run_until_parked(); + conflict_set.update(cx, |conflict_set, _| { + assert_eq!(conflict_set.has_conflict, false); + assert_eq!(conflict_set.snapshot.conflicts.len(), 0); + }); + + // Simulate the conflict being re-added. + fs.with_git_state(path!("/project/.git").as_ref(), true, |state| { + state.unmerged_paths.insert( + "a.txt".into(), + UnmergedStatus { + first_head: UnmergedStatusCode::Updated, + second_head: UnmergedStatusCode::Updated, + }, + ) + }) + .unwrap(); + + cx.run_until_parked(); + conflict_set.update(cx, |conflict_set, cx| { + let conflict_range = conflict_set.snapshot().conflicts[0] + .range + .to_point(buffer.read(cx)); + assert_eq!(conflict_range, Point::new(1, 0)..Point::new(6, 0)); + }); + } }