From bbda8ad59fa41507bbe5ff120b521686043a2ac8 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sat, 15 Aug 2026 00:01:06 +0800 Subject: [PATCH] feat(remote): keep the machine a window is on visible, and stop offering SSH commands that do nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A window on another computer looked exactly like one on this computer for as long as nothing went wrong: the home screen's status strip stays silent while the link is Attached, the workspace head shows a name someone made up, and a connected pane's row is titled by the remote shell's own cwd. The only trace was a 6px dot on the tab avatar, and the host behind it was hover-only. - The sidebar carries a machine badge under the workspace head — the same dot-plus-name the switcher's rows use — whenever the workspace is remote. - A row names its own machine when that contradicts the badge: an ssh pane inside a local workspace, or a wsl shell beside Windows ones. It stays quiet when it would only echo the badge. - The home screen, which has no sidebar to carry the badge, names the machine a new tab would open on even while the link is healthy. - The pane shown while a connection is being made asked the target for its own Display, which spells a saved profile as a bare uuid: "Connecting to 51d32f65-e669-496b-8f7d-e09cb4991cb6…". It asks the host listing now, the same fix #485 made in the switcher. - SSH: Reconnect, SSH: Remote Files and SSH: Port Forwarding were offered from every pane and returned without doing anything on most of them. They are offered where they would act, the way #549 made Save Connection as Host earn its row. - Port Forwarding also disagreed with the panel it opens: the Info panel draws a Forwards section for a remote-workspace pane, and the command only accepted a native ssh one. Both read the same rule now. --- src/ui/app.rs | 44 +++++++-- src/ui/home.rs | 10 +- src/ui/i18n/en.rs | 8 ++ src/ui/i18n/ja.rs | 8 ++ src/ui/i18n/mod.rs | 8 ++ src/ui/i18n/zh.rs | 8 ++ src/ui/palette.rs | 48 +++++++++- src/ui/remote_connect.rs | 30 ++++++ src/ui/remote_workspace.rs | 106 ++++++++++++++++++++ src/ui/right_panel.rs | 27 ++++-- src/ui/ssh_connect.rs | 27 ++++-- src/ui/tab_sidebar.rs | 191 +++++++++++++++++++++++++++++++++---- 12 files changed, 466 insertions(+), 49 deletions(-) diff --git a/src/ui/app.rs b/src/ui/app.rs index ddc47a06..7cc611c6 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -2615,8 +2615,17 @@ impl Tty7App { cx.notify(); } + /// The focused pane, when it has a connection forwards can be opened over. + /// The palette asks this to decide whether to offer the command at all, so + /// the row and what pressing it does cannot drift apart. + pub(crate) fn forwardable_pane(&self, window: &Window, cx: &App) -> Option { + let leaf = self.tabs.get(self.active)?.detail_pane(window, cx)?; + let view = leaf.read(cx); + crate::ui::right_panel::pane_holds_forwards(view).then_some(view.pane_id) + } + pub(crate) fn show_ssh_forwards(&mut self, window: &mut Window, cx: &mut Context) { - let Some((pane_id, _)) = self.active_connected_native_ssh_pane(window, cx) else { + let Some(pane_id) = self.forwardable_pane(window, cx) else { return; }; self.set_right_panel_tab(crate::core::config::RightPanelTab::Info, cx); @@ -4174,10 +4183,29 @@ impl Tty7App { }, ); - // Offered only where it would do something. A connection opened from a - // saved host has nothing to save, and a pane that is not an SSH one has - // no connection at all — either would be a row that quietly did nothing - // (#549). + // The four commands that act on *the connection in the focused pane*. + // Every one of them used to be offered from every pane, and every one + // of them returned without doing anything when the pane had no + // connection — a row that looks like it works, does nothing, and says + // nothing about why. They are offered where they would act (#549). + if self.ssh_session_to_restart(window, cx).is_some() { + commands.push( + Command::localized(L10nKey::CmdSshReconnect, CommandKind::RestartSshSession) + .in_group(CommandGroup::Ssh), + ); + } + if self.active_connected_native_ssh_pane(window, cx).is_some() { + commands.push( + Command::localized(L10nKey::CmdSshRemoteFiles, CommandKind::ToggleSftp) + .in_group(CommandGroup::Ssh), + ); + } + if self.forwardable_pane(window, cx).is_some() { + commands.push( + Command::localized(L10nKey::CmdSshPortForwarding, CommandKind::ShowSshForwards) + .in_group(CommandGroup::Ssh), + ); + } if self.unsaved_ssh_session(window, cx).is_some() { commands.push( Command::localized( @@ -7267,10 +7295,14 @@ pub(crate) fn new_terminal( owner, font_size, }; + // Asked of the host listing, not of the target's own `Display`: a + // `Profile` target spells itself as a bare uuid, and this pane fills the + // whole window while a connection is being made — "Connecting to + // 51d32f65-e669-…" is the same thing #485 took out of the switcher. let machine = spawn .workspace .as_ref() - .map(|w| w.target.to_string()) + .map(|w| crate::ui::remote_connect::target_label(cx, &w.target)) .unwrap_or_else(|| t(L10nKey::AppLocalServerName).to_string()); let pending = cx.new(|cx| crate::ui::pending_pane::PendingPane::new(machine, spawn, cx)); cx.subscribe_in( diff --git a/src/ui/home.rs b/src/ui/home.rs index f7df385b..fe45dc41 100644 --- a/src/ui/home.rs +++ b/src/ui/home.rs @@ -258,7 +258,15 @@ impl Tty7App { ) -> Option> { let machine = self.remote_machine_label(cx); let status = self.remote_status(cx)?; - let message = status.strip_message(&machine)?; + // A healthy link says nothing over a *working* window — the sidebar + // banner is already carrying it there. Here there are no tabs and so no + // sidebar, and the screen is a logo and four shortcuts that look exactly + // the same whichever machine they will run on. Say which one, before ⌘T + // opens a shell somewhere the reader did not mean. + let message = match status.strip_message(&machine) { + Some(message) => message, + None => t_fmt(L10nKey::HomeRemoteConnected, &[("machine", &machine)]), + }; // An install in flight replaces both halves of the strip: its own line // instead of the complaint that is being answered, and no button, since // pressing Update Server again would start a second one on top of it. diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index b54020b4..998446c0 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -18,6 +18,7 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::NewFileName => "New file name", L10nKey::HomeNewTab => "New Tab", L10nKey::HomeReopenClosedTab => "Reopen Closed Tab", + L10nKey::HomeRemoteConnected => "Connected to {machine}", L10nKey::HomeSwitchWorkspace => "Switch Workspace…", L10nKey::HomeCommandPalette => "Command Palette…", L10nKey::HomeSplitRight => "Split Right", @@ -1193,6 +1194,13 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::RemoteStripRouteLost => { "The connection profile for {machine} no longer exists — it cannot reconnect" } + L10nKey::RemoteStateConnecting => "connecting", + L10nKey::RemoteStateReconnecting => "reconnecting", + L10nKey::RemoteStatePreempted => "open elsewhere", + L10nKey::RemoteStateDisconnected => "not connected", + L10nKey::RemoteStateFailed => "unreachable", + L10nKey::RemoteStateServerMismatch => "server mismatch", + L10nKey::RemoteStateRouteLost => "profile gone", L10nKey::RemoteRouteParkedHint => { "Its connection profile no longer exists, so it will not reconnect on its own. \ The remote session is still there — connect to the machine with a new profile \ diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 0891032e..7c921ad4 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -18,6 +18,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::NewFileName => "新しいファイル名", L10nKey::HomeNewTab => "新規タブ", L10nKey::HomeReopenClosedTab => "閉じたタブをもう一度開く", + L10nKey::HomeRemoteConnected => "{machine} に接続済み", L10nKey::HomeSwitchWorkspace => "ワークスペースを切り替える…", L10nKey::HomeCommandPalette => "コマンドパレット…", L10nKey::HomeSplitRight => "右に分割", @@ -1246,6 +1247,13 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::RemoteStripPreempted => "このワークスペースは {by} で開かれました", L10nKey::RemoteStripFailed => "{machine} に未接続です — {error}", L10nKey::RemoteStripRouteLost => "{machine} の接続設定は存在しません — 再接続できません", + L10nKey::RemoteStateConnecting => "接続中", + L10nKey::RemoteStateReconnecting => "再接続中", + L10nKey::RemoteStatePreempted => "他で開いています", + L10nKey::RemoteStateDisconnected => "未接続", + L10nKey::RemoteStateFailed => "接続できません", + L10nKey::RemoteStateServerMismatch => "サーバー不一致", + L10nKey::RemoteStateRouteLost => "プロファイル消失", L10nKey::RemoteRouteParkedHint => { "接続設定が存在しないため、自動再接続しません。リモートのセッションは残っています — \ 新しいプロファイルでこのマシンに接続すると、ワークスペース一覧に再表示されます。" diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 715130e1..ee6b7b39 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -87,6 +87,7 @@ l10n_keys! { HomeNewTab, HomeReopenClosedTab, HomeSwitchWorkspace, + HomeRemoteConnected, HomeCommandPalette, HomeSplitRight, HomeSplitDown, @@ -991,6 +992,13 @@ l10n_keys! { RemoteStripPreempted, RemoteStripFailed, RemoteStripRouteLost, + RemoteStateConnecting, + RemoteStateReconnecting, + RemoteStatePreempted, + RemoteStateDisconnected, + RemoteStateFailed, + RemoteStateServerMismatch, + RemoteStateRouteLost, RemoteRouteParkedHint, RemoteNoticePreempted, RemoteNoticeDisconnected, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index bd99fc05..0cf0ff59 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -18,6 +18,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::NewFileName => "新文件名", L10nKey::HomeNewTab => "新标签页", L10nKey::HomeReopenClosedTab => "重新打开已关闭的标签页", + L10nKey::HomeRemoteConnected => "已连接到 {machine}", L10nKey::HomeSwitchWorkspace => "切换工作区…", L10nKey::HomeCommandPalette => "命令面板…", L10nKey::HomeSplitRight => "向右分屏", @@ -1121,6 +1122,13 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::RemoteStripPreempted => "此工作区已在 {by} 上打开", L10nKey::RemoteStripFailed => "未连接到 {machine}——{error}", L10nKey::RemoteStripRouteLost => "{machine} 的连接配置已不存在，无法重连", + L10nKey::RemoteStateConnecting => "连接中", + L10nKey::RemoteStateReconnecting => "重连中", + L10nKey::RemoteStatePreempted => "已在别处打开", + L10nKey::RemoteStateDisconnected => "未连接", + L10nKey::RemoteStateFailed => "无法连接", + L10nKey::RemoteStateServerMismatch => "服务端版本不匹配", + L10nKey::RemoteStateRouteLost => "配置已丢失", L10nKey::RemoteRouteParkedHint => { "其连接配置已不存在，不会再自动重连。远端会话仍在——\ 新建配置连上该机器后，可在工作区列表中找回。" diff --git a/src/ui/palette.rs b/src/ui/palette.rs index e49b6155..78512d38 100644 --- a/src/ui/palette.rs +++ b/src/ui/palette.rs @@ -14,7 +14,7 @@ use crate::core::config::{Config, RightPanelTab, TabBarPosition}; use crate::core::ssh_profile::parse_quick_connect; use crate::ui::i18n::{L10nKey, alias_translations, t, t_fmt}; -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum CommandKind { NewTab, NewWorkspace, @@ -527,12 +527,14 @@ impl Command { Command::localized(L10nKey::CmdSelectAll, SelectAllText), ]; + // Reconnect, Remote Files and Port Forwarding are not here: each of + // them acts on the connection in the focused pane, and each returned + // without doing anything when there was not one. `palette_commands` + // adds them where they would do something, the way #549 made Save + // Connection as Host earn its row. let ssh = [ Command::localized(L10nKey::CmdSshAddConnection, OpenSshConnectInput), Command::localized(L10nKey::CmdSshManageProfiles, OpenSshProfiles), - Command::localized(L10nKey::CmdSshReconnect, RestartSshSession), - Command::localized(L10nKey::CmdSshRemoteFiles, ToggleSftp), - Command::localized(L10nKey::CmdSshPortForwarding, ShowSshForwards), ]; let agents = [ @@ -1327,6 +1329,44 @@ mod tests { .collect() } + /// The commands that act on the connection in the focused pane are added by + /// `Tty7App::palette_commands`, which can see whether there *is* one. Any + /// of them left in the static list is offered from every pane, and returns + /// without doing anything on most of them — the row that looks like it + /// works, does nothing, and says nothing about why (#549). + #[gpui::test] + fn the_static_list_offers_nothing_that_needs_a_live_connection(cx: &mut gpui::TestAppContext) { + cx.update(|cx| { + cx.set_global(crate::core::config::Config::default()); + let kinds: Vec = Command::base_commands( + cx, + ChromeState { + rail_collapsed: false, + right_panel_visible: false, + }, + ) + .into_iter() + .map(|c| c.kind) + .collect(); + + for needs_connection in [ + CommandKind::RestartSshSession, + CommandKind::ToggleSftp, + CommandKind::ShowSshForwards, + CommandKind::SaveSshSessionAsHost, + ] { + assert!( + !kinds.contains(&needs_connection), + "{needs_connection:?} acts on a connection, so only a pane that has one may offer it" + ); + } + assert!( + kinds.contains(&CommandKind::OpenSshConnectInput), + "the commands that make a connection are always available" + ); + }); + } + #[test] fn frecency_nudges_without_overruling_the_match() { assert_eq!(frecency_bonus(0.0), 0, "an unused command gets nothing"); diff --git a/src/ui/remote_connect.rs b/src/ui/remote_connect.rs index ec15c8cb..53c226d4 100644 --- a/src/ui/remote_connect.rs +++ b/src/ui/remote_connect.rs @@ -1262,4 +1262,34 @@ mod tests { adopt_probe(&mut state, Some(vec!["Arch".to_string()])); assert_eq!(state.names, vec!["Arch".to_string()]); } + + /// The pane that fills a window while a connection is being made names the + /// machine it is waiting on, and a `Profile` target spells itself as a bare + /// uuid. Reading `target.to_string()` there put "Connecting to + /// 51d32f65-e669-496b-8f7d-e09cb4991cb6…" on screen — the same leak #485 + /// took out of the switcher. Every caller that shows a target to a reader + /// goes through here instead, so this pins the two answers it can give. + #[gpui::test] + fn a_profile_target_never_labels_itself_with_its_uuid(cx: &mut gpui::TestAppContext) { + let mut profile = crate::core::ssh_profile::SshProfile::new("build-box"); + profile.host = "10.0.0.5".into(); + profile.user = "deploy".into(); + let saved = RemoteTarget::Profile { id: profile.id }; + let deleted = RemoteTarget::Profile { + id: uuid::Uuid::new_v4(), + }; + + cx.update(|cx| { + let mut cfg = Config::default(); + cfg.ssh_profiles = vec![profile.clone()]; + cx.set_global(cfg); + + assert_eq!(target_label(cx, &saved), "build-box"); + assert_eq!( + target_label(cx, &deleted), + t(L10nKey::RemoteProfileGone), + "a profile that is gone still may not be spelled as its uuid" + ); + }); + } } diff --git a/src/ui/remote_workspace.rs b/src/ui/remote_workspace.rs index 798f741f..24f1136d 100644 --- a/src/ui/remote_workspace.rs +++ b/src/ui/remote_workspace.rs @@ -151,6 +151,55 @@ impl RemoteStatus { pub fn accepts_input(&self) -> bool { matches!(self, RemoteStatus::Attached) } + + /// One or two words for a badge that has already spent its width on the + /// machine's name. + /// + /// `None` for `Attached`, which is the state every reader assumes: the dot + /// beside it is green, and spelling "connected" out on every healthy frame + /// is how a status line stops being read. Everything else is worth a word, + /// and the reason behind that word is a sentence the strip already carries. + pub fn short_label(&self) -> Option<&'static str> { + match self { + RemoteStatus::Attached => None, + RemoteStatus::Connecting => Some(t(L10nKey::RemoteStateConnecting)), + RemoteStatus::Reconnecting { .. } => Some(t(L10nKey::RemoteStateReconnecting)), + RemoteStatus::Preempted { .. } => Some(t(L10nKey::RemoteStatePreempted)), + RemoteStatus::Disconnected => Some(t(L10nKey::RemoteStateDisconnected)), + RemoteStatus::Failed(_) => Some(t(L10nKey::RemoteStateFailed)), + RemoteStatus::ServerMismatch(_) => Some(t(L10nKey::RemoteStateServerMismatch)), + RemoteStatus::RouteLost => Some(t(L10nKey::RemoteStateRouteLost)), + } + } + + /// The colour a status dot wears for this state. + /// + /// The same ramp the switcher paints its machine rows with — green while + /// the link is up, amber while it is being made or remade, red once it has + /// given up, grey for a machine nobody is talking to. One machine has to + /// read the same in the switcher and in the window it is open in, so both + /// ask here. + pub fn dot(&self, theme: &gpui_component::Theme) -> gpui::Hsla { + match self { + RemoteStatus::Attached => gpui::rgb(crate::ui::tab_strip::LIVE_DOT).into(), + RemoteStatus::Connecting | RemoteStatus::Reconnecting { .. } => theme.warning, + RemoteStatus::Preempted { .. } => theme.warning, + RemoteStatus::Failed(_) | RemoteStatus::ServerMismatch(_) | RemoteStatus::RouteLost => { + theme.danger + } + RemoteStatus::Disconnected => gpui::rgb(crate::ui::tab_strip::UNKNOWN_DOT).into(), + } + } +} + +/// The machine a window is on, when that is not this computer. +/// +/// `None` for a local workspace on purpose: a badge shown everywhere means +/// nothing anywhere, and every surface that asks here is asking "is there +/// something the reader does not already assume?". +pub(crate) struct WindowMachine { + pub label: String, + pub status: RemoteStatus, } /// Which way the one button points. @@ -366,6 +415,32 @@ impl Tty7App { pane_workspace_for(cx, self.workspace) } + /// The machine this window is on, when it is not this computer — for the + /// surfaces that have to keep saying so while everything is going *right*. + /// + /// `remote_status` already answers `None` for a local workspace, which is + /// the whole condition; the label costs a config read, so it is only taken + /// once that has passed. + pub(crate) fn window_machine(&self, cx: &gpui::App) -> Option { + let status = self.remote_status(cx)?; + Some(WindowMachine { + label: self.remote_machine_label(cx), + status, + }) + } + + /// The address behind the machine's name — `deploy@10.0.0.5:2222` — for + /// the surfaces that show the name and can spare a tooltip for the rest. + /// + /// `None` when the snapshot spells the endpoint the same way the label + /// does, which is every host saved without a name: repeating a string + /// under itself is not an explanation. + pub(crate) fn remote_endpoint_label(&self, cx: &gpui::App) -> Option { + let host = WorkspaceStore::remote_ref(cx, self.workspace)?; + let endpoint = host.via.as_ref()?.endpoint(); + (endpoint != remote_connect::route_label(cx, &host)).then_some(endpoint) + } + pub(crate) fn remote_machine_label(&self, cx: &gpui::App) -> String { match WorkspaceStore::remote_ref(cx, self.workspace) { Some(host) => remote_connect::route_label(cx, &host), @@ -2414,6 +2489,37 @@ mod tests { assert!(!RemoteStatus::Failed("x".into()).accepts_input()); } + /// The badge under the workspace head has already spent its width on the + /// machine's name, so the state beside it is one or two words — and the + /// state every reader assumes is no words at all. A green dot next to + /// "java-box · connected" says the same thing twice. + #[test] + fn only_a_state_worth_reading_earns_a_word() { + assert_eq!(RemoteStatus::Attached.short_label(), None); + for status in [ + RemoteStatus::Connecting, + RemoteStatus::Reconnecting { + attempt: 2, + last_error: None, + }, + RemoteStatus::Preempted { + by: "laptop".into(), + }, + RemoteStatus::Disconnected, + RemoteStatus::Failed("connection refused".into()), + RemoteStatus::ServerMismatch("speaks control v9".into()), + RemoteStatus::RouteLost, + ] { + let word = status + .short_label() + .unwrap_or_else(|| panic!("{status:?} has nothing to say")); + assert!( + word.split_whitespace().count() <= 2, + "{status:?} said {word:?}, which is a sentence, not a badge" + ); + } + } + #[test] fn every_flow_state_names_its_machine() { let choice = HostChoice { diff --git a/src/ui/right_panel.rs b/src/ui/right_panel.rs index 2ae44ca1..0ff9ce03 100644 --- a/src/ui/right_panel.rs +++ b/src/ui/right_panel.rs @@ -700,14 +700,7 @@ impl Tty7App { if let Some(ssh) = view.ssh_spec() { rows.push(InfoRow::text(t(L10nKey::PanelSsh), ssh.host.clone()).copyable()); } - let connected_ssh = view - .remote_context() - .is_some_and(|c| c.kind == crate::daemon::protocol::RemoteKind::NativeSsh) - && matches!( - view.ssh_phase(), - Some(crate::daemon::protocol::SshPhase::Connected) - ); - if connected_ssh || view.workspace().is_some() { + if pane_holds_forwards(view) { forwards_pane = Some(view.pane_id); } git = view.git_status(cx); @@ -1360,6 +1353,24 @@ impl Tty7App { } } +/// Whether a pane has a connection that forwards can be opened over: a native +/// ssh session that finished authenticating, or a pane the daemon reaches +/// through a remote workspace's link. +/// +/// The Info panel decides whether to draw the Forwards section from this, and +/// the palette decides whether to offer *SSH: Port Forwarding* from it too. +/// They used to answer separately and disagree: the panel showed the section +/// for a remote-workspace pane, and the command — which asked only for a native +/// ssh pane — returned without doing anything on exactly those panes. +pub(crate) fn pane_holds_forwards(view: &crate::terminal::view::TerminalView) -> bool { + use crate::daemon::protocol::{RemoteKind, SshPhase}; + let connected_ssh = view + .remote_context() + .is_some_and(|c| c.kind == RemoteKind::NativeSsh) + && matches!(view.ssh_phase(), Some(SshPhase::Connected)); + connected_ssh || view.workspace().is_some() +} + /// Width of the fixed cell a git status letter is centred in. /// /// Load-bearing beyond this function: `scm/panel.rs` gives its group-header diff --git a/src/ui/ssh_connect.rs b/src/ui/ssh_connect.rs index 3903b12f..f7d288c6 100644 --- a/src/ui/ssh_connect.rs +++ b/src/ui/ssh_connect.rs @@ -83,22 +83,31 @@ impl Tty7App { self.open_native_ssh_tab(spec, window, cx); } + /// The connection Reconnect would put back: one in the focused pane that + /// has dropped and still remembers what it was dialled with. + /// + /// What the command *does* and whether the palette offers it are the same + /// question, so both read it here — a Reconnect row on a pane with nothing + /// to reconnect is a button that does nothing and does not say so. + pub(crate) fn ssh_session_to_restart( + &self, + window: &gpui::Window, + cx: &gpui::App, + ) -> Option> { + let view = self.focused_pane_view(window, cx)?; + let view = view.read(cx); + view.ssh_disconnected().then(|| view.ssh_spec()).flatten() + } + pub(crate) fn restart_ssh_session( &mut self, window: &mut gpui::Window, cx: &mut gpui::Context, ) { - let Some(view) = self.focused_pane_view(window, cx) else { + let Some(spec) = self.ssh_session_to_restart(window, cx) else { return; }; - let dead_spec = { - let v = view.read(cx); - if !v.ssh_disconnected() { - return; - } - v.ssh_spec() - }; - let Some(spec) = dead_spec else { + let Some(view) = self.focused_pane_view(window, cx) else { return; }; let resolved = self.resolve_restart_spec(spec, cx); diff --git a/src/ui/tab_sidebar.rs b/src/ui/tab_sidebar.rs index bee35363..6b3e4151 100644 --- a/src/ui/tab_sidebar.rs +++ b/src/ui/tab_sidebar.rs @@ -69,6 +69,21 @@ struct SidebarRowShown { title: Option<(SharedString, SharedString)>, branch: Option<(SharedString, SharedString, u32, u32)>, cwd: Option<(SharedString, SharedString)>, + /// The machine the pane is on, when the row named it because the window's + /// own banner does not. + machine: Option<(SharedString, SharedString)>, +} + +/// Whether a sidebar row names the machine its pane is on. +/// +/// The window's own machine is already on the banner under the workspace head, +/// so a row only speaks up when it would be *contradicting* that: an ssh pane +/// opened inside a local workspace, or a `wsl` shell beside Windows ones. +/// Repeating the workspace's machine on every row of a remote window is how a +/// badge stops being read. +fn row_machine(pane: Option, window: Option<&str>) -> Option { + let pane = pane?; + (!pane.trim().is_empty() && Some(pane.as_str()) != window).then_some(pane) } #[derive(Clone)] @@ -192,6 +207,9 @@ impl Tty7App { .collect(); let pointer = window.mouse_position(); + // What the banner under the workspace head already says, so a row can + // tell whether naming its own machine would be news or an echo. + let window_machine = self.window_machine(cx).map(|m| m.label); // The row text is measured against real glyphs before it is elided: // `text_sm` is 0.875rem and `text_xs` 0.75rem, resolved here so the // measurement and the render use the same sizes and family. @@ -442,10 +460,36 @@ impl Tty7App { } line }); + // A pane somewhere other than where the rest of the window is + // says so on its own line. The window's machine is already on + // the banner under the workspace head, so a row only speaks up + // when it would be *contradicting* that — an ssh pane opened + // inside a local workspace, or a `wsl` shell beside Windows + // ones. Repeating the workspace's own machine on every row of a + // remote window is how a badge stops being read. + let machine_shown = row_machine( + tab.pane + .focused_or_first(window, cx) + .and_then(|leaf| leaf.read(cx).remote_context()) + .map(|remote| remote.target), + window_machine.as_deref(), + ) + .map(|target| { + let full = SharedString::from(target); + let shown = elide_keep_edges( + &window.text_system(), + &font, + meta_size, + &full, + (label_avail - row_metrics::BRANCH_ICON - row_metrics::META_GAP).max(0.), + ); + (shown, full) + }); // Outside a repo there is no branch line; the second line then // carries the compressed cwd with its root marker, so a tab // whose title is just a shell name still says where it lives. - if git_line.is_none() { + // A machine line has already answered "where", and better. + if git_line.is_none() && machine_shown.is_none() { cwd_shown = tab .pane .focused_or_first(window, cx) @@ -481,8 +525,9 @@ impl Tty7App { title: full_title.map(|full| (shown_title.clone(), full)), branch: branch_shown.clone(), cwd: cwd_shown.clone(), + machine: machine_shown.clone(), }; - let info = self.sidebar_info(tab, window, cx, &shown); + let info = Self::sidebar_info(&shown); // Colors are captured by value so the tooltip builder (which // borrows no app state) can style the card on its own. let muted = cx.theme().muted_foreground; @@ -596,6 +641,25 @@ impl Tty7App { .when(is_active, |d| d.font_weight(FontWeight::MEDIUM)) .child(shown_title), ) + .when_some(machine_shown, |col, (machine, _)| { + col.child( + h_flex() + .id(("sidebar-machine-row", i)) + .w_full() + .items_center() + .gap_1p5() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child( + gpui::svg() + .path("icons/machine-remote.svg") + .flex_shrink_0() + .size(px(row_metrics::BRANCH_ICON)) + .text_color(cx.theme().muted_foreground), + ) + .child(div().flex_1().min_w_0().truncate().child(machine)), + ) + }) .children(git_line) .when_some(cwd_shown, |col, (cwd, _)| { col.child( @@ -980,6 +1044,8 @@ impl Tty7App { .pt(px(4.)) .child(self.workspace_head(cx)); + let machine_banner = self.remote_head_banner(cx); + let chip_inset = crate::ui::app::CONTENT_INSET - 7. + 4.; let top_bar = h_flex() .flex_shrink_0() @@ -1115,6 +1181,7 @@ impl Tty7App { cx, )) .child(workspace_head) + .children(machine_banner) .child(top_bar) .child(crate::ui::scrollbar::with_vertical_scrollbar( "tab-sidebar-scrollbar", @@ -1125,24 +1192,71 @@ impl Tty7App { .child(handle) } + /// The line under the workspace head naming the machine this window is on. + /// + /// A window on another computer used to look exactly like one on this one + /// for as long as nothing went wrong: the home screen's strip stays silent + /// while the link is `Attached`, the workspace head shows a name someone + /// made up, and a connected pane's row is titled by the remote shell's own + /// cwd. The only trace was a 6px dot on the tab avatar. Running a command + /// on the wrong machine is the mistake this product can make that costs the + /// most, so the machine is stated where the window's identity already lives + /// and left there — the same dot-plus-name the switcher's rows use, so one + /// machine reads the same in both places. + /// + /// `None` on a local workspace: a badge that is always there stops being + /// read. + fn remote_head_banner(&self, cx: &mut Context) -> Option> { + let machine = self.window_machine(cx)?; + let theme = cx.theme(); + let muted = theme.muted_foreground; + let dot = machine.status.dot(theme); + // Only when it is not the state a reader already assumes. "Connected" + // spelled out on every frame of a healthy link is the noise this + // banner is trying not to be; the dot carries that case alone. + let state = machine.status.short_label(); + let endpoint = self.remote_endpoint_label(cx); + + Some( + h_flex() + .id("sidebar-machine") + .flex_shrink_0() + .items_center() + .gap(px(6.)) + .w_full() + .px(px(crate::ui::app::CONTENT_INSET - 3.)) + .pt(px(5.)) + .text_xs() + .text_color(muted) + .when_some(endpoint, |row, endpoint| { + row.tooltip(move |window, cx| { + gpui_component::tooltip::Tooltip::new(endpoint.clone()).build(window, cx) + }) + }) + .child(div().flex_shrink_0().size(px(6.)).rounded_full().bg(dot)) + .child( + div() + .min_w_0() + .truncate() + .child(SharedString::from(machine.label)), + ) + .when_some(state, |row, state| { + row.child(div().flex_shrink_0().child("·")) + .child(div().flex_shrink_0().child(state)) + }), + ) + } + /// What the sidebar row hid: the full title, the full branch and diff - /// counts, the working directory, and the remote host the avatar only - /// dots. `None` when the row showed everything — a card would add noise, - /// not information. The host is included even for an untruncated row, - /// because the title strips the `user@host:` prefix the avatar cannot - /// spell out. + /// counts, the working directory, and the machine the pane is on. + /// `None` when the row showed everything — a card would add noise, not + /// information. /// /// Every line is decided by comparing what the row rendered against the /// string it was elided from. Both come from the row itself: deriving /// them here a second time is how a renamed tab ended up with a name the /// row shortened and the card refused to expand. - fn sidebar_info( - &self, - tab: &crate::ui::app::Tab, - window: &mut Window, - cx: &gpui::App, - shown: &SidebarRowShown, - ) -> Option { + fn sidebar_info(shown: &SidebarRowShown) -> Option { let elided = |pair: &Option<(SharedString, SharedString)>| { pair.as_ref() .filter(|(shown, full)| shown != full) @@ -1164,13 +1278,16 @@ impl Tty7App { // The host is read off the same leaf the title and cwd came from; a // split tab whose panes sit on different machines would otherwise // name whichever one happens to be first. - if let Some(target) = tab.pane.focused_or_first(window, cx).and_then(|leaf| { - leaf.read(cx) - .remote_context() - .map(|r| SharedString::from(r.target.clone())) - }) { - info.host = Some(target); - } + // + // The row now carries the machine itself whenever it differs from the + // window's, so the card only takes it when the row could not: either it + // had to elide the name, or the row stayed quiet because the banner + // over the sidebar already says it — and a card that repeats what is + // two rows above it is noise. + info.host = match &shown.machine { + Some((shown, full)) => (shown != full).then(|| full.clone()), + None => None, + }; (info.title.is_some() || info.branch.is_some() || info.cwd.is_some() || info.host.is_some()) .then_some(info) } @@ -1611,4 +1728,36 @@ mod tests { let names = group_names(&[&short, &long]); assert_eq!(names, vec!["app", "x/app"]); } + + /// The banner under the workspace head names the window's machine, so the + /// only rows worth a machine line are the ones that would contradict it. + /// Repeating "java-box" down every row of a java-box window is how the + /// badge stops being read; leaving an ssh pane inside a *local* workspace + /// unlabelled is how a command lands on the wrong computer. + #[test] + fn a_row_names_its_machine_only_when_the_window_does_not() { + let some = |s: &str| Some(s.to_string()); + + assert_eq!( + row_machine(some("java-box"), None), + some("java-box"), + "an ssh pane in a local workspace has to say so" + ); + assert_eq!( + row_machine(some("java-box"), Some("java-box")), + None, + "every row of a java-box window would just echo the banner" + ); + assert_eq!( + row_machine(some("build-box"), Some("java-box")), + some("build-box"), + "a pane ssh'd on somewhere else still contradicts the banner" + ); + assert_eq!(row_machine(None, Some("java-box")), None); + assert_eq!( + row_machine(some(" "), None), + None, + "a remote context with no target names nothing" + ); + } }