From 7587340fb14ee30aef1501bd0c1fa92777fea7d2 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Wed, 30 Apr 2025 08:01:03 -0400 Subject: [PATCH] Fix a bug that interfered with sorting of conflicted paths in the git panel (#29534) This PR updates the git store to not register a change in a repository's merge heads until conflicted paths are seen. We currently use the repository's merge heads only to decide when the list of conflicted paths should be refreshed. Previously, the logic looked like this: - Whenever we see a change in the merge heads, set the list of conflicted paths by filtering the output of `git status`. It turns out that when a conflicting merge takes a while, we can see this sequence of events: 1. We get an event in .git and reload statuses and merge heads. Previously there were no merge heads, and now we have some, but git hasn't finished figuring out which paths have conflicts, so we set the list of conflicted paths to `[]`. 2. Git finishes computing the list of conflicted paths, and we run another scan that picks these up from `git status`, but then we throw them away because the merge heads are the same as in (1). By not updating our stored merge heads until we see some conflicts in `git status`, we delay this step until (2), and so the conflicted paths show up in the git panel as intended. This means that our merge heads state no longer matches what's on disk (in particular, during a clean merge we'll never update them at all), but that's okay because we only keep this state for the purpose of organizing conflicts. Release Notes: - Fixed a bug that could cause conflicted paths to not appear in their own section in the git panel. --- crates/project/src/git_store.rs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index aaf66e8ea98110fda096e56aa4287af3b15b4725..debb1c643b06b06c0fd52cfcc65c92c210502b7b 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -2692,6 +2692,7 @@ impl MergeDetails { status: &SumTree, prev_snapshot: &RepositorySnapshot, ) -> Result<(MergeDetails, bool)> { + log::debug!("load merge details"); let message = backend.merge_message().await; let heads = backend .revparse_batch(vec![ @@ -2709,12 +2710,33 @@ impl MergeDetails { .collect::>(); let merge_heads_changed = heads != prev_snapshot.merge.heads; let conflicted_paths = if merge_heads_changed { - TreeSet::from_ordered_entries( + let current_conflicted_paths = TreeSet::from_ordered_entries( status .iter() .filter(|entry| entry.status.is_conflicted()) .map(|entry| entry.repo_path.clone()), - ) + ); + + // It can happen that we run a scan while a lengthy merge is in progress + // that will eventually result in conflicts, but before those conflicts + // are reported by `git status`. Since for the moment we only care about + // the merge heads state for the purposes of tracking conflicts, don't update + // this state until we see some conflicts. + if heads.iter().any(Option::is_some) + && !prev_snapshot.merge.heads.iter().any(Option::is_some) + && current_conflicted_paths.is_empty() + { + log::debug!("not updating merge heads because no conflicts found"); + return Ok(( + MergeDetails { + message: message.map(SharedString::from), + ..prev_snapshot.merge.clone() + }, + false, + )); + } + + current_conflicted_paths } else { prev_snapshot.merge.conflicted_paths.clone() }; @@ -3998,6 +4020,8 @@ impl Repository { Some(GitJobKey::ReloadGitState), None, |state, mut cx| async move { + log::debug!("run scheduled git status scan"); + let Some(this) = this.upgrade() else { return Ok(()); }; @@ -4535,6 +4559,7 @@ async fn compute_snapshot( ); let (merge_details, merge_heads_changed) = MergeDetails::load(&backend, &statuses_by_path, &prev_snapshot).await?; + log::debug!("new merge details (changed={merge_heads_changed:?}): {merge_details:?}"); if merge_heads_changed || branch != prev_snapshot.branch