git: Git Panel UI, continued (#22960)

TODO:

- [ ] Investigate incorrect hit target for `stage all` button
- [ ] Add top level context menu
- [ ] Add entry context menus
- [x] Show paths in list view
- [ ] For now, `enter` can just open the file
- [ ] 🐞: Hover deadzone in list caused by scrollbar
- [x] 🐞: Incorrect status/nothing shown when multiple worktrees are
added

---

This PR continues work on the feature flagged git panel.

Changes:
- Defines and wires up git panel actions & keybindings
- Re-scopes some actions from `git_ui` -> `git`.
- General git actions (StageAll, CommitChanges, ...) are scoped to
`git`.
- Git panel specific actions (Close, FocusCommitEditor, ...) are scoped
to `git_panel.
- Staging actions & UI are now connected to git!
- Unify more reusable git status into the GitState global over being
tied to the panel directly.
- Uses the new git status codepaths instead of filtering all workspace
entries

Release Notes:

- N/A

---------

Co-authored-by: Cole Miller <53574922+cole-miller@users.noreply.github.com>
Co-authored-by: Cole Miller <cole@zed.dev>
This commit is contained in:
Nate Butler 2025-01-13 11:47:09 -05:00 committed by GitHub
parent 1c6dd03e50
commit 102e70816c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 1006 additions and 840 deletions

View file

@ -21,6 +21,7 @@ use fuzzy::CharBag;
use git::GitHostingProviderRegistry;
use git::{
repository::{GitFileStatus, GitRepository, RepoPath},
status::GitStatusPair,
COOKIES, DOT_GIT, FSMONITOR_DAEMON, GITIGNORE,
};
use gpui::{
@ -193,8 +194,8 @@ pub struct RepositoryEntry {
/// - my_sub_folder_1/project_root/changed_file_1
/// - my_sub_folder_2/changed_file_2
pub(crate) statuses_by_path: SumTree<StatusEntry>,
pub(crate) work_directory_id: ProjectEntryId,
pub(crate) work_directory: WorkDirectory,
pub work_directory_id: ProjectEntryId,
pub work_directory: WorkDirectory,
pub(crate) branch: Option<Arc<str>>,
}
@ -225,6 +226,12 @@ impl RepositoryEntry {
self.statuses_by_path.iter().cloned()
}
pub fn status_for_path(&self, path: &RepoPath) -> Option<StatusEntry> {
self.statuses_by_path
.get(&PathKey(path.0.clone()), &())
.cloned()
}
pub fn initial_update(&self) -> proto::RepositoryEntry {
proto::RepositoryEntry {
work_directory_id: self.work_directory_id.to_proto(),
@ -234,7 +241,7 @@ impl RepositoryEntry {
.iter()
.map(|entry| proto::StatusEntry {
repo_path: entry.repo_path.to_string_lossy().to_string(),
status: git_status_to_proto(entry.status),
status: status_pair_to_proto(entry.status.clone()),
})
.collect(),
removed_statuses: Default::default(),
@ -259,7 +266,7 @@ impl RepositoryEntry {
current_new_entry = new_statuses.next();
}
Ordering::Equal => {
if new_entry.status != old_entry.status {
if new_entry.combined_status() != old_entry.combined_status() {
updated_statuses.push(new_entry.to_proto());
}
current_old_entry = old_statuses.next();
@ -2360,7 +2367,7 @@ impl Snapshot {
let repo_path = repo.relativize(path).unwrap();
repo.statuses_by_path
.get(&PathKey(repo_path.0), &())
.map(|entry| entry.status)
.map(|entry| entry.combined_status())
})
}
@ -2574,8 +2581,8 @@ impl Snapshot {
.map(|repo| repo.status().collect())
}
pub fn repositories(&self) -> impl Iterator<Item = &RepositoryEntry> {
self.repositories.iter()
pub fn repositories(&self) -> &SumTree<RepositoryEntry> {
&self.repositories
}
/// Get the repository whose work directory corresponds to the given path.
@ -2609,7 +2616,7 @@ impl Snapshot {
entries: impl 'a + Iterator<Item = &'a Entry>,
) -> impl 'a + Iterator<Item = (&'a Entry, Option<&'a RepositoryEntry>)> {
let mut containing_repos = Vec::<&RepositoryEntry>::new();
let mut repositories = self.repositories().peekable();
let mut repositories = self.repositories().iter().peekable();
entries.map(move |entry| {
while let Some(repository) = containing_repos.last() {
if repository.directory_contains(&entry.path) {
@ -3626,14 +3633,31 @@ pub type UpdatedGitRepositoriesSet = Arc<[(Arc<Path>, GitRepositoryChange)]>;
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct StatusEntry {
pub repo_path: RepoPath,
pub status: GitFileStatus,
pub status: GitStatusPair,
}
impl StatusEntry {
// TODO revisit uses of this
pub fn combined_status(&self) -> GitFileStatus {
self.status.combined()
}
pub fn index_status(&self) -> Option<GitFileStatus> {
self.status.index_status
}
pub fn worktree_status(&self) -> Option<GitFileStatus> {
self.status.worktree_status
}
pub fn is_staged(&self) -> Option<bool> {
self.status.is_staged()
}
fn to_proto(&self) -> proto::StatusEntry {
proto::StatusEntry {
repo_path: self.repo_path.to_proto(),
status: git_status_to_proto(self.status),
status: status_pair_to_proto(self.status.clone()),
}
}
}
@ -3641,11 +3665,10 @@ impl StatusEntry {
impl TryFrom<proto::StatusEntry> for StatusEntry {
type Error = anyhow::Error;
fn try_from(value: proto::StatusEntry) -> Result<Self, Self::Error> {
Ok(Self {
repo_path: RepoPath(Path::new(&value.repo_path).into()),
status: git_status_from_proto(Some(value.status))
.ok_or_else(|| anyhow!("Unable to parse status value {}", value.status))?,
})
let repo_path = RepoPath(Path::new(&value.repo_path).into());
let status = status_pair_from_proto(value.status)
.ok_or_else(|| anyhow!("Unable to parse status value {}", value.status))?;
Ok(Self { repo_path, status })
}
}
@ -3729,7 +3752,7 @@ impl sum_tree::Item for StatusEntry {
fn summary(&self, _: &<Self::Summary as Summary>::Context) -> Self::Summary {
PathSummary {
max_path: self.repo_path.0.clone(),
item_summary: match self.status {
item_summary: match self.combined_status() {
GitFileStatus::Added => GitStatuses {
added: 1,
..Default::default()
@ -4820,15 +4843,15 @@ impl BackgroundScanner {
for (repo_path, status) in &*status.entries {
paths.remove_repo_path(repo_path);
if cursor.seek_forward(&PathTarget::Path(&repo_path), Bias::Left, &()) {
if cursor.item().unwrap().status == *status {
if cursor.seek_forward(&PathTarget::Path(repo_path), Bias::Left, &()) {
if &cursor.item().unwrap().status == status {
continue;
}
}
changed_path_statuses.push(Edit::Insert(StatusEntry {
repo_path: repo_path.clone(),
status: *status,
status: status.clone(),
}));
}
@ -5257,7 +5280,7 @@ impl BackgroundScanner {
new_entries_by_path.insert_or_replace(
StatusEntry {
repo_path: repo_path.clone(),
status: *status,
status: status.clone(),
},
&(),
);
@ -5771,7 +5794,7 @@ impl<'a> GitTraversal<'a> {
} else if entry.is_file() {
// For a file entry, park the cursor on the corresponding status
if statuses.seek_forward(&PathTarget::Path(repo_path.as_ref()), Bias::Left, &()) {
self.current_entry_status = Some(statuses.item().unwrap().status);
self.current_entry_status = Some(statuses.item().unwrap().combined_status());
}
}
}
@ -6136,19 +6159,23 @@ impl<'a> TryFrom<(&'a CharBag, &PathMatcher, proto::Entry)> for Entry {
}
}
fn git_status_from_proto(git_status: Option<i32>) -> Option<GitFileStatus> {
git_status.and_then(|status| {
proto::GitStatus::from_i32(status).map(|status| match status {
proto::GitStatus::Added => GitFileStatus::Added,
proto::GitStatus::Modified => GitFileStatus::Modified,
proto::GitStatus::Conflict => GitFileStatus::Conflict,
proto::GitStatus::Deleted => GitFileStatus::Deleted,
})
// TODO pass the status pair all the way through
fn status_pair_from_proto(proto: i32) -> Option<GitStatusPair> {
let proto = proto::GitStatus::from_i32(proto)?;
let worktree_status = match proto {
proto::GitStatus::Added => GitFileStatus::Added,
proto::GitStatus::Modified => GitFileStatus::Modified,
proto::GitStatus::Conflict => GitFileStatus::Conflict,
proto::GitStatus::Deleted => GitFileStatus::Deleted,
};
Some(GitStatusPair {
index_status: None,
worktree_status: Some(worktree_status),
})
}
fn git_status_to_proto(status: GitFileStatus) -> i32 {
match status {
fn status_pair_to_proto(status: GitStatusPair) -> i32 {
match status.combined() {
GitFileStatus::Added => proto::GitStatus::Added as i32,
GitFileStatus::Modified => proto::GitStatus::Modified as i32,
GitFileStatus::Conflict => proto::GitStatus::Conflict as i32,

View file

@ -2179,7 +2179,7 @@ async fn test_rename_work_directory(cx: &mut TestAppContext) {
cx.read(|cx| {
let tree = tree.read(cx);
let repo = tree.repositories().next().unwrap();
let repo = tree.repositories().iter().next().unwrap();
assert_eq!(repo.path.as_ref(), Path::new("projects/project1"));
assert_eq!(
tree.status_for_file(Path::new("projects/project1/a")),
@ -2200,7 +2200,7 @@ async fn test_rename_work_directory(cx: &mut TestAppContext) {
cx.read(|cx| {
let tree = tree.read(cx);
let repo = tree.repositories().next().unwrap();
let repo = tree.repositories().iter().next().unwrap();
assert_eq!(repo.path.as_ref(), Path::new("projects/project2"));
assert_eq!(
tree.status_for_file(Path::new("projects/project2/a")),
@ -2380,8 +2380,8 @@ async fn test_file_status(cx: &mut TestAppContext) {
// Check that the right git state is observed on startup
tree.read_with(cx, |tree, _cx| {
let snapshot = tree.snapshot();
assert_eq!(snapshot.repositories().count(), 1);
let repo_entry = snapshot.repositories().next().unwrap();
assert_eq!(snapshot.repositories().iter().count(), 1);
let repo_entry = snapshot.repositories().iter().next().unwrap();
assert_eq!(repo_entry.path.as_ref(), Path::new("project"));
assert!(repo_entry.location_in_repo.is_none());
@ -2554,16 +2554,16 @@ async fn test_git_repository_status(cx: &mut TestAppContext) {
// Check that the right git state is observed on startup
tree.read_with(cx, |tree, _cx| {
let snapshot = tree.snapshot();
let repo = snapshot.repositories().next().unwrap();
let repo = snapshot.repositories().iter().next().unwrap();
let entries = repo.status().collect::<Vec<_>>();
assert_eq!(entries.len(), 3);
assert_eq!(entries[0].repo_path.as_ref(), Path::new("a.txt"));
assert_eq!(entries[0].status, GitFileStatus::Modified);
assert_eq!(entries[0].worktree_status(), Some(GitFileStatus::Modified));
assert_eq!(entries[1].repo_path.as_ref(), Path::new("b.txt"));
assert_eq!(entries[1].status, GitFileStatus::Untracked);
assert_eq!(entries[1].worktree_status(), Some(GitFileStatus::Untracked));
assert_eq!(entries[2].repo_path.as_ref(), Path::new("d.txt"));
assert_eq!(entries[2].status, GitFileStatus::Deleted);
assert_eq!(entries[2].worktree_status(), Some(GitFileStatus::Deleted));
});
std::fs::write(work_dir.join("c.txt"), "some changes").unwrap();
@ -2576,19 +2576,19 @@ async fn test_git_repository_status(cx: &mut TestAppContext) {
tree.read_with(cx, |tree, _cx| {
let snapshot = tree.snapshot();
let repository = snapshot.repositories().next().unwrap();
let repository = snapshot.repositories().iter().next().unwrap();
let entries = repository.status().collect::<Vec<_>>();
std::assert_eq!(entries.len(), 4, "entries: {entries:?}");
assert_eq!(entries[0].repo_path.as_ref(), Path::new("a.txt"));
assert_eq!(entries[0].status, GitFileStatus::Modified);
assert_eq!(entries[0].worktree_status(), Some(GitFileStatus::Modified));
assert_eq!(entries[1].repo_path.as_ref(), Path::new("b.txt"));
assert_eq!(entries[1].status, GitFileStatus::Untracked);
assert_eq!(entries[1].worktree_status(), Some(GitFileStatus::Untracked));
// Status updated
assert_eq!(entries[2].repo_path.as_ref(), Path::new("c.txt"));
assert_eq!(entries[2].status, GitFileStatus::Modified);
assert_eq!(entries[2].worktree_status(), Some(GitFileStatus::Modified));
assert_eq!(entries[3].repo_path.as_ref(), Path::new("d.txt"));
assert_eq!(entries[3].status, GitFileStatus::Deleted);
assert_eq!(entries[3].worktree_status(), Some(GitFileStatus::Deleted));
});
git_add("a.txt", &repo);
@ -2609,7 +2609,7 @@ async fn test_git_repository_status(cx: &mut TestAppContext) {
tree.read_with(cx, |tree, _cx| {
let snapshot = tree.snapshot();
let repo = snapshot.repositories().next().unwrap();
let repo = snapshot.repositories().iter().next().unwrap();
let entries = repo.status().collect::<Vec<_>>();
// Deleting an untracked entry, b.txt, should leave no status
@ -2621,7 +2621,7 @@ async fn test_git_repository_status(cx: &mut TestAppContext) {
&entries
);
assert_eq!(entries[0].repo_path.as_ref(), Path::new("a.txt"));
assert_eq!(entries[0].status, GitFileStatus::Deleted);
assert_eq!(entries[0].worktree_status(), Some(GitFileStatus::Deleted));
});
}
@ -2676,8 +2676,8 @@ async fn test_repository_subfolder_git_status(cx: &mut TestAppContext) {
// Ensure that the git status is loaded correctly
tree.read_with(cx, |tree, _cx| {
let snapshot = tree.snapshot();
assert_eq!(snapshot.repositories().count(), 1);
let repo = snapshot.repositories().next().unwrap();
assert_eq!(snapshot.repositories().iter().count(), 1);
let repo = snapshot.repositories().iter().next().unwrap();
// Path is blank because the working directory of
// the git repository is located at the root of the project
assert_eq!(repo.path.as_ref(), Path::new(""));
@ -2707,7 +2707,7 @@ async fn test_repository_subfolder_git_status(cx: &mut TestAppContext) {
tree.read_with(cx, |tree, _cx| {
let snapshot = tree.snapshot();
assert!(snapshot.repositories().next().is_some());
assert!(snapshot.repositories().iter().next().is_some());
assert_eq!(snapshot.status_for_file("c.txt"), None);
assert_eq!(snapshot.status_for_file("d/e.txt"), None);