From 2b6cf31ace8aada0003a0f07ee9efef4ef3100a3 Mon Sep 17 00:00:00 2001 From: Coenen Benjamin Date: Thu, 6 Nov 2025 08:06:45 +0100 Subject: [PATCH] file_finder: Display duplicated file in file finder history (#41917) Closes #41850 When digging into this I figured out that basically what was going on is in the history of the file finder it doesn't update the name of the file duplicated because when you duplicate a file it's named automatically with `filename copy` and so this filename was added to the history but not updated so once you wanted to go back into this file it was not part of file finder displayed history anymore because this file doesn't exist anymore but the entity id remains the same. I was also to reproduce this bug when just renaming a file. Release Notes: - Fixed: Display duplicated file in file finder history --------- Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com> Co-authored-by: Conrad Irwin Co-authored-by: Lukas Wirth --- crates/editor/src/editor.rs | 19 +- crates/project/src/project.rs | 8 +- .../project_panel/src/project_panel_tests.rs | 390 ++++++++++++++++++ crates/workspace/src/pane.rs | 15 +- crates/workspace/src/workspace.rs | 4 + 5 files changed, 431 insertions(+), 5 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 6dcc0f2eefbd3b5b4b5d6faf5b838f9ec39add6c..9df60acb3226318b2aee860b43dbd4d9b5178323 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1905,7 +1905,7 @@ impl Editor { } } - project::Event::EntryRenamed(transaction) => { + project::Event::EntryRenamed(transaction, project_path, abs_path) => { let Some(workspace) = editor.workspace() else { return; }; @@ -1913,7 +1913,23 @@ impl Editor { else { return; }; + if active_editor.entity_id() == cx.entity_id() { + let entity_id = cx.entity_id(); + workspace.update(cx, |this, cx| { + this.panes_mut() + .iter_mut() + .filter(|pane| pane.entity_id() != entity_id) + .for_each(|p| { + p.update(cx, |pane, _| { + pane.nav_history_mut().rename_item( + entity_id, + project_path.clone(), + abs_path.clone().into(), + ); + }) + }); + }); let edited_buffers_already_open = { let other_editors: Vec> = workspace .read(cx) @@ -1936,7 +1952,6 @@ impl Editor { }) }) }; - if !edited_buffers_already_open { let workspace = workspace.downgrade(); let transaction = transaction.clone(); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 489b72e9c91839a4a2adeaf486343e6e8765418a..38a75726859c3c4e7d5b7835365e6faac8634e3e 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -344,7 +344,7 @@ pub enum Event { RevealInProjectPanel(ProjectEntryId), SnippetEdit(BufferId, Vec<(lsp::Range, Snippet)>), ExpandedAllForEntry(WorktreeId, ProjectEntryId), - EntryRenamed(ProjectTransaction), + EntryRenamed(ProjectTransaction, ProjectPath, PathBuf), AgentLocationChanged, } @@ -2248,7 +2248,11 @@ impl Project { project .update(cx, |_, cx| { - cx.emit(Event::EntryRenamed(transaction)); + cx.emit(Event::EntryRenamed( + transaction, + new_path.clone(), + new_abs_path.clone(), + )); }) .ok(); diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index b6cd1da132ad5c1633001bd53fe365f24870cd7c..2ff389a6abd8298156b93671e105667c03a4889d 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -2199,6 +2199,396 @@ async fn test_create_duplicate_items(cx: &mut gpui::TestAppContext) { ); } +// NOTE: This test is skipped on Windows, because on Windows, +// when it triggers the lsp store it converts `/src/test/first copy.txt` into an uri +// but it fails with message `"/src\\test\\first copy.txt" is not parseable as an URI` +#[gpui::test] +#[cfg_attr(target_os = "windows", ignore)] +async fn test_create_duplicate_items_and_check_history(cx: &mut gpui::TestAppContext) { + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/src", + json!({ + "test": { + "first.txt": "// First Txt file", + "second.txt": "// Second Txt file", + "third.txt": "// Third Txt file", + } + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/src".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.run_until_parked(); + + select_path(&panel, "src", cx); + panel.update_in(cx, |panel, window, cx| panel.confirm(&Confirm, window, cx)); + cx.executor().run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + // + "v src <== selected", + " > test" + ] + ); + panel.update_in(cx, |panel, window, cx| { + panel.new_directory(&NewDirectory, window, cx) + }); + cx.run_until_parked(); + panel.update_in(cx, |panel, window, cx| { + assert!(panel.filename_editor.read(cx).is_focused(window)); + }); + cx.executor().run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + // + "v src", + " > [EDITOR: ''] <== selected", + " > test" + ] + ); + panel.update_in(cx, |panel, window, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text("test", window, cx)); + assert!( + panel.confirm_edit(true, window, cx).is_none(), + "Should not allow to confirm on conflicting new directory name" + ); + }); + cx.executor().run_until_parked(); + panel.update_in(cx, |panel, window, cx| { + assert!( + panel.state.edit_state.is_some(), + "Edit state should not be None after conflicting new directory name" + ); + panel.cancel(&menu::Cancel, window, cx); + panel.update_visible_entries(None, false, false, window, cx); + }); + cx.run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + // + "v src <== selected", + " > test" + ], + "File list should be unchanged after failed folder create confirmation" + ); + + select_path(&panel, "src/test", cx); + panel.update_in(cx, |panel, window, cx| panel.confirm(&Confirm, window, cx)); + cx.executor().run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + // + "v src", + " > test <== selected" + ] + ); + 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)); + }); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v src", + " v test", + " [EDITOR: ''] <== selected", + " first.txt", + " second.txt", + " third.txt" + ] + ); + panel.update_in(cx, |panel, window, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text("first.txt", window, cx)); + assert!( + panel.confirm_edit(true, window, cx).is_none(), + "Should not allow to confirm on conflicting new file name" + ); + }); + cx.executor().run_until_parked(); + panel.update_in(cx, |panel, window, cx| { + assert!( + panel.state.edit_state.is_some(), + "Edit state should not be None after conflicting new file name" + ); + panel.cancel(&menu::Cancel, window, cx); + panel.update_visible_entries(None, false, false, window, cx); + }); + cx.run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v src", + " v test <== selected", + " first.txt", + " second.txt", + " third.txt" + ], + "File list should be unchanged after failed file create confirmation" + ); + + select_path(&panel, "src/test/first.txt", cx); + panel.update_in(cx, |panel, window, cx| panel.confirm(&Confirm, window, cx)); + cx.executor().run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v src", + " v test", + " first.txt <== selected", + " second.txt", + " third.txt" + ], + ); + panel.update_in(cx, |panel, window, cx| panel.rename(&Rename, window, cx)); + panel.update_in(cx, |panel, window, cx| { + assert!(panel.filename_editor.read(cx).is_focused(window)); + }); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v src", + " v test", + " [EDITOR: 'first.txt'] <== selected", + " second.txt", + " third.txt" + ] + ); + panel.update_in(cx, |panel, window, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text("second.txt", window, cx)); + assert!( + panel.confirm_edit(true, window, cx).is_none(), + "Should not allow to confirm on conflicting file rename" + ) + }); + cx.executor().run_until_parked(); + panel.update_in(cx, |panel, window, cx| { + assert!( + panel.state.edit_state.is_some(), + "Edit state should not be None after conflicting file rename" + ); + panel.cancel(&menu::Cancel, window, cx); + }); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v src", + " v test", + " first.txt <== selected", + " second.txt", + " third.txt" + ], + "File list should be unchanged after failed rename confirmation" + ); + panel.update_in(cx, |panel, window, cx| panel.open(&Open, window, cx)); + cx.executor().run_until_parked(); + // Try to duplicate and check history + panel.update_in(cx, |panel, window, cx| { + panel.duplicate(&Duplicate, window, cx) + }); + cx.executor().run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v src", + " v test", + " first.txt", + " [EDITOR: 'first copy.txt'] <== selected <== marked", + " second.txt", + " third.txt" + ], + ); + + let confirm = panel.update_in(cx, |panel, window, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text("fourth.txt", window, cx)); + panel.confirm_edit(true, window, cx).unwrap() + }); + confirm.await.unwrap(); + cx.executor().run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v src", + " v test", + " first.txt", + " fourth.txt <== selected", + " second.txt", + " third.txt" + ], + "File list should be different after rename confirmation" + ); + + panel.update_in(cx, |panel, window, cx| { + panel.update_visible_entries(None, false, false, window, cx); + }); + cx.executor().run_until_parked(); + + select_path(&panel, "src/test/first.txt", cx); + panel.update_in(cx, |panel, window, cx| panel.open(&Open, window, cx)); + cx.executor().run_until_parked(); + + workspace + .read_with(cx, |this, cx| { + assert!( + this.recent_navigation_history_iter(cx) + .any(|(project_path, abs_path)| { + project_path.path == Arc::from(rel_path("test/fourth.txt")) + && abs_path == Some(PathBuf::from(path!("/src/test/fourth.txt"))) + }) + ); + }) + .unwrap(); +} + +// NOTE: This test is skipped on Windows, because on Windows, +// when it triggers the lsp store it converts `/src/test/first.txt` into an uri +// but it fails with message `"/src\\test\\first.txt" is not parseable as an URI` +#[gpui::test] +#[cfg_attr(target_os = "windows", ignore)] +async fn test_rename_item_and_check_history(cx: &mut gpui::TestAppContext) { + init_test_with_editor(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/src", + json!({ + "test": { + "first.txt": "// First Txt file", + "second.txt": "// Second Txt file", + "third.txt": "// Third Txt file", + } + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/src".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.run_until_parked(); + + select_path(&panel, "src", cx); + panel.update_in(cx, |panel, window, cx| panel.confirm(&Confirm, window, cx)); + cx.executor().run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + // + "v src <== selected", + " > test" + ] + ); + + select_path(&panel, "src/test", cx); + panel.update_in(cx, |panel, window, cx| panel.confirm(&Confirm, window, cx)); + cx.executor().run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + // + "v src", + " > test <== selected" + ] + ); + 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)); + }); + + select_path(&panel, "src/test/first.txt", cx); + panel.update_in(cx, |panel, window, cx| panel.open(&Open, window, cx)); + cx.executor().run_until_parked(); + + panel.update_in(cx, |panel, window, cx| panel.rename(&Rename, window, cx)); + cx.executor().run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v src", + " v test", + " [EDITOR: 'first.txt'] <== selected <== marked", + " second.txt", + " third.txt" + ], + ); + + let confirm = panel.update_in(cx, |panel, window, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text("fourth.txt", window, cx)); + panel.confirm_edit(true, window, cx).unwrap() + }); + confirm.await.unwrap(); + cx.executor().run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &[ + "v src", + " v test", + " fourth.txt <== selected", + " second.txt", + " third.txt" + ], + "File list should be different after rename confirmation" + ); + + panel.update_in(cx, |panel, window, cx| { + panel.update_visible_entries(None, false, false, window, cx); + }); + cx.executor().run_until_parked(); + + select_path(&panel, "src/test/second.txt", cx); + panel.update_in(cx, |panel, window, cx| panel.open(&Open, window, cx)); + cx.executor().run_until_parked(); + + workspace + .read_with(cx, |this, cx| { + assert!( + this.recent_navigation_history_iter(cx) + .any(|(project_path, abs_path)| { + project_path.path == Arc::from(rel_path("test/fourth.txt")) + && abs_path == Some(PathBuf::from(path!("/src/test/fourth.txt"))) + }) + ); + }) + .unwrap(); +} + #[gpui::test] async fn test_select_git_entry(cx: &mut gpui::TestAppContext) { init_test_with_editor(cx); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index c2c79b6a5fc3cc337f6dd7273d529fd40f04c8a1..c72183240061018bd979b2eb34c28c517da620f5 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1123,7 +1123,6 @@ impl Pane { false } }); - if let Some(existing_item_index) = existing_item_index { // If the item already exists, move it to the desired destination and activate it @@ -4132,6 +4131,20 @@ impl NavHistory { .retain(|entry| entry.item.id() != item_id); } + pub fn rename_item( + &mut self, + item_id: EntityId, + project_path: ProjectPath, + abs_path: Option, + ) { + let mut state = self.0.lock(); + let path_for_item = state.paths_by_item.get_mut(&item_id); + if let Some(path_for_item) = path_for_item { + path_for_item.0 = project_path; + path_for_item.1 = abs_path; + } + } + pub fn path_for_item(&self, item_id: EntityId) -> Option<(ProjectPath, Option)> { self.0.lock().paths_by_item.get(&item_id).cloned() } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 84b86a92ec5bdae9e6184037c8bf25488b308ff0..2f8095deab4717f95180439f57c295c7d8a7ed14 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -4325,6 +4325,10 @@ impl Workspace { cx.emit(Event::PaneRemoved); } + pub fn panes_mut(&mut self) -> &mut [Entity] { + &mut self.panes + } + pub fn panes(&self) -> &[Entity] { &self.panes }