From bf2171d3f09743c594574a452f6a34830c834776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Eriksson?= Date: Mon, 9 Feb 2026 15:14:40 +0100 Subject: [PATCH] project_panel: Improve file/folder creation behavior in folded paths (#46750) Closes #45550 This fix, adds a new field to `EditState` to track and temporarily unfold the given directory when creating files or directories in collapsed paths. Release Notes: - Fixed the file and folder creation input appearing in the wrong location when creating inside a collapsed folder path.. Recording: https://github.com/user-attachments/assets/15e48863-26b9-4dc9-888a-b31fb3208d98 --------- Co-authored-by: Smit Barmase --- crates/project_panel/src/project_panel.rs | 118 ++-- .../project_panel/src/project_panel_tests.rs | 506 +++++++++++++++++- 2 files changed, 571 insertions(+), 53 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index b7476f459dfb4d96807beca4096e1a6591235319..b1c07a3f94f1317dd5169b68072cc701c3fde548 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -95,11 +95,19 @@ struct State { visible_entries: Vec, max_width_item_index: Option, edit_state: Option, + temporarily_unfolded_pending_state: Option, unfolded_dir_ids: HashSet, expanded_dir_ids: HashMap>, } impl State { + fn is_unfolded(&self, entry_id: &ProjectEntryId) -> bool { + self.unfolded_dir_ids.contains(entry_id) + || self.edit_state.as_ref().map_or(false, |edit_state| { + edit_state.temporarily_unfolded == Some(*entry_id) + }) + } + fn derive(old: &Self) -> Self { Self { last_worktree_root_id: None, @@ -107,6 +115,7 @@ impl State { visible_entries: Default::default(), max_width_item_index: None, edit_state: old.edit_state.clone(), + temporarily_unfolded_pending_state: None, unfolded_dir_ids: old.unfolded_dir_ids.clone(), expanded_dir_ids: old.expanded_dir_ids.clone(), } @@ -152,6 +161,12 @@ struct UpdateVisibleEntriesTask { autoscroll: bool, } +#[derive(Debug)] +struct TemporaryUnfoldedPendingState { + previously_focused_leaf_entry: SelectedEntry, + temporarily_unfolded_active_entry_id: ProjectEntryId, +} + impl Default for UpdateVisibleEntriesTask { fn default() -> Self { UpdateVisibleEntriesTask { @@ -199,6 +214,7 @@ struct EditState { processing_filename: Option>, previously_focused: Option, validation_state: ValidationState, + temporarily_unfolded: Option, } impl EditState { @@ -833,6 +849,7 @@ impl ProjectPanel { state: State { max_width_item_index: None, edit_state: None, + temporarily_unfolded_pending_state: None, last_worktree_root_id: Default::default(), visible_entries: Default::default(), ancestors: Default::default(), @@ -1934,22 +1951,31 @@ impl ProjectPanel { } fn discard_edit_state(&mut self, window: &mut Window, cx: &mut Context) { - let previously_focused = self - .state - .edit_state - .take() - .and_then(|edit_state| edit_state.previously_focused) - .map(|entry| (entry.worktree_id, entry.entry_id)); - self.marked_entries.clear(); - cx.notify(); - window.focus(&self.focus_handle, cx); - self.update_visible_entries( - previously_focused, - false, - previously_focused.is_some(), - window, - cx, - ); + if let Some(edit_state) = self.state.edit_state.take() { + self.state.temporarily_unfolded_pending_state = edit_state + .temporarily_unfolded + .and_then(|temporarily_unfolded_entry_id| { + let previously_focused_leaf_entry = edit_state.previously_focused?; + let folded_ancestors = + self.state.ancestors.get(&temporarily_unfolded_entry_id)?; + Some(TemporaryUnfoldedPendingState { + previously_focused_leaf_entry, + temporarily_unfolded_active_entry_id: folded_ancestors + .active_ancestor() + .unwrap_or(temporarily_unfolded_entry_id), + }) + }); + let previously_focused = edit_state + .previously_focused + .map(|entry| (entry.worktree_id, entry.entry_id)); + self.update_visible_entries( + previously_focused, + false, + previously_focused.is_some(), + window, + cx, + ); + } } fn cancel(&mut self, _: &menu::Cancel, window: &mut Window, cx: &mut Context) { @@ -1958,7 +1984,10 @@ impl ProjectPanel { self.hover_expand_task.take(); return; } + self.marked_entries.clear(); + cx.notify(); self.discard_edit_state(window, cx); + window.focus(&self.focus_handle, cx); } fn open_entry( @@ -2067,6 +2096,7 @@ impl ProjectPanel { previously_focused: self.selection, depth: 0, validation_state: ValidationState::None, + temporarily_unfolded: (new_entry_id != entry_id).then_some(new_entry_id), }); self.filename_editor.update(cx, |editor, cx| { editor.clear(window, cx); @@ -2124,6 +2154,7 @@ impl ProjectPanel { previously_focused: None, depth: 0, validation_state: ValidationState::None, + temporarily_unfolded: None, }); let file_name = entry.path.file_name().unwrap_or_default().to_string(); let selection = selection.unwrap_or_else(|| { @@ -3750,6 +3781,7 @@ impl ProjectPanel { let repo_snapshots = project.git_store().read(cx).repo_snapshots(cx); let old_ancestors = self.state.ancestors.clone(); + let temporary_unfolded_pending_state = self.state.temporarily_unfolded_pending_state.take(); let mut new_state = State::derive(&self.state); new_state.last_worktree_root_id = project .visible_worktrees(cx) @@ -3771,19 +3803,6 @@ impl ProjectPanel { for worktree_snapshot in visible_worktrees { let worktree_id = worktree_snapshot.id(); - let expanded_dir_ids = match new_state.expanded_dir_ids.entry(worktree_id) { - hash_map::Entry::Occupied(e) => e.into_mut(), - hash_map::Entry::Vacant(e) => { - // The first time a worktree's root entry becomes available, - // mark that root entry as expanded. - if let Some(entry) = worktree_snapshot.root_entry() { - e.insert(vec![entry.id]).as_slice() - } else { - &[] - } - } - }; - let mut new_entry_parent_id = None; let mut new_entry_kind = EntryKind::Dir; if let Some(edit_state) = &new_state.edit_state @@ -3818,7 +3837,7 @@ impl ProjectPanel { } if auto_collapse_dirs && entry.kind.is_dir() { auto_folded_ancestors.push(entry.id); - if !new_state.unfolded_dir_ids.contains(&entry.id) + if !new_state.is_unfolded(&entry.id) && let Some(root_path) = worktree_snapshot.root_entry() { let mut child_entries = @@ -3833,10 +3852,27 @@ impl ProjectPanel { continue; } } - let depth = old_ancestors - .get(&entry.id) - .map(|ancestor| ancestor.current_ancestor_depth) - .unwrap_or_default() + let depth = temporary_unfolded_pending_state + .as_ref() + .and_then(|state| { + if state.previously_focused_leaf_entry.worktree_id + == worktree_id + && state.previously_focused_leaf_entry.entry_id + == entry.id + { + auto_folded_ancestors.iter().rev().position(|id| { + *id == state.temporarily_unfolded_active_entry_id + }) + } else { + None + } + }) + .unwrap_or_else(|| { + old_ancestors + .get(&entry.id) + .map(|ancestor| ancestor.current_ancestor_depth) + .unwrap_or_default() + }) .min(auto_folded_ancestors.len()); if let Some(edit_state) = &mut new_state.edit_state && edit_state.entry_id == entry.id @@ -3950,6 +3986,20 @@ impl ProjectPanel { } } + let expanded_dir_ids = + match new_state.expanded_dir_ids.entry(worktree_id) { + hash_map::Entry::Occupied(e) => e.into_mut(), + hash_map::Entry::Vacant(e) => { + // The first time a worktree's root entry becomes available, + // mark that root entry as expanded. + if let Some(entry) = worktree_snapshot.root_entry() { + e.insert(vec![entry.id]).as_slice() + } else { + &[] + } + } + }; + if expanded_dir_ids.binary_search(&entry.id).is_err() && entry_iter.advance_to_sibling() { diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index f9fa85d70b2cb154a05dc3aa412de862a6a6b393..62f1a4906a6c8c51d459b1b593f176250e077956 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -2,6 +2,7 @@ use super::*; use collections::HashSet; use editor::MultiBufferOffset; use gpui::{Empty, Entity, TestAppContext, VisualTestContext, WindowHandle}; +use menu::Cancel; use pretty_assertions::assert_eq; use project::FakeFs; use serde_json::json; @@ -8517,7 +8518,7 @@ fn select_path_with_mark(panel: &Entity, path: &str, cx: &mut Visu } /// `leaf_path` is the full path to the leaf entry (e.g., "root/a/b/c") -/// `active_ancestor_path` is the path to the ancestor that should be "active" (e.g., "root/a/b") +/// `active_ancestor_path` is the path to the folded component that should be active. fn select_folded_path_with_mark( panel: &Entity, leaf_path: &str, @@ -8525,28 +8526,60 @@ fn select_folded_path_with_mark( cx: &mut VisualTestContext, ) { select_path_with_mark(panel, leaf_path, cx); + set_folded_active_ancestor(panel, leaf_path, active_ancestor_path, cx); +} + +fn set_folded_active_ancestor( + panel: &Entity, + leaf_path: &str, + active_ancestor_path: &str, + cx: &mut VisualTestContext, +) { + let leaf_path = rel_path(leaf_path); let active_ancestor_path = rel_path(active_ancestor_path); panel.update(cx, |panel, cx| { - let leaf_entry_id = panel.selection.unwrap().entry_id; - if let Some(folded_ancestors) = panel.state.ancestors.get_mut(&leaf_entry_id) { - for worktree in panel.project.read(cx).worktrees(cx).collect::>() { - let worktree = worktree.read(cx); - if let Ok(active_relative_path) = - active_ancestor_path.strip_prefix(worktree.root_name()) - { - let active_entry_id = worktree.entry_for_path(active_relative_path).unwrap().id; - if let Some(index) = folded_ancestors - .ancestors - .iter() - .position(|&id| id == active_entry_id) - { - folded_ancestors.current_ancestor_depth = - folded_ancestors.ancestors.len() - 1 - index; - } - return; - } + let mut leaf_entry_id = None; + let mut target_entry_id = None; + + for worktree in panel.project.read(cx).worktrees(cx).collect::>() { + let worktree = worktree.read(cx); + if let Ok(relative_path) = leaf_path.strip_prefix(worktree.root_name()) { + leaf_entry_id = worktree.entry_for_path(relative_path).map(|entry| entry.id); + } + if let Ok(relative_path) = active_ancestor_path.strip_prefix(worktree.root_name()) { + target_entry_id = worktree.entry_for_path(relative_path).map(|entry| entry.id); + } + } + + let leaf_entry_id = + leaf_entry_id.unwrap_or_else(|| panic!("no entry for leaf path {leaf_path:?}")); + let target_entry_id = target_entry_id + .unwrap_or_else(|| panic!("no entry for active path {active_ancestor_path:?}")); + let folded_ancestors = panel + .state + .ancestors + .get_mut(&leaf_entry_id) + .unwrap_or_else(|| panic!("leaf path {leaf_path:?} should be folded")); + let ancestor_ids = folded_ancestors.ancestors.clone(); + + let mut depth_for_target = None; + for depth in 0..ancestor_ids.len() { + let resolved_entry_id = if depth == 0 { + leaf_entry_id + } else { + ancestor_ids.get(depth).copied().unwrap_or(leaf_entry_id) + }; + if resolved_entry_id == target_entry_id { + depth_for_target = Some(depth); + break; } } + + folded_ancestors.current_ancestor_depth = depth_for_target.unwrap_or_else(|| { + panic!( + "active path {active_ancestor_path:?} is not part of folded ancestors {ancestor_ids:?}" + ) + }); }); } @@ -8859,6 +8892,441 @@ async fn test_sort_mode_toggle(cx: &mut gpui::TestAppContext) { ); } +#[gpui::test] +async fn test_ensure_temporary_folding_when_creating_in_different_nested_dirs( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + // parent: accept + run_create_file_in_folded_path_case( + "parent", + "root1/parent", + "file_in_parent.txt", + &[ + "v root1", + " v parent", + " > subdir/child", + " [EDITOR: ''] <== selected", + ], + &[ + "v root1", + " v parent", + " > subdir/child", + " file_in_parent.txt <== selected <== marked", + ], + true, + cx, + ) + .await; + + // parent: cancel + run_create_file_in_folded_path_case( + "parent", + "root1/parent", + "file_in_parent.txt", + &[ + "v root1", + " v parent", + " > subdir/child", + " [EDITOR: ''] <== selected", + ], + &["v root1", " > parent/subdir/child <== selected"], + false, + cx, + ) + .await; + + // subdir: accept + run_create_file_in_folded_path_case( + "subdir", + "root1/parent/subdir", + "file_in_subdir.txt", + &[ + "v root1", + " v parent/subdir", + " > child", + " [EDITOR: ''] <== selected", + ], + &[ + "v root1", + " v parent/subdir", + " > child", + " file_in_subdir.txt <== selected <== marked", + ], + true, + cx, + ) + .await; + + // subdir: cancel + run_create_file_in_folded_path_case( + "subdir", + "root1/parent/subdir", + "file_in_subdir.txt", + &[ + "v root1", + " v parent/subdir", + " > child", + " [EDITOR: ''] <== selected", + ], + &["v root1", " > parent/subdir/child <== selected"], + false, + cx, + ) + .await; + + // child: accept + run_create_file_in_folded_path_case( + "child", + "root1/parent/subdir/child", + "file_in_child.txt", + &[ + "v root1", + " v parent/subdir/child", + " [EDITOR: ''] <== selected", + ], + &[ + "v root1", + " v parent/subdir/child", + " file_in_child.txt <== selected <== marked", + ], + true, + cx, + ) + .await; + + // child: cancel + run_create_file_in_folded_path_case( + "child", + "root1/parent/subdir/child", + "file_in_child.txt", + &[ + "v root1", + " v parent/subdir/child", + " [EDITOR: ''] <== selected", + ], + &["v root1", " v parent/subdir/child <== selected"], + false, + cx, + ) + .await; +} + +#[gpui::test] +async fn test_preserve_temporary_unfolded_active_index_on_blur_from_context_menu( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/root1", + json!({ + "parent": { + "subdir": { + "child": {}, + } + } + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root1".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, |workspace, window, cx| { + let panel = ProjectPanel::new(workspace, window, cx); + workspace.add_panel(panel.clone(), window, cx); + panel + }) + .unwrap(); + + cx.update(|_, cx| { + let settings = *ProjectPanelSettings::get_global(cx); + ProjectPanelSettings::override_global( + ProjectPanelSettings { + auto_fold_dirs: true, + ..settings + }, + cx, + ); + }); + + panel.update_in(cx, |panel, window, cx| { + panel.collapse_all_entries(&CollapseAllEntries, window, cx); + }); + cx.run_until_parked(); + + select_folded_path_with_mark( + &panel, + "root1/parent/subdir/child", + "root1/parent/subdir", + cx, + ); + panel.update(cx, |panel, _| { + panel.marked_entries.clear(); + }); + + let parent_entry_id = find_project_entry(&panel, "root1/parent", cx) + .expect("parent directory should exist for this test"); + let subdir_entry_id = find_project_entry(&panel, "root1/parent/subdir", cx) + .expect("subdir directory should exist for this test"); + let child_entry_id = find_project_entry(&panel, "root1/parent/subdir/child", cx) + .expect("child directory should exist for this test"); + + panel.update(cx, |panel, _| { + let selection = panel + .selection + .expect("leaf directory should be selected before creating a new entry"); + assert_eq!( + selection.entry_id, child_entry_id, + "initial selection should be the folded leaf entry" + ); + assert_eq!( + panel.resolve_entry(selection.entry_id), + subdir_entry_id, + "active folded component should start at subdir" + ); + }); + + panel.update_in(cx, |panel, window, cx| { + panel.deploy_context_menu( + gpui::point(gpui::px(1.), gpui::px(1.)), + child_entry_id, + window, + cx, + ); + panel.new_file(&NewFile, window, cx); + }); + cx.run_until_parked(); + panel.update_in(cx, |panel, window, cx| { + assert!(panel.filename_editor.read(cx).is_focused(window)); + }); + cx.run_until_parked(); + + set_folded_active_ancestor(&panel, "root1/parent/subdir", "root1/parent", cx); + + panel.update_in(cx, |panel, window, cx| { + panel.deploy_context_menu( + gpui::point(gpui::px(2.), gpui::px(2.)), + subdir_entry_id, + window, + cx, + ); + }); + cx.run_until_parked(); + + panel.update(cx, |panel, _| { + assert!( + panel.state.edit_state.is_none(), + "opening another context menu should blur the filename editor and discard edit state" + ); + let selection = panel + .selection + .expect("selection should restore to the previously focused leaf entry"); + assert_eq!( + selection.entry_id, child_entry_id, + "blur-driven cancellation should restore the previous leaf selection" + ); + assert_eq!( + panel.resolve_entry(selection.entry_id), + parent_entry_id, + "temporary unfolded pending state should preserve the active ancestor chosen before blur" + ); + }); + + panel.update_in(cx, |panel, window, cx| { + panel.new_file(&NewFile, window, cx); + }); + cx.run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v root1", + " v parent", + " > subdir/child", + " [EDITOR: ''] <== selected", + ], + "new file after blur should use the preserved active ancestor" + ); + panel.update(cx, |panel, _| { + let edit_state = panel + .state + .edit_state + .as_ref() + .expect("new file should enter edit state"); + assert_eq!( + edit_state.temporarily_unfolded, + Some(parent_entry_id), + "temporary unfolding should now target parent after restoring the active ancestor" + ); + }); + + let file_name = "created_after_blur.txt"; + panel + .update_in(cx, |panel, window, cx| { + panel.filename_editor.update(cx, |editor, cx| { + editor.set_text(file_name, window, cx); + }); + panel.confirm_edit(true, window, cx).expect( + "confirm_edit should start creation for the file created after blur transition", + ) + }) + .await + .expect("creating file after blur transition should succeed"); + cx.run_until_parked(); + + assert!( + fs.is_file(Path::new("/root1/parent/created_after_blur.txt")) + .await, + "file should be created under parent after active ancestor is restored to parent" + ); + assert!( + !fs.is_file(Path::new("/root1/parent/subdir/created_after_blur.txt")) + .await, + "file should not be created under subdir when parent is the active ancestor" + ); +} + +async fn run_create_file_in_folded_path_case( + case_name: &str, + active_ancestor_path: &str, + created_file_name: &str, + expected_temporary_state: &[&str], + expected_final_state: &[&str], + accept_creation: bool, + cx: &mut gpui::TestAppContext, +) { + let expected_collapsed_state = &["v root1", " > parent/subdir/child <== selected"]; + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/root1", + json!({ + "parent": { + "subdir": { + "child": {}, + } + } + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root1".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, |workspace, window, cx| { + let panel = ProjectPanel::new(workspace, window, cx); + workspace.add_panel(panel.clone(), window, cx); + panel + }) + .unwrap(); + + cx.update(|_, cx| { + let settings = *ProjectPanelSettings::get_global(cx); + ProjectPanelSettings::override_global( + ProjectPanelSettings { + auto_fold_dirs: true, + ..settings + }, + cx, + ); + }); + + panel.update_in(cx, |panel, window, cx| { + panel.collapse_all_entries(&CollapseAllEntries, window, cx); + }); + cx.run_until_parked(); + + select_folded_path_with_mark( + &panel, + "root1/parent/subdir/child", + active_ancestor_path, + cx, + ); + panel.update(cx, |panel, _| { + panel.marked_entries.clear(); + }); + + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + expected_collapsed_state, + "case '{}' should start from a folded state", + case_name + ); + + panel.update_in(cx, |panel, window, cx| { + panel.new_file(&NewFile, window, cx); + }); + cx.run_until_parked(); + panel.update_in(cx, |panel, window, cx| { + assert!(panel.filename_editor.read(cx).is_focused(window)); + }); + cx.run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + expected_temporary_state, + "case '{}' ({}) should temporarily unfold the active ancestor while editing", + case_name, + if accept_creation { "accept" } else { "cancel" } + ); + + let relative_directory = active_ancestor_path + .strip_prefix("root1/") + .expect("active_ancestor_path should start with root1/"); + let created_file_path = PathBuf::from("/root1") + .join(relative_directory) + .join(created_file_name); + + if accept_creation { + panel + .update_in(cx, |panel, window, cx| { + panel.filename_editor.update(cx, |editor, cx| { + editor.set_text(created_file_name, window, cx); + }); + panel.confirm_edit(true, window, cx).unwrap() + }) + .await + .unwrap(); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + expected_final_state, + "case '{}' should keep the newly created file selected and marked after accept", + case_name + ); + assert!( + fs.is_file(created_file_path.as_path()).await, + "case '{}' should create file '{}'", + case_name, + created_file_path.display() + ); + } else { + panel.update_in(cx, |panel, window, cx| { + panel.cancel(&Cancel, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + expected_final_state, + "case '{}' should keep the expected panel state after cancel", + case_name + ); + assert!( + !fs.is_file(created_file_path.as_path()).await, + "case '{}' should not create a file after cancel", + case_name + ); + } +} + fn init_test(cx: &mut TestAppContext) { cx.update(|cx| { let settings_store = SettingsStore::test(cx);