Enable merge conflict parsing for currently-unmerged files (#31549)

Max Brunsfeld created

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`

Change summary

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(-)

Detailed changes

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;

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

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<Self>,
     ) {
         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.

crates/project/src/git_store/conflict_set.rs 🔗

@@ -254,7 +254,7 @@ impl EventEmitter<ConflictSetUpdate> 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));
+        });
+    }
 }