From 696fdd8fed12da625357dc75939b0787c171b5d7 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Tue, 18 Nov 2025 11:22:34 +0100 Subject: [PATCH] git: Remove JobStatus from PendingOp in favour of in-flight pruning (#42955) The idea is that we only store running (`!self.finished`) or finished (`self.finished`) pending ops, while everything else (skipped, errored) jobs are pruned out immediately. We don't really need them in the grand scheme of things anyway. Release Notes: - N/A --- 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, 39 insertions(+), 62 deletions(-) diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 0691ba78560e38f5d3a297d033bd41459dff78c4..85b4b67a5586f157d4337a0564051712279258d6 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/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; } diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 94af9859df1156d7a10286a843a31e8351fe050c..dda60a2482b7bb08f0f3d73366e96b59bc7fe636 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -5404,18 +5404,27 @@ impl Repository { let ids = self.new_pending_ops_for_paths(paths, git_status); cx.spawn(async move |this, cx| { - let (job_status, result) = match f(this.clone(), cx).await { - Ok(()) => (pending_op::JobStatus::Finished, Ok(())), - Err(err) if err.is::() => (pending_op::JobStatus::Skipped, Ok(())), - Err(err) => (pending_op::JobStatus::Error, Err(err)), + let (finished, result) = match f(this.clone(), cx).await { + Ok(()) => (true, Ok(())), + Err(err) if err.is::() => (false, Ok(())), + Err(err) => (false, 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 let Some(op) = ops.op_by_id_mut(id) { - op.job_status = job_status; + 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); } edits.push(sum_tree::Edit::Insert(ops)); } @@ -5443,7 +5452,7 @@ impl Repository { ops.ops.push(PendingOp { id, git_status, - job_status: pending_op::JobStatus::Running, + finished: false, }); edits.push(sum_tree::Edit::Insert(ops)); ids.push((id, path)); @@ -5721,7 +5730,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 = - ops.ops.iter().filter(|op| op.running()).cloned().collect(); + ops.ops.iter().filter(|op| !op.finished).cloned().collect(); if inner_ops.is_empty() { None } else { diff --git a/crates/project/src/git_store/pending_op.rs b/crates/project/src/git_store/pending_op.rs index 1991eed407833d47fd35f6f573fbb46c692aed91..b5c7d9f00ec639576d498aaa3107c83758085c2e 100644 --- a/crates/project/src/git_store/pending_op.rs +++ b/crates/project/src/git_store/pending_op.rs @@ -11,14 +11,6 @@ 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, @@ -29,7 +21,7 @@ pub struct PendingOps { pub struct PendingOp { pub id: PendingOpId, pub git_status: GitStatus, - pub job_status: JobStatus, + pub finished: bool, } #[derive(Clone, Debug)] @@ -114,7 +106,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.job_status == JobStatus::Finished { + if last.git_status == GitStatus::Staged && last.finished { return true; } } @@ -124,24 +116,10 @@ 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.job_status != JobStatus::Finished { + if last.git_status == GitStatus::Staged && !last.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 - } -} diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index f3c935f3390305c8c78074439084f20b4d1562b2..f65f28c6045e55c669b35592738c08f6abbe26d4 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -8538,11 +8538,8 @@ 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]; - 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!(), + if s_op.finished { + t_op.finished = true; } } else { t_ops.ops.push(s_op); @@ -8634,7 +8631,7 @@ async fn test_repository_pending_ops_staging( Some(&pending_op::PendingOp { id: id.into(), git_status, - job_status: pending_op::JobStatus::Running + finished: false, }) ); task @@ -8649,7 +8646,7 @@ async fn test_repository_pending_ops_staging( Some(&pending_op::PendingOp { id: id.into(), git_status, - job_status: pending_op::JobStatus::Finished + finished: true, }) ); }); @@ -8675,27 +8672,27 @@ async fn test_repository_pending_ops_staging( pending_op::PendingOp { id: 1u16.into(), git_status: pending_op::GitStatus::Staged, - job_status: pending_op::JobStatus::Finished + finished: true, }, pending_op::PendingOp { id: 2u16.into(), git_status: pending_op::GitStatus::Unstaged, - job_status: pending_op::JobStatus::Finished + finished: true, }, pending_op::PendingOp { id: 3u16.into(), git_status: pending_op::GitStatus::Staged, - job_status: pending_op::JobStatus::Finished + finished: true, }, pending_op::PendingOp { id: 4u16.into(), git_status: pending_op::GitStatus::Unstaged, - job_status: pending_op::JobStatus::Finished + finished: true, }, pending_op::PendingOp { id: 5u16.into(), git_status: pending_op::GitStatus::Staged, - job_status: pending_op::JobStatus::Finished + finished: true, } ], ); @@ -8792,18 +8789,11 @@ 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: 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 - } - ], + vec![pending_op::PendingOp { + id: 2u16.into(), + git_status: pending_op::GitStatus::Staged, + finished: true, + }], ); repo.update(cx, |repo, _cx| { @@ -8904,12 +8894,12 @@ async fn test_repository_pending_ops_stage_all( pending_op::PendingOp { id: 1u16.into(), git_status: pending_op::GitStatus::Staged, - job_status: pending_op::JobStatus::Finished + finished: true, }, pending_op::PendingOp { id: 2u16.into(), git_status: pending_op::GitStatus::Unstaged, - job_status: pending_op::JobStatus::Finished + finished: true, }, ], ); @@ -8923,12 +8913,12 @@ async fn test_repository_pending_ops_stage_all( pending_op::PendingOp { id: 1u16.into(), git_status: pending_op::GitStatus::Staged, - job_status: pending_op::JobStatus::Finished + finished: true, }, pending_op::PendingOp { id: 2u16.into(), git_status: pending_op::GitStatus::Unstaged, - job_status: pending_op::JobStatus::Finished + finished: true, }, ], );