diff --git a/Cargo.lock b/Cargo.lock index d1b0a39869a44af1295235214836d446c509c360..c4ec49e50c3aa75e5e470414da470301e6f77e04 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10025,6 +10025,7 @@ dependencies = [ "ctor", "futures 0.3.31", "gpui", + "gpui_util", "log", "lsp-types", "parking_lot", diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 71e39fb595656e0dcdc53d97705b87a216ceb0f3..3e4b5c2ce211f68ef7e12895b542db5e6e3ea47c 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -122,6 +122,8 @@ CREATE TABLE "project_repository_statuses" ( "status_kind" INT4 NOT NULL, "first_status" INT4 NULL, "second_status" INT4 NULL, + "lines_added" INT4 NULL, + "lines_deleted" INT4 NULL, "scan_id" INT8 NOT NULL, "is_deleted" BOOL NOT NULL, PRIMARY KEY (project_id, repository_id, repo_path) diff --git a/crates/collab/migrations/20251208000000_test_schema.sql b/crates/collab/migrations/20251208000000_test_schema.sql index 493be3823e25a433d4a6a27a21c508f218dc68d1..0f4e4f2d2e3925ea1e4d2b964c5e4f159f393b4f 100644 --- a/crates/collab/migrations/20251208000000_test_schema.sql +++ b/crates/collab/migrations/20251208000000_test_schema.sql @@ -315,6 +315,8 @@ CREATE TABLE public.project_repository_statuses ( status_kind integer NOT NULL, first_status integer, second_status integer, + lines_added integer, + lines_deleted integer, scan_id bigint NOT NULL, is_deleted boolean NOT NULL ); diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 57fb0df86495dc2013e7cd780c2e62e57298bd11..d8803c253f5feef8ef5e040f3ea112abcc688f52 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -732,6 +732,8 @@ fn db_status_to_proto( status: Some(proto::GitFileStatus { variant: Some(variant), }), + diff_stat_added: entry.lines_added.map(|v| v as u32), + diff_stat_deleted: entry.lines_deleted.map(|v| v as u32), }) } diff --git a/crates/collab/src/db/queries/projects.rs b/crates/collab/src/db/queries/projects.rs index fa3f99e1483e8a5d8410378493556b189eff78f1..24cf639a715aa9b88da80375b389debaea0c4295 100644 --- a/crates/collab/src/db/queries/projects.rs +++ b/crates/collab/src/db/queries/projects.rs @@ -334,147 +334,6 @@ impl Database { .await?; } - // Backward-compatibility for old Zed clients. - // - // Remove this block when Zed 1.80 stable has been out for a week. - { - if !update.updated_repositories.is_empty() { - project_repository::Entity::insert_many( - update.updated_repositories.iter().map(|repository| { - project_repository::ActiveModel { - project_id: ActiveValue::set(project_id), - legacy_worktree_id: ActiveValue::set(Some(worktree_id)), - id: ActiveValue::set(repository.repository_id as i64), - scan_id: ActiveValue::set(update.scan_id as i64), - is_deleted: ActiveValue::set(false), - branch_summary: ActiveValue::Set( - repository - .branch_summary - .as_ref() - .map(|summary| serde_json::to_string(summary).unwrap()), - ), - current_merge_conflicts: ActiveValue::Set(Some( - serde_json::to_string(&repository.current_merge_conflicts) - .unwrap(), - )), - // Old clients do not use abs path, entry ids, head_commit_details, or merge_message. - abs_path: ActiveValue::set(String::new()), - entry_ids: ActiveValue::set("[]".into()), - head_commit_details: ActiveValue::set(None), - merge_message: ActiveValue::set(None), - remote_upstream_url: ActiveValue::set(None), - remote_origin_url: ActiveValue::set(None), - } - }), - ) - .on_conflict( - OnConflict::columns([ - project_repository::Column::ProjectId, - project_repository::Column::Id, - ]) - .update_columns([ - project_repository::Column::ScanId, - project_repository::Column::BranchSummary, - project_repository::Column::CurrentMergeConflicts, - ]) - .to_owned(), - ) - .exec(&*tx) - .await?; - - let has_any_statuses = update - .updated_repositories - .iter() - .any(|repository| !repository.updated_statuses.is_empty()); - - if has_any_statuses { - project_repository_statuses::Entity::insert_many( - update.updated_repositories.iter().flat_map( - |repository: &proto::RepositoryEntry| { - repository.updated_statuses.iter().map(|status_entry| { - let (repo_path, status_kind, first_status, second_status) = - proto_status_to_db(status_entry.clone()); - project_repository_statuses::ActiveModel { - project_id: ActiveValue::set(project_id), - repository_id: ActiveValue::set( - repository.repository_id as i64, - ), - scan_id: ActiveValue::set(update.scan_id as i64), - is_deleted: ActiveValue::set(false), - repo_path: ActiveValue::set(repo_path), - status: ActiveValue::set(0), - status_kind: ActiveValue::set(status_kind), - first_status: ActiveValue::set(first_status), - second_status: ActiveValue::set(second_status), - } - }) - }, - ), - ) - .on_conflict( - OnConflict::columns([ - project_repository_statuses::Column::ProjectId, - project_repository_statuses::Column::RepositoryId, - project_repository_statuses::Column::RepoPath, - ]) - .update_columns([ - project_repository_statuses::Column::ScanId, - project_repository_statuses::Column::StatusKind, - project_repository_statuses::Column::FirstStatus, - project_repository_statuses::Column::SecondStatus, - ]) - .to_owned(), - ) - .exec(&*tx) - .await?; - } - - for repo in &update.updated_repositories { - if !repo.removed_statuses.is_empty() { - project_repository_statuses::Entity::update_many() - .filter( - project_repository_statuses::Column::ProjectId - .eq(project_id) - .and( - project_repository_statuses::Column::RepositoryId - .eq(repo.repository_id), - ) - .and( - project_repository_statuses::Column::RepoPath - .is_in(repo.removed_statuses.iter()), - ), - ) - .set(project_repository_statuses::ActiveModel { - is_deleted: ActiveValue::Set(true), - scan_id: ActiveValue::Set(update.scan_id as i64), - ..Default::default() - }) - .exec(&*tx) - .await?; - } - } - } - - if !update.removed_repositories.is_empty() { - project_repository::Entity::update_many() - .filter( - project_repository::Column::ProjectId - .eq(project_id) - .and(project_repository::Column::LegacyWorktreeId.eq(worktree_id)) - .and(project_repository::Column::Id.is_in( - update.removed_repositories.iter().map(|id| *id as i64), - )), - ) - .set(project_repository::ActiveModel { - is_deleted: ActiveValue::Set(true), - scan_id: ActiveValue::Set(update.scan_id as i64), - ..Default::default() - }) - .exec(&*tx) - .await?; - } - } - let connection_ids = self.project_guest_connection_ids(project_id, &tx).await?; Ok(connection_ids) }) @@ -552,6 +411,12 @@ impl Database { status_kind: ActiveValue::set(status_kind), first_status: ActiveValue::set(first_status), second_status: ActiveValue::set(second_status), + lines_added: ActiveValue::set( + status_entry.diff_stat_added.map(|v| v as i32), + ), + lines_deleted: ActiveValue::set( + status_entry.diff_stat_deleted.map(|v| v as i32), + ), } }), ) @@ -566,6 +431,8 @@ impl Database { project_repository_statuses::Column::StatusKind, project_repository_statuses::Column::FirstStatus, project_repository_statuses::Column::SecondStatus, + project_repository_statuses::Column::LinesAdded, + project_repository_statuses::Column::LinesDeleted, ]) .to_owned(), ) diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index 7c007a570a0cb25c5302495d7342882eec0e1942..b4cbd83167b227542d8de1022b7e2cf49f5a7645 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -738,7 +738,7 @@ impl Database { while let Some(db_status) = db_statuses.next().await { let db_status: project_repository_statuses::Model = db_status?; if db_status.is_deleted { - removed_statuses.push(db_status.repo_path); + removed_statuses.push(db_status.repo_path.clone()); } else { updated_statuses.push(db_status_to_proto(db_status)?); } diff --git a/crates/collab/src/db/tables/project_repository_statuses.rs b/crates/collab/src/db/tables/project_repository_statuses.rs index 7bb903d45085467a3285a58f8afdd7a29339731a..8160d8a03c2a3b4dd0db7675489eeafcef020a9a 100644 --- a/crates/collab/src/db/tables/project_repository_statuses.rs +++ b/crates/collab/src/db/tables/project_repository_statuses.rs @@ -17,6 +17,8 @@ pub struct Model { pub first_status: Option, /// For unmerged entries, this is the `second_head` status. For tracked entries, this is the `worktree_status`. pub second_status: Option, + pub lines_added: Option, + pub lines_deleted: Option, pub scan_id: i64, pub is_deleted: bool, } diff --git a/crates/collab/tests/integration/git_tests.rs b/crates/collab/tests/integration/git_tests.rs index 6e50e41bade5f5dfdf124f5a6d659e81fc2ce0f6..dccc99a07769e66a3eb318a8201d8e14a29ef4f2 100644 --- a/crates/collab/tests/integration/git_tests.rs +++ b/crates/collab/tests/integration/git_tests.rs @@ -1,16 +1,40 @@ use std::path::{Path, PathBuf}; use call::ActiveCall; -use git::status::{FileStatus, StatusCode, TrackedStatus}; -use git_ui::project_diff::ProjectDiff; +use collections::HashMap; +use git::{ + repository::RepoPath, + status::{DiffStat, FileStatus, StatusCode, TrackedStatus}, +}; +use git_ui::{git_panel::GitPanel, project_diff::ProjectDiff}; use gpui::{AppContext as _, BackgroundExecutor, TestAppContext, VisualTestContext}; use project::ProjectPath; use serde_json::json; + use util::{path, rel_path::rel_path}; use workspace::{MultiWorkspace, Workspace}; use crate::TestServer; +fn collect_diff_stats( + panel: &gpui::Entity, + cx: &C, +) -> HashMap { + panel.read_with(cx, |panel, cx| { + let Some(repo) = panel.active_repository() else { + return HashMap::default(); + }; + let snapshot = repo.read(cx).snapshot(); + let mut stats = HashMap::default(); + for entry in snapshot.statuses_by_path.iter() { + if let Some(diff_stat) = entry.diff_stat { + stats.insert(entry.repo_path.clone(), diff_stat); + } + } + stats + }) +} + #[gpui::test] async fn test_project_diff(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) { let mut server = TestServer::start(cx_a.background_executor.clone()).await; @@ -279,3 +303,198 @@ async fn test_remote_git_worktrees( ); assert_eq!(bugfix_worktree.sha.as_ref(), "fake-sha"); } + +#[gpui::test] +async fn test_diff_stat_sync_between_host_and_downstream_client( + cx_a: &mut TestAppContext, + cx_b: &mut TestAppContext, + cx_c: &mut TestAppContext, +) { + let mut server = TestServer::start(cx_a.background_executor.clone()).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + let client_c = server.create_client(cx_c, "user_c").await; + + server + .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b), (&client_c, cx_c)]) + .await; + + let fs = client_a.fs(); + fs.insert_tree( + path!("/code"), + json!({ + "project1": { + ".git": {}, + "src": { + "lib.rs": "line1\nline2\nline3\n", + "new_file.rs": "added1\nadded2\n", + }, + "README.md": "# project 1", + } + }), + ) + .await; + + let dot_git = Path::new(path!("/code/project1/.git")); + fs.set_head_for_repo( + dot_git, + &[ + ("src/lib.rs", "line1\nold_line2\n".into()), + ("src/deleted.rs", "was_here\n".into()), + ], + "deadbeef", + ); + fs.set_index_for_repo( + dot_git, + &[ + ("src/lib.rs", "line1\nold_line2\nline3\nline4\n".into()), + ("src/staged_only.rs", "x\ny\n".into()), + ("src/new_file.rs", "added1\nadded2\n".into()), + ("README.md", "# project 1".into()), + ], + ); + + let (project_a, worktree_id) = client_a + .build_local_project(path!("/code/project1"), cx_a) + .await; + let active_call_a = cx_a.read(ActiveCall::global); + let project_id = active_call_a + .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) + .await + .unwrap(); + let project_b = client_b.join_remote_project(project_id, cx_b).await; + let _project_c = client_c.join_remote_project(project_id, cx_c).await; + cx_a.run_until_parked(); + + let (workspace_a, cx_a) = client_a.build_workspace(&project_a, cx_a); + let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b); + + let panel_a = workspace_a.update_in(cx_a, GitPanel::new_test); + workspace_a.update_in(cx_a, |workspace, window, cx| { + workspace.add_panel(panel_a.clone(), window, cx); + }); + + let panel_b = workspace_b.update_in(cx_b, GitPanel::new_test); + workspace_b.update_in(cx_b, |workspace, window, cx| { + workspace.add_panel(panel_b.clone(), window, cx); + }); + + cx_a.run_until_parked(); + + let stats_a = collect_diff_stats(&panel_a, cx_a); + let stats_b = collect_diff_stats(&panel_b, cx_b); + + let mut expected: HashMap = HashMap::default(); + expected.insert( + RepoPath::new("src/lib.rs").unwrap(), + DiffStat { + added: 3, + deleted: 2, + }, + ); + expected.insert( + RepoPath::new("src/deleted.rs").unwrap(), + DiffStat { + added: 0, + deleted: 1, + }, + ); + expected.insert( + RepoPath::new("src/new_file.rs").unwrap(), + DiffStat { + added: 2, + deleted: 0, + }, + ); + expected.insert( + RepoPath::new("README.md").unwrap(), + DiffStat { + added: 1, + deleted: 0, + }, + ); + assert_eq!(stats_a, expected, "host diff stats should match expected"); + assert_eq!(stats_a, stats_b, "host and remote should agree"); + + let buffer_a = project_a + .update(cx_a, |p, cx| { + p.open_buffer((worktree_id, rel_path("src/lib.rs")), cx) + }) + .await + .unwrap(); + + let _buffer_b = project_b + .update(cx_b, |p, cx| { + p.open_buffer((worktree_id, rel_path("src/lib.rs")), cx) + }) + .await + .unwrap(); + cx_a.run_until_parked(); + + buffer_a.update(cx_a, |buf, cx| { + buf.edit([(buf.len()..buf.len(), "line4\n")], None, cx); + }); + project_a + .update(cx_a, |project, cx| { + project.save_buffer(buffer_a.clone(), cx) + }) + .await + .unwrap(); + cx_a.run_until_parked(); + + let stats_a = collect_diff_stats(&panel_a, cx_a); + let stats_b = collect_diff_stats(&panel_b, cx_b); + + let mut expected_after_edit = expected.clone(); + expected_after_edit.insert( + RepoPath::new("src/lib.rs").unwrap(), + DiffStat { + added: 4, + deleted: 2, + }, + ); + assert_eq!( + stats_a, expected_after_edit, + "host diff stats should reflect the edit" + ); + assert_eq!( + stats_b, expected_after_edit, + "remote diff stats should reflect the host's edit" + ); + + let active_call_b = cx_b.read(ActiveCall::global); + active_call_b + .update(cx_b, |call, cx| call.hang_up(cx)) + .await + .unwrap(); + cx_a.run_until_parked(); + + let user_id_b = client_b.current_user_id(cx_b).to_proto(); + active_call_a + .update(cx_a, |call, cx| call.invite(user_id_b, None, cx)) + .await + .unwrap(); + cx_b.run_until_parked(); + let active_call_b = cx_b.read(ActiveCall::global); + active_call_b + .update(cx_b, |call, cx| call.accept_incoming(cx)) + .await + .unwrap(); + cx_a.run_until_parked(); + + let project_b = client_b.join_remote_project(project_id, cx_b).await; + cx_a.run_until_parked(); + + let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b); + let panel_b = workspace_b.update_in(cx_b, GitPanel::new_test); + workspace_b.update_in(cx_b, |workspace, window, cx| { + workspace.add_panel(panel_b.clone(), window, cx); + }); + cx_b.run_until_parked(); + + let stats_b = collect_diff_stats(&panel_b, cx_b); + assert_eq!( + stats_b, expected_after_edit, + "remote diff stats should be restored from the database after rejoining the call" + ); +} diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 06ebea9157f97a0323297cd3ae142c4b306fe4ef..85489b6057cd8214ee512fb477428c93cdb32219 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -795,8 +795,8 @@ impl GitRepository for FakeGitRepository { fn diff_stat( &self, - diff_type: git::repository::DiffType, - ) -> BoxFuture<'_, Result>> { + path_prefixes: &[RepoPath], + ) -> BoxFuture<'_, Result> { fn count_lines(s: &str) -> u32 { if s.is_empty() { 0 @@ -805,122 +805,95 @@ impl GitRepository for FakeGitRepository { } } - match diff_type { - git::repository::DiffType::HeadToIndex => self - .with_state_async(false, |state| { - let mut result = HashMap::default(); - let all_paths: HashSet<&RepoPath> = state - .head_contents - .keys() - .chain(state.index_contents.keys()) - .collect(); - for path in all_paths { - let head = state.head_contents.get(path); - let index = state.index_contents.get(path); - match (head, index) { - (Some(old), Some(new)) if old != new => { - result.insert( - path.clone(), - git::status::DiffStat { - added: count_lines(new), - deleted: count_lines(old), - }, - ); - } - (Some(old), None) => { - result.insert( - path.clone(), - git::status::DiffStat { - added: 0, - deleted: count_lines(old), - }, - ); - } - (None, Some(new)) => { - result.insert( - path.clone(), - git::status::DiffStat { - added: count_lines(new), - deleted: 0, - }, - ); - } - _ => {} - } - } - Ok(result) - }) - .boxed(), - git::repository::DiffType::HeadToWorktree => { - let workdir_path = self.dot_git_path.parent().unwrap().to_path_buf(); - let worktree_files: HashMap = self + fn matches_prefixes(path: &RepoPath, prefixes: &[RepoPath]) -> bool { + if prefixes.is_empty() { + return true; + } + prefixes.iter().any(|prefix| { + let prefix_str = prefix.as_unix_str(); + if prefix_str == "." { + return true; + } + path == prefix || path.starts_with(&prefix) + }) + } + + let path_prefixes = path_prefixes.to_vec(); + + let workdir_path = self.dot_git_path.parent().unwrap().to_path_buf(); + let worktree_files: HashMap = self + .fs + .files() + .iter() + .filter_map(|path| { + let repo_path = path.strip_prefix(&workdir_path).ok()?; + if repo_path.starts_with(".git") { + return None; + } + let content = self .fs - .files() - .iter() - .filter_map(|path| { - let repo_path = path.strip_prefix(&workdir_path).ok()?; - if repo_path.starts_with(".git") { - return None; - } - let content = self - .fs - .read_file_sync(path) - .ok() - .and_then(|bytes| String::from_utf8(bytes).ok())?; - let repo_path = RelPath::new(repo_path, PathStyle::local()).ok()?; - Some((RepoPath::from_rel_path(&repo_path), content)) - }) - .collect(); + .read_file_sync(path) + .ok() + .and_then(|bytes| String::from_utf8(bytes).ok())?; + let repo_path = RelPath::new(repo_path, PathStyle::local()).ok()?; + Some((RepoPath::from_rel_path(&repo_path), content)) + }) + .collect(); - self.with_state_async(false, move |state| { - let mut result = HashMap::default(); - let all_paths: HashSet<&RepoPath> = state - .head_contents + 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() - .chain(worktree_files.keys()) - .collect(); - for path in all_paths { - let head = state.head_contents.get(path); - let worktree = worktree_files.get(path); - match (head, worktree) { - (Some(old), Some(new)) if old != new => { - result.insert( - path.clone(), - git::status::DiffStat { - added: count_lines(new), - deleted: count_lines(old), - }, - ); - } - (Some(old), None) => { - result.insert( - path.clone(), - git::status::DiffStat { - added: 0, - deleted: count_lines(old), - }, - ); - } - (None, Some(new)) => { - result.insert( - path.clone(), - git::status::DiffStat { - added: count_lines(new), - deleted: 0, - }, - ); - } - _ => {} - } + .filter(|p| state.index_contents.contains_key(*p)), + ) + .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) { + (Some(old), Some(new)) if old != new => { + entries.push(( + path.clone(), + git::status::DiffStat { + added: count_lines(new), + deleted: count_lines(old), + }, + )); } - Ok(result) - }) - .boxed() - } - git::repository::DiffType::MergeBase { .. } => { - future::ready(Ok(HashMap::default())).boxed() + (Some(old), None) => { + entries.push(( + path.clone(), + git::status::DiffStat { + added: 0, + deleted: count_lines(old), + }, + )); + } + (None, Some(new)) => { + entries.push(( + path.clone(), + git::status::DiffStat { + added: count_lines(new), + deleted: 0, + }, + )); + } + _ => {} + } } - } + entries.sort_by(|(a, _), (b, _)| a.cmp(b)); + Ok(git::status::GitDiffStat { + entries: entries.into(), + }) + }) + .boxed() } fn checkpoint(&self) -> BoxFuture<'static, Result> { diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index f5a856325cc80071f2c8ef500e7b07aa24035f59..c36c70935522836eeea4a83a889109dc807604c8 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -924,8 +924,8 @@ pub trait GitRepository: Send + Sync { fn diff_stat( &self, - diff: DiffType, - ) -> BoxFuture<'_, Result>>; + path_prefixes: &[RepoPath], + ) -> BoxFuture<'_, Result>; /// Creates a checkpoint for the repository. fn checkpoint(&self) -> BoxFuture<'static, Result>; @@ -1997,42 +1997,30 @@ impl GitRepository for RealGitRepository { fn diff_stat( &self, - diff: DiffType, - ) -> BoxFuture<'_, Result>> { + path_prefixes: &[RepoPath], + ) -> BoxFuture<'_, Result> { + let path_prefixes = path_prefixes.to_vec(); let git_binary = self.git_binary(); + self.executor .spawn(async move { - let git = git_binary?; - let output = match diff { - DiffType::HeadToIndex => { - git.build_command(["diff", "--numstat", "--staged"]) - .output() - .await? - } - DiffType::HeadToWorktree => { - git.build_command(["diff", "--numstat"]).output().await? - } - DiffType::MergeBase { base_ref } => { - git.build_command([ - "diff", - "--numstat", - "--merge-base", - base_ref.as_ref(), - "HEAD", - ]) - .output() - .await? - } - }; - - anyhow::ensure!( - output.status.success(), - "Failed to run git diff --numstat:\n{}", - String::from_utf8_lossy(&output.stderr) - ); - Ok(crate::status::parse_numstat(&String::from_utf8_lossy( - &output.stdout, - ))) + let git_binary = git_binary?; + let mut args: Vec = vec![ + "diff".into(), + "--numstat".into(), + "--no-renames".into(), + "HEAD".into(), + ]; + if !path_prefixes.is_empty() { + args.push("--".into()); + args.extend( + path_prefixes + .iter() + .map(|p| p.as_std_path().to_string_lossy().into_owned()), + ); + } + let output = git_binary.run(&args).await?; + Ok(crate::status::parse_numstat(&output)) }) .boxed() } @@ -2942,11 +2930,6 @@ fn git_status_args(path_prefixes: &[RepoPath]) -> Vec { OsString::from("--no-renames"), OsString::from("-z"), ]; - args.extend( - path_prefixes - .iter() - .map(|path_prefix| path_prefix.as_std_path().into()), - ); args.extend(path_prefixes.iter().map(|path_prefix| { if path_prefix.is_empty() { Path::new(".").into() diff --git a/crates/git/src/status.rs b/crates/git/src/status.rs index b20919e7ecf4748d0035a003ed5eadebae752dd7..e8b5caec505f7bf65cb4f5cd7d789207ccd8784f 100644 --- a/crates/git/src/status.rs +++ b/crates/git/src/status.rs @@ -586,13 +586,18 @@ pub struct DiffStat { pub deleted: u32, } +#[derive(Clone, Debug)] +pub struct GitDiffStat { + pub entries: Arc<[(RepoPath, DiffStat)]>, +} + /// Parses the output of `git diff --numstat` where output looks like: /// /// ```text /// 24 12 dir/file.txt /// ``` -pub fn parse_numstat(output: &str) -> HashMap { - let mut stats = HashMap::default(); +pub fn parse_numstat(output: &str) -> GitDiffStat { + let mut entries = Vec::new(); for line in output.lines() { let line = line.trim(); if line.is_empty() { @@ -613,10 +618,14 @@ pub fn parse_numstat(output: &str) -> HashMap { let Ok(path) = RepoPath::new(path_str) else { continue; }; - let stat = DiffStat { added, deleted }; - stats.insert(path, stat); + entries.push((path, DiffStat { added, deleted })); + } + entries.sort_by(|(a, _), (b, _)| a.cmp(b)); + entries.dedup_by(|(a, _), (b, _)| a == b); + + GitDiffStat { + entries: entries.into(), } - stats } #[cfg(test)] @@ -629,20 +638,25 @@ mod tests { use super::{DiffStat, parse_numstat}; + fn lookup<'a>(entries: &'a [(RepoPath, DiffStat)], path: &str) -> Option<&'a DiffStat> { + let path = RepoPath::new(path).unwrap(); + entries.iter().find(|(p, _)| p == &path).map(|(_, s)| s) + } + #[test] fn test_parse_numstat_normal() { let input = "10\t5\tsrc/main.rs\n3\t1\tREADME.md\n"; let result = parse_numstat(input); - assert_eq!(result.len(), 2); + assert_eq!(result.entries.len(), 2); assert_eq!( - result.get(&RepoPath::new("src/main.rs").unwrap()), + lookup(&result.entries, "src/main.rs"), Some(&DiffStat { added: 10, deleted: 5 }) ); assert_eq!( - result.get(&RepoPath::new("README.md").unwrap()), + lookup(&result.entries, "README.md"), Some(&DiffStat { added: 3, deleted: 1 @@ -655,10 +669,10 @@ mod tests { // git diff --numstat outputs "-\t-\tpath" for binary files let input = "-\t-\timage.png\n5\t2\tsrc/lib.rs\n"; let result = parse_numstat(input); - assert_eq!(result.len(), 1); - assert!(!result.contains_key(&RepoPath::new("image.png").unwrap())); + assert_eq!(result.entries.len(), 1); + assert!(lookup(&result.entries, "image.png").is_none()); assert_eq!( - result.get(&RepoPath::new("src/lib.rs").unwrap()), + lookup(&result.entries, "src/lib.rs"), Some(&DiffStat { added: 5, deleted: 2 @@ -668,18 +682,18 @@ mod tests { #[test] fn test_parse_numstat_empty_input() { - assert!(parse_numstat("").is_empty()); - assert!(parse_numstat("\n\n").is_empty()); - assert!(parse_numstat(" \n \n").is_empty()); + assert!(parse_numstat("").entries.is_empty()); + assert!(parse_numstat("\n\n").entries.is_empty()); + assert!(parse_numstat(" \n \n").entries.is_empty()); } #[test] fn test_parse_numstat_malformed_lines_skipped() { let input = "not_a_number\t5\tfile.rs\n10\t5\tvalid.rs\n"; let result = parse_numstat(input); - assert_eq!(result.len(), 1); + assert_eq!(result.entries.len(), 1); assert_eq!( - result.get(&RepoPath::new("valid.rs").unwrap()), + lookup(&result.entries, "valid.rs"), Some(&DiffStat { added: 10, deleted: 5 @@ -692,9 +706,9 @@ mod tests { // Lines with fewer than 3 tab-separated fields are skipped let input = "10\t5\n7\t3\tok.rs\n"; let result = parse_numstat(input); - assert_eq!(result.len(), 1); + assert_eq!(result.entries.len(), 1); assert_eq!( - result.get(&RepoPath::new("ok.rs").unwrap()), + lookup(&result.entries, "ok.rs"), Some(&DiffStat { added: 7, deleted: 3 @@ -707,7 +721,7 @@ mod tests { let input = "0\t0\tunchanged_but_present.rs\n"; let result = parse_numstat(input); assert_eq!( - result.get(&RepoPath::new("unchanged_but_present.rs").unwrap()), + lookup(&result.entries, "unchanged_but_present.rs"), Some(&DiffStat { added: 0, deleted: 0 diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 1fabc387247e3f0889749463e3aabd89ef0bff42..61d94b68a118525bd9b67217a929ce7462696dc7 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -41,7 +41,7 @@ use gpui::{ WeakEntity, actions, anchored, deferred, point, size, uniform_list, }; use itertools::Itertools; -use language::{Buffer, BufferEvent, File}; +use language::{Buffer, File}; use language_model::{ ConfiguredModel, LanguageModelRegistry, LanguageModelRequest, LanguageModelRequestMessage, Role, }; @@ -51,7 +51,6 @@ use notifications::status_toast::{StatusToast, ToastIcon}; use panel::{PanelHeader, panel_button, panel_filled_button, panel_icon_button}; use project::{ Fs, Project, ProjectPath, - buffer_store::BufferStoreEvent, git_store::{GitStoreEvent, Repository, RepositoryEvent, RepositoryId, pending_op}, project_settings::{GitPathStyle, ProjectSettings}, }; @@ -533,6 +532,7 @@ pub struct GitStatusEntry { pub(crate) repo_path: RepoPath, pub(crate) status: FileStatus, pub(crate) staging: StageStatus, + pub(crate) diff_stat: Option, } impl GitStatusEntry { @@ -653,8 +653,7 @@ pub struct GitPanel { local_committer_task: Option>, bulk_staging: Option, stash_entries: GitStash, - diff_stats: HashMap, - diff_stats_task: Task<()>, + _settings_subscription: Subscription, } @@ -723,18 +722,14 @@ impl GitPanel { if tree_view != was_tree_view { this.view_mode = GitPanelViewMode::from_settings(cx); } + + let mut update_entries = false; if sort_by_path != was_sort_by_path || tree_view != was_tree_view { this.bulk_staging.take(); - this.update_visible_entries(window, cx); + update_entries = true; } - if diff_stats != was_diff_stats { - if diff_stats { - this.fetch_diff_stats(cx); - } else { - this.diff_stats.clear(); - this.diff_stats_task = Task::ready(()); - cx.notify(); - } + if (diff_stats != was_diff_stats) || update_entries { + this.update_visible_entries(window, cx); } was_sort_by_path = sort_by_path; was_tree_view = tree_view; @@ -791,33 +786,6 @@ impl GitPanel { ) .detach(); - let buffer_store = project.read(cx).buffer_store().clone(); - - for buffer in project.read(cx).opened_buffers(cx) { - cx.subscribe(&buffer, |this, _buffer, event, cx| { - if matches!(event, BufferEvent::Saved) { - if GitPanelSettings::get_global(cx).diff_stats { - this.fetch_diff_stats(cx); - } - } - }) - .detach(); - } - - cx.subscribe(&buffer_store, |_this, _store, event, cx| { - if let BufferStoreEvent::BufferAdded(buffer) = event { - cx.subscribe(buffer, |this, _buffer, event, cx| { - if matches!(event, BufferEvent::Saved) { - if GitPanelSettings::get_global(cx).diff_stats { - this.fetch_diff_stats(cx); - } - } - }) - .detach(); - } - }) - .detach(); - let mut this = Self { active_repository, commit_editor, @@ -858,8 +826,6 @@ impl GitPanel { entry_count: 0, bulk_staging: None, stash_entries: Default::default(), - diff_stats: HashMap::default(), - diff_stats_task: Task::ready(()), _settings_subscription, }; @@ -3575,6 +3541,7 @@ impl GitPanel { repo_path: entry.repo_path.clone(), status: entry.status, staging, + diff_stat: entry.diff_stat, }; if staging.has_staged() { @@ -3611,6 +3578,7 @@ impl GitPanel { repo_path: ops.repo_path.clone(), status: status.status, staging: StageStatus::Staged, + diff_stat: status.diff_stat, }); } } @@ -3743,60 +3711,9 @@ impl GitPanel { editor.set_placeholder_text(&placeholder_text, window, cx) }); - if GitPanelSettings::get_global(cx).diff_stats { - self.fetch_diff_stats(cx); - } - cx.notify(); } - fn fetch_diff_stats(&mut self, cx: &mut Context) { - let Some(repo) = self.active_repository.clone() else { - self.diff_stats.clear(); - return; - }; - - let unstaged_rx = repo.update(cx, |repo, cx| repo.diff_stat(DiffType::HeadToWorktree, cx)); - let staged_rx = repo.update(cx, |repo, cx| repo.diff_stat(DiffType::HeadToIndex, cx)); - - self.diff_stats_task = cx.spawn(async move |this, cx| { - let (unstaged_result, staged_result) = - futures::future::join(unstaged_rx, staged_rx).await; - - let mut combined = match unstaged_result { - Ok(Ok(stats)) => stats, - Ok(Err(err)) => { - log::warn!("Failed to fetch unstaged diff stats: {err:?}"); - HashMap::default() - } - Err(_) => HashMap::default(), - }; - - let staged = match staged_result { - Ok(Ok(stats)) => Some(stats), - Ok(Err(err)) => { - log::warn!("Failed to fetch staged diff stats: {err:?}"); - None - } - Err(_) => None, - }; - - if let Some(staged) = staged { - for (path, stat) in staged { - let entry = combined.entry(path).or_default(); - entry.added += stat.added; - entry.deleted += stat.deleted; - } - } - - this.update(cx, |this, cx| { - this.diff_stats = combined; - cx.notify(); - }) - .ok(); - }); - } - fn header_state(&self, header_type: Section) -> ToggleState { let (staged_count, count) = match header_type { Section::New => (self.new_staged_count, self.new_count), @@ -5227,17 +5144,14 @@ impl GitPanel { .active(|s| s.bg(active_bg)) .child(name_row) .when(GitPanelSettings::get_global(cx).diff_stats, |el| { - el.when_some( - self.diff_stats.get(&entry.repo_path).copied(), - move |this, stat| { - let id = format!("diff-stat-{}", id_for_diff_stat); - this.child(ui::DiffStat::new( - id, - stat.added as usize, - stat.deleted as usize, - )) - }, - ) + el.when_some(entry.diff_stat, move |this, stat| { + let id = format!("diff-stat-{}", id_for_diff_stat); + this.child(ui::DiffStat::new( + id, + stat.added as usize, + stat.deleted as usize, + )) + }) }) .child( div() @@ -5629,6 +5543,21 @@ impl GitPanel { } } +#[cfg(any(test, feature = "test-support"))] +impl GitPanel { + pub fn new_test( + workspace: &mut Workspace, + window: &mut Window, + cx: &mut Context, + ) -> Entity { + Self::new(workspace, window, cx) + } + + pub fn active_repository(&self) -> Option<&Entity> { + self.active_repository.as_ref() + } +} + impl Render for GitPanel { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { let project = self.project.read(cx); @@ -6606,11 +6535,19 @@ mod tests { repo_path: repo_path("crates/gpui/gpui.rs"), status: StatusCode::Modified.worktree(), staging: StageStatus::Unstaged, + diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), }), GitListEntry::Status(GitStatusEntry { repo_path: repo_path("crates/util/util.rs"), status: StatusCode::Modified.worktree(), staging: StageStatus::Unstaged, + diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), },), ], ); @@ -6631,11 +6568,19 @@ mod tests { repo_path: repo_path("crates/gpui/gpui.rs"), status: StatusCode::Modified.worktree(), staging: StageStatus::Unstaged, + diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), }), GitListEntry::Status(GitStatusEntry { repo_path: repo_path("crates/util/util.rs"), status: StatusCode::Modified.worktree(), staging: StageStatus::Unstaged, + diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), },), ], ); diff --git a/crates/lsp/Cargo.toml b/crates/lsp/Cargo.toml index 9533ddb600b18213de4d6e50599c62aa182b9b8a..2c48575a648a9eba12b16ce8edb2cf959d7cc8b3 100644 --- a/crates/lsp/Cargo.toml +++ b/crates/lsp/Cargo.toml @@ -13,12 +13,13 @@ path = "src/lsp.rs" doctest = false [features] -test-support = ["async-pipe"] +test-support = ["async-pipe", "gpui_util"] [dependencies] anyhow.workspace = true async-pipe = { workspace = true, optional = true } collections.workspace = true +gpui_util = { workspace = true, optional = true } futures.workspace = true gpui.workspace = true log.workspace = true @@ -34,6 +35,7 @@ release_channel.workspace = true [dev-dependencies] async-pipe.workspace = true +gpui_util.workspace = true ctor.workspace = true gpui = { workspace = true, features = ["test-support"] } semver.workspace = true diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index e552c21d701cefa8aa1f4b6e14e826892e3b25b6..2e2318065292ffdc2ac39b577afc7a264d36473d 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -1970,10 +1970,14 @@ impl FakeLanguageServer { let responded_tx = responded_tx.clone(); let executor = cx.background_executor().clone(); async move { + let _guard = gpui_util::defer({ + let responded_tx = responded_tx.clone(); + move || { + responded_tx.unbounded_send(()).ok(); + } + }); executor.simulate_random_delay().await; - let result = result.await; - responded_tx.unbounded_send(()).ok(); - result + result.await } }) .detach(); diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index b03c7d69ab05daf94254a9d47cb2ae23da3043d1..fdafea73fd0ca797616cc58fc9e4b6a3c2101224 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -24,7 +24,7 @@ use futures::{ mpsc, oneshot::{self, Canceled}, }, - future::{self, Shared}, + future::{self, BoxFuture, Shared}, stream::FuturesOrdered, }; use git::{ @@ -39,8 +39,8 @@ use git::{ }, stash::{GitStash, StashEntry}, status::{ - DiffTreeType, FileStatus, GitSummary, StatusCode, TrackedStatus, TreeDiff, TreeDiffStatus, - UnmergedStatus, UnmergedStatusCode, + self, DiffStat, DiffTreeType, FileStatus, GitSummary, StatusCode, TrackedStatus, TreeDiff, + TreeDiffStatus, UnmergedStatus, UnmergedStatusCode, }, }; use gpui::{ @@ -195,6 +195,7 @@ pub struct GitStoreCheckpoint { pub struct StatusEntry { pub repo_path: RepoPath, pub status: FileStatus, + pub diff_stat: Option, } impl StatusEntry { @@ -216,6 +217,8 @@ impl StatusEntry { repo_path: self.repo_path.to_proto(), simple_status, 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), } } } @@ -226,7 +229,15 @@ impl TryFrom for StatusEntry { fn try_from(value: proto::StatusEntry) -> Result { let repo_path = RepoPath::from_proto(&value.repo_path).context("invalid repo path")?; let status = status_from_proto(value.simple_status, value.status)?; - Ok(Self { repo_path, status }) + let diff_stat = match (value.diff_stat_added, value.diff_stat_deleted) { + (Some(added), Some(deleted)) => Some(DiffStat { added, deleted }), + _ => None, + }; + Ok(Self { + repo_path, + status, + diff_stat, + }) } } @@ -555,7 +566,6 @@ impl GitStore { client.add_entity_request_handler(Self::handle_askpass); client.add_entity_request_handler(Self::handle_check_for_pushed_commits); client.add_entity_request_handler(Self::handle_git_diff); - client.add_entity_request_handler(Self::handle_git_diff_stat); client.add_entity_request_handler(Self::handle_tree_diff); client.add_entity_request_handler(Self::handle_get_blob_content); client.add_entity_request_handler(Self::handle_open_unstaged_diff); @@ -2761,45 +2771,6 @@ impl GitStore { Ok(proto::GitDiffResponse { diff }) } - async fn handle_git_diff_stat( - this: Entity, - envelope: TypedEnvelope, - mut cx: AsyncApp, - ) -> Result { - let repository_id = RepositoryId::from_proto(envelope.payload.repository_id); - let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?; - let diff_type = match envelope.payload.diff_type() { - proto::git_diff_stat::DiffType::HeadToIndex => DiffType::HeadToIndex, - proto::git_diff_stat::DiffType::HeadToWorktree => DiffType::HeadToWorktree, - proto::git_diff_stat::DiffType::MergeBase => { - let base_ref = envelope - .payload - .merge_base_ref - .ok_or_else(|| anyhow!("merge_base_ref is required for MergeBase diff type"))?; - DiffType::MergeBase { - base_ref: base_ref.into(), - } - } - }; - - let stats = repository_handle - .update(&mut cx, |repository_handle, cx| { - repository_handle.diff_stat(diff_type, cx) - }) - .await??; - - let entries = stats - .into_iter() - .map(|(path, stat)| proto::GitDiffStatEntry { - path: path.to_proto(), - added: stat.added, - deleted: stat.deleted, - }) - .collect(); - - Ok(proto::GitDiffStatResponse { entries }) - } - async fn handle_tree_diff( this: Entity, request: TypedEnvelope, @@ -3623,7 +3594,9 @@ impl RepositorySnapshot { current_new_entry = new_statuses.next(); } Ordering::Equal => { - if new_entry.status != old_entry.status { + if new_entry.status != old_entry.status + || new_entry.diff_stat != old_entry.diff_stat + { updated_statuses.push(new_entry.to_proto()); } current_old_entry = old_statuses.next(); @@ -3693,6 +3666,12 @@ impl RepositorySnapshot { .cloned() } + pub fn diff_stat_for_path(&self, path: &RepoPath) -> Option { + self.statuses_by_path + .get(&PathKey(path.as_ref().clone()), ()) + .and_then(|entry| entry.diff_stat) + } + pub fn abs_path_to_repo_path(&self, abs_path: &Path) -> Option { Self::abs_path_to_repo_path_inner(&self.work_directory_abs_path, abs_path, self.path_style) } @@ -4193,6 +4172,10 @@ impl Repository { self.snapshot.status() } + pub fn diff_stat_for_path(&self, path: &RepoPath) -> Option { + self.snapshot.diff_stat_for_path(path) + } + pub fn cached_stash(&self) -> GitStash { self.snapshot.stash_entries.clone() } @@ -5884,63 +5867,6 @@ impl Repository { }) } - /// Fetches per-line diff statistics (additions/deletions) via `git diff --numstat`. - pub fn diff_stat( - &mut self, - diff_type: DiffType, - _cx: &App, - ) -> oneshot::Receiver< - Result>, - > { - let id = self.id; - self.send_job(None, move |repo, _cx| async move { - match repo { - RepositoryState::Local(LocalRepositoryState { backend, .. }) => { - backend.diff_stat(diff_type).await - } - RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => { - let (proto_diff_type, merge_base_ref) = match &diff_type { - DiffType::HeadToIndex => { - (proto::git_diff_stat::DiffType::HeadToIndex.into(), None) - } - DiffType::HeadToWorktree => { - (proto::git_diff_stat::DiffType::HeadToWorktree.into(), None) - } - DiffType::MergeBase { base_ref } => ( - proto::git_diff_stat::DiffType::MergeBase.into(), - Some(base_ref.to_string()), - ), - }; - let response = client - .request(proto::GitDiffStat { - project_id: project_id.0, - repository_id: id.to_proto(), - diff_type: proto_diff_type, - merge_base_ref, - }) - .await?; - - let stats = response - .entries - .into_iter() - .filter_map(|entry| { - let path = RepoPath::from_proto(&entry.path).log_err()?; - Some(( - path, - git::status::DiffStat { - added: entry.added, - deleted: entry.deleted, - }, - )) - }) - .collect(); - - Ok(stats) - } - } - }) - } - pub fn create_branch( &mut self, branch_name: String, @@ -6165,6 +6091,7 @@ impl Repository { cx.emit(RepositoryEvent::StatusesChanged); } self.snapshot.statuses_by_path.edit(edits, ()); + if update.is_last_update { self.snapshot.scan_id = update.scan_id; } @@ -6479,22 +6406,43 @@ impl Repository { return Ok(()); } + let has_head = prev_snapshot.head_commit.is_some(); + let stash_entries = backend.stash_entries().await?; let changed_path_statuses = cx .background_spawn(async move { let mut changed_paths = changed_paths.into_iter().flatten().collect::>(); - let statuses = backend - .status(&changed_paths.iter().cloned().collect::>()) - .await?; + 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 (statuses, diff_stats) = + futures::future::try_join(status_task, diff_stat_future).await?; + + let diff_stats: HashMap = + HashMap::from_iter(diff_stats.entries.into_iter().cloned()); + let mut changed_path_statuses = Vec::new(); let prev_statuses = prev_snapshot.statuses_by_path.clone(); let mut cursor = prev_statuses.cursor::(()); for (repo_path, status) in &*statuses.entries { + let current_diff_stat = diff_stats.get(repo_path).copied(); + changed_paths.remove(repo_path); if cursor.seek_forward(&PathTarget::Path(repo_path), Bias::Left) - && cursor.item().is_some_and(|entry| entry.status == *status) + && cursor.item().is_some_and(|entry| { + entry.status == *status && entry.diff_stat == current_diff_stat + }) { continue; } @@ -6502,6 +6450,7 @@ impl Repository { changed_path_statuses.push(Edit::Insert(StatusEntry { repo_path: repo_path.clone(), status: *status, + diff_stat: current_diff_stat, })); } let mut cursor = prev_statuses.cursor::(()); @@ -6859,11 +6808,31 @@ async fn compute_snapshot( let mut events = Vec::new(); let branches = backend.branches().await?; let branch = branches.into_iter().find(|branch| branch.is_head); - let statuses = backend - .status(&[RepoPath::from_rel_path( + + // Useful when branch is None in detached head state + let head_commit = match backend.head_sha().await { + Some(head_sha) => backend.show(head_sha).await.log_err(), + None => None, + }; + + let diff_stat_future: BoxFuture<'_, Result> = if head_commit.is_some() { + backend.diff_stat(&[]) + } else { + future::ready(Ok(status::GitDiffStat { + entries: Arc::default(), + })) + .boxed() + }; + let (statuses, diff_stats) = futures::future::try_join( + backend.status(&[RepoPath::from_rel_path( &RelPath::new(".".as_ref(), PathStyle::local()).unwrap(), - )]) - .await?; + )]), + diff_stat_future, + ) + .await?; + + let diff_stat_map: HashMap<&RepoPath, DiffStat> = + diff_stats.entries.iter().map(|(p, s)| (p, *s)).collect(); let stash_entries = backend.stash_entries().await?; let mut conflicted_paths = Vec::new(); let statuses_by_path = SumTree::from_iter( @@ -6874,6 +6843,7 @@ async fn compute_snapshot( StatusEntry { repo_path: repo_path.clone(), status: *status, + diff_stat: diff_stat_map.get(repo_path).copied(), } }), (), @@ -6886,12 +6856,6 @@ async fn compute_snapshot( events.push(RepositoryEvent::StatusesChanged) } - // Useful when branch is None in detached head state - let head_commit = match backend.head_sha().await { - Some(head_sha) => backend.show(head_sha).await.log_err(), - None => None, - }; - if branch != prev_snapshot.branch || head_commit != prev_snapshot.head_commit { events.push(RepositoryEvent::BranchChanged); } diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index d597377910a2a837e456ac4384b06c333887dfb3..d86b969e61ed173ee314cde6f584f2dbab6859f9 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -31,7 +31,7 @@ use futures::{StreamExt, future}; use git::{ GitHostingProviderRegistry, repository::{RepoPath, repo_path}, - status::{FileStatus, StatusCode, TrackedStatus}, + status::{DiffStat, FileStatus, StatusCode, TrackedStatus}, }; use git2::RepositoryInitOptions; use gpui::{ @@ -9253,14 +9253,23 @@ async fn test_git_repository_status(cx: &mut gpui::TestAppContext) { StatusEntry { repo_path: repo_path("a.txt"), status: StatusCode::Modified.worktree(), + diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), }, StatusEntry { repo_path: repo_path("b.txt"), status: FileStatus::Untracked, + diff_stat: None, }, StatusEntry { repo_path: repo_path("d.txt"), status: StatusCode::Deleted.worktree(), + diff_stat: Some(DiffStat { + added: 0, + deleted: 1, + }), }, ] ); @@ -9282,18 +9291,31 @@ async fn test_git_repository_status(cx: &mut gpui::TestAppContext) { StatusEntry { repo_path: repo_path("a.txt"), status: StatusCode::Modified.worktree(), + diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), }, StatusEntry { repo_path: repo_path("b.txt"), status: FileStatus::Untracked, + diff_stat: None, }, StatusEntry { repo_path: repo_path("c.txt"), status: StatusCode::Modified.worktree(), + diff_stat: Some(DiffStat { + added: 1, + deleted: 1, + }), }, StatusEntry { repo_path: repo_path("d.txt"), status: StatusCode::Deleted.worktree(), + diff_stat: Some(DiffStat { + added: 0, + deleted: 1, + }), }, ] ); @@ -9327,6 +9349,10 @@ async fn test_git_repository_status(cx: &mut gpui::TestAppContext) { [StatusEntry { repo_path: repo_path("a.txt"), status: StatusCode::Deleted.worktree(), + diff_stat: Some(DiffStat { + added: 0, + deleted: 1, + }), }] ); }); @@ -9391,6 +9417,7 @@ async fn test_git_status_postprocessing(cx: &mut gpui::TestAppContext) { worktree_status: StatusCode::Added } .into(), + diff_stat: None, }] ) }); @@ -9593,6 +9620,10 @@ async fn test_repository_pending_ops_staging( worktree_status: StatusCode::Unmodified } .into(), + diff_stat: Some(DiffStat { + added: 1, + deleted: 0, + }), }] ); }); @@ -9699,6 +9730,10 @@ async fn test_repository_pending_ops_long_running_staging( worktree_status: StatusCode::Unmodified } .into(), + diff_stat: Some(DiffStat { + added: 1, + deleted: 0, + }), }] ); }); @@ -9823,10 +9858,12 @@ async fn test_repository_pending_ops_stage_all( StatusEntry { repo_path: repo_path("a.txt"), status: FileStatus::Untracked, + diff_stat: None, }, StatusEntry { repo_path: repo_path("b.txt"), status: FileStatus::Untracked, + diff_stat: None, }, ] ); diff --git a/crates/proto/proto/git.proto b/crates/proto/proto/git.proto index 6cb3acfcd878c8f970c4e99789939424a3835709..736abcdaa49f62d72582750a8a28ea785baee282 100644 --- a/crates/proto/proto/git.proto +++ b/crates/proto/proto/git.proto @@ -229,29 +229,6 @@ message GitDiffResponse { string diff = 1; } -message GitDiffStat { - uint64 project_id = 1; - uint64 repository_id = 2; - DiffType diff_type = 3; - optional string merge_base_ref = 4; - - enum DiffType { - HEAD_TO_WORKTREE = 0; - HEAD_TO_INDEX = 1; - MERGE_BASE = 2; - } -} - -message GitDiffStatResponse { - repeated GitDiffStatEntry entries = 1; -} - -message GitDiffStatEntry { - string path = 1; - uint32 added = 2; - uint32 deleted = 3; -} - message GitInit { uint64 project_id = 1; string abs_path = 2; @@ -360,6 +337,8 @@ message StatusEntry { // Can be removed once collab's min version is >=0.171.0. GitStatus simple_status = 2; GitFileStatus status = 3; + optional uint32 diff_stat_added = 4; + optional uint32 diff_stat_deleted = 5; } message StashEntry { diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index d6139f5342d153221d13917e26565a4c0eb5a707..c129b6eff26404b66b38439c29f0b83289b37172 100644 --- a/crates/proto/proto/zed.proto +++ b/crates/proto/proto/zed.proto @@ -474,9 +474,7 @@ message Envelope { SpawnKernel spawn_kernel = 426; SpawnKernelResponse spawn_kernel_response = 427; - KillKernel kill_kernel = 428; - GitDiffStat git_diff_stat = 429; - GitDiffStatResponse git_diff_stat_response = 430; // current max + KillKernel kill_kernel = 428; // current max } reserved 87 to 88; @@ -501,6 +499,7 @@ message Envelope { reserved 280 to 281; reserved 332 to 333; reserved 394 to 396; + reserved 429 to 430; } message Hello { diff --git a/crates/proto/src/proto.rs b/crates/proto/src/proto.rs index 3d30551557000c305a82b328828b566c9d78f75e..dd0a77beb29345021563b21bafd261d02b87e1ab 100644 --- a/crates/proto/src/proto.rs +++ b/crates/proto/src/proto.rs @@ -322,8 +322,6 @@ messages!( (CheckForPushedCommitsResponse, Background), (GitDiff, Background), (GitDiffResponse, Background), - (GitDiffStat, Background), - (GitDiffStatResponse, Background), (GitInit, Background), (GetDebugAdapterBinary, Background), (DebugAdapterBinary, Background), @@ -541,7 +539,6 @@ request_messages!( (GitRenameBranch, Ack), (CheckForPushedCommits, CheckForPushedCommitsResponse), (GitDiff, GitDiffResponse), - (GitDiffStat, GitDiffStatResponse), (GitInit, Ack), (ToggleBreakpoint, Ack), (GetDebugAdapterBinary, DebugAdapterBinary), @@ -730,7 +727,6 @@ entity_messages!( GitRemoveRemote, CheckForPushedCommits, GitDiff, - GitDiffStat, GitInit, BreakpointsForFile, ToggleBreakpoint, diff --git a/crates/remote_server/src/remote_editing_tests.rs b/crates/remote_server/src/remote_editing_tests.rs index 9b9fe9948ace530d7e55d2843952ca5c9efb3749..7f9953c8a4e746d9586b663330badb38149cfb64 100644 --- a/crates/remote_server/src/remote_editing_tests.rs +++ b/crates/remote_server/src/remote_editing_tests.rs @@ -6,7 +6,6 @@ use agent::{AgentTool, ReadFileTool, ReadFileToolInput, ToolCallEventStream, Too use client::{Client, UserStore}; use clock::FakeSystemClock; use collections::{HashMap, HashSet}; -use git::repository::DiffType; use language_model::LanguageModelToolResultContent; use extension::ExtensionHostProxy; @@ -1917,129 +1916,6 @@ async fn test_remote_git_branches(cx: &mut TestAppContext, server_cx: &mut TestA assert_eq!(server_branch.name(), "totally-new-branch"); } -#[gpui::test] -async fn test_remote_git_diff_stat(cx: &mut TestAppContext, server_cx: &mut TestAppContext) { - let fs = FakeFs::new(server_cx.executor()); - fs.insert_tree( - path!("/code"), - json!({ - "project1": { - ".git": {}, - "src": { - "lib.rs": "line1\nline2\nline3\n", - "new_file.rs": "added1\nadded2\n", - }, - "README.md": "# project 1", - }, - }), - ) - .await; - - let dot_git = Path::new(path!("/code/project1/.git")); - - // HEAD: lib.rs (2 lines), deleted.rs (1 line) - fs.set_head_for_repo( - dot_git, - &[ - ("src/lib.rs", "line1\nold_line2\n".into()), - ("src/deleted.rs", "was_here\n".into()), - ], - "deadbeef", - ); - // Index: lib.rs modified (4 lines), staged_only.rs new (2 lines) - fs.set_index_for_repo( - dot_git, - &[ - ("src/lib.rs", "line1\nold_line2\nline3\nline4\n".into()), - ("src/staged_only.rs", "x\ny\n".into()), - ], - ); - - let (project, _headless) = init_test(&fs, cx, server_cx).await; - let (_worktree, _) = project - .update(cx, |project, cx| { - project.find_or_create_worktree(path!("/code/project1"), true, cx) - }) - .await - .unwrap(); - cx.run_until_parked(); - - let repo_path = |s: &str| git::repository::RepoPath::new(s).unwrap(); - - let repository = project.update(cx, |project, cx| project.active_repository(cx).unwrap()); - - // --- HeadToWorktree --- - let stats = cx - .update(|cx| repository.update(cx, |repo, cx| repo.diff_stat(DiffType::HeadToWorktree, cx))) - .await - .unwrap() - .unwrap(); - - // src/lib.rs: worktree 3 lines vs HEAD 2 lines - let stat = stats.get(&repo_path("src/lib.rs")).expect("src/lib.rs"); - assert_eq!((stat.added, stat.deleted), (3, 2)); - - // src/new_file.rs: only in worktree (2 lines) - let stat = stats - .get(&repo_path("src/new_file.rs")) - .expect("src/new_file.rs"); - assert_eq!((stat.added, stat.deleted), (2, 0)); - - // src/deleted.rs: only in HEAD (1 line) - let stat = stats - .get(&repo_path("src/deleted.rs")) - .expect("src/deleted.rs"); - assert_eq!((stat.added, stat.deleted), (0, 1)); - - // README.md: only in worktree (1 line) - let stat = stats.get(&repo_path("README.md")).expect("README.md"); - assert_eq!((stat.added, stat.deleted), (1, 0)); - - // --- HeadToIndex --- - let stats = cx - .update(|cx| repository.update(cx, |repo, cx| repo.diff_stat(DiffType::HeadToIndex, cx))) - .await - .unwrap() - .unwrap(); - - // src/lib.rs: index 4 lines vs HEAD 2 lines - let stat = stats.get(&repo_path("src/lib.rs")).expect("src/lib.rs"); - assert_eq!((stat.added, stat.deleted), (4, 2)); - - // src/staged_only.rs: only in index (2 lines) - let stat = stats - .get(&repo_path("src/staged_only.rs")) - .expect("src/staged_only.rs"); - assert_eq!((stat.added, stat.deleted), (2, 0)); - - // src/deleted.rs: in HEAD but not in index - let stat = stats - .get(&repo_path("src/deleted.rs")) - .expect("src/deleted.rs"); - assert_eq!((stat.added, stat.deleted), (0, 1)); - - // --- MergeBase (not implemented in FakeGitRepository) --- - let stats = cx - .update(|cx| { - repository.update(cx, |repo, cx| { - repo.diff_stat( - DiffType::MergeBase { - base_ref: "main".into(), - }, - cx, - ) - }) - }) - .await - .unwrap() - .unwrap(); - - assert!( - stats.is_empty(), - "MergeBase diff_stat should return empty from FakeGitRepository" - ); -} - #[gpui::test] async fn test_remote_agent_fs_tool_calls(cx: &mut TestAppContext, server_cx: &mut TestAppContext) { let fs = FakeFs::new(server_cx.executor());