Fix file missing or duplicated when copying multiple items in project panel + Fix marked files not being deselected after selecting a directory (#20859)

Closes #20858

This fix depends on the sanitization logic implemented in PR #20577.
Since that branch may undergo further changes, this branch will be
periodically rebased on it. Once #20577 is merged, the dependency will
no longer apply.

Release Notes:

- Fix missing or duplicated files when copying multiple items in the
project panel.
- Fix marked files not being deselected after selecting a directory on
primary click.
- Fix "copy path" and "copy path relative" with multiple items selected
in project panel.

**Problem**:

In this case, `dir1` is selected while `dir2`, `dir3`, and `dir1/file`
are marked. Using the `marked_entries` function results in only `dir1`,
which is incorrect.

<img height="120"
src="https://github.com/user-attachments/assets/d4d92cc5-c998-4948-9a58-25c4f54167f2"
/>

Currently, the `marked_entries` function is used in five actions, which
all produce incorrect results:

1. Delete (via the disjoint function)
2. Copy 
3. Cut
4. Copy Path
5. Copy Path Relative

**Solution**:

1. `marked_entries` function should not use "When currently selected
entry is not marked, it's treated as the only marked entry." logic.
There is no grand scheme behind this logic as confirmed by piotr
[here](https://github.com/zed-industries/zed/issues/17746#issuecomment-2464765963).
2. `copy` and `cut` actions should use the disjoint function to prevent
obivous failures.
3. `copy path` and `copy path relative` should keep using *fixed*
`marked_entries` as that is expected behavior for these actions.

---

1. Before/After: Partial Copy

Select `dir1` and `c.txt` (in that order, reverse order works!), and
copy it and paste in `dir2`. `c.txt` is not copied in `dir2`.

<img height="170"
src="https://github.com/user-attachments/assets/a09fcb40-f38f-46ef-b0b4-e44ec01dda18"
/>
<img height="170"
src="https://github.com/user-attachments/assets/bb87dbe5-8e2e-4ca4-a565-42be5755ec8a"
/>

---
2. Before/After: Duplicate Copy

Select `a.txt`, `dir1` and `c.txt` (in that order), and copy it and
paste in `dir2`. `a.txt` is duplicated in `dir2`.

<img height="170"
src="https://github.com/user-attachments/assets/6f999d22-3607-48d7-9ff6-2e27494002f8"
/>
<img height="170"
src="https://github.com/user-attachments/assets/b4b6ff7d-0df7-45ea-83e4-50a0acb18457"
/>

---
3. Before/After: Directory Selection

Simply primary click on any file, now primary click on any dir. That
previous file is still marked.

<img height="170"
src="https://github.com/user-attachments/assets/9f1948ce-7445-4377-9733-06490ed6a324"
/>
<img height="170"
src="https://github.com/user-attachments/assets/e78203bc-96ba-424b-b588-c038992a9f0e"
/>

---
4. Before/After: Copy Path and Copy Path Relative

Upon `copy path` (ctrl + alt + c):

Before: Only `/home/tims/test/dir2/a.txt` was copied.
After: All three paths `/home/tims/test/dir2`, `/home/tims/test/c.txt`
and `/home/tims/test/dir2/a.txt` are copied.

This is also how VSCode also copies path when multiple are selected.

<img height="170"
src="https://github.com/user-attachments/assets/e20423ea-1682-4efd-b208-631e2edd3771"
/>
This commit is contained in:
tims 2024-11-27 04:53:01 +05:30 committed by GitHub
parent 57e4540759
commit d75d34576a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -1185,7 +1185,7 @@ impl ProjectPanel {
fn remove(&mut self, trash: bool, skip_prompt: bool, cx: &mut ViewContext<'_, ProjectPanel>) { fn remove(&mut self, trash: bool, skip_prompt: bool, cx: &mut ViewContext<'_, ProjectPanel>) {
maybe!({ maybe!({
let items_to_delete = self.disjoint_entries_for_removal(cx); let items_to_delete = self.disjoint_entries(cx);
if items_to_delete.is_empty() { if items_to_delete.is_empty() {
return None; return None;
} }
@ -1546,7 +1546,7 @@ impl ProjectPanel {
} }
fn cut(&mut self, _: &Cut, cx: &mut ViewContext<Self>) { fn cut(&mut self, _: &Cut, cx: &mut ViewContext<Self>) {
let entries = self.marked_entries(); let entries = self.disjoint_entries(cx);
if !entries.is_empty() { if !entries.is_empty() {
self.clipboard = Some(ClipboardEntry::Cut(entries)); self.clipboard = Some(ClipboardEntry::Cut(entries));
cx.notify(); cx.notify();
@ -1554,7 +1554,7 @@ impl ProjectPanel {
} }
fn copy(&mut self, _: &Copy, cx: &mut ViewContext<Self>) { fn copy(&mut self, _: &Copy, cx: &mut ViewContext<Self>) {
let entries = self.marked_entries(); let entries = self.disjoint_entries(cx);
if !entries.is_empty() { if !entries.is_empty() {
self.clipboard = Some(ClipboardEntry::Copied(entries)); self.clipboard = Some(ClipboardEntry::Copied(entries));
cx.notify(); cx.notify();
@ -1928,7 +1928,7 @@ impl ProjectPanel {
None None
} }
fn disjoint_entries_for_removal(&self, cx: &AppContext) -> BTreeSet<SelectedEntry> { fn disjoint_entries(&self, cx: &AppContext) -> BTreeSet<SelectedEntry> {
let marked_entries = self.marked_entries(); let marked_entries = self.marked_entries();
let mut sanitized_entries = BTreeSet::new(); let mut sanitized_entries = BTreeSet::new();
if marked_entries.is_empty() { if marked_entries.is_empty() {
@ -1976,25 +1976,25 @@ impl ProjectPanel {
sanitized_entries sanitized_entries
} }
// Returns list of entries that should be affected by an operation. // Returns the union of the currently selected entry and all marked entries.
// When currently selected entry is not marked, it's treated as the only marked entry.
fn marked_entries(&self) -> BTreeSet<SelectedEntry> { fn marked_entries(&self) -> BTreeSet<SelectedEntry> {
let Some(mut selection) = self.selection else { let mut entries = self
return Default::default(); .marked_entries
}; .iter()
if self.marked_entries.contains(&selection) { .map(|entry| SelectedEntry {
self.marked_entries entry_id: self.resolve_entry(entry.entry_id),
.iter() worktree_id: entry.worktree_id,
.copied() })
.map(|mut entry| { .collect::<BTreeSet<_>>();
entry.entry_id = self.resolve_entry(entry.entry_id);
entry if let Some(selection) = self.selection {
}) entries.insert(SelectedEntry {
.collect() entry_id: self.resolve_entry(selection.entry_id),
} else { worktree_id: selection.worktree_id,
selection.entry_id = self.resolve_entry(selection.entry_id); });
BTreeSet::from_iter([selection])
} }
entries
} }
/// Finds the currently selected subentry for a given leaf entry id. If a given entry /// Finds the currently selected subentry for a given leaf entry id. If a given entry
@ -2915,6 +2915,7 @@ impl ProjectPanel {
this.marked_entries.remove(&selection); this.marked_entries.remove(&selection);
} }
} else if kind.is_dir() { } else if kind.is_dir() {
this.marked_entries.clear();
this.toggle_expanded(entry_id, cx); this.toggle_expanded(entry_id, cx);
} else { } else {
let preview_tabs_enabled = PreviewTabsSettings::get_global(cx).enabled; let preview_tabs_enabled = PreviewTabsSettings::get_global(cx).enabled;
@ -3051,7 +3052,8 @@ impl ProjectPanel {
.single_line() .single_line()
.color(filename_text_color) .color(filename_text_color)
.when( .when(
is_active && index == active_index, index == active_index
&& (is_active || is_marked),
|this| this.underline(true), |this| this.underline(true),
), ),
); );
@ -5177,6 +5179,163 @@ mod tests {
); );
} }
#[gpui::test]
async fn test_copy_paste_directory_with_sibling_file(cx: &mut gpui::TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.executor().clone());
fs.insert_tree(
"/test",
json!({
"dir1": {
"a.txt": "",
"b.txt": "",
},
"dir2": {},
"c.txt": "",
"d.txt": "",
}),
)
.await;
let project = Project::test(fs.clone(), ["/test".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, "test/dir1", cx);
cx.simulate_modifiers_change(gpui::Modifiers {
control: true,
..Default::default()
});
select_path_with_mark(&panel, "test/dir1", cx);
select_path_with_mark(&panel, "test/c.txt", cx);
assert_eq!(
visible_entries_as_strings(&panel, 0..15, cx),
&[
"v test",
" v dir1 <== marked",
" a.txt",
" b.txt",
" > dir2",
" c.txt <== selected <== marked",
" d.txt",
],
"Initial state before copying dir1 and c.txt"
);
panel.update(cx, |panel, cx| {
panel.copy(&Default::default(), cx);
});
select_path(&panel, "test/dir2", cx);
panel.update(cx, |panel, cx| {
panel.paste(&Default::default(), cx);
});
cx.executor().run_until_parked();
toggle_expand_dir(&panel, "test/dir2/dir1", cx);
assert_eq!(
visible_entries_as_strings(&panel, 0..15, cx),
&[
"v test",
" v dir1 <== marked",
" a.txt",
" b.txt",
" v dir2",
" v dir1 <== selected",
" a.txt",
" b.txt",
" c.txt",
" c.txt <== marked",
" d.txt",
],
"Should copy dir1 as well as c.txt into dir2"
);
}
#[gpui::test]
async fn test_copy_paste_nested_and_root_entries(cx: &mut gpui::TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.executor().clone());
fs.insert_tree(
"/test",
json!({
"dir1": {
"a.txt": "",
"b.txt": "",
},
"dir2": {},
"c.txt": "",
"d.txt": "",
}),
)
.await;
let project = Project::test(fs.clone(), ["/test".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, "test/dir1", cx);
cx.simulate_modifiers_change(gpui::Modifiers {
control: true,
..Default::default()
});
select_path_with_mark(&panel, "test/dir1/a.txt", cx);
select_path_with_mark(&panel, "test/dir1", cx);
select_path_with_mark(&panel, "test/c.txt", cx);
assert_eq!(
visible_entries_as_strings(&panel, 0..15, cx),
&[
"v test",
" v dir1 <== marked",
" a.txt <== marked",
" b.txt",
" > dir2",
" c.txt <== selected <== marked",
" d.txt",
],
"Initial state before copying a.txt, dir1 and c.txt"
);
panel.update(cx, |panel, cx| {
panel.copy(&Default::default(), cx);
});
select_path(&panel, "test/dir2", cx);
panel.update(cx, |panel, cx| {
panel.paste(&Default::default(), cx);
});
cx.executor().run_until_parked();
toggle_expand_dir(&panel, "test/dir2/dir1", cx);
assert_eq!(
visible_entries_as_strings(&panel, 0..20, cx),
&[
"v test",
" v dir1 <== marked",
" a.txt <== marked",
" b.txt",
" v dir2",
" v dir1 <== selected",
" a.txt",
" b.txt",
" c.txt",
" c.txt <== marked",
" d.txt",
],
"Should copy dir1 and c.txt into dir2. a.txt is already present in copied dir1."
);
}
#[gpui::test] #[gpui::test]
async fn test_remove_opened_file(cx: &mut gpui::TestAppContext) { async fn test_remove_opened_file(cx: &mut gpui::TestAppContext) {
init_test_with_editor(cx); init_test_with_editor(cx);