Follow up on: https://github.com/zed-industries/zed/pull/49519 This PR reworks how Zed calculates diff num stats by moving the calculation to the `RepositorySnapshot` layer, instead of the `GitPanel`. This has a couple of benefits: 1. Snapshot recalculations are already set up to recompute on file system changes and only update the affected files. This means that diff stats don't need to manage their own subscription or states anymore like they did in the original PR. 2. We're able to further separate the data layer from the UI. Before, the git panel owned all the subscriptions and tasks that refreshed the diff stat, now the repository does, which is more inline with the code base. 3. Integration tests are cleaner because `FakeRepository` can handle all the data and calculations of diff stat and make it accessible to more tests in the codebase. Because a lot of tests wouldn't initialize the git panel when they used the git repository. 4. This made implementing remote/collab support for this feature streamline. Remote clients wouldn't get the same buffer events as local clients, so they wouldn't know that the diff stat state has been updated and invalidate their data. 5. File system changes that happened outside of Zed now trigger the diff stat refresh because we're using the `RepositorySnapshot`. I added some integration tests as well to make sure collab support is working this time. Finally, adding the initial diff calculation to `compute_snapshot` didn't affect performance for me when checking against chromium's diff with HEAD~1000. So this should be a safe change to make. I decided to add diff stats on the status entry struct because it made updating changed paths and the collab database much simpler than having two separate SumTrees. Also whenever the UI got a file's status it would check its diff stat as well, so this change makes that code more streamlined as well. Before you mark this PR as ready for review, make sure that you have: - [x] Added a solid test coverage and/or screenshots from doing manual testing. - [x] Done a self-review taking into account security and performance aspects. Release Notes: - N/A
501 lines
15 KiB
Rust
501 lines
15 KiB
Rust
use std::path::{Path, PathBuf};
|
|
|
|
use call::ActiveCall;
|
|
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<C: gpui::AppContext>(
|
|
panel: &gpui::Entity<GitPanel>,
|
|
cx: &C,
|
|
) -> HashMap<RepoPath, DiffStat> {
|
|
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;
|
|
let client_a = server.create_client(cx_a, "user_a").await;
|
|
let client_b = server.create_client(cx_b, "user_b").await;
|
|
cx_a.set_name("cx_a");
|
|
cx_b.set_name("cx_b");
|
|
|
|
server
|
|
.create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
|
|
.await;
|
|
|
|
client_a
|
|
.fs()
|
|
.insert_tree(
|
|
path!("/a"),
|
|
json!({
|
|
".git": {},
|
|
"changed.txt": "after\n",
|
|
"unchanged.txt": "unchanged\n",
|
|
"created.txt": "created\n",
|
|
"secret.pem": "secret-changed\n",
|
|
}),
|
|
)
|
|
.await;
|
|
|
|
client_a.fs().set_head_and_index_for_repo(
|
|
Path::new(path!("/a/.git")),
|
|
&[
|
|
("changed.txt", "before\n".to_string()),
|
|
("unchanged.txt", "unchanged\n".to_string()),
|
|
("deleted.txt", "deleted\n".to_string()),
|
|
("secret.pem", "shh\n".to_string()),
|
|
],
|
|
);
|
|
let (project_a, worktree_id) = client_a.build_local_project(path!("/a"), 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();
|
|
|
|
cx_b.update(editor::init);
|
|
cx_b.update(git_ui::init);
|
|
let project_b = client_b.join_remote_project(project_id, cx_b).await;
|
|
let window_b = cx_b.add_window(|window, cx| {
|
|
let workspace = cx.new(|cx| {
|
|
Workspace::new(
|
|
None,
|
|
project_b.clone(),
|
|
client_b.app_state.clone(),
|
|
window,
|
|
cx,
|
|
)
|
|
});
|
|
MultiWorkspace::new(workspace, window, cx)
|
|
});
|
|
let cx_b = &mut VisualTestContext::from_window(*window_b, cx_b);
|
|
let workspace_b = window_b
|
|
.root(cx_b)
|
|
.unwrap()
|
|
.read_with(cx_b, |multi_workspace, _| {
|
|
multi_workspace.workspace().clone()
|
|
});
|
|
|
|
cx_b.update(|window, cx| {
|
|
window
|
|
.focused(cx)
|
|
.unwrap()
|
|
.dispatch_action(&git_ui::project_diff::Diff, window, cx)
|
|
});
|
|
let diff = workspace_b.update(cx_b, |workspace, cx| {
|
|
workspace.active_item(cx).unwrap().act_as::<ProjectDiff>(cx)
|
|
});
|
|
let diff = diff.unwrap();
|
|
cx_b.run_until_parked();
|
|
|
|
diff.update(cx_b, |diff, cx| {
|
|
assert_eq!(
|
|
diff.excerpt_paths(cx),
|
|
vec![
|
|
rel_path("changed.txt").into_arc(),
|
|
rel_path("deleted.txt").into_arc(),
|
|
rel_path("created.txt").into_arc()
|
|
]
|
|
);
|
|
});
|
|
|
|
client_a
|
|
.fs()
|
|
.insert_tree(
|
|
path!("/a"),
|
|
json!({
|
|
".git": {},
|
|
"changed.txt": "before\n",
|
|
"unchanged.txt": "changed\n",
|
|
"created.txt": "created\n",
|
|
"secret.pem": "secret-changed\n",
|
|
}),
|
|
)
|
|
.await;
|
|
cx_b.run_until_parked();
|
|
|
|
project_b.update(cx_b, |project, cx| {
|
|
let project_path = ProjectPath {
|
|
worktree_id,
|
|
path: rel_path("unchanged.txt").into(),
|
|
};
|
|
let status = project.project_path_git_status(&project_path, cx);
|
|
assert_eq!(
|
|
status.unwrap(),
|
|
FileStatus::Tracked(TrackedStatus {
|
|
worktree_status: StatusCode::Modified,
|
|
index_status: StatusCode::Unmodified,
|
|
})
|
|
);
|
|
});
|
|
|
|
diff.update(cx_b, |diff, cx| {
|
|
assert_eq!(
|
|
diff.excerpt_paths(cx),
|
|
vec![
|
|
rel_path("deleted.txt").into_arc(),
|
|
rel_path("unchanged.txt").into_arc(),
|
|
rel_path("created.txt").into_arc()
|
|
]
|
|
);
|
|
});
|
|
}
|
|
|
|
#[gpui::test]
|
|
async fn test_remote_git_worktrees(
|
|
executor: BackgroundExecutor,
|
|
cx_a: &mut TestAppContext,
|
|
cx_b: &mut TestAppContext,
|
|
) {
|
|
let mut server = TestServer::start(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;
|
|
server
|
|
.create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
|
|
.await;
|
|
let active_call_a = cx_a.read(ActiveCall::global);
|
|
|
|
client_a
|
|
.fs()
|
|
.insert_tree(
|
|
path!("/project"),
|
|
json!({ ".git": {}, "file.txt": "content" }),
|
|
)
|
|
.await;
|
|
|
|
let (project_a, _) = client_a.build_local_project(path!("/project"), cx_a).await;
|
|
|
|
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;
|
|
|
|
executor.run_until_parked();
|
|
|
|
let repo_b = cx_b.update(|cx| project_b.read(cx).active_repository(cx).unwrap());
|
|
|
|
// Initially only the main worktree (the repo itself) should be present
|
|
let worktrees = cx_b
|
|
.update(|cx| repo_b.update(cx, |repository, _| repository.worktrees()))
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
assert_eq!(worktrees.len(), 1);
|
|
assert_eq!(worktrees[0].path, PathBuf::from(path!("/project")));
|
|
|
|
// Client B creates a git worktree via the remote project
|
|
let worktree_directory = PathBuf::from(path!("/project"));
|
|
cx_b.update(|cx| {
|
|
repo_b.update(cx, |repository, _| {
|
|
repository.create_worktree(
|
|
"feature-branch".to_string(),
|
|
worktree_directory.clone(),
|
|
Some("abc123".to_string()),
|
|
)
|
|
})
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
executor.run_until_parked();
|
|
|
|
// Client B lists worktrees — should see main + the one just created
|
|
let worktrees = cx_b
|
|
.update(|cx| repo_b.update(cx, |repository, _| repository.worktrees()))
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
assert_eq!(worktrees.len(), 2);
|
|
assert_eq!(worktrees[0].path, PathBuf::from(path!("/project")));
|
|
assert_eq!(worktrees[1].path, worktree_directory.join("feature-branch"));
|
|
assert_eq!(worktrees[1].ref_name.as_ref(), "refs/heads/feature-branch");
|
|
assert_eq!(worktrees[1].sha.as_ref(), "abc123");
|
|
|
|
// Verify from the host side that the worktree was actually created
|
|
let host_worktrees = {
|
|
let repo_a = cx_a.update(|cx| {
|
|
project_a
|
|
.read(cx)
|
|
.repositories(cx)
|
|
.values()
|
|
.next()
|
|
.unwrap()
|
|
.clone()
|
|
});
|
|
cx_a.update(|cx| repo_a.update(cx, |repository, _| repository.worktrees()))
|
|
.await
|
|
.unwrap()
|
|
.unwrap()
|
|
};
|
|
assert_eq!(host_worktrees.len(), 2);
|
|
assert_eq!(host_worktrees[0].path, PathBuf::from(path!("/project")));
|
|
assert_eq!(
|
|
host_worktrees[1].path,
|
|
worktree_directory.join("feature-branch")
|
|
);
|
|
|
|
// Client B creates a second git worktree without an explicit commit
|
|
cx_b.update(|cx| {
|
|
repo_b.update(cx, |repository, _| {
|
|
repository.create_worktree(
|
|
"bugfix-branch".to_string(),
|
|
worktree_directory.clone(),
|
|
None,
|
|
)
|
|
})
|
|
})
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
|
|
executor.run_until_parked();
|
|
|
|
// Client B lists worktrees — should now have main + two created
|
|
let worktrees = cx_b
|
|
.update(|cx| repo_b.update(cx, |repository, _| repository.worktrees()))
|
|
.await
|
|
.unwrap()
|
|
.unwrap();
|
|
assert_eq!(worktrees.len(), 3);
|
|
|
|
let feature_worktree = worktrees
|
|
.iter()
|
|
.find(|worktree| worktree.ref_name.as_ref() == "refs/heads/feature-branch")
|
|
.expect("should find feature-branch worktree");
|
|
assert_eq!(
|
|
feature_worktree.path,
|
|
worktree_directory.join("feature-branch")
|
|
);
|
|
|
|
let bugfix_worktree = worktrees
|
|
.iter()
|
|
.find(|worktree| worktree.ref_name.as_ref() == "refs/heads/bugfix-branch")
|
|
.expect("should find bugfix-branch worktree");
|
|
assert_eq!(
|
|
bugfix_worktree.path,
|
|
worktree_directory.join("bugfix-branch")
|
|
);
|
|
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<RepoPath, DiffStat> = 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"
|
|
);
|
|
}
|