Git fix repo selection (#25996)

Conrad Irwin created

Release Notes:

- git: Fixed a bug where staging/unstaging of hunks could use the wrong
git repository if you had many open

Change summary

crates/project/src/buffer_store.rs | 48 +++++++++++++++++++++++++++++--
crates/project/src/git.rs          | 29 ++++++++++++++++++
crates/project/src/project.rs      | 30 +++++++++++++------
3 files changed, 92 insertions(+), 15 deletions(-)

Detailed changes

crates/project/src/buffer_store.rs 🔗

@@ -136,6 +136,15 @@ impl BufferDiffState {
         let _ = self.diff_bases_changed(buffer, diff_bases_change, cx);
     }
 
+    pub fn wait_for_recalculation(&mut self) -> Option<oneshot::Receiver<()>> {
+        if self.diff_updated_futures.is_empty() {
+            return None;
+        }
+        let (tx, rx) = oneshot::channel();
+        self.diff_updated_futures.push(tx);
+        Some(rx)
+    }
+
     fn diff_bases_changed(
         &mut self,
         buffer: text::BufferSnapshot,
@@ -1362,8 +1371,23 @@ impl BufferStore {
         cx: &mut Context<Self>,
     ) -> Task<Result<Entity<BufferDiff>>> {
         let buffer_id = buffer.read(cx).remote_id();
-        if let Some(diff) = self.get_unstaged_diff(buffer_id, cx) {
-            return Task::ready(Ok(diff));
+        if let Some(OpenBuffer::Complete { diff_state, .. }) = self.opened_buffers.get(&buffer_id) {
+            if let Some(unstaged_diff) = diff_state
+                .read(cx)
+                .unstaged_diff
+                .as_ref()
+                .and_then(|weak| weak.upgrade())
+            {
+                if let Some(task) =
+                    diff_state.update(cx, |diff_state, _| diff_state.wait_for_recalculation())
+                {
+                    return cx.background_executor().spawn(async move {
+                        task.await?;
+                        Ok(unstaged_diff)
+                    });
+                }
+                return Task::ready(Ok(unstaged_diff));
+            }
         }
 
         let task = match self.loading_diffs.entry((buffer_id, DiffKind::Unstaged)) {
@@ -1402,8 +1426,24 @@ impl BufferStore {
         cx: &mut Context<Self>,
     ) -> Task<Result<Entity<BufferDiff>>> {
         let buffer_id = buffer.read(cx).remote_id();
-        if let Some(diff) = self.get_uncommitted_diff(buffer_id, cx) {
-            return Task::ready(Ok(diff));
+
+        if let Some(OpenBuffer::Complete { diff_state, .. }) = self.opened_buffers.get(&buffer_id) {
+            if let Some(uncommitted_diff) = diff_state
+                .read(cx)
+                .uncommitted_diff
+                .as_ref()
+                .and_then(|weak| weak.upgrade())
+            {
+                if let Some(task) =
+                    diff_state.update(cx, |diff_state, _| diff_state.wait_for_recalculation())
+                {
+                    return cx.background_executor().spawn(async move {
+                        task.await?;
+                        Ok(uncommitted_diff)
+                    });
+                }
+                return Task::ready(Ok(uncommitted_diff));
+            }
         }
 
         let task = match self.loading_diffs.entry((buffer_id, DiffKind::Uncommitted)) {

crates/project/src/git.rs 🔗

@@ -24,7 +24,7 @@ use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use text::BufferId;
 use util::{maybe, ResultExt};
-use worktree::{ProjectEntryId, RepositoryEntry, StatusEntry};
+use worktree::{ProjectEntryId, RepositoryEntry, StatusEntry, WorkDirectory};
 
 pub struct GitStore {
     buffer_store: Entity<BufferStore>,
@@ -691,6 +691,33 @@ impl Repository {
         self.worktree_id_path_to_repo_path(path.worktree_id, &path.path)
     }
 
+    // note: callers must verify these come from the same worktree
+    pub fn contains_sub_repo(&self, other: &Entity<Self>, cx: &App) -> bool {
+        let other_work_dir = &other.read(cx).repository_entry.work_directory;
+        match (&self.repository_entry.work_directory, other_work_dir) {
+            (WorkDirectory::InProject { .. }, WorkDirectory::AboveProject { .. }) => false,
+            (WorkDirectory::AboveProject { .. }, WorkDirectory::InProject { .. }) => true,
+            (
+                WorkDirectory::InProject {
+                    relative_path: this_path,
+                },
+                WorkDirectory::InProject {
+                    relative_path: other_path,
+                },
+            ) => other_path.starts_with(this_path),
+            (
+                WorkDirectory::AboveProject {
+                    absolute_path: this_path,
+                    ..
+                },
+                WorkDirectory::AboveProject {
+                    absolute_path: other_path,
+                    ..
+                },
+            ) => other_path.starts_with(this_path),
+        }
+    }
+
     pub fn worktree_id_path_to_repo_path(
         &self,
         worktree_id: WorktreeId,

crates/project/src/project.rs 🔗

@@ -4310,16 +4310,26 @@ impl Project {
             .buffer_for_id(buffer_id, cx)?
             .read(cx)
             .project_path(cx)?;
-        self.git_store
-            .read(cx)
-            .all_repositories()
-            .into_iter()
-            .find_map(|repo| {
-                Some((
-                    repo.clone(),
-                    repo.read(cx).repository_entry.relativize(&path.path).ok()?,
-                ))
-            })
+
+        let mut found: Option<(Entity<Repository>, RepoPath)> = None;
+        for repo_handle in self.git_store.read(cx).all_repositories() {
+            let repo = repo_handle.read(cx);
+            if repo.worktree_id != path.worktree_id {
+                continue;
+            }
+            let Ok(relative_path) = repo.repository_entry.relativize(&path.path) else {
+                continue;
+            };
+            if found
+                .as_ref()
+                .is_some_and(|(found, _)| repo.contains_sub_repo(found, cx))
+            {
+                continue;
+            }
+            found = Some((repo_handle.clone(), relative_path))
+        }
+
+        found
     }
 }