From 114c4621433eb948b87fa5f4f41df8af6601f66e Mon Sep 17 00:00:00 2001 From: tims <0xtimsb@gmail.com> Date: Fri, 22 Nov 2024 16:29:04 +0530 Subject: [PATCH] Maintain selection on file/dir deletion in project panel (#20577) Closes #20444 - Focus on next file/dir on deletion. - Focus on prev file/dir in case where it's last item in worktree. - Tested when multiple files/dirs are being deleted. Release Notes: - Maintain selection on file/dir deletion in project panel. --------- Co-authored-by: Kirill Bulatov --- crates/project_panel/src/project_panel.rs | 800 +++++++++++++++++++++- crates/util/src/paths.rs | 32 +- crates/workspace/src/pane.rs | 2 +- 3 files changed, 813 insertions(+), 21 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 5ad2c2d12e..c757924727 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -40,6 +40,7 @@ use serde::{Deserialize, Serialize}; use smallvec::SmallVec; use std::{ cell::OnceCell, + cmp, collections::HashSet, ffi::OsStr, ops::Range, @@ -53,7 +54,7 @@ use ui::{ IndentGuideColors, IndentGuideLayout, KeyBinding, Label, ListItem, Scrollbar, ScrollbarState, Tooltip, }; -use util::{maybe, ResultExt, TryFutureExt}; +use util::{maybe, paths::compare_paths, ResultExt, TryFutureExt}; use workspace::{ dock::{DockPosition, Panel, PanelEvent}, notifications::{DetachAndPromptErr, NotifyTaskExt}, @@ -550,7 +551,7 @@ impl ProjectPanel { .entry((project_path.worktree_id, path_buffer.clone())) .and_modify(|strongest_diagnostic_severity| { *strongest_diagnostic_severity = - std::cmp::min(*strongest_diagnostic_severity, diagnostic_severity); + cmp::min(*strongest_diagnostic_severity, diagnostic_severity); }) .or_insert(diagnostic_severity); } @@ -1184,15 +1185,15 @@ impl ProjectPanel { fn remove(&mut self, trash: bool, skip_prompt: bool, cx: &mut ViewContext<'_, ProjectPanel>) { maybe!({ - if self.marked_entries.is_empty() && self.selection.is_none() { + let items_to_delete = self.disjoint_entries_for_removal(cx); + if items_to_delete.is_empty() { return None; } let project = self.project.read(cx); - let items_to_delete = self.marked_entries(); let mut dirty_buffers = 0; let file_paths = items_to_delete - .into_iter() + .iter() .filter_map(|selection| { let project_path = project.path_for_entry(selection.entry_id, cx)?; dirty_buffers += @@ -1261,28 +1262,120 @@ impl ProjectPanel { } else { None }; - - cx.spawn(|this, mut cx| async move { + let next_selection = self.find_next_selection_after_deletion(items_to_delete, cx); + cx.spawn(|panel, mut cx| async move { if let Some(answer) = answer { if answer.await != Ok(0) { - return Result::<(), anyhow::Error>::Ok(()); + return anyhow::Ok(()); } } for (entry_id, _) in file_paths { - this.update(&mut cx, |this, cx| { - this.project - .update(cx, |project, cx| project.delete_entry(entry_id, trash, cx)) - .ok_or_else(|| anyhow!("no such entry")) - })?? - .await?; + panel + .update(&mut cx, |panel, cx| { + panel + .project + .update(cx, |project, cx| project.delete_entry(entry_id, trash, cx)) + .context("no such entry") + })?? + .await?; } - Result::<(), anyhow::Error>::Ok(()) + panel.update(&mut cx, |panel, cx| { + if let Some(next_selection) = next_selection { + panel.selection = Some(next_selection); + panel.autoscroll(cx); + } else { + panel.select_last(&SelectLast {}, cx); + } + })?; + Ok(()) }) .detach_and_log_err(cx); Some(()) }); } + fn find_next_selection_after_deletion( + &self, + sanitized_entries: BTreeSet, + cx: &mut ViewContext, + ) -> Option { + if sanitized_entries.is_empty() { + return None; + } + + let project = self.project.read(cx); + let (worktree_id, worktree) = sanitized_entries + .iter() + .map(|entry| entry.worktree_id) + .filter_map(|id| project.worktree_for_id(id, cx).map(|w| (id, w.read(cx)))) + .max_by(|(_, a), (_, b)| a.root_name().cmp(b.root_name()))?; + + let marked_entries_in_worktree = sanitized_entries + .iter() + .filter(|e| e.worktree_id == worktree_id) + .collect::>(); + let latest_entry = marked_entries_in_worktree + .iter() + .max_by(|a, b| { + match ( + worktree.entry_for_id(a.entry_id), + worktree.entry_for_id(b.entry_id), + ) { + (Some(a), Some(b)) => { + compare_paths((&a.path, a.is_file()), (&b.path, b.is_file())) + } + _ => cmp::Ordering::Equal, + } + }) + .and_then(|e| worktree.entry_for_id(e.entry_id))?; + + let parent_path = latest_entry.path.parent()?; + let parent_entry = worktree.entry_for_path(parent_path)?; + + // Remove all siblings that are being deleted except the last marked entry + let mut siblings: Vec = worktree + .snapshot() + .child_entries(parent_path) + .filter(|sibling| { + sibling.id == latest_entry.id + || !marked_entries_in_worktree.contains(&&SelectedEntry { + worktree_id, + entry_id: sibling.id, + }) + }) + .cloned() + .collect(); + + project::sort_worktree_entries(&mut siblings); + let sibling_entry_index = siblings + .iter() + .position(|sibling| sibling.id == latest_entry.id)?; + + if let Some(next_sibling) = sibling_entry_index + .checked_add(1) + .and_then(|i| siblings.get(i)) + { + return Some(SelectedEntry { + worktree_id, + entry_id: next_sibling.id, + }); + } + if let Some(prev_sibling) = sibling_entry_index + .checked_sub(1) + .and_then(|i| siblings.get(i)) + { + return Some(SelectedEntry { + worktree_id, + entry_id: prev_sibling.id, + }); + } + // No neighbour sibling found, fall back to parent + Some(SelectedEntry { + worktree_id, + entry_id: parent_entry.id, + }) + } + fn unfold_directory(&mut self, _: &UnfoldDirectory, cx: &mut ViewContext) { if let Some((worktree, entry)) = self.selected_entry(cx) { self.unfolded_dir_ids.insert(entry.id); @@ -1835,6 +1928,54 @@ impl ProjectPanel { None } + fn disjoint_entries_for_removal(&self, cx: &AppContext) -> BTreeSet { + let marked_entries = self.marked_entries(); + let mut sanitized_entries = BTreeSet::new(); + if marked_entries.is_empty() { + return sanitized_entries; + } + + let project = self.project.read(cx); + let marked_entries_by_worktree: HashMap> = marked_entries + .into_iter() + .filter(|entry| !project.entry_is_worktree_root(entry.entry_id, cx)) + .fold(HashMap::default(), |mut map, entry| { + map.entry(entry.worktree_id).or_default().push(entry); + map + }); + + for (worktree_id, marked_entries) in marked_entries_by_worktree { + if let Some(worktree) = project.worktree_for_id(worktree_id, cx) { + let worktree = worktree.read(cx); + let marked_dir_paths = marked_entries + .iter() + .filter_map(|entry| { + worktree.entry_for_id(entry.entry_id).and_then(|entry| { + if entry.is_dir() { + Some(entry.path.as_ref()) + } else { + None + } + }) + }) + .collect::>(); + + sanitized_entries.extend(marked_entries.into_iter().filter(|entry| { + let Some(entry_info) = worktree.entry_for_id(entry.entry_id) else { + return false; + }; + let entry_path = entry_info.path.as_ref(); + let inside_marked_dir = marked_dir_paths.iter().any(|&marked_dir_path| { + entry_path != marked_dir_path && entry_path.starts_with(marked_dir_path) + }); + !inside_marked_dir + })); + } + } + + sanitized_entries + } + // Returns list of entries that should be affected by an operation. // When currently selected entry is not marked, it's treated as the only marked entry. fn marked_entries(&self) -> BTreeSet { @@ -5080,14 +5221,13 @@ mod tests { &[ "v src", " v test", - " second.rs", + " second.rs <== selected", " third.rs" ], "Project panel should have no deleted file, no other file is selected in it" ); ensure_no_open_items_and_panes(&workspace, cx); - select_path(&panel, "src/test/second.rs", cx); panel.update(cx, |panel, cx| panel.open(&Open, cx)); cx.executor().run_until_parked(); assert_eq!( @@ -5121,7 +5261,7 @@ mod tests { submit_deletion_skipping_prompt(&panel, cx); assert_eq!( visible_entries_as_strings(&panel, 0..10, cx), - &["v src", " v test", " third.rs"], + &["v src", " v test", " third.rs <== selected"], "Project panel should have no deleted file, with one last file remaining" ); ensure_no_open_items_and_panes(&workspace, cx); @@ -5630,7 +5770,11 @@ mod tests { submit_deletion(&panel, cx); assert_eq!( visible_entries_as_strings(&panel, 0..10, cx), - &["v project_root", " v dir_1", " v nested_dir",] + &[ + "v project_root", + " v dir_1", + " v nested_dir <== selected", + ] ); } #[gpui::test] @@ -6327,6 +6471,598 @@ mod tests { ); } + #[gpui::test] + async fn test_basic_file_deletion_scenarios(cx: &mut gpui::TestAppContext) { + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root", + json!({ + "dir1": { + "subdir1": {}, + "file1.txt": "", + "file2.txt": "", + }, + "dir2": { + "subdir2": {}, + "file3.txt": "", + "file4.txt": "", + }, + "file5.txt": "", + "file6.txt": "", + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let panel = workspace.update(cx, ProjectPanel::new).unwrap(); + + toggle_expand_dir(&panel, "root/dir1", cx); + toggle_expand_dir(&panel, "root/dir2", cx); + + // Test Case 1: Delete middle file in directory + select_path(&panel, "root/dir1/file1.txt", cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1", + " > subdir1", + " file1.txt <== selected", + " file2.txt", + " v dir2", + " > subdir2", + " file3.txt", + " file4.txt", + " file5.txt", + " file6.txt", + ], + "Initial state before deleting middle file" + ); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1", + " > subdir1", + " file2.txt <== selected", + " v dir2", + " > subdir2", + " file3.txt", + " file4.txt", + " file5.txt", + " file6.txt", + ], + "Should select next file after deleting middle file" + ); + + // Test Case 2: Delete last file in directory + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1", + " > subdir1 <== selected", + " v dir2", + " > subdir2", + " file3.txt", + " file4.txt", + " file5.txt", + " file6.txt", + ], + "Should select next directory when last file is deleted" + ); + + // Test Case 3: Delete root level file + select_path(&panel, "root/file6.txt", cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1", + " > subdir1", + " v dir2", + " > subdir2", + " file3.txt", + " file4.txt", + " file5.txt", + " file6.txt <== selected", + ], + "Initial state before deleting root level file" + ); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1", + " > subdir1", + " v dir2", + " > subdir2", + " file3.txt", + " file4.txt", + " file5.txt <== selected", + ], + "Should select prev entry at root level" + ); + } + + #[gpui::test] + async fn test_complex_selection_scenarios(cx: &mut gpui::TestAppContext) { + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root", + json!({ + "dir1": { + "subdir1": { + "a.txt": "", + "b.txt": "" + }, + "file1.txt": "", + }, + "dir2": { + "subdir2": { + "c.txt": "", + "d.txt": "" + }, + "file2.txt": "", + }, + "file3.txt": "", + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let panel = workspace.update(cx, ProjectPanel::new).unwrap(); + + toggle_expand_dir(&panel, "root/dir1", cx); + toggle_expand_dir(&panel, "root/dir1/subdir1", cx); + toggle_expand_dir(&panel, "root/dir2", cx); + toggle_expand_dir(&panel, "root/dir2/subdir2", cx); + + // Test Case 1: Select and delete nested directory with parent + cx.simulate_modifiers_change(gpui::Modifiers { + control: true, + ..Default::default() + }); + select_path_with_mark(&panel, "root/dir1/subdir1", cx); + select_path_with_mark(&panel, "root/dir1", cx); + + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir1 <== selected <== marked", + " v subdir1 <== marked", + " a.txt", + " b.txt", + " file1.txt", + " v dir2", + " v subdir2", + " c.txt", + " d.txt", + " file2.txt", + " file3.txt", + ], + "Initial state before deleting nested directory with parent" + ); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir2 <== selected", + " v subdir2", + " c.txt", + " d.txt", + " file2.txt", + " file3.txt", + ], + "Should select next directory after deleting directory with parent" + ); + + // Test Case 2: Select mixed files and directories across levels + select_path_with_mark(&panel, "root/dir2/subdir2/c.txt", cx); + select_path_with_mark(&panel, "root/dir2/file2.txt", cx); + select_path_with_mark(&panel, "root/file3.txt", cx); + + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir2", + " v subdir2", + " c.txt <== marked", + " d.txt", + " file2.txt <== marked", + " file3.txt <== selected <== marked", + ], + "Initial state before deleting" + ); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v root", + " v dir2 <== selected", + " v subdir2", + " d.txt", + ], + "Should select sibling directory" + ); + } + + #[gpui::test] + async fn test_delete_all_files_and_directories(cx: &mut gpui::TestAppContext) { + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root", + json!({ + "dir1": { + "subdir1": { + "a.txt": "", + "b.txt": "" + }, + "file1.txt": "", + }, + "dir2": { + "subdir2": { + "c.txt": "", + "d.txt": "" + }, + "file2.txt": "", + }, + "file3.txt": "", + "file4.txt": "", + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let panel = workspace.update(cx, ProjectPanel::new).unwrap(); + + toggle_expand_dir(&panel, "root/dir1", cx); + toggle_expand_dir(&panel, "root/dir1/subdir1", cx); + toggle_expand_dir(&panel, "root/dir2", cx); + toggle_expand_dir(&panel, "root/dir2/subdir2", cx); + + // Test Case 1: Select all root files and directories + cx.simulate_modifiers_change(gpui::Modifiers { + control: true, + ..Default::default() + }); + select_path_with_mark(&panel, "root/dir1", cx); + select_path_with_mark(&panel, "root/dir2", cx); + select_path_with_mark(&panel, "root/file3.txt", cx); + select_path_with_mark(&panel, "root/file4.txt", cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root", + " v dir1 <== marked", + " v subdir1", + " a.txt", + " b.txt", + " file1.txt", + " v dir2 <== marked", + " v subdir2", + " c.txt", + " d.txt", + " file2.txt", + " file3.txt <== marked", + " file4.txt <== selected <== marked", + ], + "State before deleting all contents" + ); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &["v root <== selected"], + "Only empty root directory should remain after deleting all contents" + ); + } + + #[gpui::test] + async fn test_nested_selection_deletion(cx: &mut gpui::TestAppContext) { + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root", + json!({ + "dir1": { + "subdir1": { + "file_a.txt": "content a", + "file_b.txt": "content b", + }, + "subdir2": { + "file_c.txt": "content c", + }, + "file1.txt": "content 1", + }, + "dir2": { + "file2.txt": "content 2", + }, + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let panel = workspace.update(cx, ProjectPanel::new).unwrap(); + + toggle_expand_dir(&panel, "root/dir1", cx); + toggle_expand_dir(&panel, "root/dir1/subdir1", cx); + toggle_expand_dir(&panel, "root/dir2", cx); + cx.simulate_modifiers_change(gpui::Modifiers { + control: true, + ..Default::default() + }); + + // Test Case 1: Select parent directory, subdirectory, and a file inside the subdirectory + select_path_with_mark(&panel, "root/dir1", cx); + select_path_with_mark(&panel, "root/dir1/subdir1", cx); + select_path_with_mark(&panel, "root/dir1/subdir1/file_a.txt", cx); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root", + " v dir1 <== marked", + " v subdir1 <== marked", + " file_a.txt <== selected <== marked", + " file_b.txt", + " > subdir2", + " file1.txt", + " v dir2", + " file2.txt", + ], + "State with parent dir, subdir, and file selected" + ); + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &["v root", " v dir2 <== selected", " file2.txt",], + "Only dir2 should remain after deletion" + ); + } + + #[gpui::test] + async fn test_multiple_worktrees_deletion(cx: &mut gpui::TestAppContext) { + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor().clone()); + // First worktree + fs.insert_tree( + "/root1", + json!({ + "dir1": { + "file1.txt": "content 1", + "file2.txt": "content 2", + }, + "dir2": { + "file3.txt": "content 3", + }, + }), + ) + .await; + + // Second worktree + fs.insert_tree( + "/root2", + json!({ + "dir3": { + "file4.txt": "content 4", + "file5.txt": "content 5", + }, + "file6.txt": "content 6", + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root1".as_ref(), "/root2".as_ref()], cx).await; + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let panel = workspace.update(cx, ProjectPanel::new).unwrap(); + + // Expand all directories for testing + toggle_expand_dir(&panel, "root1/dir1", cx); + toggle_expand_dir(&panel, "root1/dir2", cx); + toggle_expand_dir(&panel, "root2/dir3", cx); + + // Test Case 1: Delete files across different worktrees + cx.simulate_modifiers_change(gpui::Modifiers { + control: true, + ..Default::default() + }); + select_path_with_mark(&panel, "root1/dir1/file1.txt", cx); + select_path_with_mark(&panel, "root2/dir3/file4.txt", cx); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " v dir1", + " file1.txt <== marked", + " file2.txt", + " v dir2", + " file3.txt", + "v root2", + " v dir3", + " file4.txt <== selected <== marked", + " file5.txt", + " file6.txt", + ], + "Initial state with files selected from different worktrees" + ); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " v dir1", + " file2.txt", + " v dir2", + " file3.txt", + "v root2", + " v dir3", + " file5.txt <== selected", + " file6.txt", + ], + "Should select next file in the last worktree after deletion" + ); + + // Test Case 2: Delete directories from different worktrees + select_path_with_mark(&panel, "root1/dir1", cx); + select_path_with_mark(&panel, "root2/dir3", cx); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " v dir1 <== marked", + " file2.txt", + " v dir2", + " file3.txt", + "v root2", + " v dir3 <== selected <== marked", + " file5.txt", + " file6.txt", + ], + "State with directories marked from different worktrees" + ); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " v dir2", + " file3.txt", + "v root2", + " file6.txt <== selected", + ], + "Should select remaining file in last worktree after directory deletion" + ); + + // Test Case 4: Delete all remaining files except roots + select_path_with_mark(&panel, "root1/dir2/file3.txt", cx); + select_path_with_mark(&panel, "root2/file6.txt", cx); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " v dir2", + " file3.txt <== marked", + "v root2", + " file6.txt <== selected <== marked", + ], + "State with all remaining files marked" + ); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &["v root1", " v dir2", "v root2 <== selected"], + "Second parent root should be selected after deleting" + ); + } + + #[gpui::test] + async fn test_selection_fallback_to_next_highest_worktree(cx: &mut gpui::TestAppContext) { + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root_b", + json!({ + "dir1": { + "file1.txt": "content 1", + "file2.txt": "content 2", + }, + }), + ) + .await; + + fs.insert_tree( + "/root_c", + json!({ + "dir2": {}, + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root_b".as_ref(), "/root_c".as_ref()], cx).await; + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let panel = workspace.update(cx, ProjectPanel::new).unwrap(); + + toggle_expand_dir(&panel, "root_b/dir1", cx); + toggle_expand_dir(&panel, "root_c/dir2", cx); + + cx.simulate_modifiers_change(gpui::Modifiers { + control: true, + ..Default::default() + }); + select_path_with_mark(&panel, "root_b/dir1/file1.txt", cx); + select_path_with_mark(&panel, "root_b/dir1/file2.txt", cx); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root_b", + " v dir1", + " file1.txt <== marked", + " file2.txt <== selected <== marked", + "v root_c", + " v dir2", + ], + "Initial state with files marked in root_b" + ); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root_b", + " v dir1 <== selected", + "v root_c", + " v dir2", + ], + "After deletion in root_b as it's last deletion, selection should be in root_b" + ); + + select_path_with_mark(&panel, "root_c/dir2", cx); + + submit_deletion(&panel, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &["v root_b", " v dir1", "v root_c <== selected",], + "After deleting from root_c, it should remain in root_c" + ); + } + fn toggle_expand_dir( panel: &View, path: impl AsRef, @@ -6364,6 +7100,32 @@ mod tests { }); } + fn select_path_with_mark( + panel: &View, + path: impl AsRef, + cx: &mut VisualTestContext, + ) { + let path = path.as_ref(); + panel.update(cx, |panel, cx| { + for worktree in panel.project.read(cx).worktrees(cx).collect::>() { + let worktree = worktree.read(cx); + if let Ok(relative_path) = path.strip_prefix(worktree.root_name()) { + let entry_id = worktree.entry_for_path(relative_path).unwrap().id; + let entry = crate::SelectedEntry { + worktree_id: worktree.id(), + entry_id, + }; + if !panel.marked_entries.contains(&entry) { + panel.marked_entries.insert(entry); + } + panel.selection = Some(entry); + return; + } + } + panic!("no worktree for path {:?}", path); + }); + } + fn find_project_entry( panel: &View, path: impl AsRef, diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index d629c8facc..f4e494f66e 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -378,7 +378,15 @@ pub fn compare_paths( .as_deref() .map(NumericPrefixWithSuffix::from_numeric_prefixed_str); - num_and_remainder_a.cmp(&num_and_remainder_b) + num_and_remainder_a.cmp(&num_and_remainder_b).then_with(|| { + if a_is_file && b_is_file { + let ext_a = path_a.extension().unwrap_or_default(); + let ext_b = path_b.extension().unwrap_or_default(); + ext_a.cmp(ext_b) + } else { + cmp::Ordering::Equal + } + }) }); if !ordering.is_eq() { return ordering; @@ -433,6 +441,28 @@ mod tests { ); } + #[test] + fn compare_paths_with_same_name_different_extensions() { + let mut paths = vec![ + (Path::new("test_dirs/file.rs"), true), + (Path::new("test_dirs/file.txt"), true), + (Path::new("test_dirs/file.md"), true), + (Path::new("test_dirs/file"), true), + (Path::new("test_dirs/file.a"), true), + ]; + paths.sort_by(|&a, &b| compare_paths(a, b)); + assert_eq!( + paths, + vec![ + (Path::new("test_dirs/file"), true), + (Path::new("test_dirs/file.a"), true), + (Path::new("test_dirs/file.md"), true), + (Path::new("test_dirs/file.rs"), true), + (Path::new("test_dirs/file.txt"), true), + ] + ); + } + #[test] fn compare_paths_case_semi_sensitive() { let mut paths = vec![ diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index e9b81d4554..4eec2f18d1 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -48,7 +48,7 @@ use ui::{v_flex, ContextMenu}; use util::{debug_panic, maybe, truncate_and_remove_front, ResultExt}; /// A selected entry in e.g. project panel. -#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct SelectedEntry { pub worktree_id: WorktreeId, pub entry_id: ProjectEntryId,