From bf8b83c93ac4fbc49e1b100a0dd424588a2013ad Mon Sep 17 00:00:00 2001 From: Mikayla Date: Wed, 18 Dec 2024 17:38:42 -0800 Subject: [PATCH] Fix several bugs in how statuses are computed for multiple sub repositories co-authored-by: Cole --- crates/worktree/src/worktree.rs | 46 ++++++++++++++++---- crates/worktree/src/worktree_tests.rs | 61 +++++++++++++++++++++++++-- 2 files changed, 94 insertions(+), 13 deletions(-) diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 6322350a5a43b..102ee60f8ab81 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -3706,7 +3706,10 @@ impl<'a, 'b> SeekTarget<'a, GitEntrySummary, (TraversalProgress<'a>, GitStatuses struct AllStatusesCursor<'a, I> { repositories: I, - current_cursor: Option, GitStatuses)>>, + current_cursor: Option<( + RepositoryWorkDirectory, + Cursor<'a, GitEntry, (TraversalProgress<'a>, GitStatuses)>, + )>, statuses_so_far: GitStatuses, } @@ -3715,33 +3718,58 @@ where I: FusedIterator + Iterator, { fn seek_forward(&mut self, target: &GitEntryTraversalTarget<'_>) { + let mut target_was_in_last_repo = false; loop { - let cursor = match &mut self.current_cursor { + let (work_dir, cursor) = match &mut self.current_cursor { Some(cursor) => cursor, None => { - let Some((_, entry)) = self.repositories.next() else { + let Some((work_dir, entry)) = self.repositories.next() else { break; }; - self.current_cursor = Some( + self.current_cursor = Some(( + work_dir.clone(), entry .git_entries_by_path .cursor::<(TraversalProgress<'_>, GitStatuses)>(&()), - ); + )); self.current_cursor.as_mut().unwrap() } }; - cursor.seek_forward(target, Bias::Left, &()); - if cursor.item().is_some() { - break; + + let path = match target { + GitEntryTraversalTarget::PathSuccessor(path) => path, + GitEntryTraversalTarget::Path(path) => path, + }; + if let Some(relative_path) = path.strip_prefix(&work_dir.0).ok() { + target_was_in_last_repo = true; + + let new_target = match target { + GitEntryTraversalTarget::PathSuccessor(_) => { + GitEntryTraversalTarget::PathSuccessor(relative_path.as_ref()) + } + GitEntryTraversalTarget::Path(_) => { + GitEntryTraversalTarget::Path(relative_path.as_ref()) + } + }; + + cursor.seek_forward(&new_target, Bias::Left, &()); + if cursor.item().is_some() { + break; + } + } else { + if target_was_in_last_repo { + break; + } } + self.statuses_so_far += cursor.start().1; self.current_cursor = None; } } fn start(&self) -> GitStatuses { - if let Some(cursor) = self.current_cursor.as_ref() { + if let Some((_, cursor)) = self.current_cursor.as_ref() { cursor.start().1 + self.statuses_so_far } else { self.statuses_so_far diff --git a/crates/worktree/src/worktree_tests.rs b/crates/worktree/src/worktree_tests.rs index f4288b25037e8..c6dd6d01dc975 100644 --- a/crates/worktree/src/worktree_tests.rs +++ b/crates/worktree/src/worktree_tests.rs @@ -2842,7 +2842,7 @@ async fn test_propagate_git_statuses(cx: &mut TestAppContext) { } #[gpui::test] -async fn test_propagate_statuses_repos_under_project(cx: &mut TestAppContext) { +async fn test_propagate_statuses_for_repos_under_project(cx: &mut TestAppContext) { init_test(cx); let fs = FakeFs::new(cx.background_executor.clone()); fs.insert_tree( @@ -2850,11 +2850,18 @@ async fn test_propagate_statuses_repos_under_project(cx: &mut TestAppContext) { json!({ "x": { ".git": {}, - "x1.txt": "foo" + "x1.txt": "foo", + "x2.txt": "bar" }, "y": { ".git": {}, - "y1.txt": "bar" + "y1.txt": "baz", + "y2.txt": "qux" + }, + "z": { + ".git": {}, + "z1.txt": "quux", + "z2.txt": "quuux" } }), ) @@ -2868,6 +2875,14 @@ async fn test_propagate_statuses_repos_under_project(cx: &mut TestAppContext) { Path::new("/root/y/.git"), &[(Path::new("y1.txt"), GitFileStatus::Conflict)], ); + fs.set_status_for_repo_via_git_operation( + Path::new("/root/y/.git"), + &[(Path::new("y2.txt"), GitFileStatus::Modified)], + ); + fs.set_status_for_repo_via_git_operation( + Path::new("/root/z/.git"), + &[(Path::new("z2.txt"), GitFileStatus::Modified)], + ); let tree = Worktree::local( Path::new("/root"), @@ -2888,11 +2903,49 @@ async fn test_propagate_statuses_repos_under_project(cx: &mut TestAppContext) { check_propagated_statuses( &snapshot, &[ - // (Path::new(""), None), // /root, doesn't have a git repository in it and so should not have a git repository status (Path::new("x"), Some(GitFileStatus::Added)), (Path::new("x/x1.txt"), Some(GitFileStatus::Added)), + ], + ); + + check_propagated_statuses( + &snapshot, + &[ + (Path::new("y"), Some(GitFileStatus::Conflict)), + (Path::new("y/y1.txt"), Some(GitFileStatus::Conflict)), + ], + ); + + check_propagated_statuses( + &snapshot, + &[ + (Path::new("z"), Some(GitFileStatus::Modified)), + (Path::new("z/z2.txt"), Some(GitFileStatus::Modified)), + ], + ); + + check_propagated_statuses( + &snapshot, + &[ + (Path::new(""), None), // /root doesn't have a git repository in it and so should not have a git repository status + (Path::new("x"), Some(GitFileStatus::Added)), + (Path::new("x/x1.txt"), Some(GitFileStatus::Added)), + ], + ); + + check_propagated_statuses( + &snapshot, + &[ + (Path::new(""), None), + (Path::new("x"), Some(GitFileStatus::Added)), + (Path::new("x/x1.txt"), Some(GitFileStatus::Added)), + (Path::new("x/x2.txt"), None), (Path::new("y"), Some(GitFileStatus::Conflict)), (Path::new("y/y1.txt"), Some(GitFileStatus::Conflict)), + (Path::new("y/y2.txt"), Some(GitFileStatus::Modified)), + (Path::new("z"), Some(GitFileStatus::Modified)), + (Path::new("z/z1.txt"), None), + (Path::new("z/z2.txt"), Some(GitFileStatus::Modified)), ], ); }