From 72cb786e5bb42b8a7764ed787eceef64c1e416bc Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Fri, 10 Jan 2025 11:50:57 -0500 Subject: [PATCH] Handle more worktree update cases Co-Authored-By: Cole Miller <53574922+cole-miller@users.noreply.github.com> --- .../random_project_collaboration_tests.rs | 2 +- crates/editor/src/git/project_diff.rs | 6 +- crates/git_ui/src/git_panel.rs | 138 +++++++++++++++--- crates/git_ui/src/git_ui.rs | 66 ++++++--- crates/worktree/src/worktree.rs | 6 +- crates/worktree/src/worktree_tests.rs | 20 +-- 6 files changed, 184 insertions(+), 54 deletions(-) diff --git a/crates/collab/src/tests/random_project_collaboration_tests.rs b/crates/collab/src/tests/random_project_collaboration_tests.rs index 3dd16b9b7a..6a3c5c131a 100644 --- a/crates/collab/src/tests/random_project_collaboration_tests.rs +++ b/crates/collab/src/tests/random_project_collaboration_tests.rs @@ -1222,7 +1222,7 @@ impl RandomizedTest for ProjectCollaborationTest { id, guest_project.remote_id(), ); - assert_eq!(guest_snapshot.repositories().collect::>(), host_snapshot.repositories().collect::>(), + assert_eq!(guest_snapshot.repositories().iter().collect::>(), host_snapshot.repositories().iter().collect::>(), "{} has different repositories than the host for worktree {:?} and project {:?}", client.username, host_snapshot.abs_path(), diff --git a/crates/editor/src/git/project_diff.rs b/crates/editor/src/git/project_diff.rs index 8464f249c8..4acadad41e 100644 --- a/crates/editor/src/git/project_diff.rs +++ b/crates/editor/src/git/project_diff.rs @@ -197,12 +197,10 @@ impl ProjectDiffEditor { let snapshot = worktree.read(cx).snapshot(); let applicable_entries = snapshot .repositories() + .iter() .flat_map(|entry| { entry.status().map(|git_entry| { - ( - git_entry.combined_status(), - entry.join(git_entry.repo_path), - ) + (git_entry.combined_status(), entry.join(git_entry.repo_path)) }) }) .filter_map(|(status, path)| { diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 822ca730fe..dec5491a34 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -1,4 +1,4 @@ -use crate::first_worktree_repository; +use crate::{first_repository_in_project, first_worktree_repository}; use crate::{ git_status_icon, settings::GitPanelSettings, CommitAllChanges, CommitChanges, GitState, GitViewMode, RevertAll, StageAll, ToggleStaged, UnstageAll, @@ -129,27 +129,97 @@ impl GitPanel { }) .detach(); cx.subscribe(&project, move |this, project, event, cx| { + use project::Event; + + let first_worktree_id = + project.read(cx).worktrees(cx).next().and_then(|worktree| { + let snapshot = worktree.read(cx).snapshot(); + Some(snapshot.id().clone()) + }); + let first_repo_in_project = first_repository_in_project(&project, cx); + // TODO: Don't get another git_state here // was running into a borrow issue let git_state = GitState::get_global(cx); match event { - project::Event::WorktreeRemoved(_) | project::Event::WorktreeOrderChanged => { - // TODO pick a new worktree if ours was removed - this.schedule_update(); + project::Event::WorktreeRemoved(id) => { + git_state.update(cx, |state, _| { + state.all_repositories.remove(id); + let Some((worktree_id, _, _)) = state.active_repository.as_ref() else { + return; + }; + if worktree_id == id { + state.active_repository = first_repo_in_project; + this.schedule_update(); + } + }); } - project::Event::WorktreeUpdatedEntries(id, _) - | project::Event::WorktreeAdded(id) - | project::Event::WorktreeUpdatedGitRepositories(id) => { - let git_state = git_state.clone(); - if let Some((first_repo, git_repo)) = - first_worktree_repository(&project, *id, cx) - { - git_state.update(cx, |state, _| { - state.activate_repository(first_repo, git_repo) - }); - } - this.schedule_update(); + project::Event::WorktreeOrderChanged => { + // activate the new first worktree if the first was moved + let Some(first_id) = first_worktree_id else { + return; + }; + git_state.update(cx, |state, _| { + if !state + .active_repository + .as_ref() + .is_some_and(|(id, _, _)| id == &first_id) + { + state.active_repository = first_repo_in_project; + this.schedule_update(); + } + }); + } + Event::WorktreeAdded(id) => { + git_state.update(cx, |state, cx| { + let Some(worktree) = project.read(cx).worktree_for_id(*id, cx) else { + return; + }; + let snapshot = worktree.read(cx).snapshot(); + state + .all_repositories + .insert(*id, snapshot.repositories().clone()); + }); + let Some(first_id) = first_worktree_id else { + return; + }; + git_state.update(cx, |state, _| { + if !state + .active_repository + .as_ref() + .is_some_and(|(id, _, _)| id == &first_id) + { + state.active_repository = first_repo_in_project; + this.schedule_update(); + } + }); + } + project::Event::WorktreeUpdatedEntries(id, _) => { + git_state.update(cx, |state, _| { + if state + .active_repository + .as_ref() + .is_some_and(|(active_id, _, _)| active_id == id) + { + this.schedule_update(); + } + }); + } + project::Event::WorktreeUpdatedGitRepositories(_) => { + let Some(first) = first_repo_in_project else { + return; + }; + git_state.update(cx, |state, _| { + if !state + .active_repository + .as_ref() + .is_some_and(|(id, _, _)| id == &first.0) + { + state.active_repository = Some(first); + } + this.schedule_update(); + }); } project::Event::Closed => { this.reveal_in_editor = Task::ready(()); @@ -222,7 +292,7 @@ impl GitPanel { if let Some((repo, git_repo)) = first_worktree_repository(&project, snapshot.id(), cx) { - state.activate_repository(repo, git_repo); + state.activate_repository(snapshot.id().clone(), repo, git_repo); } }); @@ -503,6 +573,38 @@ impl GitPanel { fn open_entry(&self, entry: &GitListEntry) { // TODO: Open entry or entry's changes. println!("Open {} triggered!", entry.repo_path); + + // cx.emit(project_panel::Event::OpenedEntry { + // entry_id, + // focus_opened_item, + // allow_preview, + // }); + // + // workspace + // .open_path_preview( + // ProjectPath { + // worktree_id, + // path: file_path.clone(), + // }, + // None, + // focus_opened_item, + // allow_preview, + // cx, + // ) + // .detach_and_prompt_err("Failed to open file", cx, move |e, _| { + // match e.error_code() { + // ErrorCode::Disconnected => if is_via_ssh { + // Some("Disconnected from SSH host".to_string()) + // } else { + // Some("Disconnected from remote project".to_string()) + // }, + // ErrorCode::UnsharedItem => Some(format!( + // "{} is not shared by the host. This could be because it has been marked as `private`", + // file_path.display() + // )), + // _ => None, + // } + // }); } fn stage_all(&mut self, _: &StageAll, cx: &mut ViewContext) { @@ -599,7 +701,7 @@ impl GitPanel { self.visible_entries.clear(); - let Some((repo, _)) = git_state.active_repository().as_ref() else { + let Some((_, repo, _)) = git_state.active_repository().as_ref() else { // Just clear entries if no repository is active. cx.notify(); return; diff --git a/crates/git_ui/src/git_ui.rs b/crates/git_ui/src/git_ui.rs index b7ecf1b7f9..cf9effc111 100644 --- a/crates/git_ui/src/git_ui.rs +++ b/crates/git_ui/src/git_ui.rs @@ -1,9 +1,7 @@ use ::settings::Settings; +use collections::HashMap; use futures::{future::FusedFuture, select, FutureExt}; -use git::{ - repository::{GitFileStatus, GitRepository, RepoPath}, - status::GitStatusPair, -}; +use git::repository::{GitFileStatus, GitRepository, RepoPath}; use gpui::{actions, AppContext, Context, Global, Hsla, Model, ModelContext}; use project::{Project, WorktreeId}; use settings::GitPanelSettings; @@ -13,7 +11,8 @@ use std::{ sync::Arc, time::Duration, }; -use ui::{Color, Icon, IconName, IntoElement, SharedString, ToggleState}; +use sum_tree::SumTree; +use ui::{Color, Icon, IconName, IntoElement, SharedString}; use worktree::RepositoryEntry; pub mod git_panel; @@ -68,10 +67,12 @@ pub struct GitState { /// When a git repository is selected, this is used to track which repository's changes /// are currently being viewed or modified in the UI. - active_repository: Option<(RepositoryEntry, Arc)>, + active_repository: Option<(WorktreeId, RepositoryEntry, Arc)>, updater_tx: mpsc::Sender<(Arc, Vec, StatusAction)>, + all_repositories: HashMap>, + list_view_mode: GitViewMode, } @@ -163,6 +164,7 @@ impl GitState { active_repository: None, updater_tx, list_view_mode: GitViewMode::default(), + all_repositories: HashMap::default(), } } @@ -172,13 +174,16 @@ impl GitState { pub fn activate_repository( &mut self, + worktree_id: WorktreeId, active_repository: RepositoryEntry, git_repo: Arc, ) { - self.active_repository = Some((active_repository, git_repo)); + self.active_repository = Some((worktree_id, active_repository, git_repo)); } - pub fn active_repository(&self) -> Option<&(RepositoryEntry, Arc)> { + pub fn active_repository( + &self, + ) -> Option<&(WorktreeId, RepositoryEntry, Arc)> { self.active_repository.as_ref() } @@ -191,7 +196,7 @@ impl GitState { } pub fn stage_entry(&mut self, repo_path: RepoPath) { - if let Some((_, git_repo)) = self.active_repository.as_ref() { + if let Some((_, _, git_repo)) = self.active_repository.as_ref() { let _ = self .updater_tx .send((git_repo.clone(), vec![repo_path], StatusAction::Stage)); @@ -199,7 +204,7 @@ impl GitState { } pub fn unstage_entry(&mut self, repo_path: RepoPath) { - if let Some((_, git_repo)) = self.active_repository.as_ref() { + if let Some((_, _, git_repo)) = self.active_repository.as_ref() { let _ = self.updater_tx .send((git_repo.clone(), vec![repo_path], StatusAction::Unstage)); @@ -207,7 +212,7 @@ impl GitState { } pub fn stage_entries(&mut self, entries: Vec) { - if let Some((_, git_repo)) = self.active_repository.as_ref() { + if let Some((_, _, git_repo)) = self.active_repository.as_ref() { let _ = self .updater_tx .send((git_repo.clone(), entries, StatusAction::Stage)); @@ -215,7 +220,7 @@ impl GitState { } fn act_on_all(&mut self, action: StatusAction) { - if let Some((active_repository, git_repo)) = self.active_repository.as_ref() { + if let Some((_, active_repository, git_repo)) = self.active_repository.as_ref() { let _ = self.updater_tx.send(( git_repo.clone(), active_repository @@ -241,12 +246,37 @@ pub fn first_worktree_repository( worktree_id: WorktreeId, cx: &mut AppContext, ) -> Option<(RepositoryEntry, Arc)> { - let worktree = project.read(cx).worktree_for_id(worktree_id, cx)?; - let snapshot = worktree.read(cx).snapshot(); - let repo = snapshot.repositories().next()?.clone(); - let local = worktree.read(cx).as_local()?; - let git_repo = local.get_local_repo(&repo)?.repo().clone(); - Some((repo, git_repo)) + project + .read(cx) + .worktree_for_id(worktree_id, cx) + .and_then(|worktree| { + let snapshot = worktree.read(cx).snapshot(); + let repo = snapshot.repositories().iter().next()?.clone(); + let git_repo = worktree + .read(cx) + .as_local()? + .get_local_repo(&repo)? + .repo() + .clone(); + Some((repo, git_repo)) + }) +} + +pub fn first_repository_in_project( + project: &Model, + cx: &mut AppContext, +) -> Option<(WorktreeId, RepositoryEntry, Arc)> { + project.read(cx).worktrees(cx).next().and_then(|worktree| { + let snapshot = worktree.read(cx).snapshot(); + let repo = snapshot.repositories().iter().next()?.clone(); + let git_repo = worktree + .read(cx) + .as_local()? + .get_local_repo(&repo)? + .repo() + .clone(); + Some((snapshot.id(), repo, git_repo)) + }) } const ADDED_COLOR: Hsla = Hsla { diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 32e7125ad5..7b79815a25 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -2581,8 +2581,8 @@ impl Snapshot { .map(|repo| repo.status().collect()) } - pub fn repositories(&self) -> impl Iterator { - self.repositories.iter() + pub fn repositories(&self) -> &SumTree { + &self.repositories } /// Get the repository whose work directory corresponds to the given path. @@ -2616,7 +2616,7 @@ impl Snapshot { entries: impl 'a + Iterator, ) -> impl 'a + Iterator)> { 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) { diff --git a/crates/worktree/src/worktree_tests.rs b/crates/worktree/src/worktree_tests.rs index 7b65402c8b..5f8144347d 100644 --- a/crates/worktree/src/worktree_tests.rs +++ b/crates/worktree/src/worktree_tests.rs @@ -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,7 +2554,7 @@ 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::>(); assert_eq!(entries.len(), 3); @@ -2576,7 +2576,7 @@ 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::>(); std::assert_eq!(entries.len(), 4, "entries: {entries:?}"); @@ -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::>(); // Deleting an untracked entry, b.txt, should leave no status @@ -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);