Implement dragging external files to remote projects (#28987)

Max Brunsfeld and Peter Tripp created

Release Notes:

- Added the ability to copy external files into remote projects by
dragging them onto the project panel.

---------

Co-authored-by: Peter Tripp <petertripp@gmail.com>

Change summary

crates/fs/src/fs.rs                              |  32 ++++
crates/project/src/project.rs                    |   2 
crates/project_panel/src/project_panel.rs        | 108 ++++++++---------
crates/proto/proto/worktree.proto                |   1 
crates/remote_server/src/remote_editing_tests.rs | 110 ++++++++++++++++++
crates/terminal_view/src/terminal_view.rs        |   6 
crates/worktree/src/worktree.rs                  |  86 +++++++++++--
crates/worktree/src/worktree_tests.rs            |  12 
8 files changed, 275 insertions(+), 82 deletions(-)

Detailed changes

crates/fs/src/fs.rs 🔗

@@ -111,6 +111,7 @@ pub trait Fs: Send + Sync {
     async fn load_bytes(&self, path: &Path) -> Result<Vec<u8>>;
     async fn atomic_write(&self, path: PathBuf, text: String) -> Result<()>;
     async fn save(&self, path: &Path, text: &Rope, line_ending: LineEnding) -> Result<()>;
+    async fn write(&self, path: &Path, content: &[u8]) -> Result<()>;
     async fn canonicalize(&self, path: &Path) -> Result<PathBuf>;
     async fn is_file(&self, path: &Path) -> bool;
     async fn is_dir(&self, path: &Path) -> bool;
@@ -567,6 +568,14 @@ impl Fs for RealFs {
         Ok(())
     }
 
+    async fn write(&self, path: &Path, content: &[u8]) -> Result<()> {
+        if let Some(path) = path.parent() {
+            self.create_dir(path).await?;
+        }
+        smol::fs::write(path, content).await?;
+        Ok(())
+    }
+
     async fn canonicalize(&self, path: &Path) -> Result<PathBuf> {
         Ok(smol::fs::canonicalize(path).await?)
     }
@@ -2105,6 +2114,16 @@ impl Fs for FakeFs {
         Ok(())
     }
 
+    async fn write(&self, path: &Path, content: &[u8]) -> Result<()> {
+        self.simulate_random_delay().await;
+        let path = normalize_path(path);
+        if let Some(path) = path.parent() {
+            self.create_dir(path).await?;
+        }
+        self.write_file_internal(path, content.to_vec(), false)?;
+        Ok(())
+    }
+
     async fn canonicalize(&self, path: &Path) -> Result<PathBuf> {
         let path = normalize_path(path);
         self.simulate_random_delay().await;
@@ -2346,7 +2365,7 @@ pub async fn copy_recursive<'a>(
     target: &'a Path,
     options: CopyOptions,
 ) -> Result<()> {
-    for (is_dir, item) in read_dir_items(fs, source).await? {
+    for (item, is_dir) in read_dir_items(fs, source).await? {
         let Ok(item_relative_path) = item.strip_prefix(source) else {
             continue;
         };
@@ -2380,7 +2399,10 @@ pub async fn copy_recursive<'a>(
     Ok(())
 }
 
-async fn read_dir_items<'a>(fs: &'a dyn Fs, source: &'a Path) -> Result<Vec<(bool, PathBuf)>> {
+/// Recursively reads all of the paths in the given directory.
+///
+/// Returns a vector of tuples of (path, is_dir).
+pub async fn read_dir_items<'a>(fs: &'a dyn Fs, source: &'a Path) -> Result<Vec<(PathBuf, bool)>> {
     let mut items = Vec::new();
     read_recursive(fs, source, &mut items).await?;
     Ok(items)
@@ -2389,7 +2411,7 @@ async fn read_dir_items<'a>(fs: &'a dyn Fs, source: &'a Path) -> Result<Vec<(boo
 fn read_recursive<'a>(
     fs: &'a dyn Fs,
     source: &'a Path,
-    output: &'a mut Vec<(bool, PathBuf)>,
+    output: &'a mut Vec<(PathBuf, bool)>,
 ) -> BoxFuture<'a, Result<()>> {
     use futures::future::FutureExt;
 
@@ -2400,7 +2422,7 @@ fn read_recursive<'a>(
             .ok_or_else(|| anyhow!("path does not exist: {}", source.display()))?;
 
         if metadata.is_dir {
-            output.push((true, source.to_path_buf()));
+            output.push((source.to_path_buf(), true));
             let mut children = fs.read_dir(source).await?;
             while let Some(child_path) = children.next().await {
                 if let Ok(child_path) = child_path {
@@ -2408,7 +2430,7 @@ fn read_recursive<'a>(
                 }
             }
         } else {
-            output.push((false, source.to_path_buf()));
+            output.push((source.to_path_buf(), false));
         }
         Ok(())
     }

crates/project/src/project.rs 🔗

@@ -1882,7 +1882,7 @@ impl Project {
             ))));
         };
         worktree.update(cx, |worktree, cx| {
-            worktree.create_entry(project_path.path, is_directory, cx)
+            worktree.create_entry(project_path.path, is_directory, None, cx)
         })
     }
 

