From 54fdf58d3a5ba58e3d71fdd862f47cf5ebc05698 Mon Sep 17 00:00:00 2001 From: Sathwik Chirivelli <146921254+chirivelli@users.noreply.github.com> Date: Sun, 19 Jul 2026 00:32:25 +0530 Subject: [PATCH] git_panel: Show staged and unstaged diff stats (#60815) # Objective - Show accurate diff stats for each staged and unstaged projection of a partially staged file in the Git panel. - This was originally considered for https://github.com/zed-industries/zed/pull/59884, but was scoped out of that already-large PR and is being submitted separately as discussed there. ## Solution - Collect HEAD-to-index and index-to-worktree diff stats alongside the existing combined HEAD-to-worktree stats. - Carry the staged and unstaged stats through repository status snapshots and remote status serialization. - Use the stat matching the projected Git panel section while preserving the combined stat for the other grouping modes. - Update the fake Git repository and add regression coverage with deliberately different staged and unstaged counts. ## Testing - `cargo check -p git_ui` - `cargo check -p collab` - `cargo test -p git_ui test_group_by_staging_section_membership_and_order --lib` - `cargo test -p project --lib --no-run` - `cargo fmt --all -- --check` - `git diff --check` ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed diff stats for partially staged files in the Git panel --- crates/collab/src/db.rs | 4 + crates/fs/src/fake_git_repo.rs | 46 +++++--- crates/git/src/repository.rs | 22 ++-- crates/git_ui/src/git_panel.rs | 55 ++++++++-- crates/project/src/git_store.rs | 100 ++++++++++++++---- .../tests/integration/project_tests.rs | 57 ++++++++++ crates/proto/proto/git.proto | 4 + 7 files changed, 241 insertions(+), 47 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index d8dad9fec1..786f750cd0 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -732,6 +732,10 @@ fn db_status_to_proto( }), diff_stat_added: entry.lines_added.map(|v| v as u32), diff_stat_deleted: entry.lines_deleted.map(|v| v as u32), + staged_diff_stat_added: None, + staged_diff_stat_deleted: None, + unstaged_diff_stat_added: None, + unstaged_diff_stat_deleted: None, }) } diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 00e91cb799..02610f0fdb 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -1156,6 +1156,7 @@ impl GitRepository for FakeGitRepository { fn diff_stat( &self, + diff: git::repository::DiffStatType, path_prefixes: &[RepoPath], ) -> BoxFuture<'static, Result> { fn count_lines(s: &str) -> u32 { @@ -1203,22 +1204,43 @@ impl GitRepository for FakeGitRepository { self.with_state_async(false, move |state| { let mut entries = Vec::new(); - let all_paths: HashSet<&RepoPath> = state - .head_contents - .keys() - .chain( - worktree_files - .keys() - .filter(|p| state.index_contents.contains_key(*p)), - ) - .collect(); + let (old_files, new_files) = match diff { + git::repository::DiffStatType::HeadToIndex => { + (&state.head_contents, &state.index_contents) + } + git::repository::DiffStatType::HeadToWorktree => { + (&state.head_contents, &worktree_files) + } + git::repository::DiffStatType::IndexToWorktree => { + (&state.index_contents, &worktree_files) + } + }; + let all_paths: HashSet<&RepoPath> = match diff { + git::repository::DiffStatType::HeadToIndex => state + .head_contents + .keys() + .chain(state.index_contents.keys()) + .collect(), + git::repository::DiffStatType::HeadToWorktree => state + .head_contents + .keys() + .chain( + worktree_files + .keys() + .filter(|path| state.index_contents.contains_key(*path)), + ) + .collect(), + git::repository::DiffStatType::IndexToWorktree => { + state.index_contents.keys().collect() + } + }; for path in all_paths { if !matches_prefixes(path, &path_prefixes) { continue; } - let head = state.head_contents.get(path); - let worktree = worktree_files.get(path); - match (head, worktree) { + let old_file = old_files.get(path); + let new_file = new_files.get(path); + match (old_file, new_file) { (Some(old), Some(new)) if old != new => { entries.push(( path.clone(), diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 857a62f1b1..c4daf3ff87 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -1109,6 +1109,7 @@ pub trait GitRepository: Send + Sync { fn diff_stat( &self, + diff: DiffStatType, path_prefixes: &[RepoPath], ) -> BoxFuture<'static, Result>; @@ -1193,6 +1194,13 @@ pub enum DiffType { MergeBase { base_ref: SharedString }, } +#[derive(Clone, Copy)] +pub enum DiffStatType { + HeadToIndex, + HeadToWorktree, + IndexToWorktree, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, JsonSchema)] pub enum PushOptions { SetUpstream, @@ -2333,6 +2341,7 @@ impl GitRepository for RealGitRepository { fn diff_stat( &self, + diff: DiffStatType, path_prefixes: &[RepoPath], ) -> BoxFuture<'static, Result> { let path_prefixes = path_prefixes.to_vec(); @@ -2341,12 +2350,13 @@ impl GitRepository for RealGitRepository { self.executor .spawn(async move { let git_binary = git_binary?; - let mut args: Vec = vec![ - "diff".into(), - "--numstat".into(), - "--no-renames".into(), - "HEAD".into(), - ]; + let mut args: Vec = + vec!["diff".into(), "--numstat".into(), "--no-renames".into()]; + match diff { + DiffStatType::HeadToIndex => args.extend(["--cached".into(), "HEAD".into()]), + DiffStatType::HeadToWorktree => args.push("HEAD".into()), + DiffStatType::IndexToWorktree => {} + } if !path_prefixes.is_empty() { args.push("--".into()); args.extend( diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index fd97769d03..d7021e1b51 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -4548,13 +4548,13 @@ impl GitPanel { self.stash_entries = repo.cached_stash(); - for entry in repo.cached_status() { + for status_entry in repo.cached_status() { self.changes_count += 1; - let is_conflict = repo.had_conflict_on_last_merge_head_change(&entry.repo_path); - let is_new = entry.status.is_created(); - let staging = entry.status.staging(); + let is_conflict = repo.had_conflict_on_last_merge_head_change(&status_entry.repo_path); + let is_new = status_entry.status.is_created(); + let staging = status_entry.status.staging(); - if let Some(pending) = repo.pending_ops_for_path(&entry.repo_path) + if let Some(pending) = repo.pending_ops_for_path(&status_entry.repo_path) && pending .ops .iter() @@ -4564,10 +4564,10 @@ impl GitPanel { } let entry = GitStatusEntry { - repo_path: entry.repo_path.clone(), - status: entry.status, + repo_path: status_entry.repo_path.clone(), + status: status_entry.status, staging, - diff_stat: entry.diff_stat, + diff_stat: status_entry.diff_stat, }; if !is_conflict && !is_new { @@ -4583,10 +4583,16 @@ impl GitPanel { conflict_entries.push(entry); } else if group_by_staging_state { if staging.has_staged() { - staged_entries.push(entry.clone()); + staged_entries.push(GitStatusEntry { + diff_stat: status_entry.staged_diff_stat, + ..entry.clone() + }); } if staging.has_unstaged() { - unstaged_entries.push(entry); + unstaged_entries.push(GitStatusEntry { + diff_stat: status_entry.unstaged_diff_stat, + ..entry + }); } } else if group_by_file_status && is_conflict { conflict_entries.push(entry); @@ -9339,7 +9345,6 @@ mod tests { ("crates/util/util.rs", StatusCode::Modified.worktree()), ], ); - let project = Project::test(fs.clone(), [path!("/root/zed/crates/gpui").as_ref()], cx).await; let window_handle = @@ -9552,6 +9557,16 @@ mod tests { ("unstaged.rs", StatusCode::Modified.worktree()), ], ); + fs.with_git_state(path!("/project/.git").as_ref(), true, |state| { + state.head_contents.insert( + repo_path("partial.rs"), + "head one\nhead two\nhead three\nhead four".into(), + ); + state + .index_contents + .insert(repo_path("partial.rs"), "index one\nindex two".into()); + }) + .expect("fake repository should exist"); let project = Project::test(fs.clone(), [Path::new(path!("/project"))], cx).await; let window_handle = @@ -9626,6 +9641,24 @@ mod tests { panel.stage_intent_for_entry_index(projections[1].index), StageIntent::Stage ); + assert_eq!( + panel.entries[projections[0].index] + .status_entry() + .and_then(|entry| entry.diff_stat), + Some(DiffStat { + added: 2, + deleted: 4, + }) + ); + assert_eq!( + panel.entries[projections[1].index] + .status_entry() + .and_then(|entry| entry.diff_stat), + Some(DiffStat { + added: 1, + deleted: 2, + }) + ); panel.entries.clone() }); diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 1b1f873516..ef6b7fe30e 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -35,10 +35,11 @@ use git::{ parse_git_remote_url, repository::{ Branch, BranchesScanResult, CommitData, CommitDetails, CommitDiff, CommitFile, - CommitOptions, CreateWorktreeTarget, DiffType, FetchOptions, FileHistoryChangedFileSets, - GitCommitTemplate, GitRepository, GitRepositoryCheckpoint, InitialGraphCommitData, - LogOrder, LogSource, PushOptions, Remote, RemoteCommandOutput, RepoPath, ResetMode, - SearchCommitArgs, UpstreamTrackingStatus, Worktree as GitWorktree, delete_branch_flag, + CommitOptions, CreateWorktreeTarget, DiffStatType, DiffType, FetchOptions, + FileHistoryChangedFileSets, GitCommitTemplate, GitRepository, GitRepositoryCheckpoint, + InitialGraphCommitData, LogOrder, LogSource, PushOptions, Remote, RemoteCommandOutput, + RepoPath, ResetMode, SearchCommitArgs, UpstreamTrackingStatus, Worktree as GitWorktree, + delete_branch_flag, }, stash::{GitStash, StashEntry}, status::{ @@ -318,6 +319,8 @@ pub struct StatusEntry { pub repo_path: RepoPath, pub status: FileStatus, pub diff_stat: Option, + pub staged_diff_stat: Option, + pub unstaged_diff_stat: Option, } impl StatusEntry { @@ -341,6 +344,10 @@ impl StatusEntry { status: Some(status_to_proto(self.status)), diff_stat_added: self.diff_stat.map(|ds| ds.added), diff_stat_deleted: self.diff_stat.map(|ds| ds.deleted), + staged_diff_stat_added: self.staged_diff_stat.map(|ds| ds.added), + staged_diff_stat_deleted: self.staged_diff_stat.map(|ds| ds.deleted), + unstaged_diff_stat_added: self.unstaged_diff_stat.map(|ds| ds.added), + unstaged_diff_stat_deleted: self.unstaged_diff_stat.map(|ds| ds.deleted), } } } @@ -355,10 +362,24 @@ impl TryFrom for StatusEntry { (Some(added), Some(deleted)) => Some(DiffStat { added, deleted }), _ => None, }; + let staged_diff_stat = match (value.staged_diff_stat_added, value.staged_diff_stat_deleted) + { + (Some(added), Some(deleted)) => Some(DiffStat { added, deleted }), + _ => None, + }; + let unstaged_diff_stat = match ( + value.unstaged_diff_stat_added, + value.unstaged_diff_stat_deleted, + ) { + (Some(added), Some(deleted)) => Some(DiffStat { added, deleted }), + _ => None, + }; Ok(Self { repo_path, status, diff_stat, + staged_diff_stat, + unstaged_diff_stat, }) } } @@ -9138,20 +9159,29 @@ impl Repository { let changed_paths_vec = changed_paths.iter().cloned().collect::>(); let status_task = backend.status(&changed_paths_vec); - let diff_stat_future = if has_head { - backend.diff_stat(&changed_paths_vec) - } else { - future::ready(Ok(status::GitDiffStat { - entries: Arc::default(), - })) - .boxed() + let diff_stat_future = |diff| { + if has_head { + backend.diff_stat(diff, &changed_paths_vec) + } else { + future::ready(Ok(status::GitDiffStat::default())).boxed() + } }; - let (statuses, diff_stats) = - futures::future::try_join(status_task, diff_stat_future).await?; + let (statuses, diff_stats, staged_diff_stats, unstaged_diff_stats) = + futures::future::try_join4( + status_task, + diff_stat_future(DiffStatType::HeadToWorktree), + diff_stat_future(DiffStatType::HeadToIndex), + diff_stat_future(DiffStatType::IndexToWorktree), + ) + .await?; let diff_stats: HashMap = HashMap::from_iter(diff_stats.entries.into_iter().cloned()); + let staged_diff_stats: HashMap = + HashMap::from_iter(staged_diff_stats.entries.into_iter().cloned()); + let unstaged_diff_stats: HashMap = + HashMap::from_iter(unstaged_diff_stats.entries.into_iter().cloned()); let mut changed_path_statuses = Vec::new(); let prev_statuses = prev_snapshot.statuses_by_path.clone(); @@ -9182,10 +9212,17 @@ impl Repository { for (repo_path, status) in &*statuses.entries { let current_diff_stat = diff_stats.get(repo_path).copied(); + let current_staged_diff_stat = + staged_diff_stats.get(repo_path).copied(); + let current_unstaged_diff_stat = + unstaged_diff_stats.get(repo_path).copied(); if cursor.seek_forward(&PathTarget::Path(repo_path), Bias::Left) && cursor.item().is_some_and(|entry| { - entry.status == *status && entry.diff_stat == current_diff_stat + entry.status == *status + && entry.diff_stat == current_diff_stat + && entry.staged_diff_stat == current_staged_diff_stat + && entry.unstaged_diff_stat == current_unstaged_diff_stat }) { continue; @@ -9195,6 +9232,8 @@ impl Repository { repo_path: repo_path.clone(), status: *status, diff_stat: current_diff_stat, + staged_diff_stat: current_staged_diff_stat, + unstaged_diff_stat: current_unstaged_diff_stat, })); } anyhow::Ok(changed_path_statuses) @@ -10538,14 +10577,23 @@ async fn compute_snapshot( .unwrap_or_default() } }; - let diff_stat_future = { + let diff_stats_future = { let snapshot = snapshot.clone(); let backend = backend.clone(); async move { if snapshot.head_commit.is_some() { - backend.diff_stat(&[]).await.log_err().unwrap_or_default() + futures::future::join3( + backend.diff_stat(DiffStatType::HeadToWorktree, &[]), + backend.diff_stat(DiffStatType::HeadToIndex, &[]), + backend.diff_stat(DiffStatType::IndexToWorktree, &[]), + ) + .await } else { - Default::default() + ( + Ok(status::GitDiffStat::default()), + Ok(status::GitDiffStat::default()), + Ok(status::GitDiffStat::default()), + ) } } }; @@ -10555,11 +10603,25 @@ async fn compute_snapshot( }; let (statuses, diff_stats, stash_entries) = - futures::future::join3(statuses_future, diff_stat_future, stash_entries_future).await; + futures::future::join3(statuses_future, diff_stats_future, stash_entries_future).await; + let (diff_stats, staged_diff_stats, unstaged_diff_stats) = diff_stats; + let diff_stats = diff_stats.log_err().unwrap_or_default(); + let staged_diff_stats = staged_diff_stats.log_err().unwrap_or_default(); + let unstaged_diff_stats = unstaged_diff_stats.log_err().unwrap_or_default(); log::debug!("fetched statuses, diff stats, stash entries"); let diff_stat_map: HashMap<&RepoPath, DiffStat> = diff_stats.entries.iter().map(|(p, s)| (p, *s)).collect(); + let staged_diff_stat_map: HashMap<&RepoPath, DiffStat> = staged_diff_stats + .entries + .iter() + .map(|(p, s)| (p, *s)) + .collect(); + let unstaged_diff_stat_map: HashMap<&RepoPath, DiffStat> = unstaged_diff_stats + .entries + .iter() + .map(|(p, s)| (p, *s)) + .collect(); let mut conflicted_paths = Vec::new(); let statuses_by_path = SumTree::from_iter( statuses.entries.iter().map(|(repo_path, status)| { @@ -10570,6 +10632,8 @@ async fn compute_snapshot( repo_path: repo_path.clone(), status: *status, diff_stat: diff_stat_map.get(repo_path).copied(), + staged_diff_stat: staged_diff_stat_map.get(repo_path).copied(), + unstaged_diff_stat: unstaged_diff_stat_map.get(repo_path).copied(), } }), (), diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index 420af4aaed..6857ce680a 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -12372,11 +12372,18 @@ async fn test_git_repository_status(cx: &mut gpui::TestAppContext) { added: 1, deleted: 1, }), + staged_diff_stat: None, + unstaged_diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), }, StatusEntry { repo_path: repo_path("b.txt"), status: FileStatus::Untracked, diff_stat: None, + staged_diff_stat: None, + unstaged_diff_stat: None, }, StatusEntry { repo_path: repo_path("d.txt"), @@ -12385,6 +12392,11 @@ async fn test_git_repository_status(cx: &mut gpui::TestAppContext) { added: 0, deleted: 1, }), + staged_diff_stat: None, + unstaged_diff_stat: Some(DiffStat { + added: 0, + deleted: 1, + }), }, ] ); @@ -12410,11 +12422,18 @@ async fn test_git_repository_status(cx: &mut gpui::TestAppContext) { added: 1, deleted: 1, }), + staged_diff_stat: None, + unstaged_diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), }, StatusEntry { repo_path: repo_path("b.txt"), status: FileStatus::Untracked, diff_stat: None, + staged_diff_stat: None, + unstaged_diff_stat: None, }, StatusEntry { repo_path: repo_path("c.txt"), @@ -12423,6 +12442,11 @@ async fn test_git_repository_status(cx: &mut gpui::TestAppContext) { added: 1, deleted: 1, }), + staged_diff_stat: None, + unstaged_diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), }, StatusEntry { repo_path: repo_path("d.txt"), @@ -12431,6 +12455,11 @@ async fn test_git_repository_status(cx: &mut gpui::TestAppContext) { added: 0, deleted: 1, }), + staged_diff_stat: None, + unstaged_diff_stat: Some(DiffStat { + added: 0, + deleted: 1, + }), }, ] ); @@ -12468,6 +12497,11 @@ async fn test_git_repository_status(cx: &mut gpui::TestAppContext) { added: 0, deleted: 1, }), + staged_diff_stat: None, + unstaged_diff_stat: Some(DiffStat { + added: 0, + deleted: 1, + }), }] ); }); @@ -12516,6 +12550,8 @@ async fn test_git_repository_status_removes_directory_descendants(cx: &mut gpui: repo_path: repo_path("ci2/Dockerfile.namespace"), status: FileStatus::Untracked, diff_stat: None, + staged_diff_stat: None, + unstaged_diff_stat: None, }] ); }); @@ -12558,6 +12594,8 @@ async fn test_git_repository_status_removes_directory_descendants(cx: &mut gpui: repo_path: repo_path("ci3/Dockerfile.namespace"), status: FileStatus::Untracked, diff_stat: None, + staged_diff_stat: None, + unstaged_diff_stat: None, }] ); }); @@ -12719,6 +12757,11 @@ async fn test_git_status_postprocessing(cx: &mut gpui::TestAppContext) { } .into(), diff_stat: None, + staged_diff_stat: Some(DiffStat { + added: 0, + deleted: 0, + }), + unstaged_diff_stat: None, }] ) }); @@ -12925,6 +12968,11 @@ async fn test_repository_pending_ops_staging( added: 1, deleted: 0, }), + staged_diff_stat: Some(DiffStat { + added: 1, + deleted: 0, + }), + unstaged_diff_stat: None, }] ); }); @@ -13035,6 +13083,11 @@ async fn test_repository_pending_ops_long_running_staging( added: 1, deleted: 0, }), + staged_diff_stat: Some(DiffStat { + added: 1, + deleted: 0, + }), + unstaged_diff_stat: None, }] ); }); @@ -13160,11 +13213,15 @@ async fn test_repository_pending_ops_stage_all( repo_path: repo_path("a.txt"), status: FileStatus::Untracked, diff_stat: None, + staged_diff_stat: None, + unstaged_diff_stat: None, }, StatusEntry { repo_path: repo_path("b.txt"), status: FileStatus::Untracked, diff_stat: None, + staged_diff_stat: None, + unstaged_diff_stat: None, }, ] ); diff --git a/crates/proto/proto/git.proto b/crates/proto/proto/git.proto index 33cbadc13c..bb8a8b89a0 100644 --- a/crates/proto/proto/git.proto +++ b/crates/proto/proto/git.proto @@ -330,6 +330,10 @@ message StatusEntry { GitFileStatus status = 3; optional uint32 diff_stat_added = 4; optional uint32 diff_stat_deleted = 5; + optional uint32 staged_diff_stat_added = 6; + optional uint32 staged_diff_stat_deleted = 7; + optional uint32 unstaged_diff_stat_added = 8; + optional uint32 unstaged_diff_stat_deleted = 9; } message StashEntry {