From d83c316e6d37130ac353e552a7430e432eaa033c Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Thu, 6 Feb 2025 22:04:02 -0500 Subject: [PATCH] Fix Project Panel `select_next_git_entry` action (#24217) ## Context I noticed that the project panel `select_next_git_entry` wasn't behaving correctly. Turns out it was searching in reverse, which caused the action to select itself or the last entry. This PR corrects the behavior and adds a unit test that should stop regressions. Note: Since select next/prev git entry uses the same function as select next/prev diagnostic, the test partially works for that as well. Release Notes: - Fix bug where `select_next_git_entry` project panel action would only select a previous entry or the currently selected entry. --------- Co-authored-by: Mikayla Maki --- Cargo.lock | 2 +- crates/project_panel/src/project_panel.rs | 282 +++++++++++++++++++++- 2 files changed, 282 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 205ffc6693..eae4c7bea9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7184,7 +7184,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc2f4eb4bc735547cfed7c0a4922cbd04a4655978c09b54f1f7b228750664c34" dependencies = [ "cfg-if", - "windows-targets 0.52.6", + "windows-targets 0.48.5", ] [[package]] diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index b2c0ab8923..c4af13f6cb 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1866,7 +1866,7 @@ impl ProjectPanel { ) { let selection = self.find_entry( self.selection.as_ref(), - true, + false, |entry, worktree_id| { (self.selection.is_none() || self.selection.is_some_and(|selection| { @@ -6726,6 +6726,286 @@ mod tests { ); } + #[gpui::test] + async fn test_select_git_entry(cx: &mut gpui::TestAppContext) { + use git::status::{FileStatus, StatusCode, TrackedStatus}; + use std::path::Path; + + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root", + json!({ + "tree1": { + ".git": {}, + "dir1": { + "modified1.txt": "", + "unmodified1.txt": "", + "modified2.txt": "", + }, + "dir2": { + "modified3.txt": "", + "unmodified2.txt": "", + }, + "modified4.txt": "", + "unmodified3.txt": "", + }, + "tree2": { + ".git": {}, + "dir3": { + "modified5.txt": "", + "unmodified4.txt": "", + }, + "modified6.txt": "", + "unmodified5.txt": "", + } + }), + ) + .await; + + // Mark files as git modified + let tree1_modified_files = [ + "dir1/modified1.txt", + "dir1/modified2.txt", + "modified4.txt", + "dir2/modified3.txt", + ]; + + let tree2_modified_files = ["dir3/modified5.txt", "modified6.txt"]; + + let root1_dot_git = Path::new("/root/tree1/.git"); + let root2_dot_git = Path::new("/root/tree2/.git"); + let set_value = FileStatus::Tracked(TrackedStatus { + index_status: StatusCode::Modified, + worktree_status: StatusCode::Modified, + }); + + fs.with_git_state(&root1_dot_git, true, |git_repo_state| { + for file_path in tree1_modified_files { + git_repo_state.statuses.insert(file_path.into(), set_value); + } + }); + + fs.with_git_state(&root2_dot_git, true, |git_repo_state| { + for file_path in tree2_modified_files { + git_repo_state.statuses.insert(file_path.into(), set_value); + } + }); + + let project = Project::test( + fs.clone(), + ["/root/tree1".as_ref(), "/root/tree2".as_ref()], + cx, + ) + .await; + let workspace = + cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let panel = workspace.update(cx, ProjectPanel::new).unwrap(); + + // Check initial state + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v tree1", + " > .git", + " > dir1", + " > dir2", + " modified4.txt", + " unmodified3.txt", + "v tree2", + " > .git", + " > dir3", + " modified6.txt", + " unmodified5.txt" + ], + ); + + // Test selecting next modified entry + panel.update_in(cx, |panel, window, cx| { + panel.select_next_git_entry(&SelectNextGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 0..6, cx), + &[ + "v tree1", + " > .git", + " v dir1", + " modified1.txt <== selected", + " modified2.txt", + " unmodified1.txt", + ], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_next_git_entry(&SelectNextGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 0..6, cx), + &[ + "v tree1", + " > .git", + " v dir1", + " modified1.txt", + " modified2.txt <== selected", + " unmodified1.txt", + ], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_next_git_entry(&SelectNextGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 6..9, cx), + &[ + " v dir2", + " modified3.txt <== selected", + " unmodified2.txt", + ], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_next_git_entry(&SelectNextGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 9..11, cx), + &[" modified4.txt <== selected", " unmodified3.txt",], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_next_git_entry(&SelectNextGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 13..16, cx), + &[ + " v dir3", + " modified5.txt <== selected", + " unmodified4.txt", + ], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_next_git_entry(&SelectNextGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 16..18, cx), + &[" modified6.txt <== selected", " unmodified5.txt",], + ); + + // Wraps around to first modified file + panel.update_in(cx, |panel, window, cx| { + panel.select_next_git_entry(&SelectNextGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 0..18, cx), + &[ + "v tree1", + " > .git", + " v dir1", + " modified1.txt <== selected", + " modified2.txt", + " unmodified1.txt", + " v dir2", + " modified3.txt", + " unmodified2.txt", + " modified4.txt", + " unmodified3.txt", + "v tree2", + " > .git", + " v dir3", + " modified5.txt", + " unmodified4.txt", + " modified6.txt", + " unmodified5.txt", + ], + ); + + // Wraps around again to last modified file + panel.update_in(cx, |panel, window, cx| { + panel.select_prev_git_entry(&SelectPrevGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 16..18, cx), + &[" modified6.txt <== selected", " unmodified5.txt",], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_prev_git_entry(&SelectPrevGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 13..16, cx), + &[ + " v dir3", + " modified5.txt <== selected", + " unmodified4.txt", + ], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_prev_git_entry(&SelectPrevGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 9..11, cx), + &[" modified4.txt <== selected", " unmodified3.txt",], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_prev_git_entry(&SelectPrevGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 6..9, cx), + &[ + " v dir2", + " modified3.txt <== selected", + " unmodified2.txt", + ], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_prev_git_entry(&SelectPrevGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 0..6, cx), + &[ + "v tree1", + " > .git", + " v dir1", + " modified1.txt", + " modified2.txt <== selected", + " unmodified1.txt", + ], + ); + + panel.update_in(cx, |panel, window, cx| { + panel.select_prev_git_entry(&SelectPrevGitEntry, window, cx); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 0..6, cx), + &[ + "v tree1", + " > .git", + " v dir1", + " modified1.txt <== selected", + " modified2.txt", + " unmodified1.txt", + ], + ); + } + #[gpui::test] async fn test_select_directory(cx: &mut gpui::TestAppContext) { init_test_with_editor(cx);