From 9820b8513f67ba370fccc3c289dd53b28ac4e9a7 Mon Sep 17 00:00:00 2001 From: iret77 <63622643+iret77@users.noreply.github.com> Date: Sat, 4 Jul 2026 12:51:59 +0200 Subject: [PATCH] =?UTF-8?q?feat(fm):=20move=20across=20connections=20(F6?= =?UTF-8?q?=20local=E2=86=94remote,=20copy-then-delete)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F6 now works between a local and a remote pane, not just same-fs: the file transfers (upload/download) and, only once that fully succeeds, the source is deleted through its own backend. The source is never removed on failure or cancellation, so a failed move cannot lose data. - spawn_transfer_with_backend gains delete_after = Some((source_backend, source_path)); the completion deletes the source on Ok(Ok(())) only. - transfer_cross_connection drops the move deferral and passes the delete intent per file; the summary reads "move transfer(s) started". Directories across connections and two-different-remote-hosts remain reported-not-done (the rest of increment C). Verified: cargo check -p warp 0/0; 248 sftp + 5 registry + 6 keynav tests green (the former deferral test now asserts the transfer starts). Co-Authored-By: Claude Fable 5 --- app/src/sftp_manager/browser.rs | 49 ++++++++++++++----- .../sftp_manager/browser_integration_tests.rs | 14 +++--- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/app/src/sftp_manager/browser.rs b/app/src/sftp_manager/browser.rs index d88d5a9edd..a63761bd4c 100644 --- a/app/src/sftp_manager/browser.rs +++ b/app/src/sftp_manager/browser.rs @@ -1014,15 +1014,6 @@ impl SftpBrowserView { is_move: bool, ctx: &mut ViewContext, ) { - if is_move { - self.show_error_toast( - "Moving across connections is coming next — copy, then delete the source." - .to_string(), - ctx, - ); - return; - } - // Upload (local source → remote target) transfers through the target's // session; download (remote source → local target) through ours. let direction = match plan_transfer(&self.fs_namespace(), &target.fs) { @@ -1048,6 +1039,9 @@ impl SftpBrowserView { } }, }; + // For a move, the source is deleted after the transfer succeeds, via the + // *source* pane's own backend (ours) — a cross-connection copy-then-delete. + let source_backend = self.sftp.clone(); let target_dir = target.current_path.clone(); let mut started = 0usize; @@ -1082,11 +1076,23 @@ impl SftpBrowserView { TransferDirection::Upload => (source.clone(), dest), TransferDirection::Download => (dest, source.clone()), }; - self.spawn_transfer_with_backend(local_path, remote_path, backend.clone(), direction, ctx); + let delete_after = match (is_move, &source_backend) { + (true, Some(b)) => Some((b.clone(), source.clone())), + _ => None, + }; + self.spawn_transfer_with_backend( + local_path, + remote_path, + backend.clone(), + direction, + delete_after, + ctx, + ); started += 1; } - let mut parts = vec![format!("{started} transfer(s) started")]; + let verb = if is_move { "move" } else { "copy" }; + let mut parts = vec![format!("{started} {verb} transfer(s) started")]; if skipped_exist > 0 { parts.push(format!("{skipped_exist} skipped (already exist)")); } @@ -1100,12 +1106,18 @@ impl SftpBrowserView { /// Spawn a single upload/download on the transfer engine using an explicit /// backend (the target's for uploads, ours for downloads), mirroring the /// drag-and-drop upload path but parameterised by direction and backend. + /// + /// `delete_after` = `Some((source_backend, source_path))` turns a copy into + /// a **move**: on success the source is deleted through its own backend + /// (copy-then-delete across connections). It is never deleted on failure or + /// cancellation, so a failed move can't lose data. fn spawn_transfer_with_backend( &mut self, local_path: PathBuf, remote_path: PathBuf, backend: Arc, direction: TransferDirection, + delete_after: Option<(Arc, PathBuf)>, ctx: &mut ViewContext, ) { let total_size = match direction { @@ -1174,7 +1186,20 @@ impl SftpBrowserView { } me.transfer_handles.remove(&task_id); match &result { - Ok(Ok(())) => me.refresh_dir(ctx), + Ok(Ok(())) => { + // Copy-then-delete: only remove the source once the + // transfer has fully succeeded (never on error/cancel). + if let Some((source_backend, source_path)) = &delete_after { + if let Err(e) = source_backend.delete_file(source_path) { + log::warn!("fm move: source delete failed: {e}"); + me.show_error_toast( + format!("Copied, but removing the source failed: {e}"), + ctx, + ); + } + } + me.refresh_dir(ctx); + } Ok(Err(e)) => { log::error!("sftp: cross-pane transfer failed: {e}"); me.show_error_toast(format!("Transfer failed: {e}"), ctx); diff --git a/app/src/sftp_manager/browser_integration_tests.rs b/app/src/sftp_manager/browser_integration_tests.rs index ed0c3684cf..1669772166 100644 --- a/app/src/sftp_manager/browser_integration_tests.rs +++ b/app/src/sftp_manager/browser_integration_tests.rs @@ -2069,10 +2069,11 @@ fn test_f5_local_to_remote_starts_an_upload_transfer() { }); } -/// F6 (move) across connections is deferred, not partially done: no transfer -/// starts and nothing is renamed. +/// F6 (move) across connections now runs as an upload transfer (copy-then-delete): +/// a transfer starts. The source removal happens on async completion, so this +/// synchronous check asserts the transfer was created, not the final deletion. #[test] -fn test_f6_move_across_connections_is_deferred() { +fn test_f6_move_across_connections_starts_transfer() { warpui::App::test((), |mut app| async move { initialize_app(&mut app); let temp = create_temp_dir_with_files(&[("left/bar.txt", b"x"), ("right/.keep", b"")]); @@ -2095,8 +2096,9 @@ fn test_f6_move_across_connections_is_deferred() { v.handle_action(&SftpBrowserAction::MoveToOtherPane, ctx); }); - view_a.read(&app, |v, _| assert!(v.transfers.is_empty(), "no transfer started")); - assert!(root.join("left/bar.txt").exists(), "source untouched"); - assert!(!root.join("right/bar.txt").exists(), "nothing moved"); + view_a.read(&app, |v, _| { + assert_eq!(v.transfers.len(), 1, "cross-connection move starts one transfer"); + assert_eq!(v.transfers[0].direction, TransferDirection::Upload); + }); }); }