diff --git a/CHANGELOG.md b/CHANGELOG.md index d79628cd..21636b5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -270,6 +270,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 its switcher row offers *Remove entry* next to a note that a new profile finds the session again. +- **Opening a folder from Explorer no longer costs you your layout** — "Open in + tty7", "Open tty7 here", and `tty7 ` restore the last window's tabs and + splits before opening the folder as one more tab in it. A launch naming a + directory used to skip the restore outright whenever no window was already + up — whatever "Restore last layout" said — so the folder arrived as a lone + blank terminal and the previous tabs were left behind, still running on the + server but reachable only through the workspace switcher. With a window + already up the same menu entry had always just added a tab, which is the + behavior both shapes of launch now share. A path still declines to follow the + layout onto a remote machine and starts a fresh local workspace there, since + the directory it names is a path on this computer. + - **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 diff --git a/src/main.rs b/src/main.rs index df8e6e5d..aeff7d86 100644 --- a/src/main.rs +++ b/src/main.rs @@ -454,10 +454,7 @@ fn main() { keymap::init(cx); crate::ui::local_link::LocalLink::install(cx); - let reopen = open_path - .is_none() - .then(|| crate::core::session::WorkspaceStore::restore_one(cx)) - .flatten(); + let reopen = crate::ui::windows::restore_target(cx, open_path.as_deref()); crate::ui::windows::open_at(cx, reopen, open_path); }); } diff --git a/src/ui/app.rs b/src/ui/app.rs index 668aa9f4..4396a4ff 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -683,7 +683,7 @@ impl Tty7App { pub fn for_workspace_at( id: Option, - initial_cwd: Option, + mut initial_cwd: Option, window: &mut Window, cx: &mut Context, ) -> Self { @@ -701,9 +701,20 @@ impl Tty7App { let on_machine = id.is_some_and(|id| crate::ui::machine_mirror::machine_holds_tabs(cx, id)); let hydrate = on_machine || (known && (restore || is_remote)); let session = hydrate.then(Session::default); + // A window that is about to pull its layout cannot also open the folder + // the launch asked for as its first terminal — the pull would find a + // window that already has a tab, decline to adopt into it, and push + // that one tab back as the whole workspace. So the folder travels with + // the hydration and becomes a tab once the layout is up. + let open_after_hydrate = hydrate.then(|| initial_cwd.take()).flatten(); let app = Self::with_session_at(Some(workspace), session, initial_cwd, window, cx); if hydrate { - crate::ui::tree_sync::hydrate_window_from_tree(cx, workspace); + match open_after_hydrate { + Some(cwd) => { + crate::ui::tree_sync::hydrate_window_then_open(cx, workspace, cwd); + } + None => crate::ui::tree_sync::hydrate_window_from_tree(cx, workspace), + } } else { if !is_remote { crate::ui::tree_sync::mark_window_informed(cx, workspace); diff --git a/src/ui/tree_sync.rs b/src/ui/tree_sync.rs index bef802f0..425421cf 100644 --- a/src/ui/tree_sync.rs +++ b/src/ui/tree_sync.rs @@ -727,6 +727,14 @@ struct WsState { /// Leaving it standing after the run ends is what makes a *first* failure /// wait the cap: the count would still be carrying an outage that is over. rehydrate_attempts: u32, + /// A folder the launch asked for, waiting for this window's layout. + /// + /// Held here rather than opened straight away because a window with a tab + /// in it is one `Adopt::IfEmpty` will not adopt into: the pull would land, + /// decline the layout, and push the single tab back as the whole workspace. + /// Parking it also means a pull that has to be retried still gets the + /// folder opened, on whichever attempt finally lands. + then_open: Option, /// Whether this window has already been told why it opened empty. /// /// The retry is as quiet as the failure was, so a window whose machine @@ -750,6 +758,7 @@ impl Default for WsState { epoch: 0, rehydrate: None, rehydrate_attempts: 0, + then_open: None, said_why_empty: false, } } @@ -1267,6 +1276,48 @@ pub(crate) fn hydrate_window_from_tree(cx: &mut App, client_ws: WorkspaceId) { hydrate(cx, client_ws, Adopt::IfEmpty); } +/// Pulls this window's layout, then opens `path` as one more tab in it. +/// +/// This is what a launch carrying a directory does — Explorer's "Open in tty7", +/// or `tty7 ` with no window up. Both halves are wanted: the layout the +/// user left, and the folder they just double-clicked. +pub(crate) fn hydrate_window_then_open( + cx: &mut App, + client_ws: WorkspaceId, + path: std::path::PathBuf, +) { + cx.default_global::() + .windows + .entry(client_ws) + .or_default() + .then_open = Some(path); + hydrate(cx, client_ws, Adopt::IfEmpty); +} + +/// Opens the folder a launch parked here, now that the layout it waited for is +/// up. Does nothing for the windows — every other one — that parked nothing. +fn open_parked_path(cx: &mut App, client_ws: WorkspaceId) { + let Some(path) = cx + .default_global::() + .windows + .get_mut(&client_ws) + .and_then(|state| state.then_open.take()) + else { + return; + }; + let Some(handle) = crate::ui::windows::WindowRegistry::window_for(cx, client_ws) else { + return; + }; + let Some(app) = + crate::ui::windows::WindowRegistry::app_for(cx, client_ws).and_then(|app| app.upgrade()) + else { + return; + }; + let _ = handle.update(cx, move |_, window, cx| { + app.update(cx, |app, cx| app.new_tab_at(path, window, cx)); + }); +} + #[derive(Clone, Copy, PartialEq)] enum Adopt { IfEmpty, @@ -1559,6 +1610,22 @@ fn finish_hydration( adopt: Adopt, outcome: io::Result<(Machine, WsMirror, Session)>, ) { + if settle_hydration(cx, client_ws, epoch, adopt, outcome) { + open_parked_path(cx, client_ws); + } +} + +/// The body of [`finish_hydration`]. Returns whether this attempt settled the +/// window's layout — false for one that was superseded or has to be retried, +/// which are the two cases where a parked folder waits for the attempt that +/// does settle it rather than opening over a layout still on its way. +fn settle_hydration( + cx: &mut App, + client_ws: WorkspaceId, + epoch: u64, + adopt: Adopt, + outcome: io::Result<(Machine, WsMirror, Session)>, +) -> bool { let current = cx .default_global::() .windows @@ -1566,7 +1633,7 @@ fn finish_hydration( .map(|s| s.epoch); if current != Some(epoch) { log::debug!("workspace {client_ws}: dropping a superseded hydration"); - return; + return false; } let (machine, mirror, session) = match outcome { Ok(pulled) => pulled, @@ -1581,7 +1648,7 @@ fn finish_hydration( "could not hydrate workspace {client_ws} from its machine: {e}" ); let _ = owe_rehydration(cx, client_ws, epoch, adopt); - return; + return false; } }; let host = WorkspaceStore::host_of(cx, client_ws); @@ -1589,7 +1656,7 @@ fn finish_hydration( let machine_was_empty = mirror.tabs.is_empty(); let was_dirty = { let Some(state) = cx.default_global::().windows.get_mut(&client_ws) else { - return; + return false; }; let dirty = matches!(state.sync, SyncPhase::Unprimed { dirty: true, .. }); state.informed |= machine_was_empty; @@ -1604,7 +1671,7 @@ fn finish_hydration( let Some(app) = crate::ui::windows::WindowRegistry::app_for(cx, client_ws).and_then(|app| app.upgrade()) else { - return; + return false; }; if adopt == Adopt::IfEmpty && !app.read(cx).tabs.is_empty() { // A full window over an empty tree has to write itself back, whether @@ -1622,7 +1689,7 @@ fn finish_hydration( if was_dirty || machine_was_empty { app.update(cx, |app, cx| sync_window(app, cx)); } - return; + return true; } if session.tabs.is_empty() && adopt == Adopt::IfEmpty { if was_dirty @@ -1631,10 +1698,10 @@ fn finish_hydration( { app.update(cx, |app, cx| sync_window(app, cx)); } - return; + return true; } let Some(handle) = crate::ui::windows::WindowRegistry::window_for(cx, client_ws) else { - return; + return false; }; let wanted = session.tabs.len(); log::info!("rebuilding {wanted} tab(s) of workspace {client_ws} from its machine's tree"); @@ -1669,6 +1736,7 @@ fn finish_hydration( window uninformed so the layout is not mistaken for an empty workspace" ); } + true } /// Someone else removed this workspace from its machine — `tty7 ws rm`, or @@ -2112,6 +2180,50 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[gpui::test] + fn a_parked_folder_outlives_the_pull_that_has_to_be_retried(cx: &mut gpui::TestAppContext) { + // The folder an Explorer launch asked for is opened by whichever + // attempt finally lands, so re-entering `hydrate` — which is how every + // retry gets here — must not reset it along with the rest of the pull + // state. Losing it would mean the double-clicked folder never opens. + cx.update(|cx| { + let ws = WorkspaceId::new(); + let path = std::path::PathBuf::from("/tmp/from-explorer"); + crate::core::session::WorkspaceStore::install_for_test( + cx, + crate::core::session::WindowViews::default(), + ); + crate::ui::windows::WindowRegistry::init(cx); + + hydrate_window_then_open(cx, ws, path.clone()); + assert_eq!( + cx.default_global::().windows[&ws] + .then_open + .as_ref(), + Some(&path), + "the request must be parked, not opened over a layout still in flight" + ); + + hydrate(cx, ws, Adopt::IfEmpty); + assert_eq!( + cx.default_global::().windows[&ws] + .then_open + .as_ref(), + Some(&path), + "a retry must still owe the folder" + ); + + // With no window to put it in there is nothing to open, and the + // request must not survive to surface in some unrelated window. + open_parked_path(cx, ws); + assert!( + cx.default_global::().windows[&ws] + .then_open + .is_none() + ); + }); + } + #[gpui::test] fn preemption_drops_the_mirror_the_queue_and_the_informed_licence( cx: &mut gpui::TestAppContext, diff --git a/src/ui/windows.rs b/src/ui/windows.rs index 9dd889f4..7b36585f 100644 --- a/src/ui/windows.rs +++ b/src/ui/windows.rs @@ -285,20 +285,40 @@ pub fn open_from_cli(cx: &mut App, path: Option) { }); } +/// The workspace a launch reopens, if any. +/// +/// A launch that carries a directory restores the last layout exactly like a +/// pathless one does, and the directory becomes another tab in it. The two used +/// to disagree: "Open in tty7" from Explorer, or `tty7 `, skipped the +/// restore outright whenever no window was already up, so a folder opened that +/// way came back as one blank terminal with the previous tabs nowhere — still +/// live on the machine, but reachable only through the switcher. +/// +/// What a path does change is that it will not follow the layout onto a remote +/// machine. The requested directory is a path on this computer and a remote +/// workspace has no business spawning it, so that case starts a fresh local +/// workspace — which is what every path-carrying launch used to do. +pub fn restore_target(cx: &mut App, path: Option<&std::path::Path>) -> Option { + if path.is_some() { + let views = WorkspaceStore::all(cx); + let candidate = views.workspace_to_restore()?; + if views.get(candidate).is_some_and(|view| view.is_remote()) { + return None; + } + } + WorkspaceStore::restore_one(cx) +} + /// Opens a window after CLI routing reaches a GUI process with no live windows. /// -/// A pathless request follows the same restoration policy as normal startup. -/// An explicit path always starts a fresh local workspace so the requested tab -/// cannot accidentally be attached to a detached remote workspace. +/// Both shapes of request follow the same restoration policy as normal startup; +/// see [`restore_target`] for the one way a path narrows it. fn open_missing_cli_window_with( cx: &mut App, path: Option, open: impl FnOnce(&mut App, Option, Option), ) { - let restore = path - .is_none() - .then(|| WorkspaceStore::restore_one(cx)) - .flatten(); + let restore = restore_target(cx, path.as_deref()); open(cx, restore, path); } @@ -757,6 +777,75 @@ mod tests { assert_eq!(opened, Some((None, None))); } + #[gpui::test] + fn a_request_carrying_a_path_restores_the_layout_and_brings_the_path_along( + cx: &mut gpui::TestAppContext, + ) { + // The Explorer "Open in tty7" case with no window up: the folder is + // wanted *and* so are the tabs that were there, which used to be + // dropped on the floor by every launch that named a directory. + let view = WindowView::default(); + let restored = view.id; + let path = std::path::PathBuf::from("/tmp/somewhere"); + let mut opened = None; + + cx.update(|cx| { + WorkspaceStore::install_for_test( + cx, + WindowViews { + views: vec![view], + active: Some(restored), + }, + ); + open_missing_cli_window_with(cx, Some(path.clone()), |_, workspace, path| { + opened = Some((workspace, path)); + }); + }); + + assert_eq!(opened, Some((Some(restored), Some(path)))); + } + + #[gpui::test] + fn a_path_will_not_follow_the_layout_onto_a_remote_machine(cx: &mut gpui::TestAppContext) { + // The directory is a path on this computer, so a remote workspace is + // the one restore candidate a path-carrying launch declines. + let remote = WindowView::on_remote(RemoteRef::new( + RemoteTarget::Wsl { + distro: "Ubuntu".into(), + }, + WorkspaceId::new(), + )); + let remote_id = remote.id; + let path = std::path::PathBuf::from("/tmp/somewhere"); + let mut opened = None; + + cx.update(|cx| { + WorkspaceStore::install_for_test( + cx, + WindowViews { + views: vec![remote], + active: Some(remote_id), + }, + ); + open_missing_cli_window_with(cx, Some(path.clone()), |_, workspace, path| { + opened = Some((workspace, path)); + }); + }); + + assert_eq!(opened, Some((None, Some(path)))); + + // Declining it is not the same as detaching it: the remote window is + // left exactly as it was for the next pathless launch to restore. + cx.update(|cx| { + assert!( + WorkspaceStore::all(cx) + .get(remote_id) + .is_some_and(|view| view.open), + "a declined remote candidate must not be closed on its behalf" + ); + }); + } + #[test] fn the_confirmation_says_which_of_the_three_answers_it_got() { set_locale("en");