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.
This commit is contained in:
Cole Miller 2025-04-30 08:01:03 -04:00 committed by GitHub
parent 5073cba08d
commit 7587340fb1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -2692,6 +2692,7 @@ impl MergeDetails {
status: &SumTree<StatusEntry>,
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::<Vec<_>>();
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