From 7e928dd615b7797f68001c1cb13639eff314b580 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 17 Apr 2025 11:06:56 -0700 Subject: [PATCH] Implement dragging external files to remote projects (#28987) Release Notes: - Added the ability to copy external files into remote projects by dragging them onto the project panel. --------- Co-authored-by: Peter Tripp --- 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 + .../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(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 7e78f513e3..93cd1eb593 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -111,6 +111,7 @@ pub trait Fs: Send + Sync { async fn load_bytes(&self, path: &Path) -> Result>; 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; 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 { 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 { 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> { +/// 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> { 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( 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(()) } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 34c4d001f4..218ba89333 100644 --- a/crates/project/src/project.rs +++ b/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) }) } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index b1d4df8a4c..3abc19aee4 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/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::(cx.listener( - move |this, event: &DragMoveEvent, _, 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::(cx.listener( + move |this, event: &DragMoveEvent, _, 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::(cx.listener( move |this, event: &DragMoveEvent, window, cx| { if event.bounds.contains(&event.event.position) { diff --git a/crates/proto/proto/worktree.proto b/crates/proto/proto/worktree.proto index 418f908d7a..0650e0a491 100644 --- a/crates/proto/proto/worktree.proto +++ b/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 { diff --git a/crates/remote_server/src/remote_editing_tests.rs b/crates/remote_server/src/remote_editing_tests.rs index bb65156658..b5c825e7e1 100644 --- a/crates/remote_server/src/remote_editing_tests.rs +++ b/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] diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 4940b655aa..76d5a26f33 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/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() diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index c85fc4e930..38e39fd774 100644 --- a/crates/worktree/src/worktree.rs +++ b/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>, is_directory: bool, + content: Option>, cx: &Context, ) -> Task> { let path: Arc = 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, paths: Vec>, - overwrite_existing_files: bool, + fs: Arc, cx: &Context, ) -> Task>> { 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::::from_proto(request.path), request.is_directory, + request.content, cx, ), ) @@ -1585,6 +1584,7 @@ impl LocalWorktree { &self, path: impl Into>, is_dir: bool, + content: Option>, cx: &Context, ) -> Task> { 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, paths: Vec>, - overwrite_existing_files: bool, cx: &Context, ) -> Task>> { + 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, + paths_to_copy: Vec>, + local_fs: Arc, + cx: &Context, + ) -> Task, 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 { diff --git a/crates/worktree/src/worktree_tests.rs b/crates/worktree/src/worktree_tests.rs index 21a67e3b49..c3f56af4c6 100644 --- a/crates/worktree/src/worktree_tests.rs +++ b/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(())