crates/project_panel/src/project_panel.rs 🔗

@@ -2983,16 +2983,18 @@ impl ProjectPanel {
 
         let open_file_after_drop = paths.len() == 1 && paths[0].is_file();
 
-        let Some((target_directory, worktree)) = maybe!({
-            let worktree = self.project.read(cx).worktree_for_entry(entry_id, cx)?;
+        let Some((target_directory, worktree, fs)) = maybe!({
+            let project = self.project.read(cx);
+            let fs = project.fs().clone();
+            let worktree = project.worktree_for_entry(entry_id, cx)?;
             let entry = worktree.read(cx).entry_for_id(entry_id)?;
-            let path = worktree.read(cx).absolutize(&entry.path).ok()?;
-            let target_directory = if path.is_dir() {
-                path
+            let path = entry.path.clone();
+            let target_directory = if entry.is_dir() {
+                path.to_path_buf()
             } else {
                 path.parent()?.to_path_buf()
             };
-            Some((target_directory, worktree))
+            Some((target_directory, worktree, fs))
         }) else {
             return;
         };
@@ -3034,10 +3036,10 @@ impl ProjectPanel {
                 }
 
                 let task = worktree.update( cx, |worktree, cx| {
-                    worktree.copy_external_entries(target_directory, paths, true, cx)
+                    worktree.copy_external_entries(target_directory.into(), paths, fs, cx)
                 })?;
 
-                let opened_entries = task.await?;
+                let opened_entries = task.await.with_context(|| "failed to copy external paths")?;
                 this.update(cx, |this, cx| {
                     if open_file_after_drop && !opened_entries.is_empty() {
                         this.open_entry(opened_entries[0], true, false, cx);
@@ -3697,7 +3699,6 @@ impl ProjectPanel {
         let depth = details.depth;
         let worktree_id = details.worktree_id;
         let selections = Arc::new(self.marked_entries.clone());
-        let is_local = self.project.read(cx).is_local();
 
         let dragged_selection = DraggedSelection {
             active_selection: selection,
@@ -3762,60 +3763,57 @@ impl ProjectPanel {
             .border_r_2()
             .border_color(border_color)
             .hover(|style| style.bg(bg_hover_color).border_color(border_hover_color))
-            .when(is_local, |div| {
-                div.on_drag_move::<ExternalPaths>(cx.listener(
-                    move |this, event: &DragMoveEvent<ExternalPaths>, _, cx| {
-                        if event.bounds.contains(&event.event.position) {
-                            if this.last_external_paths_drag_over_entry == Some(entry_id) {
-                                return;
-                            }
-                            this.last_external_paths_drag_over_entry = Some(entry_id);
-                            this.marked_entries.clear();
+            .on_drag_move::<ExternalPaths>(cx.listener(
+                move |this, event: &DragMoveEvent<ExternalPaths>, _, cx| {
+                    if event.bounds.contains(&event.event.position) {
+                        if this.last_external_paths_drag_over_entry == Some(entry_id) {
+                            return;
+                        }
+                        this.last_external_paths_drag_over_entry = Some(entry_id);
+                        this.marked_entries.clear();
 
-                            let Some((worktree, path, entry)) = maybe!({
-                                let worktree = this
-                                    .project
-                                    .read(cx)
-                                    .worktree_for_id(selection.worktree_id, cx)?;
-                                let worktree = worktree.read(cx);
-                                let abs_path = worktree.absolutize(&path).log_err()?;
-                                let path = if abs_path.is_dir() {
-                                    path.as_ref()
-                                } else {
-                                    path.parent()?
-                                };
-                                let entry = worktree.entry_for_path(path)?;
-                                Some((worktree, path, entry))
-                            }) else {
-                                return;
+                        let Some((worktree, path, entry)) = maybe!({
+                            let worktree = this
+                                .project
+                                .read(cx)
+                                .worktree_for_id(selection.worktree_id, cx)?;
+                            let worktree = worktree.read(cx);
+                            let entry = worktree.entry_for_path(&path)?;
+                            let path = if entry.is_dir() {
+                                path.as_ref()
+                            } else {
+                                path.parent()?
                             };
+                            Some((worktree, path, entry))
+                        }) else {
+                            return;
+                        };
 
+                        this.marked_entries.insert(SelectedEntry {
+                            entry_id: entry.id,
+                            worktree_id: worktree.id(),
+                        });
+
+                        for entry in worktree.child_entries(path) {
                             this.marked_entries.insert(SelectedEntry {
                                 entry_id: entry.id,
                                 worktree_id: worktree.id(),
                             });
-
-                            for entry in worktree.child_entries(path) {
-                                this.marked_entries.insert(SelectedEntry {
-                                    entry_id: entry.id,
-                                    worktree_id: worktree.id(),
-                                });
-                            }
-
-                            cx.notify();
                         }
-                    },
-                ))
-                .on_drop(cx.listener(
-                    move |this, external_paths: &ExternalPaths, window, cx| {
-                        this.hover_scroll_task.take();
-                        this.last_external_paths_drag_over_entry = None;
-                        this.marked_entries.clear();
-                        this.drop_external_files(external_paths.paths(), entry_id, window, cx);
-                        cx.stop_propagation();
-                    },
-                ))
-            })
+
+                        cx.notify();
+                    }
+                },
+            ))
+            .on_drop(cx.listener(
+                move |this, external_paths: &ExternalPaths, window, cx| {
+                    this.hover_scroll_task.take();
+                    this.last_external_paths_drag_over_entry = None;
+                    this.marked_entries.clear();
+                    this.drop_external_files(external_paths.paths(), entry_id, window, cx);
+                    cx.stop_propagation();
+                },
+            ))
             .on_drag_move::<DraggedSelection>(cx.listener(
                 move |this, event: &DragMoveEvent<DraggedSelection>, window, cx| {
                     if event.bounds.contains(&event.event.position) {

crates/proto/proto/worktree.proto 🔗

@@ -91,6 +91,7 @@ message CreateProjectEntry {
     uint64 worktree_id = 2;
     string path = 3;
     bool is_directory = 4;
+    optional bytes content = 5;
 }
 
 message RenameProjectEntry {

crates/remote_server/src/remote_editing_tests.rs 🔗

@@ -1204,6 +1204,116 @@ async fn test_remote_rename_entry(cx: &mut TestAppContext, server_cx: &mut TestA
     });
 }
 
+#[gpui::test]
+async fn test_copy_file_into_remote_project(
+    cx: &mut TestAppContext,
+    server_cx: &mut TestAppContext,
+) {
+    let remote_fs = FakeFs::new(server_cx.executor());
+    remote_fs
+        .insert_tree(
+            path!("/code"),
+            json!({
+                "project1": {
+                    ".git": {},
+                    "README.md": "# project 1",
+                    "src": {
+                        "main.rs": ""
+                    }
+                },
+            }),
+        )
+        .await;
+
+    let (project, _) = init_test(&remote_fs, cx, server_cx).await;
+    let (worktree, _) = project
+        .update(cx, |project, cx| {
+            project.find_or_create_worktree(path!("/code/project1"), true, cx)
+        })
+        .await
+        .unwrap();
+
+    cx.run_until_parked();
+
+    let local_fs = project
+        .read_with(cx, |project, _| project.fs().clone())
+        .as_fake();
+    local_fs
+        .insert_tree(
+            path!("/local-code"),
+            json!({
+                "dir1": {
+                    "file1": "file 1 content",
+                    "dir2": {
+                        "file2": "file 2 content",
+                        "dir3": {
+                            "file3": ""
+                        },
+                        "dir4": {}
+                    },
+                    "dir5": {}
+                },
+                "file4": "file 4 content"
+            }),
+        )
+        .await;
+
+    worktree
+        .update(cx, |worktree, cx| {
+            worktree.copy_external_entries(
+                Path::new("src").into(),
+                vec![
+                    Path::new(path!("/local-code/dir1/file1")).into(),
+                    Path::new(path!("/local-code/dir1/dir2")).into(),
+                ],
+                local_fs.clone(),
+                cx,
+            )
+        })
+        .await
+        .unwrap();
+
+    assert_eq!(
+        remote_fs.paths(true),
+        vec![
+            PathBuf::from(path!("/")),
+            PathBuf::from(path!("/code")),
+            PathBuf::from(path!("/code/project1")),
+            PathBuf::from(path!("/code/project1/.git")),
+            PathBuf::from(path!("/code/project1/README.md")),
+            PathBuf::from(path!("/code/project1/src")),
+            PathBuf::from(path!("/code/project1/src/dir2")),
+            PathBuf::from(path!("/code/project1/src/file1")),
+            PathBuf::from(path!("/code/project1/src/main.rs")),
+            PathBuf::from(path!("/code/project1/src/dir2/dir3")),
+            PathBuf::from(path!("/code/project1/src/dir2/dir4")),
+            PathBuf::from(path!("/code/project1/src/dir2/file2")),
+            PathBuf::from(path!("/code/project1/src/dir2/dir3/file3")),
+        ]
+    );
+    assert_eq!(
+        remote_fs
+            .load(path!("/code/project1/src/file1").as_ref())
+            .await
+            .unwrap(),
+        "file 1 content"
+    );
+    assert_eq!(
+        remote_fs
+            .load(path!("/code/project1/src/dir2/file2").as_ref())
+            .await
+            .unwrap(),
+        "file 2 content"
+    );
+    assert_eq!(
+        remote_fs
+            .load(path!("/code/project1/src/dir2/dir3/file3").as_ref())
+            .await
+            .unwrap(),
+        ""
+    );
+}
+
 // TODO: this test fails on Windows.
 #[cfg(not(windows))]
 #[gpui::test]

crates/terminal_view/src/terminal_view.rs 🔗

@@ -1944,7 +1944,11 @@ mod tests {
             .unwrap();
 
         let entry = cx
-            .update(|cx| wt.update(cx, |wt, cx| wt.create_entry(Path::new(""), is_dir, cx)))
+            .update(|cx| {
+                wt.update(cx, |wt, cx| {
+                    wt.create_entry(Path::new(""), is_dir, None, cx)
+                })
+            })
             .await
             .unwrap()
             .to_included()

crates/worktree/src/worktree.rs 🔗

@@ -7,7 +7,7 @@ use ::ignore::gitignore::{Gitignore, GitignoreBuilder};
 use anyhow::{Context as _, Result, anyhow};
 use clock::ReplicaId;
 use collections::{HashMap, HashSet, VecDeque};
-use fs::{Fs, MTime, PathEvent, RemoveOptions, Watcher, copy_recursive};
+use fs::{Fs, MTime, PathEvent, RemoveOptions, Watcher, copy_recursive, read_dir_items};
 use futures::{
     FutureExt as _, Stream, StreamExt,
     channel::{
@@ -847,18 +847,20 @@ impl Worktree {
         &mut self,
         path: impl Into<Arc<Path>>,
         is_directory: bool,
+        content: Option<Vec<u8>>,
         cx: &Context<Worktree>,
     ) -> Task<Result<CreatedEntry>> {
         let path: Arc<Path> = path.into();
         let worktree_id = self.id();
         match self {
-            Worktree::Local(this) => this.create_entry(path, is_directory, cx),
+            Worktree::Local(this) => this.create_entry(path, is_directory, content, cx),
             Worktree::Remote(this) => {
                 let project_id = this.project_id;
                 let request = this.client.request(proto::CreateProjectEntry {
                     worktree_id: worktree_id.to_proto(),
                     project_id,
                     path: path.as_ref().to_proto(),
+                    content,
                     is_directory,
                 });
                 cx.spawn(async move |this, cx| {
@@ -979,18 +981,14 @@ impl Worktree {
 
     pub fn copy_external_entries(
         &mut self,
-        target_directory: PathBuf,
+        target_directory: Arc<Path>,
         paths: Vec<Arc<Path>>,
-        overwrite_existing_files: bool,
+        fs: Arc<dyn Fs>,
         cx: &Context<Worktree>,
     ) -> Task<Result<Vec<ProjectEntryId>>> {
         match self {
-            Worktree::Local(this) => {
-                this.copy_external_entries(target_directory, paths, overwrite_existing_files, cx)
-            }
-            _ => Task::ready(Err(anyhow!(
-                "Copying external entries is not supported for remote worktrees"
-            ))),
+            Worktree::Local(this) => this.copy_external_entries(target_directory, paths, cx),
+            Worktree::Remote(this) => this.copy_external_entries(target_directory, paths, fs, cx),
         }
     }
 
@@ -1057,6 +1055,7 @@ impl Worktree {
                 this.create_entry(
                     Arc::<Path>::from_proto(request.path),
                     request.is_directory,
+                    request.content,
                     cx,
                 ),
             )
@@ -1585,6 +1584,7 @@ impl LocalWorktree {
         &self,
         path: impl Into<Arc<Path>>,
         is_dir: bool,
+        content: Option<Vec<u8>>,
         cx: &Context<Worktree>,
     ) -> Task<Result<CreatedEntry>> {
         let path = path.into();
@@ -1601,7 +1601,7 @@ impl LocalWorktree {
                     .await
                     .with_context(|| format!("creating directory {task_abs_path:?}"))
             } else {
-                fs.save(&task_abs_path, &Rope::default(), LineEnding::default())
+                fs.write(&task_abs_path, content.as_deref().unwrap_or(&[]))
                     .await
                     .with_context(|| format!("creating file {task_abs_path:?}"))
             }
@@ -1877,11 +1877,13 @@ impl LocalWorktree {
 
     pub fn copy_external_entries(
         &self,
-        target_directory: PathBuf,
+        target_directory: Arc<Path>,
         paths: Vec<Arc<Path>>,
-        overwrite_existing_files: bool,
         cx: &Context<Worktree>,
     ) -> Task<Result<Vec<ProjectEntryId>>> {
+        let Ok(target_directory) = self.absolutize(&target_directory) else {
+            return Task::ready(Err(anyhow!("invalid target path")));
+        };
         let worktree_path = self.abs_path().clone();
         let fs = self.fs.clone();
         let paths = paths
@@ -1913,7 +1915,7 @@ impl LocalWorktree {
                         &source,
                         &target,
                         fs::CopyOptions {
-                            overwrite: overwrite_existing_files,
+                            overwrite: true,
                             ..Default::default()
                         },
                     )
@@ -2283,6 +2285,62 @@ impl RemoteWorktree {
             }
         })
     }
+
+    fn copy_external_entries(
+        &self,
+        target_directory: Arc<Path>,
+        paths_to_copy: Vec<Arc<Path>>,
+        local_fs: Arc<dyn Fs>,
+        cx: &Context<Worktree>,
+    ) -> Task<Result<Vec<ProjectEntryId>, anyhow::Error>> {
+        let client = self.client.clone();
+        let worktree_id = self.id().to_proto();
+        let project_id = self.project_id;
+
+        cx.background_spawn(async move {
+            let mut requests = Vec::new();
+            for root_path_to_copy in paths_to_copy {
+                let Some(filename) = root_path_to_copy.file_name() else {
+                    continue;
+                };
+                for (abs_path, is_directory) in
+                    read_dir_items(local_fs.as_ref(), &root_path_to_copy).await?
+                {
+                    let Ok(relative_path) = abs_path.strip_prefix(&root_path_to_copy) else {
+                        continue;
+                    };
+                    let content = if is_directory {
+                        None
+                    } else {
+                        Some(local_fs.load_bytes(&abs_path).await?)
+                    };
+
+                    let mut target_path = target_directory.join(filename);
+                    if relative_path.file_name().is_some() {
+                        target_path.push(relative_path)
+                    }
+
+                    requests.push(proto::CreateProjectEntry {
+                        project_id,
+                        worktree_id,
+                        path: target_path.to_string_lossy().to_string(),
+                        is_directory,
+                        content,
+                    });
+                }
+            }
+            requests.sort_unstable_by(|a, b| a.path.cmp(&b.path));
+            requests.dedup();
+
+            let mut copied_entry_ids = Vec::new();
+            for request in requests {
+                let response = client.request(request).await?;
+                copied_entry_ids.extend(response.entry.map(|e| ProjectEntryId::from_proto(e.id)));
+            }
+
+            Ok(copied_entry_ids)
+        })
+    }
 }
 
 impl Snapshot {

crates/worktree/src/worktree_tests.rs 🔗

@@ -1270,7 +1270,7 @@ async fn test_create_directory_during_initial_scan(cx: &mut TestAppContext) {
         .update(cx, |tree, cx| {
             tree.as_local_mut()
                 .unwrap()
-                .create_entry("a/e".as_ref(), true, cx)
+                .create_entry("a/e".as_ref(), true, None, cx)
         })
         .await
         .unwrap()
@@ -1319,7 +1319,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) {
         .update(cx, |tree, cx| {
             tree.as_local_mut()
                 .unwrap()
-                .create_entry("a/b/c/d.txt".as_ref(), false, cx)
+                .create_entry("a/b/c/d.txt".as_ref(), false, None, cx)
         })
         .await
         .unwrap()
@@ -1353,7 +1353,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) {
         .update(cx, |tree, cx| {
             tree.as_local_mut()
                 .unwrap()
-                .create_entry("a/b/c/d.txt".as_ref(), false, cx)
+                .create_entry("a/b/c/d.txt".as_ref(), false, None, cx)
         })
         .await
         .unwrap()
@@ -1373,7 +1373,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) {
         .update(cx, |tree, cx| {
             tree.as_local_mut()
                 .unwrap()
-                .create_entry("a/b/c/e.txt".as_ref(), false, cx)
+                .create_entry("a/b/c/e.txt".as_ref(), false, None, cx)
         })
         .await
         .unwrap()
@@ -1391,7 +1391,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) {
         .update(cx, |tree, cx| {
             tree.as_local_mut()
                 .unwrap()
-                .create_entry("d/e/f/g.txt".as_ref(), false, cx)
+                .create_entry("d/e/f/g.txt".as_ref(), false, None, cx)
         })
         .await
         .unwrap()
@@ -1739,7 +1739,7 @@ fn randomly_mutate_worktree(
                     if is_dir { "dir" } else { "file" },
                     child_path,
                 );
-                let task = worktree.create_entry(child_path, is_dir, cx);
+                let task = worktree.create_entry(child_path, is_dir, None, cx);
                 cx.background_spawn(async move {
                     task.await?;
                     Ok(())