git: Make GitPanel more responsive to long-running staging ops (#41667)

Jakub Konka created

Currently, this only applies to long-running individually selected
unstaged files in the git panel. Next up I would like to make this work
for `Stage All`/`Unstage All` however this will most likely require
pushing `PendingOperation` into `GitStore` (from the `GitPanel`).

Release Notes:

- N/A

Change summary

crates/git_ui/src/git_panel.rs  |  71 ++++++++------
crates/project/src/git_store.rs | 171 +++++++++++++++++++---------------
2 files changed, 135 insertions(+), 107 deletions(-)

Detailed changes

crates/git_ui/src/git_panel.rs 🔗

@@ -286,6 +286,12 @@ struct PendingOperation {
     op_id: usize,
 }
 
+impl PendingOperation {
+    fn contains_path(&self, path: &RepoPath) -> bool {
+        self.entries.iter().any(|p| &p.repo_path == path)
+    }
+}
+
 pub struct GitPanel {
     pub(crate) active_repository: Option<Entity<Repository>>,
     pub(crate) commit_editor: Entity<Editor>,
@@ -1240,19 +1246,21 @@ impl GitPanel {
         };
         let (stage, repo_paths) = match entry {
             GitListEntry::Status(status_entry) => {
-                if status_entry.status.staging().is_fully_staged() {
+                let repo_paths = vec![status_entry.clone()];
+                let stage = if let Some(status) = self.entry_staging(&status_entry) {
+                    !status.is_fully_staged()
+                } else if status_entry.status.staging().is_fully_staged() {
                     if let Some(op) = self.bulk_staging.clone()
                         && op.anchor == status_entry.repo_path
                     {
                         self.bulk_staging = None;
                     }
-
-                    (false, vec![status_entry.clone()])
+                    false
                 } else {
                     self.set_bulk_staging_anchor(status_entry.repo_path.clone(), cx);
-
-                    (true, vec![status_entry.clone()])
-                }
+                    true
+                };
+                (stage, repo_paths)
             }
             GitListEntry::Header(section) => {
                 let goal_staged_state = !self.header_state(section.header).selected();
@@ -2677,10 +2685,7 @@ impl GitPanel {
             if self.pending.iter().any(|pending| {
                 pending.target_status == TargetStatus::Reverted
                     && !pending.finished
-                    && pending
-                        .entries
-                        .iter()
-                        .any(|pending| pending.repo_path == entry.repo_path)
+                    && pending.contains_path(&entry.repo_path)
             }) {
                 continue;
             }
@@ -2731,10 +2736,7 @@ impl GitPanel {
                 last_pending_staged = pending.entries.first().cloned();
             }
             if let Some(single_staged) = &single_staged_entry
-                && pending
-                    .entries
-                    .iter()
-                    .any(|entry| entry.repo_path == single_staged.repo_path)
+                && pending.contains_path(&single_staged.repo_path)
             {
                 pending_status_for_single_staged = Some(pending.target_status);
             }
@@ -2797,7 +2799,7 @@ impl GitPanel {
             && let Some(index) = bulk_staging_anchor_new_index
             && let Some(entry) = self.entries.get(index)
             && let Some(entry) = entry.status_entry()
-            && self.entry_staging(entry) == StageStatus::Staged
+            && self.entry_staging(entry).unwrap_or(entry.staging) == StageStatus::Staged
         {
             self.bulk_staging = bulk_staging;
         }
@@ -2845,39 +2847,47 @@ impl GitPanel {
             self.entry_count += 1;
             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() {
+                if self
+                    .entry_staging(status_entry)
+                    .unwrap_or(status_entry.staging)
+                    .has_staged()
+                {
                     self.conflicted_staged_count += 1;
                 }
             } else if status_entry.status.is_created() {
                 self.new_count += 1;
-                if self.entry_staging(status_entry).has_staged() {
+                if self
+                    .entry_staging(status_entry)
+                    .unwrap_or(status_entry.staging)
+                    .has_staged()
+                {
                     self.new_staged_count += 1;
                 }
             } else {
                 self.tracked_count += 1;
-                if self.entry_staging(status_entry).has_staged() {
+                if self
+                    .entry_staging(status_entry)
+                    .unwrap_or(status_entry.staging)
+                    .has_staged()
+                {
                     self.tracked_staged_count += 1;
                 }
             }
         }
     }
 
-    fn entry_staging(&self, entry: &GitStatusEntry) -> StageStatus {
+    fn entry_staging(&self, entry: &GitStatusEntry) -> Option<StageStatus> {
         for pending in self.pending.iter().rev() {
-            if pending
-                .entries
-                .iter()
-                .any(|pending_entry| pending_entry.repo_path == entry.repo_path)
-            {
+            if pending.contains_path(&entry.repo_path) {
                 match pending.target_status {
-                    TargetStatus::Staged => return StageStatus::Staged,
-                    TargetStatus::Unstaged => return StageStatus::Unstaged,
+                    TargetStatus::Staged => return Some(StageStatus::Staged),
+                    TargetStatus::Unstaged => return Some(StageStatus::Unstaged),
                     TargetStatus::Reverted => continue,
                     TargetStatus::Unchanged => continue,
                 }
             }
         }
-        entry.staging
+        None
     }
 
     pub(crate) fn has_staged_changes(&self) -> bool {
@@ -3710,7 +3720,8 @@ impl GitPanel {
         let ix = self.entry_by_path(&repo_path, cx)?;
         let entry = self.entries.get(ix)?;
 
-        let entry_staging = self.entry_staging(entry.status_entry()?);
+        let status = entry.status_entry()?;
+        let entry_staging = self.entry_staging(status).unwrap_or(status.staging);
 
         let checkbox = Checkbox::new("stage-file", entry_staging.as_bool().into())
             .disabled(!self.has_write_access(cx))
@@ -4004,8 +4015,8 @@ impl GitPanel {
         let checkbox_id: ElementId =
             ElementId::Name(format!("entry_{}_{}_checkbox", display_name, ix).into());
 
-        let entry_staging = self.entry_staging(entry);
-        let mut is_staged: ToggleState = self.entry_staging(entry).as_bool().into();
+        let entry_staging = self.entry_staging(entry).unwrap_or(entry.staging);
+        let mut is_staged: ToggleState = entry_staging.as_bool().into();
         if self.show_placeholders && !self.has_staged_changes() && !entry.status.is_created() {
             is_staged = ToggleState::Selected;
         }

crates/project/src/git_store.rs 🔗

@@ -3717,20 +3717,15 @@ impl Repository {
         Some(self.git_store.upgrade()?.read(cx).buffer_store.clone())
     }
 
-    pub fn stage_entries(
+    fn save_buffers<'a>(
         &self,
-        entries: Vec<RepoPath>,
+        entries: impl IntoIterator<Item = &'a RepoPath>,
         cx: &mut Context<Self>,
-    ) -> Task<anyhow::Result<()>> {
-        if entries.is_empty() {
-            return Task::ready(Ok(()));
-        }
-        let id = self.id;
-
+    ) -> Vec<Task<anyhow::Result<()>>> {
         let mut save_futures = Vec::new();
         if let Some(buffer_store) = self.buffer_store(cx) {
             buffer_store.update(cx, |buffer_store, cx| {
-                for path in &entries {
+                for path in entries {
                     let Some(project_path) = self.repo_path_to_project_path(path, cx) else {
                         continue;
                     };
@@ -3746,37 +3741,64 @@ impl Repository {
                 }
             })
         }
+        save_futures
+    }
+
+    pub fn stage_entries(
+        &self,
+        entries: Vec<RepoPath>,
+        cx: &mut Context<Self>,
+    ) -> Task<anyhow::Result<()>> {
+        if entries.is_empty() {
+            return Task::ready(Ok(()));
+        }
+        let id = self.id;
+        let save_tasks = self.save_buffers(&entries, cx);
+        let paths = entries
+            .iter()
+            .map(|p| p.as_unix_str())
+            .collect::<Vec<_>>()
+            .join(" ");
+        let status = format!("git add {paths}");
+        let job_key = match entries.len() {
+            1 => Some(GitJobKey::WriteIndex(entries[0].clone())),
+            _ => None,
+        };
 
         cx.spawn(async move |this, cx| {
-            for save_future in save_futures {
-                save_future.await?;
+            for save_task in save_tasks {
+                save_task.await?;
             }
 
             this.update(cx, |this, _| {
-                this.send_job(None, move |git_repo, _cx| async move {
-                    match git_repo {
-                        RepositoryState::Local {
-                            backend,
-                            environment,
-                            ..
-                        } => backend.stage_paths(entries, environment.clone()).await,
-                        RepositoryState::Remote { project_id, client } => {
-                            client
-                                .request(proto::Stage {
-                                    project_id: project_id.0,
-                                    repository_id: id.to_proto(),
-                                    paths: entries
-                                        .into_iter()
-                                        .map(|repo_path| repo_path.to_proto())
-                                        .collect(),
-                                })
-                                .await
-                                .context("sending stage request")?;
+                this.send_keyed_job(
+                    job_key,
+                    Some(status.into()),
+                    move |git_repo, _cx| async move {
+                        match git_repo {
+                            RepositoryState::Local {
+                                backend,
+                                environment,
+                                ..
+                            } => backend.stage_paths(entries, environment.clone()).await,
+                            RepositoryState::Remote { project_id, client } => {
+                                client
+                                    .request(proto::Stage {
+                                        project_id: project_id.0,
+                                        repository_id: id.to_proto(),
+                                        paths: entries
+                                            .into_iter()
+                                            .map(|repo_path| repo_path.to_proto())
+                                            .collect(),
+                                    })
+                                    .await
+                                    .context("sending stage request")?;
 
-                            Ok(())
+                                Ok(())
+                            }
                         }
-                    }
-                })
+                    },
+                )
             })?
             .await??;
 
@@ -3793,57 +3815,52 @@ impl Repository {
             return Task::ready(Ok(()));
         }
         let id = self.id;
-
-        let mut save_futures = Vec::new();
-        if let Some(buffer_store) = self.buffer_store(cx) {
-            buffer_store.update(cx, |buffer_store, cx| {
-                for path in &entries {
-                    let Some(project_path) = self.repo_path_to_project_path(path, cx) else {
-                        continue;
-                    };
-                    if let Some(buffer) = buffer_store.get_by_path(&project_path)
-                        && buffer
-                            .read(cx)
-                            .file()
-                            .is_some_and(|file| file.disk_state().exists())
-                        && buffer.read(cx).has_unsaved_edits()
-                    {
-                        save_futures.push(buffer_store.save_buffer(buffer, cx));
-                    }
-                }
-            })
-        }
+        let save_tasks = self.save_buffers(&entries, cx);
+        let paths = entries
+            .iter()
+            .map(|p| p.as_unix_str())
+            .collect::<Vec<_>>()
+            .join(" ");
+        let status = format!("git reset {paths}");
+        let job_key = match entries.len() {
+            1 => Some(GitJobKey::WriteIndex(entries[0].clone())),
+            _ => None,
+        };
 
         cx.spawn(async move |this, cx| {
-            for save_future in save_futures {
-                save_future.await?;
+            for save_task in save_tasks {
+                save_task.await?;
             }
 
             this.update(cx, |this, _| {
-                this.send_job(None, move |git_repo, _cx| async move {
-                    match git_repo {
-                        RepositoryState::Local {
-                            backend,
-                            environment,
-                            ..
-                        } => backend.unstage_paths(entries, environment).await,
-                        RepositoryState::Remote { project_id, client } => {
-                            client
-                                .request(proto::Unstage {
-                                    project_id: project_id.0,
-                                    repository_id: id.to_proto(),
-                                    paths: entries
-                                        .into_iter()
-                                        .map(|repo_path| repo_path.to_proto())
-                                        .collect(),
-                                })
-                                .await
-                                .context("sending unstage request")?;
+                this.send_keyed_job(
+                    job_key,
+                    Some(status.into()),
+                    move |git_repo, _cx| async move {
+                        match git_repo {
+                            RepositoryState::Local {
+                                backend,
+                                environment,
+                                ..
+                            } => backend.unstage_paths(entries, environment).await,
+                            RepositoryState::Remote { project_id, client } => {
+                                client
+                                    .request(proto::Unstage {
+                                        project_id: project_id.0,
+                                        repository_id: id.to_proto(),
+                                        paths: entries
+                                            .into_iter()
+                                            .map(|repo_path| repo_path.to_proto())
+                                            .collect(),
+                                    })
+                                    .await
+                                    .context("sending unstage request")?;
 
-                            Ok(())
+                                Ok(())
+                            }
                         }
-                    }
-                })
+                    },
+                )
             })?
             .await??;