Revert "git: Remove JobStatus from PendingOp in favour of in-flight p… (#42970)

Jakub Konka created

…runing (#42955)"

This reverts commit 696fdd8fed12da625357dc75939b0787c171b5d7.

Release Notes:

- N/A

Change summary

crates/git_ui/src/git_panel.rs             |  2 
crates/project/src/git_store.rs            | 25 ++++--------
crates/project/src/git_store/pending_op.rs | 28 +++++++++++++-
crates/project/src/project_tests.rs        | 46 ++++++++++++++---------
4 files changed, 62 insertions(+), 39 deletions(-)

Detailed changes

crates/git_ui/src/git_panel.rs 🔗

@@ -2662,7 +2662,7 @@ impl GitPanel {
                 && pending
                     .ops
                     .iter()
-                    .any(|op| op.git_status == pending_op::GitStatus::Reverted && op.finished)
+                    .any(|op| op.git_status == pending_op::GitStatus::Reverted && op.finished())
             {
                 continue;
             }

crates/project/src/git_store.rs 🔗

@@ -5404,27 +5404,18 @@ impl Repository {
         let ids = self.new_pending_ops_for_paths(paths, git_status);
 
         cx.spawn(async move |this, cx| {
-            let (finished, result) = match f(this.clone(), cx).await {
-                Ok(()) => (true, Ok(())),
-                Err(err) if err.is::<Canceled>() => (false, Ok(())),
-                Err(err) => (false, Err(err)),
+            let (job_status, result) = match f(this.clone(), cx).await {
+                Ok(()) => (pending_op::JobStatus::Finished, Ok(())),
+                Err(err) if err.is::<Canceled>() => (pending_op::JobStatus::Skipped, Ok(())),
+                Err(err) => (pending_op::JobStatus::Error, Err(err)),
             };
 
             this.update(cx, |this, _| {
                 let mut edits = Vec::with_capacity(ids.len());
                 for (id, entry) in ids {
                     if let Some(mut ops) = this.snapshot.pending_ops_for_path(&entry) {
-                        if finished {
-                            if let Some(op) = ops.op_by_id_mut(id) {
-                                op.finished = true;
-                            }
-                        } else {
-                            let idx = ops
-                                .ops
-                                .iter()
-                                .position(|op| op.id == id)
-                                .expect("pending operation must exist");
-                            ops.ops.remove(idx);
+                        if let Some(op) = ops.op_by_id_mut(id) {
+                            op.job_status = job_status;
                         }
                         edits.push(sum_tree::Edit::Insert(ops));
                     }
@@ -5452,7 +5443,7 @@ impl Repository {
             ops.ops.push(PendingOp {
                 id,
                 git_status,
-                finished: false,
+                job_status: pending_op::JobStatus::Running,
             });
             edits.push(sum_tree::Edit::Insert(ops));
             ids.push((id, path));
@@ -5730,7 +5721,7 @@ async fn compute_snapshot(
     let pending_ops_by_path = SumTree::from_iter(
         prev_snapshot.pending_ops_by_path.iter().filter_map(|ops| {
             let inner_ops: Vec<PendingOp> =
-                ops.ops.iter().filter(|op| !op.finished).cloned().collect();
+                ops.ops.iter().filter(|op| op.running()).cloned().collect();
             if inner_ops.is_empty() {
                 None
             } else {

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

@@ -11,6 +11,14 @@ pub enum GitStatus {
     Unchanged,
 }
 
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum JobStatus {
+    Running,
+    Finished,
+    Skipped,
+    Error,
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct PendingOps {
     pub repo_path: RepoPath,
@@ -21,7 +29,7 @@ pub struct PendingOps {
 pub struct PendingOp {
     pub id: PendingOpId,
     pub git_status: GitStatus,
-    pub finished: bool,
+    pub job_status: JobStatus,
 }
 
 #[derive(Clone, Debug)]
@@ -106,7 +114,7 @@ impl PendingOps {
     /// File is staged if the last job is finished and has status Staged.
     pub fn staged(&self) -> bool {
         if let Some(last) = self.ops.last() {
-            if last.git_status == GitStatus::Staged && last.finished {
+            if last.git_status == GitStatus::Staged && last.job_status == JobStatus::Finished {
                 return true;
             }
         }
@@ -116,10 +124,24 @@ impl PendingOps {
     /// File is staged if the last job is not finished and has status Staged.
     pub fn staging(&self) -> bool {
         if let Some(last) = self.ops.last() {
-            if last.git_status == GitStatus::Staged && !last.finished {
+            if last.git_status == GitStatus::Staged && last.job_status != JobStatus::Finished {
                 return true;
             }
         }
         false
     }
 }
+
+impl PendingOp {
+    pub fn running(&self) -> bool {
+        self.job_status == JobStatus::Running
+    }
+
+    pub fn finished(&self) -> bool {
+        matches!(self.job_status, JobStatus::Finished | JobStatus::Skipped)
+    }
+
+    pub fn error(&self) -> bool {
+        self.job_status == JobStatus::Error
+    }
+}

crates/project/src/project_tests.rs 🔗

@@ -8538,8 +8538,11 @@ fn merge_pending_ops_snapshots(
                     .find_map(|(op, idx)| if op.id == s_op.id { Some(idx) } else { None })
                 {
                     let t_op = &mut t_ops.ops[op_idx];
-                    if s_op.finished {
-                        t_op.finished = true;
+                    match (s_op.job_status, t_op.job_status) {
+                        (pending_op::JobStatus::Running, _) => {}
+                        (s_st, pending_op::JobStatus::Running) => t_op.job_status = s_st,
+                        (s_st, t_st) if s_st == t_st => {}
+                        _ => unreachable!(),
                     }
                 } else {
                     t_ops.ops.push(s_op);
@@ -8631,7 +8634,7 @@ async fn test_repository_pending_ops_staging(
                 Some(&pending_op::PendingOp {
                     id: id.into(),
                     git_status,
-                    finished: false,
+                    job_status: pending_op::JobStatus::Running
                 })
             );
             task
@@ -8646,7 +8649,7 @@ async fn test_repository_pending_ops_staging(
                 Some(&pending_op::PendingOp {
                     id: id.into(),
                     git_status,
-                    finished: true,
+                    job_status: pending_op::JobStatus::Finished
                 })
             );
         });
@@ -8672,27 +8675,27 @@ async fn test_repository_pending_ops_staging(
             pending_op::PendingOp {
                 id: 1u16.into(),
                 git_status: pending_op::GitStatus::Staged,
-                finished: true,
+                job_status: pending_op::JobStatus::Finished
             },
             pending_op::PendingOp {
                 id: 2u16.into(),
                 git_status: pending_op::GitStatus::Unstaged,
-                finished: true,
+                job_status: pending_op::JobStatus::Finished
             },
             pending_op::PendingOp {
                 id: 3u16.into(),
                 git_status: pending_op::GitStatus::Staged,
-                finished: true,
+                job_status: pending_op::JobStatus::Finished
             },
             pending_op::PendingOp {
                 id: 4u16.into(),
                 git_status: pending_op::GitStatus::Unstaged,
-                finished: true,
+                job_status: pending_op::JobStatus::Finished
             },
             pending_op::PendingOp {
                 id: 5u16.into(),
                 git_status: pending_op::GitStatus::Staged,
-                finished: true,
+                job_status: pending_op::JobStatus::Finished
             }
         ],
     );
@@ -8789,11 +8792,18 @@ async fn test_repository_pending_ops_long_running_staging(
             .get(&worktree::PathKey(repo_path("a.txt").as_ref().clone()), ())
             .unwrap()
             .ops,
-        vec![pending_op::PendingOp {
-            id: 2u16.into(),
-            git_status: pending_op::GitStatus::Staged,
-            finished: true,
-        }],
+        vec![
+            pending_op::PendingOp {
+                id: 1u16.into(),
+                git_status: pending_op::GitStatus::Staged,
+                job_status: pending_op::JobStatus::Skipped
+            },
+            pending_op::PendingOp {
+                id: 2u16.into(),
+                git_status: pending_op::GitStatus::Staged,
+                job_status: pending_op::JobStatus::Finished
+            }
+        ],
     );
 
     repo.update(cx, |repo, _cx| {
@@ -8894,12 +8904,12 @@ async fn test_repository_pending_ops_stage_all(
             pending_op::PendingOp {
                 id: 1u16.into(),
                 git_status: pending_op::GitStatus::Staged,
-                finished: true,
+                job_status: pending_op::JobStatus::Finished
             },
             pending_op::PendingOp {
                 id: 2u16.into(),
                 git_status: pending_op::GitStatus::Unstaged,
-                finished: true,
+                job_status: pending_op::JobStatus::Finished
             },
         ],
     );
@@ -8913,12 +8923,12 @@ async fn test_repository_pending_ops_stage_all(
             pending_op::PendingOp {
                 id: 1u16.into(),
                 git_status: pending_op::GitStatus::Staged,
-                finished: true,
+                job_status: pending_op::JobStatus::Finished
             },
             pending_op::PendingOp {
                 id: 2u16.into(),
                 git_status: pending_op::GitStatus::Unstaged,
-                finished: true,
+                job_status: pending_op::JobStatus::Finished
             },
         ],
     );