From 96c72c252d24565541cfefe220434ae95bc64aec Mon Sep 17 00:00:00 2001 From: Mayank Verma Date: Wed, 18 Feb 2026 00:38:28 +0530 Subject: [PATCH] git_ui: Fix tree view next selection out of bounds (#49283) Closes #49259 Release Notes: - This change ensures that when the last visible collapsed directory is selected, the selection remains on that directory. --- crates/git_ui/src/git_panel.rs | 161 +++++++++++++++++++++++++++++++-- 1 file changed, 155 insertions(+), 6 deletions(-) diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 9e4b125c3766af11894ec952808ffa48078d2362..4856af8594f110cb060917db1e1e38c1b367da8e 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -1112,12 +1112,14 @@ impl GitPanel { return; }; - if selected_entry == item_count - 1 { - return; - } - let new_index = match &self.view_mode { - GitPanelViewMode::Flat => selected_entry.saturating_add(1), + GitPanelViewMode::Flat => { + if selected_entry >= item_count.saturating_sub(1) { + return; + } + + selected_entry.saturating_add(1) + } GitPanelViewMode::Tree(state) => { let Some(current_logical_index) = state .logical_indices @@ -1127,7 +1129,15 @@ impl GitPanel { return; }; - state.logical_indices[current_logical_index.saturating_add(1)] + let Some(new_index) = state + .logical_indices + .get(current_logical_index.saturating_add(1)) + .copied() + else { + return; + }; + + new_index } }; @@ -7244,6 +7254,145 @@ mod tests { }); } + #[gpui::test] + async fn test_tree_view_select_next_at_last_visible_collapsed_directory( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.background_executor.clone()); + fs.insert_tree( + path!("/project"), + json!({ + ".git": {}, + "bar": { + "bar1.py": "print('bar1')", + "bar2.py": "print('bar2')", + }, + "foo": { + "foo1.py": "print('foo1')", + "foo2.py": "print('foo2')", + }, + "foobar.py": "print('foobar')", + }), + ) + .await; + + fs.set_status_for_repo( + path!("/project/.git").as_ref(), + &[ + ("bar/bar1.py", StatusCode::Modified.worktree()), + ("bar/bar2.py", StatusCode::Modified.worktree()), + ("foo/foo1.py", StatusCode::Modified.worktree()), + ("foo/foo2.py", StatusCode::Modified.worktree()), + ("foobar.py", FileStatus::Untracked), + ], + ); + + let project = Project::test(fs.clone(), [Path::new(path!("/project"))], cx).await; + let window_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = window_handle + .read_with(cx, |mw, _| mw.workspace().clone()) + .unwrap(); + let cx = &mut VisualTestContext::from_window(window_handle.into(), cx); + + cx.read(|cx| { + project + .read(cx) + .worktrees(cx) + .next() + .unwrap() + .read(cx) + .as_local() + .unwrap() + .scan_complete() + }) + .await; + + cx.executor().run_until_parked(); + cx.update(|_window, cx| { + SettingsStore::update_global(cx, |store, cx| { + store.update_user_settings(cx, |settings| { + settings.git_panel.get_or_insert_default().tree_view = Some(true); + }) + }); + }); + + let panel = workspace.update_in(cx, GitPanel::new); + let handle = cx.update_window_entity(&panel, |panel, _, _| { + std::mem::replace(&mut panel.update_visible_entries_task, Task::ready(())) + }); + + cx.executor().advance_clock(2 * UPDATE_DEBOUNCE); + handle.await; + + let foo_key = panel.read_with(cx, |panel, _| { + panel + .entries + .iter() + .find_map(|entry| match entry { + GitListEntry::Directory(dir) if dir.key.path == repo_path("foo") => { + Some(dir.key.clone()) + } + _ => None, + }) + .expect("foo directory should exist in tree view") + }); + + panel.update_in(cx, |panel, window, cx| { + panel.toggle_directory(&foo_key, window, cx); + }); + + let foo_idx = panel.read_with(cx, |panel, _| { + let state = panel + .view_mode + .tree_state() + .expect("tree view state should exist"); + assert_eq!(state.expanded_dirs.get(&foo_key).copied(), Some(false)); + + let foo_idx = panel + .entries + .iter() + .enumerate() + .find_map(|(index, entry)| match entry { + GitListEntry::Directory(dir) if dir.key.path == repo_path("foo") => Some(index), + _ => None, + }) + .expect("foo directory should exist in tree view"); + + let foo_logical_idx = state + .logical_indices + .iter() + .position(|&index| index == foo_idx) + .expect("foo directory should be visible"); + let next_logical_idx = state.logical_indices[foo_logical_idx + 1]; + assert!(matches!( + panel.entries.get(next_logical_idx), + Some(GitListEntry::Header(GitHeaderEntry { + header: Section::New + })) + )); + + foo_idx + }); + + panel.update_in(cx, |panel, window, cx| { + panel.selected_entry = Some(foo_idx); + panel.select_next(&menu::SelectNext, window, cx); + }); + + panel.read_with(cx, |panel, _| { + let selected_idx = panel.selected_entry.expect("selection should be set"); + let selected_entry = panel + .entries + .get(selected_idx) + .and_then(|entry| entry.status_entry()) + .expect("selected entry should be a status entry"); + assert_eq!(selected_entry.repo_path, repo_path("foobar.py")); + }); + } + fn assert_entry_paths(entries: &[GitListEntry], expected_paths: &[Option<&str>]) { assert_eq!(entries.len(), expected_paths.len()); for (entry, expected_path) in entries.iter().zip(expected_paths) {