From 8cb397cf6c95eea869447746b1757f4ed22acffa Mon Sep 17 00:00:00 2001 From: Josef Zoller Date: Thu, 2 Jan 2025 22:33:51 +0100 Subject: [PATCH] project_panel: Open rename file editor if pasted file was disambiguated (#19975) Closes #19974. When a file is pasted in the project panel at a location where a file with that name already exists, the new file's name is disambiguated by appending " copy" at the end. This happens on the paste and the duplicate actions, as well as when Alt-dragging files. With this PR, this will now open the file rename editor with the disambiguator pre-selected. Open question: With this PR's current implementation, this won't always work when pasting multiple files at once. In this case, the file rename editor only opens for the last pasted file, if that file was disambiguated. If only other files were disambiguated instead, it won't open. This roughly mimics the previous paste behaviour, namely that only the last pasted file was selected. I see two options here: If multiple files were pasted and some of them were disambiguated, we could select and open the rename editor for the last file that was actually disambiguated (easy), or we could open a kind of multi-editor for all files (hard, but maybe a multi-rename editor could actually be interesting in general...). Release Notes: - Open rename file editor if pasted file was disambiguated --- crates/project_panel/src/project_panel.rs | 216 ++++++++++++++++++---- 1 file changed, 180 insertions(+), 36 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index c2afc7aefd..03d335c3c6 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1159,7 +1159,7 @@ impl ProjectPanel { } } - fn rename(&mut self, _: &Rename, cx: &mut ViewContext) { + fn rename_impl(&mut self, selection: Option>, cx: &mut ViewContext) { if let Some(SelectedEntry { worktree_id, entry_id, @@ -1183,13 +1183,16 @@ impl ProjectPanel { .map(|s| s.to_string_lossy()) .unwrap_or_default() .to_string(); - let file_stem = entry.path.file_stem().map(|s| s.to_string_lossy()); - let selection_end = - file_stem.map_or(file_name.len(), |file_stem| file_stem.len()); + let selection = selection.unwrap_or_else(|| { + let file_stem = entry.path.file_stem().map(|s| s.to_string_lossy()); + let selection_end = + file_stem.map_or(file_name.len(), |file_stem| file_stem.len()); + 0..selection_end + }); self.filename_editor.update(cx, |editor, cx| { editor.set_text(file_name, cx); editor.change_selections(Some(Autoscroll::fit()), cx, |s| { - s.select_ranges([0..selection_end]) + s.select_ranges([selection]) }); editor.focus(cx); }); @@ -1201,6 +1204,10 @@ impl ProjectPanel { } } + fn rename(&mut self, _: &Rename, cx: &mut ViewContext) { + self.rename_impl(None, cx); + } + fn trash(&mut self, action: &Trash, cx: &mut ViewContext) { self.remove(true, action.skip_prompt, cx); } @@ -1762,7 +1769,7 @@ impl ProjectPanel { source: &SelectedEntry, (worktree, target_entry): (Model, &Entry), cx: &AppContext, - ) -> Option { + ) -> Option<(PathBuf, Option>)> { let mut new_path = target_entry.path.to_path_buf(); // If we're pasting into a file, or a directory into itself, go up one level. if target_entry.is_file() || (target_entry.is_dir() && target_entry.id == source.entry_id) { @@ -1778,6 +1785,8 @@ impl ProjectPanel { new_path.push(&clipboard_entry_file_name); let extension = new_path.extension().map(|e| e.to_os_string()); let file_name_without_extension = Path::new(&clipboard_entry_file_name).file_stem()?; + let file_name_len = file_name_without_extension.to_string_lossy().len(); + let mut disambiguation_range = None; let mut ix = 0; { let worktree = worktree.read(cx); @@ -1785,9 +1794,17 @@ impl ProjectPanel { new_path.pop(); let mut new_file_name = file_name_without_extension.to_os_string(); - new_file_name.push(" copy"); + + let disambiguation = " copy"; + let mut disambiguation_len = disambiguation.len(); + + new_file_name.push(disambiguation); + if ix > 0 { - new_file_name.push(format!(" {}", ix)); + let extra_disambiguation = format!(" {}", ix); + disambiguation_len += extra_disambiguation.len(); + + new_file_name.push(extra_disambiguation); } if let Some(extension) = extension.as_ref() { new_file_name.push("."); @@ -1795,10 +1812,11 @@ impl ProjectPanel { } new_path.push(new_file_name); + disambiguation_range = Some(file_name_len..(file_name_len + disambiguation_len)); ix += 1; } } - Some(new_path) + Some((new_path, disambiguation_range)) } fn paste(&mut self, _: &Paste, cx: &mut ViewContext) { @@ -1816,9 +1834,10 @@ impl ProjectPanel { } let mut paste_entry_tasks: IndexMap<(ProjectEntryId, bool), PasteTask> = IndexMap::default(); + let mut disambiguation_range = None; let clip_is_cut = clipboard_entries.is_cut(); for clipboard_entry in clipboard_entries.items() { - let new_path = + let (new_path, new_disambiguation_range) = self.create_paste_path(clipboard_entry, self.selected_sub_entry(cx)?, cx)?; let clip_entry_id = clipboard_entry.entry_id; let is_same_worktree = clipboard_entry.worktree_id == worktree_id; @@ -1855,8 +1874,11 @@ impl ProjectPanel { }; let needs_delete = !is_same_worktree && clip_is_cut; paste_entry_tasks.insert((clip_entry_id, needs_delete), task); + disambiguation_range = new_disambiguation_range.or(disambiguation_range); } + let item_count = paste_entry_tasks.len(); + cx.spawn(|project_panel, mut cx| async move { let mut last_succeed = None; let mut need_delete_ids = Vec::new(); @@ -1877,17 +1899,6 @@ impl ProjectPanel { } } } - // update selection - if let Some(entry_id) = last_succeed { - project_panel - .update(&mut cx, |project_panel, _cx| { - project_panel.selection = Some(SelectedEntry { - worktree_id, - entry_id, - }); - }) - .ok(); - } // remove entry for cut in difference worktree for entry_id in need_delete_ids { project_panel @@ -1899,6 +1910,22 @@ impl ProjectPanel { })?? .await?; } + // update selection + if let Some(entry_id) = last_succeed { + project_panel + .update(&mut cx, |project_panel, cx| { + project_panel.selection = Some(SelectedEntry { + worktree_id, + entry_id, + }); + + // if only one entry was pasted and it was disambiguated, open the rename editor + if item_count == 1 && disambiguation_range.is_some() { + project_panel.rename_impl(disambiguation_range, cx); + } + }) + .ok(); + } anyhow::Ok(()) }) @@ -2606,23 +2633,55 @@ impl ProjectPanel { let _ = maybe!({ let project = self.project.read(cx); let target_worktree = project.worktree_for_entry(target_entry_id, cx)?; + let worktree_id = target_worktree.read(cx).id(); let target_entry = target_worktree .read(cx) .entry_for_id(target_entry_id)? .clone(); + + let mut copy_tasks = Vec::new(); + let mut disambiguation_range = None; for selection in selections.items() { - let new_path = self.create_paste_path( + let (new_path, new_disambiguation_range) = self.create_paste_path( selection, (target_worktree.clone(), &target_entry), cx, )?; - self.project - .update(cx, |project, cx| { - project.copy_entry(selection.entry_id, None, new_path, cx) - }) - .detach_and_log_err(cx) + + let task = self.project.update(cx, |project, cx| { + project.copy_entry(selection.entry_id, None, new_path, cx) + }); + copy_tasks.push(task); + disambiguation_range = new_disambiguation_range.or(disambiguation_range); } + let item_count = copy_tasks.len(); + + cx.spawn(|project_panel, mut cx| async move { + let mut last_succeed = None; + for task in copy_tasks.into_iter() { + if let Some(Some(entry)) = task.await.log_err() { + last_succeed = Some(entry.id); + } + } + // update selection + if let Some(entry_id) = last_succeed { + project_panel + .update(&mut cx, |project_panel, cx| { + project_panel.selection = Some(SelectedEntry { + worktree_id, + entry_id, + }); + + // if only one entry was dragged and it was disambiguated, open the rename editor + if item_count == 1 && disambiguation_range.is_some() { + project_panel.rename_impl(disambiguation_range, cx); + } + }) + .ok(); + } + }) + .detach(); Some(()) }); } else { @@ -5283,11 +5342,22 @@ mod tests { // "v root1", " one.txt", - " one copy.txt <== selected", + " [EDITOR: 'one copy.txt'] <== selected", " one.two.txt", ] ); + panel.update(cx, |panel, cx| { + panel.filename_editor.update(cx, |editor, cx| { + let file_name_selections = editor.selections.all::(cx); + assert_eq!(file_name_selections.len(), 1, "File editing should have a single selection, but got: {file_name_selections:?}"); + let file_name_selection = &file_name_selections[0]; + assert_eq!(file_name_selection.start, "one".len(), "Should select the file name disambiguation after the original file name"); + assert_eq!(file_name_selection.end, "one copy".len(), "Should select the file name disambiguation until the extension"); + }); + assert!(panel.confirm_edit(cx).is_none()); + }); + panel.update(cx, |panel, cx| { panel.paste(&Default::default(), cx); }); @@ -5300,10 +5370,12 @@ mod tests { "v root1", " one.txt", " one copy.txt", - " one copy 1.txt <== selected", + " [EDITOR: 'one copy 1.txt'] <== selected", " one.two.txt", ] ); + + panel.update(cx, |panel, cx| assert!(panel.confirm_edit(cx).is_none())); } #[gpui::test] @@ -5495,11 +5567,14 @@ mod tests { " four.txt", " one.txt", " three.txt", - " three copy.txt <== selected", + " [EDITOR: 'three copy.txt'] <== selected", " two.txt", ] ); + panel.update(cx, |panel, cx| panel.cancel(&menu::Cancel {}, cx)); + cx.executor().run_until_parked(); + select_path(&panel, "root1/a", cx); panel.update(cx, |panel, cx| { panel.copy(&Default::default(), cx); @@ -5603,6 +5678,48 @@ mod tests { select_path(&panel, "root", cx); panel.update(cx, |panel, cx| panel.paste(&Default::default(), cx)); cx.executor().run_until_parked(); + assert_eq!( + visible_entries_as_strings(&panel, 0..50, cx), + &[ + // + "v root", + " > a", + " > [EDITOR: 'a copy'] <== selected", + " v b", + " v a", + " v inner_dir", + " four.txt", + " three.txt", + " one.txt", + " two.txt" + ] + ); + + let confirm = panel.update(cx, |panel, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text("c", cx)); + panel.confirm_edit(cx).unwrap() + }); + assert_eq!( + visible_entries_as_strings(&panel, 0..50, cx), + &[ + // + "v root", + " > a", + " > [PROCESSING: 'c'] <== selected", + " v b", + " v a", + " v inner_dir", + " four.txt", + " three.txt", + " one.txt", + " two.txt" + ] + ); + + confirm.await.unwrap(); + panel.update(cx, |panel, cx| panel.paste(&Default::default(), cx)); cx.executor().run_until_parked(); assert_eq!( @@ -5611,18 +5728,18 @@ mod tests { // "v root", " > a", - " v a copy", - " > a <== selected", - " > inner_dir", - " one.txt", - " two.txt", " v b", " v a", " v inner_dir", " four.txt", " three.txt", " one.txt", - " two.txt" + " two.txt", + " v c", + " > a <== selected", + " > inner_dir", + " one.txt", + " two.txt", ] ); } @@ -5703,6 +5820,33 @@ mod tests { ], "Should copy dir1 as well as c.txt into dir2" ); + + // Disambiguating multiple files should not open the rename editor. + select_path(&panel, "test/dir2", cx); + panel.update(cx, |panel, cx| { + panel.paste(&Default::default(), cx); + }); + cx.executor().run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..15, cx), + &[ + "v test", + " v dir1 <== marked", + " a.txt", + " b.txt", + " v dir2", + " v dir1", + " a.txt", + " b.txt", + " > dir1 copy <== selected", + " c.txt", + " c copy.txt", + " c.txt <== marked", + " d.txt", + ], + "Should copy dir1 as well as c.txt into dir2 and disambiguate them without opening the rename editor" + ); } #[gpui::test]