From 1df43b72b5a795deb0d372fd09bb6bdcd7371dc8 Mon Sep 17 00:00:00 2001 From: l0ng-ai Date: Mon, 10 Aug 2026 12:41:17 +0800 Subject: [PATCH] feat(files): copy dropped files into the folder they were dropped on (#458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(files): copy dropped files into the folder they were dropped on The Files panel has only ever been a drag *source* — a row dragged into a terminal inserts its path. Nothing on the tree ever registered a drop, so a file dragged in from the desktop did nothing at all, not even a highlight. Closes #453. The drop is the whole gesture: files land where the cursor was, not somewhere a dialog asks about afterwards. A folder row takes them itself, a file row stands in for the folder holding it — "next to this one" — and the space the rows do not cover belongs to the top of the tree. The placeholder inside an empty folder takes a drop too; it is the only thing drawn there, and letting it fall through to the root would put files somewhere the cursor never was. A row under the cursor wins over the column, which is what gpui's innermost-first dispatch already does. The copy itself goes through the `Host` the tree is listing, so a remote workspace reads here and writes there. Locally it is `fs::copy`, which is what keeps the executable bit that `write_file` would drop; remotely the bytes ride one control frame, and a file too big for that is refused with the advice to use SFTP rather than half-sent. Names already taken are asked about before anything is written, and the answer governs the whole drop — a half-done copy would have to be undone to honour a "no". Replacing a folder replaces it rather than merging into it. A drag let go where it started is a miss, not an error, so it says nothing. * fix(sftp): list the directory again once an upload lands An upload is written to `.tty7-upload-` and renamed into place at the very end. The browser listed the directory the moment the transfer was handed to the daemon, so it caught that temporary name — and nothing ever listed again, so a finished upload sat on screen as a file with a hash glued to its name until the directory was navigated by hand. The premature listing is gone, and the panel now remembers the job ids it started: once one stops running — done, failed, cancelled, or dropped off the job list entirely — the directory is listed once more. Two uploads in flight settle independently, so the second one finishing does not depend on the first. * docs(changelog): note the SFTP upload listing fix * ci(host-boundary): allow the source side of a file drop, and stop scanning two files as empty The Files panel now copies dropped files in, and what the desktop hands over is by construction a path on the desktop's own machine: reading it is a local read even when the tree being dropped on is remote. The destination side goes through `Host`, and the one `std::fs::copy` that touches a destination sits inside a branch already gated on `host.id().is_local()`. While adding that entry: `attr` starts unset, which awk reads as 0, so a file whose first line is `mod something` matched `attr == NR - 1` and cut its body at line 0. `head -n -1` then errored and the file was scanned as empty — `src/terminal/mod.rs` and `src/ui/tray/mod.rs` both open that way, and the guard had been blind to both. Neither contains a violation, so seeing them is free. --- .github/scripts/check-host-boundary.sh | 12 + CHANGELOG.md | 20 ++ src/ui/file_copy.rs | 374 +++++++++++++++++++++++++ src/ui/file_tree.rs | 267 +++++++++++++++++- src/ui/i18n/en.rs | 12 + src/ui/i18n/ja.rs | 12 + src/ui/i18n/mod.rs | 10 + src/ui/i18n/zh.rs | 10 + src/ui/mod.rs | 1 + src/ui/sftp.rs | 114 +++++++- 10 files changed, 818 insertions(+), 14 deletions(-) create mode 100644 src/ui/file_copy.rs diff --git a/.github/scripts/check-host-boundary.sh b/.github/scripts/check-host-boundary.sh index b1dc7216..642c173b 100755 --- a/.github/scripts/check-host-boundary.sh +++ b/.github/scripts/check-host-boundary.sh @@ -84,6 +84,13 @@ src/terminal/search.rs|.is_absolute() # can be handed a path. Always `std::env::temp_dir()` on this machine. src/terminal/view.rs|std::fs::create_dir_all src/terminal/view.rs|std::fs::write + +# The source side of a file drop. What the desktop hands over is by +# construction a path on the desktop's own machine, so reading it is a local +# read even when the tree being dropped on is remote — the destination side of +# that copy goes through `Host`, and the one `std::fs::copy` that touches a +# destination sits inside a branch already gated on `host.id().is_local()`. +src/ui/file_copy.rs|std::fs:: EOF ) @@ -91,7 +98,12 @@ EOF # preserved because only the tail is dropped. body_of() { local file=$1 cut + # `attr` starts unset, which awk reads as 0 — so a file whose *first* line + # is `mod something` used to match `attr == NR - 1` and cut the body at + # line 0, leaving `head -n -1` to error out and the file to be scanned as + # empty. Two files opened that way, and the guard was blind to both. cut=$(awk ' + BEGIN { attr = -1 } /^#\[cfg\(test\)\]$/ { attr = NR } /^mod [A-Za-z_]/ { if (attr == NR - 1) { print NR - 1; exit } } ' "$file") diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f049930..cef5e28e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Drop files into the Files panel to copy them in** — the panel has always + been a drag *source*; it now takes a drop as well. Files dragged in from the + desktop land in the folder under the cursor: a folder row takes them itself, + a file row stands in for the folder holding it, and the space below the tree + belongs to the top of it. The landing is highlighted while the drag is in + flight. Folders come in whole, the executable bit survives the copy, and a + name already taken is asked about rather than silently replaced — a "no" + leaves every file in the drop untouched. It works over a remote workspace + too, reading here and writing there, up to the size one control frame can + carry; past that the panel says to use SFTP. + - **Rearrange splits by dragging a pane** — hovering a pane now floats a small grip along its top edge; dragging it moves that pane elsewhere in the tab. A drop on a pane's **side** goes in beside it: facing a neighbour in the same @@ -39,6 +50,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **An SFTP upload no longer sits in the browser under its temporary name** — + an upload is written as `.tty7-upload-` and renamed into place at + the end, and the browser listed the directory the moment the transfer + started, catching exactly that name. Nothing listed again afterwards, so a + finished upload read as a file with a hash glued to its name until the + directory was navigated by hand. The premature listing is gone and the + directory is listed once the upload stops running — finished, failed or + cancelled. + - **Drag cursors on Windows** — the grip on a pane's top edge, and a sidebar group being dragged, now change the pointer on Windows too. Win32 ships no open- or closed-hand cursor and gpui answers both with the plain arrow, so diff --git a/src/ui/file_copy.rs b/src/ui/file_copy.rs new file mode 100644 index 00000000..5eba4b81 --- /dev/null +++ b/src/ui/file_copy.rs @@ -0,0 +1,374 @@ +//! Copying dropped files into a directory of the Files panel's tree. +//! +//! The panel has always been a drag *source* — a row can be dragged into a +//! terminal to insert its path. This is the other direction: whatever the +//! desktop (or another row) drops on the tree gets copied into the directory +//! under the cursor. +//! +//! Everything here runs on a `HostOps` worker thread, never on the UI thread, +//! and the destination is reached through the [`Host`] the tree is listing — +//! which is this machine for a local workspace and the far end of a control +//! connection for a remote one. The *sources* are always local paths: a file +//! drop from the desktop can only name files on the desktop's own machine. + +use std::io; +use std::path::{Path, PathBuf}; + +use crate::ui::host_ops::Host; +use crate::ui::i18n::{L10nKey, t, t_fmt}; + +/// How deep a dropped folder is followed before the copy gives up. +/// +/// `metadata` follows symlinks, which is what makes a dropped alias copy the +/// thing it points at rather than a broken link — and what makes a link back +/// up its own tree an infinite walk. This is the floor that walk stops on. +const MAX_DEPTH: usize = 64; + +/// Ceiling on one file copied to a host that is not this machine. +/// +/// `write_file` puts the whole file in a single control frame, alongside the +/// JSON naming the path, and the frame as a whole has to fit in `MAX_FRAME`. +/// The megabyte of slack is for that JSON and the framing around it. +const REMOTE_FILE_MAX: u64 = (crate::daemon::protocol::MAX_FRAME - 1024 * 1024) as u64; + +/// What one drop did, in the terms the panel has to answer in: rows to +/// refresh, names to ask about, failures to report. +#[derive(Default)] +pub(crate) struct DropReport { + pub copied: Vec, + /// Names that already exist in the destination. Non-empty only when the + /// caller asked without `overwrite`, and then nothing at all was written — + /// the answer to "replace?" governs every name in the drop, so a half-done + /// copy would have to be undone to honour a "no". + pub conflicts: Vec, + pub errors: Vec<(String, io::Error)>, +} + +impl DropReport { + fn fail(&mut self, name: &str, message: String) { + self.errors + .push((name.to_string(), io::Error::other(message))); + } +} + +/// Copy `sources` into `dir` on `host`. +/// +/// Two passes: name and vet every source first, then write. That is what lets +/// the panel ask about name collisions before anything has been overwritten. +pub(crate) fn copy_into_dir( + host: &dyn Host, + sources: &[PathBuf], + dir: &Path, + overwrite: bool, +) -> DropReport { + let mut report = DropReport::default(); + let mut planned: Vec<(PathBuf, PathBuf, String)> = Vec::new(); + + for src in sources { + let Some(name) = src.file_name().map(|n| n.to_string_lossy().to_string()) else { + // A root directory, or a path ending in `..`: nothing to name the + // copy after. + continue; + }; + // A row dropped back where it already is: on the folder holding it, or + // — since every row is both a drag source and a drop target — on + // itself, which is where an abandoned drag lands. Both are misses, and + // a miss is not worth a notification. + if src.parent() == Some(dir) || dir == src { + continue; + } + if dir.starts_with(src) { + report.fail(&name, t(L10nKey::FileDropIntoItself).to_string()); + continue; + } + // The drag carries whatever `on_drag` put in it, and a row of a remote + // tree carries a path on the *far* machine. Reading it here would + // either fail or, worse, find a local file of the same name. + if !src.exists() { + report.fail(&name, t(L10nKey::FileDropNotHere).to_string()); + continue; + } + if host.exists(&host.join(dir, &name)) { + report.conflicts.push(name.clone()); + } + planned.push((src.clone(), host.join(dir, &name), name)); + } + + if !overwrite && !report.conflicts.is_empty() { + return report; + } + + for (src, dest, name) in planned { + // Replace rather than merge: a folder dropped onto a folder of the + // same name should end up as what was dropped, not as the union of the + // two, which is what writing into it one file at a time would leave. + if host.exists(&dest) + && let Err(e) = host.remove(&dest, true) + { + report.errors.push((name, e)); + continue; + } + match copy_tree(host, &src, &dest, 0) { + Ok(()) => report.copied.push(name), + Err(e) => report.errors.push((name, e)), + } + } + report +} + +fn copy_tree(host: &dyn Host, src: &Path, dest: &Path, depth: usize) -> io::Result<()> { + if depth > MAX_DEPTH { + return Err(io::Error::other(t_fmt( + L10nKey::FileDropTooDeep, + &[("n", &MAX_DEPTH.to_string())], + ))); + } + let meta = std::fs::metadata(src)?; + if !meta.is_dir() { + return copy_file(host, src, dest, meta.len()); + } + host.create_dir(dest, true)?; + for entry in std::fs::read_dir(src)? { + let entry = entry?; + let name = entry.file_name(); + // `Host::join` builds paths out of `&str`, so a name that is not UTF-8 + // would land under a lossy spelling of itself. Locally there is no + // reason to go through it at all; over the wire the path is a String + // either way, and lossy is the best that can be done. + let child = match host.id().is_local() { + true => dest.join(&name), + false => host.join(dest, &name.to_string_lossy()), + }; + copy_tree(host, &entry.path(), &child, depth + 1)?; + } + Ok(()) +} + +fn copy_file(host: &dyn Host, src: &Path, dest: &Path, len: u64) -> io::Result<()> { + // One syscall path locally, and the one that keeps the mode bits: + // `write_file` would drop the executable bit off every script copied in. + if host.id().is_local() { + std::fs::copy(src, dest)?; + return Ok(()); + } + if len > REMOTE_FILE_MAX { + return Err(io::Error::other(t_fmt( + L10nKey::FileDropTooLarge, + &[("limit", &(REMOTE_FILE_MAX / (1024 * 1024)).to_string())], + ))); + } + let bytes = std::fs::read(src)?; + host.write_file(dest, &bytes)?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use tty7_core::host::local::LocalHost; + + fn scratch(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("tty7-drop-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::canonicalize(&dir).unwrap() + } + + fn write(path: &Path, body: &str) { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, body).unwrap(); + } + + #[test] + fn a_dropped_file_lands_in_the_directory_it_was_dropped_on() { + let root = scratch("one-file"); + let src = root.join("from/note.txt"); + write(&src, "hello"); + let dest_dir = root.join("into"); + std::fs::create_dir_all(&dest_dir).unwrap(); + + let report = copy_into_dir(&*LocalHost::shared(), &[src], &dest_dir, false); + + assert_eq!(report.copied, vec!["note.txt".to_string()]); + assert!(report.errors.is_empty()); + assert_eq!( + std::fs::read_to_string(dest_dir.join("note.txt")).unwrap(), + "hello" + ); + } + + #[test] + fn a_dropped_folder_brings_everything_under_it() { + let root = scratch("folder"); + write(&root.join("from/pkg/a.txt"), "a"); + write(&root.join("from/pkg/deep/b.txt"), "b"); + let dest_dir = root.join("into"); + std::fs::create_dir_all(&dest_dir).unwrap(); + + let report = copy_into_dir( + &*LocalHost::shared(), + &[root.join("from/pkg")], + &dest_dir, + false, + ); + + assert_eq!(report.copied, vec!["pkg".to_string()]); + assert_eq!( + std::fs::read_to_string(dest_dir.join("pkg/deep/b.txt")).unwrap(), + "b" + ); + } + + #[test] + fn an_executable_stays_executable() { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + let root = scratch("mode"); + let src = root.join("from/run.sh"); + write(&src, "#!/bin/sh\n"); + std::fs::set_permissions(&src, std::fs::Permissions::from_mode(0o755)).unwrap(); + let dest_dir = root.join("into"); + std::fs::create_dir_all(&dest_dir).unwrap(); + + copy_into_dir(&*LocalHost::shared(), &[src], &dest_dir, false); + + let mode = std::fs::metadata(dest_dir.join("run.sh")) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o111, 0o111, "the executable bit did not survive"); + } + } + + #[test] + fn a_name_already_there_is_reported_and_nothing_is_written() { + let root = scratch("conflict"); + let src = root.join("from/note.txt"); + write(&src, "new"); + let dest_dir = root.join("into"); + write(&dest_dir.join("note.txt"), "old"); + let other = root.join("from/fresh.txt"); + write(&other, "fresh"); + + let report = copy_into_dir( + &*LocalHost::shared(), + &[src.clone(), other], + &dest_dir, + false, + ); + + assert_eq!(report.conflicts, vec!["note.txt".to_string()]); + assert!( + report.copied.is_empty(), + "the answer governs the whole drop" + ); + assert_eq!( + std::fs::read_to_string(dest_dir.join("note.txt")).unwrap(), + "old" + ); + assert!(!dest_dir.join("fresh.txt").exists()); + } + + #[test] + fn replacing_a_folder_leaves_what_was_dropped_and_not_the_union() { + let root = scratch("replace"); + write(&root.join("from/pkg/new.txt"), "new"); + let dest_dir = root.join("into"); + write(&dest_dir.join("pkg/stale.txt"), "stale"); + + let report = copy_into_dir( + &*LocalHost::shared(), + &[root.join("from/pkg")], + &dest_dir, + true, + ); + + assert_eq!(report.copied, vec!["pkg".to_string()]); + assert!(dest_dir.join("pkg/new.txt").exists()); + assert!( + !dest_dir.join("pkg/stale.txt").exists(), + "replacing a folder must not merge into it" + ); + } + + #[test] + fn a_row_dropped_back_on_its_own_folder_does_nothing() { + let root = scratch("same-dir"); + let src = root.join("here/note.txt"); + write(&src, "hello"); + + let report = copy_into_dir(&*LocalHost::shared(), &[src], &root.join("here"), false); + + assert!(report.copied.is_empty()); + assert!(report.errors.is_empty(), "a miss is not an error"); + assert!(report.conflicts.is_empty()); + } + + #[test] + fn a_drag_abandoned_on_the_folder_it_started_from_says_nothing() { + let root = scratch("abandoned"); + let pkg = root.join("pkg"); + std::fs::create_dir_all(&pkg).unwrap(); + + let report = copy_into_dir(&*LocalHost::shared(), &[pkg.clone()], &pkg, false); + + assert!(report.copied.is_empty()); + assert!( + report.errors.is_empty(), + "letting go where you picked it up is not an error" + ); + } + + #[test] + fn a_folder_cannot_be_copied_into_itself() { + let root = scratch("into-itself"); + write(&root.join("pkg/deep/a.txt"), "a"); + + let report = copy_into_dir( + &*LocalHost::shared(), + &[root.join("pkg")], + &root.join("pkg/deep"), + false, + ); + + assert_eq!(report.errors.len(), 1); + assert!(report.copied.is_empty()); + } + + #[test] + fn a_path_that_is_not_on_this_machine_is_refused_rather_than_guessed_at() { + let root = scratch("elsewhere"); + let dest_dir = root.join("into"); + std::fs::create_dir_all(&dest_dir).unwrap(); + + let report = copy_into_dir( + &*LocalHost::shared(), + &[PathBuf::from("/srv/on-the-far-end/note.txt")], + &dest_dir, + false, + ); + + assert_eq!(report.errors.len(), 1); + assert_eq!(report.errors[0].0, "note.txt"); + assert!(report.copied.is_empty()); + } + + #[cfg(unix)] + #[test] + fn a_symlink_cycle_cannot_make_the_copy_walk_forever() { + let root = scratch("cycle"); + let src = root.join("from/pkg"); + std::fs::create_dir_all(&src).unwrap(); + std::os::unix::fs::symlink(&src, src.join("loop")).unwrap(); + let dest_dir = root.join("into"); + std::fs::create_dir_all(&dest_dir).unwrap(); + + let report = copy_into_dir(&*LocalHost::shared(), &[src], &dest_dir, false); + + assert_eq!(report.errors.len(), 1, "the walk has to stop and say so"); + assert!(report.copied.is_empty()); + } +} diff --git a/src/ui/file_tree.rs b/src/ui/file_tree.rs index 2b1b2dc1..6378f1b6 100644 --- a/src/ui/file_tree.rs +++ b/src/ui/file_tree.rs @@ -4,6 +4,7 @@ use std::sync::Arc; use crate::core::config::RightPanelTab; use crate::ui::app::Tty7App; +use crate::ui::file_copy; use crate::ui::host_ops::{ByHost, HostId, HostOps, InFlight, SharedHost, WatchSub}; use crate::ui::host_registry::HostRegistry; use crate::ui::i18n::{L10nKey, t, t_fmt}; @@ -1214,6 +1215,98 @@ impl Tty7App { .detach(); } + /// Copy what was dropped on the tree into `dir`. + /// + /// The drop is the whole gesture: the panel does not ask where to put the + /// files, it puts them where the cursor was. The one question it does ask + /// is about replacing something already there, and that question is asked + /// before anything has been written. + fn file_tree_drop_paths( + &mut self, + sources: Vec, + dir: PathBuf, + window: &mut Window, + cx: &mut Context, + ) { + self.file_tree_copy_into(sources, dir, false, window, cx); + } + + fn file_tree_copy_into( + &mut self, + sources: Vec, + dir: PathBuf, + overwrite: bool, + window: &mut Window, + cx: &mut Context, + ) { + if sources.is_empty() { + return; + } + let Some(host) = self.active_host(cx) else { + return; + }; + let id = host.id(); + let asked_for = sources.clone(); + let target = dir.clone(); + HostOps::run_in( + host, + window, + cx, + move |h| file_copy::copy_into_dir(h, &sources, &target, overwrite), + move |app, report: file_copy::DropReport, window, cx| { + if !report.copied.is_empty() { + app.file_tree.invalidate_dir(id, &dir); + } + if !report.conflicts.is_empty() { + app.file_tree_confirm_replace(asked_for, dir, report.conflicts, window, cx); + } else if let Some((name, e)) = report.errors.first() { + // One notification for the drop, not one per file: a folder + // of unreadable files would otherwise bury the screen. + let context = match report.errors.len() { + 1 => t_fmt(L10nKey::FileDropFailed, &[("name", name)]), + n => t_fmt( + L10nKey::FileDropFailedMany, + &[("name", name), ("n", &(n - 1).to_string())], + ), + }; + HostOps::notify_err(window, cx, &context, e); + } + cx.notify(); + }, + ); + } + + fn file_tree_confirm_replace( + &mut self, + sources: Vec, + dir: PathBuf, + conflicts: Vec, + window: &mut Window, + cx: &mut Context, + ) { + let title = match conflicts.as_slice() { + [one] => t_fmt(L10nKey::FileDropReplaceTitle, &[("name", one)]), + many => t_fmt( + L10nKey::FileDropReplaceManyTitle, + &[("n", &many.len().to_string())], + ), + }; + let answer = window.prompt( + PromptLevel::Warning, + &title, + Some(t(L10nKey::FileDropReplaceBody)), + &crate::ui::confirm_answers(t(L10nKey::FileDropReplace), t(L10nKey::Cancel)), + cx, + ); + cx.spawn_in(window, async move |app, cx| { + let Ok(0) = answer.await else { return }; + let _ = app.update_in(cx, |app, window, cx| { + app.file_tree_copy_into(sources, dir, true, window, cx); + }); + }) + .detach(); + } + fn file_tree_cd(&mut self, dir: &Path, window: &mut Window, cx: &mut Context) { let Some(leaf) = self .tabs @@ -1316,7 +1409,21 @@ impl Tty7App { .children( rows.iter() .flat_map(|row| self.render_tree_row(row, window, cx)), - ); + ) + // Everything the rows do not cover — the gap below the last one, + // and the whole column while the tree is still empty — belongs to + // the top of the tree. A row under the cursor wins: gpui hands a + // drop to the innermost target first, and it stops there. + .when_some(roots.first().cloned(), |d, root| { + d.drag_over::(|s, _, _, cx| { + s.bg(cx.theme().drag_border.opacity(0.06)) + }) + .on_drop(cx.listener( + move |this, paths: &ExternalPaths, window, cx| { + this.file_tree_drop_paths(paths.paths().to_vec(), root.clone(), window, cx); + }, + )) + }); crate::ui::scrollbar::with_vertical_scrollbar( "right-panel-tree-scrollbar", column, @@ -1337,7 +1444,10 @@ impl Tty7App { // A placeholder standing in for children that are not there. Not a // file, so it takes none of the row machinery below — no hover, no - // selection, no context menu, no drag. + // selection, no context menu, no drag. It does take a drop: it is + // drawn inside a folder and it is the only thing in an empty one, so + // letting it fall through to the root would put files somewhere the + // cursor never was. if let Some(note) = row.note { let (key, ink) = match note { TreeNote::Loading => (L10nKey::TreeDirLoading, muted), @@ -1361,6 +1471,24 @@ impl Tty7App { TreeNote::SearchCapped => t_fmt(key, &[("n", &SEARCH_LIMIT.to_string())]), _ => t(key).to_string(), }) + // Every note but the capped-search one stands for a real + // directory, and carries its path; that one stands for the + // rest of a search and has nowhere to put anything. + .when(!path.as_os_str().is_empty(), |d| { + d.drag_over::(|s, _, _, cx| { + s.bg(cx.theme().drag_border.opacity(0.14)) + }) + .on_drop(cx.listener( + move |this, paths: &ExternalPaths, window, cx| { + this.file_tree_drop_paths( + paths.paths().to_vec(), + path.clone(), + window, + cx, + ); + }, + )) + }) .into_any_element(), ]; } @@ -1447,6 +1575,19 @@ impl Tty7App { cx.new(|_| DragGhost { name }) } }) + // The other direction: files dropped on this row are copied in. + // A folder takes them itself; a file stands in for the folder it + // is in, which is where "put it next to this one" lands. + .drag_over::(|s, _, _, cx| s.bg(cx.theme().drag_border.opacity(0.14))) + .on_drop(cx.listener({ + let dir = match is_dir { + true => path.clone(), + false => path.parent().unwrap_or(&path).to_path_buf(), + }; + move |this, paths: &ExternalPaths, window, cx| { + this.file_tree_drop_paths(paths.paths().to_vec(), dir.clone(), window, cx); + } + })) .context_menu({ let app = cx.entity().downgrade(); let path = path.clone(); @@ -2112,19 +2253,19 @@ mod render_idle_gpui_tests { const BUDGET: u64 = 200; - fn serial() -> std::sync::MutexGuard<'static, ()> { + pub(super) fn serial() -> std::sync::MutexGuard<'static, ()> { static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); LOCK.lock().unwrap_or_else(|e| e.into_inner()) } - fn scratch(name: &str) -> PathBuf { + pub(super) fn scratch(name: &str) -> PathBuf { let dir = std::env::temp_dir().join(format!("tty7-idle-{name}-{}", std::process::id())); let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); std::fs::canonicalize(&dir).unwrap() } - fn files_panel_on( + pub(super) fn files_panel_on( cx: &mut TestAppContext, root: &Path, ) -> ( @@ -2177,7 +2318,7 @@ mod render_idle_gpui_tests { (app, vcx, pane) } - fn rows(app: &Entity, vcx: &mut VisualTestContext) -> usize { + pub(super) fn rows(app: &Entity, vcx: &mut VisualTestContext) -> usize { app.update_in(vcx, |app, _, _| { let code = app.tab_code().expect("panel state"); app.file_tree @@ -2194,7 +2335,7 @@ mod render_idle_gpui_tests { }); } - fn settle(app: &Entity, vcx: &mut VisualTestContext, root: &Path) { + pub(super) fn settle(app: &Entity, vcx: &mut VisualTestContext, root: &Path) { let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); while std::time::Instant::now() < deadline { vcx.background_executor.run_until_parked(); @@ -2623,3 +2764,115 @@ mod render_idle_gpui_tests { let _ = std::fs::remove_dir_all(&root); } } + +/// The drop end of the panel, driven through the real app: the copy runs on a +/// `HostOps` worker and the tree has to catch up with what it wrote. +/// +/// What these cannot reach is the hit test — whether the row under the cursor +/// is the one that gets the drop is decided by gpui's hitbox stack, and there +/// is no headless way to put a cursor over a row. +#[cfg(all(test, unix))] +mod drop_gpui_tests { + use super::render_idle_gpui_tests::{files_panel_on, rows, scratch, serial, settle}; + use super::*; + use gpui::{TestAppContext, VisualTestContext}; + + /// The copy runs on a `HostOps` worker — a real OS thread the test + /// executor does not own, so parking it proves nothing about whether the + /// worker is done. This waits for the result instead of assuming it. + fn wait_until( + vcx: &mut VisualTestContext, + what: &str, + mut done: impl FnMut(&mut VisualTestContext) -> bool, + ) { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); + loop { + vcx.background_executor.run_until_parked(); + if done(vcx) { + return; + } + assert!(std::time::Instant::now() < deadline, "{what}"); + std::thread::sleep(std::time::Duration::from_millis(10)); + } + } + + #[gpui::test] + fn a_dropped_file_is_copied_in_and_shows_up_in_the_tree(cx: &mut TestAppContext) { + let _serial = serial(); + let root = scratch("drop-lands"); + let from = scratch("drop-source"); + std::fs::write(from.join("note.txt"), "hello").unwrap(); + let (app, mut vcx, _pane) = files_panel_on(cx, &root); + let before = rows(&app, &mut vcx); + + app.update_in(&mut vcx, |app, window, cx| { + app.file_tree_drop_paths(vec![from.join("note.txt")], root.clone(), window, cx); + }); + wait_until(&mut vcx, "the copy never landed", |_| { + root.join("note.txt").exists() + }); + settle(&app, &mut vcx, &root); + + assert_eq!( + std::fs::read_to_string(root.join("note.txt")).unwrap(), + "hello" + ); + assert_eq!(rows(&app, &mut vcx), before + 1, "the tree never caught up"); + let _ = std::fs::remove_dir_all(&root); + let _ = std::fs::remove_dir_all(&from); + } + + #[gpui::test] + fn a_drop_over_a_name_that_is_taken_asks_before_it_writes(cx: &mut TestAppContext) { + let _serial = serial(); + let root = scratch("drop-conflict-no"); + std::fs::write(root.join("note.txt"), "old").unwrap(); + let from = scratch("drop-conflict-no-source"); + std::fs::write(from.join("note.txt"), "new").unwrap(); + let (app, mut vcx, _pane) = files_panel_on(cx, &root); + + app.update_in(&mut vcx, |app, window, cx| { + app.file_tree_drop_paths(vec![from.join("note.txt")], root.clone(), window, cx); + }); + wait_until(&mut vcx, "it overwrote without asking", |vcx| { + vcx.has_pending_prompt() + }); + vcx.simulate_prompt_answer(t(L10nKey::Cancel)); + settle(&app, &mut vcx, &root); + + assert_eq!( + std::fs::read_to_string(root.join("note.txt")).unwrap(), + "old", + "answering no still replaced the file" + ); + let _ = std::fs::remove_dir_all(&root); + let _ = std::fs::remove_dir_all(&from); + } + + #[gpui::test] + fn answering_yes_replaces_what_was_there(cx: &mut TestAppContext) { + let _serial = serial(); + let root = scratch("drop-conflict-yes"); + std::fs::write(root.join("note.txt"), "old").unwrap(); + let from = scratch("drop-conflict-yes-source"); + std::fs::write(from.join("note.txt"), "new").unwrap(); + let (app, mut vcx, _pane) = files_panel_on(cx, &root); + + app.update_in(&mut vcx, |app, window, cx| { + app.file_tree_drop_paths(vec![from.join("note.txt")], root.clone(), window, cx); + }); + wait_until(&mut vcx, "it never asked", |vcx| vcx.has_pending_prompt()); + vcx.simulate_prompt_answer(t(L10nKey::FileDropReplace)); + wait_until(&mut vcx, "the replacement never landed", |_| { + std::fs::read_to_string(root.join("note.txt")).is_ok_and(|s| s == "new") + }); + settle(&app, &mut vcx, &root); + + assert_eq!( + std::fs::read_to_string(root.join("note.txt")).unwrap(), + "new" + ); + let _ = std::fs::remove_dir_all(&root); + let _ = std::fs::remove_dir_all(&from); + } +} diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 4f61aa1b..7e654576 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -855,6 +855,18 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::FileTreeContextCopyPath => "Copy Path", L10nKey::FileTreeContextHideDotfiles => "Hide Dotfiles", L10nKey::FileTreeContextShowDotfiles => "Show Dotfiles", + L10nKey::FileDropIntoItself => "A folder cannot be copied into itself.", + L10nKey::FileDropNotHere => "Not on this machine.", + L10nKey::FileDropTooDeep => "Nested more than {n} folders deep.", + L10nKey::FileDropTooLarge => "Larger than {limit} MB — send it over SFTP instead.", + L10nKey::FileDropReplaceTitle => "Replace \"{name}\"?", + L10nKey::FileDropReplaceManyTitle => "Replace {n} items?", + L10nKey::FileDropReplaceBody => { + "This folder already has something by that name. Replacing it cannot be undone." + } + L10nKey::FileDropReplace => "Replace", + L10nKey::FileDropFailed => "Could not copy {name}", + L10nKey::FileDropFailedMany => "Could not copy {name}, and {n} more failed", L10nKey::SshPromptNewKey => "new {fingerprint}", L10nKey::SshPromptOldKey => "old {old_fingerprint}", L10nKey::EditorCantOpen => "Could not open {path}: {e}", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index a51f982f..ad9b2d1b 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -897,6 +897,18 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::FileTreeContextCopyPath => "パスをコピー", L10nKey::FileTreeContextHideDotfiles => "ドットファイルを非表示", L10nKey::FileTreeContextShowDotfiles => "ドットファイルを表示", + L10nKey::FileDropIntoItself => "フォルダを自分自身の中にはコピーできません", + L10nKey::FileDropNotHere => "このマシンにはありません", + L10nKey::FileDropTooDeep => "フォルダの入れ子が {n} 階層を超えています", + L10nKey::FileDropTooLarge => "{limit} MB を超えています。SFTP で転送してください", + L10nKey::FileDropReplaceTitle => "「{name}」を置き換えますか？", + L10nKey::FileDropReplaceManyTitle => "{n} 項目を置き換えますか？", + L10nKey::FileDropReplaceBody => { + "このフォルダには同じ名前のものがすでにあります。置き換えると元に戻せません" + } + L10nKey::FileDropReplace => "置き換える", + L10nKey::FileDropFailed => "{name} をコピーできませんでした", + L10nKey::FileDropFailedMany => "{name} をコピーできませんでした。他に {n} 件も失敗しました", L10nKey::SshPromptNewKey => "新しいキー {fingerprint}", L10nKey::SshPromptOldKey => "以前のキー {old_fingerprint}", L10nKey::EditorCantOpen => "{path} を開けません: {e}", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index a98edf70..edc62a9b 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -653,6 +653,16 @@ l10n_keys! { FileTreeContextCopyPath, FileTreeContextHideDotfiles, FileTreeContextShowDotfiles, + FileDropIntoItself, + FileDropNotHere, + FileDropTooDeep, + FileDropTooLarge, + FileDropReplaceTitle, + FileDropReplaceManyTitle, + FileDropReplaceBody, + FileDropReplace, + FileDropFailed, + FileDropFailedMany, SshPromptNewKey, SshPromptOldKey, EditorCantOpen, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 77f240eb..3b21768a 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -820,6 +820,16 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::FileTreeContextCopyPath => "复制路径", L10nKey::FileTreeContextHideDotfiles => "隐藏点文件", L10nKey::FileTreeContextShowDotfiles => "显示点文件", + L10nKey::FileDropIntoItself => "文件夹不能复制到它自己里面。", + L10nKey::FileDropNotHere => "不在这台机器上。", + L10nKey::FileDropTooDeep => "文件夹嵌套超过 {n} 层。", + L10nKey::FileDropTooLarge => "超过 {limit} MB，请改用 SFTP 传输。", + L10nKey::FileDropReplaceTitle => "替换“{name}”？", + L10nKey::FileDropReplaceManyTitle => "替换 {n} 个项目？", + L10nKey::FileDropReplaceBody => "这个目录下已经有同名的东西了，替换后无法撤销。", + L10nKey::FileDropReplace => "替换", + L10nKey::FileDropFailed => "无法复制 {name}", + L10nKey::FileDropFailedMany => "无法复制 {name}，另有 {n} 个也失败了", L10nKey::SshPromptNewKey => "新 {fingerprint}", L10nKey::SshPromptOldKey => "旧 {old_fingerprint}", L10nKey::EditorCantOpen => "无法打开 {path}：{e}", diff --git a/src/ui/mod.rs b/src/ui/mod.rs index e5ccdd0d..9bc7880e 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -2,6 +2,7 @@ pub mod app; pub mod assets; pub mod code_editor; pub mod diff_overlay; +pub mod file_copy; pub mod file_tree; pub mod forwards; pub mod hints; diff --git a/src/ui/sftp.rs b/src/ui/sftp.rs index a762a91e..6e3d702a 100644 --- a/src/ui/sftp.rs +++ b/src/ui/sftp.rs @@ -143,6 +143,12 @@ pub(crate) struct SftpPanelState { pub(crate) filter_input: gpui::Entity, pub(crate) error: Option, pub(crate) jobs: Vec, + /// Uploads this panel started whose landing it has not listed yet. + /// + /// An upload is written to `.tty7-upload-` and renamed into + /// place at the very end, so any listing taken while one is in flight + /// shows the temporary name. These are the jobs a listing is owed to. + uploads_awaiting_listing: HashSet, /// Local names handed to a download that has not created its file yet. /// Two quick downloads of the same remote file would otherwise both find /// the name free and the second would write over the first. @@ -182,6 +188,7 @@ impl SftpPanelState { filter_input, error: None, jobs: Vec::new(), + uploads_awaiting_listing: HashSet::new(), claimed_downloads: HashSet::new(), dismissed_jobs: HashSet::new(), show_history: false, @@ -200,6 +207,22 @@ impl SftpPanelState { } } +/// Of the uploads a listing is owed to, the ones that are still writing. +/// +/// A job that has dropped off the list entirely counts as done: whether it +/// finished, failed or was trimmed from the history, it is not going to rename +/// anything into place later. +fn uploads_still_running(owed: &HashSet, jobs: &[SftpJobProgress]) -> HashSet { + owed.iter() + .copied() + .filter(|id| { + jobs.iter() + .find(|job| job.job_id == *id) + .is_some_and(|job| job.state == SftpJobState::Running) + }) + .collect() +} + fn is_dir_like(e: &SftpEntry) -> bool { matches!(e.kind, SftpEntryKind::Dir) || (matches!(e.kind, SftpEntryKind::Symlink) && e.target_is_dir) @@ -929,13 +952,18 @@ impl Tty7App { remote: remote_join(&cwd, &name), recursive, }; - if let Err(e) = self.sftp_route().transfer_start(spec) { - self.sftp_panel.error = Some(e); + match self.sftp_route().transfer_start(spec) { + Ok(job_id) => { + self.sftp_panel.uploads_awaiting_listing.insert(job_id); + } + Err(e) => self.sftp_panel.error = Some(e), } } self.sftp_poll_jobs(cx); self.sftp_start_polling(cx); - self.sftp_refresh(cx); + // No listing here on purpose. The upload has only just been handed to + // the daemon, so a listing taken now catches the temporary name it + // writes under; the one owed for it is taken when it settles. } pub(crate) fn sftp_cancel_job(&mut self, job_id: u64, cx: &mut Context) { @@ -966,8 +994,28 @@ impl Tty7App { fn sftp_poll_jobs(&mut self, cx: &mut Context) { if self.sftp_panel.open_pane_id.is_some() { - self.sftp_panel.jobs = self.sftp_route().transfer_list(); - cx.notify(); + let jobs = self.sftp_route().transfer_list(); + self.sftp_apply_jobs(jobs, cx); + } + } + + /// Take a fresh job list, and list the directory again once the uploads + /// that were running have stopped running. + /// + /// Nothing used to ask for that listing. An upload lands under + /// `.tty7-upload-` and is renamed into place at the end, so the + /// listing on screen was the one taken while the temporary name existed — + /// and it stayed, so a finished upload read as a file with a hash glued to + /// its name. + fn sftp_apply_jobs(&mut self, jobs: Vec, cx: &mut Context) { + let owed = &self.sftp_panel.uploads_awaiting_listing; + let still_running = uploads_still_running(owed, &jobs); + let settled = still_running.len() != owed.len(); + self.sftp_panel.uploads_awaiting_listing = still_running; + self.sftp_panel.jobs = jobs; + cx.notify(); + if settled { + self.sftp_refresh(cx); } } @@ -1004,8 +1052,7 @@ impl Tty7App { if this.sftp_panel.poll_gen != generation { return false; } - this.sftp_panel.jobs = jobs; - cx.notify(); + this.sftp_apply_jobs(jobs, cx); true }) .unwrap_or(false); @@ -1774,6 +1821,59 @@ impl Tty7App { mod tests { use super::*; + fn upload(job_id: u64, state: SftpJobState) -> SftpJobProgress { + SftpJobProgress { + job_id, + pane_id: 1, + kind: SftpTransferKind::Upload, + state, + current: String::new(), + bytes_done: 0, + bytes_total: 0, + error: None, + local: "/here/note.txt".into(), + remote: "/there/note.txt".into(), + } + } + + #[test] + fn an_upload_owes_a_listing_until_it_stops_running() { + let owed = HashSet::from([7]); + let running = uploads_still_running(&owed, &[upload(7, SftpJobState::Running)]); + assert_eq!( + running, owed, + "a listing taken now shows the temporary name" + ); + + for done in [ + SftpJobState::Done, + SftpJobState::Error, + SftpJobState::Cancelled, + ] { + let running = uploads_still_running(&owed, &[upload(7, done)]); + assert!(running.is_empty(), "{done:?} still owes the listing"); + } + } + + #[test] + fn a_job_that_falls_off_the_list_is_not_waited_on_forever() { + let owed = HashSet::from([7]); + assert!(uploads_still_running(&owed, &[]).is_empty()); + } + + #[test] + fn one_upload_finishing_does_not_settle_the_one_beside_it() { + let owed = HashSet::from([7, 8]); + let running = uploads_still_running( + &owed, + &[ + upload(7, SftpJobState::Done), + upload(8, SftpJobState::Running), + ], + ); + assert_eq!(running, HashSet::from([8])); + } + #[test] fn a_second_download_is_numbered_rather_than_written_over_the_first() { let dir = tempfile::tempdir().expect("tempdir");