From e1efae280975ce5ad6fa33ec9ee64f26a044d7f6 Mon Sep 17 00:00:00 2001 From: Hongwei Qin <122079993+shihuaidexianyu@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:38:49 +0800 Subject: [PATCH] fix(ui): read the Info panel agent row off one pane, and shorten Windows paths to their leaf (#543, #544) (#570) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ui): read the Info panel agent row off one pane, and shorten Windows paths to their leaf (#543, #544) Two rows in the Info panel were misreading a split tab or a Windows path. The agent row took its name from `tab.agent` (the first leaf with an agent) and its status from `tab.agent_status` (the most urgent across the whole tab); the two resolve independently, so a split tab running two agents could show one pane name beside the other pane state — a row no leaf ever had. Both now come from the detail pane when it has an agent, falling back to the tab aggregate only when the focused leaf has none, so the row holds while focus sits on a plain shell but never splices. The cwd row split the path on `/` only, so a backslash-spelled path — any agent-reported cwd (`agent_session.cwd` arrives as `C:\…`), a cmd pane, a shell-integration-off pane, or the seed cwd before the first prompt — elided its tail and hid the directory own name. The split now takes the last of either separator, and is not cfg-gated: the panel shows remote paths, so a Windows build describes Unix paths and vice versa. The `~` shortening moves into a shared `path_display` helper reading `USERPROFILE` as well as `HOME` and comparing with separators normalized and case folded; the tab strip `abbreviate_home` and the home picker `display_path` had the same HOME-only miss and now use it too. Tests pin backslash, mixed-separator, drive-root and UNC shapes. * fix(ui): hold the agent row's fallback to one leaf, and stop the home test leaking Review follow-ups on the #543/#544 pair. - The fallback branch still spliced. When the focused leaf carries no agent the row fell back to `tab.agent` + `tab.agent_status`, which is exactly the pair the fix set out to break up: the first leaf with an agent, beside the highest urgency anywhere in the tab. A three-way split with focus on a plain shell could still read `Claude · Working` off two different panes. `Tab::agent_row` now picks the most urgent agent leaf and answers both halves out of it; `agent_status` is that pair's status, so the tab strip's badge is unchanged. - `path_display`'s test home was a process-global that no test ever cleared, so once one of these tests ran, every later `abbreviate_home` in the binary read the pinned home — including `ui::home`'s own test, which sets `HOME` and expects to see it. Ordering decided whether it passed. The comparison moves into `abbreviate_under(path, home)` and the tests hand their home in; nothing global is left to leak. Adds the trailing-separator home and non-ASCII component cases the byte boundary reasoning turns on. --------- Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> --- CHANGELOG.md | 16 +++++ src/ui/app.rs | 34 +++++++++-- src/ui/home.rs | 9 +-- src/ui/mod.rs | 1 + src/ui/path_display.rs | 129 +++++++++++++++++++++++++++++++++++++++++ src/ui/right_panel.rs | 86 ++++++++++++++++++++++----- src/ui/tab_strip.rs | 19 ++---- 7 files changed, 253 insertions(+), 41 deletions(-) create mode 100644 src/ui/path_display.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c2d1d21..b6f1f9e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 command exists; only a *failed* hang-up (which `ws rm` reports by pane id) leaves orphans behind. The site reference, the bundled skill reference, and `ws rm --help` all read the same way now (#539). +- **The Info panel's agent row reads one pane, not a splice of two** — the + name came from `tab.agent` (whichever leaf has an agent) while the status + came from `tab.agent_status` (the most urgent across the whole tab), so a + split tab running two agents could show one pane's name beside the other + pane's state. The row now takes both from the detail pane when it has an + agent, and otherwise from the tab's most urgent agent pane — so the row + still holds while focus sits on a plain shell, and its two halves always + describe the same pane (#543). +- **Windows paths in the Info panel shorten to their leaf again** — the cwd + row split on `/` only, so a backslash-spelled path (any agent-reported cwd, + a cmd pane, the shell-integration-off case) elided its *tail* and hid the + directory's own name. The split now takes the last of either separator, + and the `~` shortening — shared by the Info panel, the tab strip, and the + home picker — reads `USERPROFILE` as well as `HOME` and compares with + separators normalized and case folded, so a `C:/Users/…` pane shortens + under a `C:\Users\…` home (#544). ## [26.8.3] - 2026-08-12 diff --git a/src/ui/app.rs b/src/ui/app.rs index 4d7574a7..753c3946 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -341,7 +341,20 @@ impl Tab { .find_map(|l| l.read(cx).agent()) } - pub(crate) fn agent_status(&self, cx: &App) -> Option { + /// The tab's most urgent agent leaf, named and reported by that one leaf. + /// + /// `agent` and `agent_status` answer independently — the *first* leaf + /// carrying an agent, and the highest urgency found *anywhere* in the tab + /// — so reading them as a pair can put one pane's name beside another + /// pane's state, a row no leaf ever had. Anywhere both halves are shown + /// at once reads them from here instead (#543). + pub(crate) fn agent_row( + &self, + cx: &App, + ) -> Option<( + crate::core::cli_agent::CLIAgent, + crate::core::cli_agent::AgentStatus, + )> { use crate::core::cli_agent::AgentStatus; let urgency = |s: AgentStatus| match s { AgentStatus::Waiting => 3, @@ -352,14 +365,23 @@ impl Tab { self.pane .terminals() .into_iter() - .filter(|l| l.read(cx).agent().is_some()) - .map(|l| { - l.read(cx) + .filter_map(|l| { + let view = l.read(cx); + let agent = view.agent()?; + // A pane whose agent is running but has never reported a + // session reads as idle, the same reading the badge has always + // given it. + let status = view .agent_session() .map(|s| s.status) - .unwrap_or(AgentStatus::Idle) + .unwrap_or(AgentStatus::Idle); + Some((agent, status)) }) - .max_by_key(|s| urgency(*s)) + .max_by_key(|(_, status)| urgency(*status)) + } + + pub(crate) fn agent_status(&self, cx: &App) -> Option { + self.agent_row(cx).map(|(_, status)| status) } pub(crate) fn agent_unread_count(&self, cx: &App) -> usize { diff --git a/src/ui/home.rs b/src/ui/home.rs index 9e06cf7a..9df9427b 100644 --- a/src/ui/home.rs +++ b/src/ui/home.rs @@ -104,12 +104,9 @@ pub(crate) fn relative_time(now: u64, then: u64) -> String { pub(crate) fn display_path(path: &std::path::Path) -> String { let text = path.to_string_lossy(); - let shortened = match std::env::var("HOME") { - Ok(home) if !home.is_empty() && text.starts_with(&home) => { - format!("~{}", &text[home.len()..]) - } - _ => text.to_string(), - }; + // Same home-abbreviation the Info panel and tab strip use: HOME with a + // USERPROFILE fallback, separators normalized, case folded (#544). + let shortened = crate::ui::path_display::abbreviate_home(&text).into_owned(); if shortened.chars().count() <= PICKER_PATH_MAX { return shortened; } diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 106b46fb..ee86b958 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -19,6 +19,7 @@ pub mod machine_mirror; pub mod palette; pub mod pane; pub mod pane_drag; +pub mod path_display; pub mod pending_pane; pub mod perf; pub mod prefill; diff --git a/src/ui/path_display.rs b/src/ui/path_display.rs new file mode 100644 index 00000000..c9f96718 --- /dev/null +++ b/src/ui/path_display.rs @@ -0,0 +1,129 @@ +//! The one place a path is shortened to start from `~`. +//! +//! Three rows shorten a path for display — the Info panel's cwd, the tab +//! strip's title, the home picker's recent list — and all three used to spell +//! the check their own way: read `HOME`, compare byte prefixes. On Windows +//! that missed twice over. `HOME` is often unset there (the variable is +//! `USERPROFILE`), and even a set home never matched a pane cwd that spells +//! itself with the other separator or different case — a PowerShell pane +//! reports `C:/Users/x/…` while `USERPROFILE` is `C:\Users\x` (#544). +//! +//! The comparison below normalizes both sides to `/` and folds case before +//! comparing, so every spelling of the same directory shortens. What comes +//! back is for reading only — a `~`-rooted path is spelled the way `~` paths +//! are spelled everywhere, with `/`. Nothing feeds it back to an API: the +//! Info panel's Copy Path and Reveal both carry the untouched `PathBuf`, and +//! the tab strip and picker only ever draw it. + +use std::borrow::Cow; + +/// The directory `~` stands for, or `None` when this machine won't say. +/// +/// `USERPROFILE` is the fallback rather than the only source on Windows so +/// the MSYS/Git-Bash environments that do export `HOME` keep working, and +/// the two agree in every case that matters. +fn home_dir() -> Option { + std::env::var_os("HOME") + .filter(|h| !h.is_empty()) + .or_else(|| std::env::var_os("USERPROFILE").filter(|h| !h.is_empty())) +} + +/// `/`-spelled, case-folded, trailing separators dropped — the form two +/// paths are compared in, never the form either is shown in. Case folding is +/// ASCII-only: drive letters and the ASCII half of real paths are where +/// Windows case instability actually lives, and a full Unicode fold would +/// fold a Unix filename that happened to differ only in case into a match it +/// is not. +fn normalized(s: &str) -> String { + s.replace('\\', "/") + .trim_end_matches('/') + .to_ascii_lowercase() +} + +/// Shortens `path` to start from `~` when it is (inside) the home directory. +/// +/// The `~` replaces the home prefix and the remainder is re-spelled with +/// `/` separators (a `~\work` hybrid reads as a root the path never had), +/// but its case and component spelling are the path's own. A path that is +/// exactly home shortens to `~`, and one whose next character is not a +/// separator (`/home/xavier` under `/home/xa`) does not match at all. +pub(crate) fn abbreviate_home(path: &str) -> Cow<'_, str> { + let Some(home) = home_dir() else { + return Cow::Borrowed(path); + }; + abbreviate_under(path, &home.to_string_lossy()) +} + +/// `abbreviate_home` with the home handed in rather than read from the +/// environment, so the tests below pin one without touching a process-global +/// the rest of the binary is also reading (`ui::home`'s own test sets `HOME` +/// and expects to see it, and everything runs in one process). +fn abbreviate_under<'a>(path: &'a str, home: &str) -> Cow<'a, str> { + let home_norm = normalized(home); + if home_norm.is_empty() { + return Cow::Borrowed(path); + } + let path_norm = normalized(path); + if path_norm == home_norm { + return Cow::Owned("~".to_string()); + } + if !path_norm.starts_with(&home_norm) { + return Cow::Borrowed(path); + } + // The byte after the home prefix has to be a separator. Where it sits in + // the *original* string is derived from the normalized one rather than + // from `home.len()`: a trailing-separator difference (`C:\Users\xa\` + // recorded as home) makes the two lengths disagree, and slicing by the + // wrong one can split a UTF-8 boundary. Separator and case substitutions + // preserve byte length, so the boundary found in the normalized string is + // the boundary in the original. The remainder is re-spelled with `/`: + // `~\work` reads as a root the path never had. + let boundary = home_norm.len(); + if !path_norm[boundary..].starts_with('/') { + return Cow::Borrowed(path); + } + Cow::Owned(format!("~/{}", path[boundary + 1..].replace('\\', "/"))) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_path_under_home_shortens_to_tilde() { + assert_eq!(abbreviate_under("/home/xa", "/home/xa"), "~"); + assert_eq!(abbreviate_under("/home/xa/work", "/home/xa"), "~/work"); + // A home recorded with its trailing separator matches the same paths, + // and the slice that follows it is found in the *normalized* string, + // so the two spellings cannot disagree about where the cut is. + assert_eq!(abbreviate_under("/home/xa/work", "/home/xa/"), "~/work"); + // A longer name that merely starts with home is not under it. + assert_eq!(abbreviate_under("/home/xavier", "/home/xa"), "/home/xavier"); + assert_eq!(abbreviate_under("/var/tmp", "/home/xa"), "/var/tmp"); + // A home the environment reports as empty leaves every path alone, + // rather than turning `/` into `~`. + assert_eq!(abbreviate_under("/var/tmp", ""), "/var/tmp"); + assert_eq!(abbreviate_under("/var/tmp", "/"), "/var/tmp"); + } + + #[test] + fn separators_and_case_do_not_change_what_counts_as_home() { + // The Windows miss: USERPROFILE spells `C:\Users\xa`, a PowerShell + // pane reports `C:/Users/xa/…`, and neither matched the other. + let home = "C:\\Users\\xa"; + assert_eq!(abbreviate_under("C:/Users/xa/work", home), "~/work"); + assert_eq!(abbreviate_under("c:\\Users\\XA\\work", home), "~/work"); + assert_eq!(abbreviate_under("C:\\Users\\xa", home), "~"); + // The remainder is re-spelled with `/`, case untouched. + assert_eq!(abbreviate_under("C:/Users/xa/Mix\\ed", home), "~/Mix/ed"); + } + + #[test] + fn a_non_ascii_component_is_sliced_on_a_character_boundary() { + // The cut is taken from the normalized string; `replace` and the + // ASCII case fold both preserve byte length, so a multi-byte + // component before or after the home prefix cannot move it. + assert_eq!(abbreviate_under("/home/日本/work", "/home/日本"), "~/work"); + assert_eq!(abbreviate_under("/home/xa/日本語", "/home/xa"), "~/日本語"); + } +} diff --git a/src/ui/right_panel.rs b/src/ui/right_panel.rs index b169c373..16d33301 100644 --- a/src/ui/right_panel.rs +++ b/src/ui/right_panel.rs @@ -625,6 +625,11 @@ impl Tty7App { // off in both places rather than in one of them. let mut diff_target: Option<(crate::ui::host_ops::HostId, PathBuf)> = None; let mut git: Option = None; + // The agent row's name and status, read off one leaf (see below). + let mut agent_row: Option<( + crate::core::cli_agent::CLIAgent, + crate::core::cli_agent::AgentStatus, + )> = None; if let Some(tab) = self.tabs.get(self.active) { if let Some(leaf) = tab.detail_pane(window, cx) { @@ -667,6 +672,26 @@ impl Tty7App { forwards_pane = Some(view.pane_id); } git = view.git_status(cx); + // Name and status come from the *same* leaf: the detail pane's + // own agent when it has one, and otherwise the tab's most + // urgent agent leaf — which still holds the row while focus + // sits on a plain shell, and still colours its dot the way the + // tab strip's badge does, but names the pane it took the + // status from. Pairing `tab.agent` with `tab.agent_status` + // would splice one pane's name onto another pane's status — a + // row no leaf ever had — because the two resolve + // independently (#543). Read here, where `view` is in scope; + // pushed beside the other rows below. + agent_row = match view.agent() { + Some(agent) => { + let status = view + .agent_session() + .map(|s| s.status) + .unwrap_or(crate::core::cli_agent::AgentStatus::Idle); + Some((agent, status)) + } + None => tab.agent_row(cx), + }; } // Read off the same pane the rows above describe, rather than off // `Tab::git_status`, which resolves a split tab to its *first* leaf @@ -691,18 +716,15 @@ impl Tty7App { reveal: None, }); } - if let Some(agent) = tab.agent(cx) { + // Name and status were read off one leaf above; push the row. + if let Some((agent, status)) = agent_row { let name = agent.display_name(); - let status = tab.agent_status(cx); rows.push(InfoRow { label: t(L10nKey::PanelAgent), value: InfoValue::Agent { - text: match status { - Some(s) => format!("{name} · {}", agent_status_label(s)), - None => name.to_string(), - }, - dot: status.and_then(|s| s.dot_rgb()), - hollow: status == Some(crate::core::cli_agent::AgentStatus::Waiting), + text: format!("{name} · {}", agent_status_label(status)), + dot: status.dot_rgb(), + hollow: status == crate::core::cli_agent::AgentStatus::Waiting, }, copy: None, reveal: None, @@ -1419,7 +1441,14 @@ fn agent_status_label(status: crate::core::cli_agent::AgentStatus) -> &'static s /// Splits a path into everything-but-the-last-segment and the last segment, /// so a row can shrink the first and keep the second. fn split_path_leaf(s: &str) -> (String, String) { - match s.rfind('/') { + // The larger of the two separator positions, not cfg-gated by platform: + // the Info panel shows remote paths too, so a Windows build describes + // Unix paths and vice versa — and a mixed-spelling path (`C:\Users\dev/ + // project`, which agent-reported cwds arrive as) still cuts at its true + // leaf (#544). A Unix filename containing a literal `\` loses a shorter + // leaf; head + leaf still rejoins exactly, so the cost is decorative. + let leaf_at = s.rfind('/').max(s.rfind('\\')); + match leaf_at { // Keep the separator with the head: "~/a/b/" + "c" rejoins exactly. Some(i) if i + 1 < s.len() => (s[..=i].to_string(), s[i + 1..].to_string()), _ => (String::new(), s.to_string()), @@ -1427,11 +1456,7 @@ fn split_path_leaf(s: &str) -> (String, String) { } fn compact_path(path: &std::path::Path) -> String { - let s = path.to_string_lossy().to_string(); - match std::env::var("HOME") { - Ok(home) if !home.is_empty() && s.starts_with(&home) => s.replacen(&home, "~", 1), - _ => s, - } + crate::ui::path_display::abbreviate_home(&path.to_string_lossy()).into_owned() } #[cfg(test)] @@ -1526,6 +1551,9 @@ mod tests { "/", "relative", "", + "C:\\Users\\dev\\project", + "C:\\Users\\dev/project", + "\\\\server\\share\\dir", ] { let (head, leaf) = split_path_leaf(p); assert_eq!(format!("{head}{leaf}"), p, "rejoining {p:?}"); @@ -1543,4 +1571,34 @@ mod tests { let (head, leaf) = split_path_leaf("/"); assert_eq!((head.as_str(), leaf.as_str()), ("", "/")); } + + #[test] + fn the_leaf_survives_windows_and_mixed_spellings() { + // Backslash-native, the shape an agent-reported cwd arrives in. + let (head, leaf) = split_path_leaf("C:\\Users\\dev\\project"); + assert_eq!( + (head.as_str(), leaf.as_str()), + ("C:\\Users\\dev\\", "project") + ); + // Mixed separators cut at the *last* one of either kind. + let (head, leaf) = split_path_leaf("C:\\Users\\dev/project"); + assert_eq!( + (head.as_str(), leaf.as_str()), + ("C:\\Users\\dev/", "project") + ); + let (head, leaf) = split_path_leaf("C:/Users/dev\\project"); + assert_eq!( + (head.as_str(), leaf.as_str()), + ("C:/Users/dev\\", "project") + ); + // A drive root has no leaf to keep. + let (head, leaf) = split_path_leaf("C:\\"); + assert_eq!((head.as_str(), leaf.as_str()), ("", "C:\\")); + // A UNC path splits at its last component, head keeping the share. + let (head, leaf) = split_path_leaf("\\\\server\\share\\dir"); + assert_eq!( + (head.as_str(), leaf.as_str()), + ("\\\\server\\share\\", "dir") + ); + } } diff --git a/src/ui/tab_strip.rs b/src/ui/tab_strip.rs index 955639b6..7d927694 100644 --- a/src/ui/tab_strip.rs +++ b/src/ui/tab_strip.rs @@ -57,21 +57,10 @@ pub(crate) fn abbreviate_home(path: &str) -> std::borrow::Cow<'_, str> { if path.starts_with('~') { return Cow::Borrowed(path); } - let Some(home) = std::env::var_os("HOME") else { - return Cow::Borrowed(path); - }; - let home = home.to_string_lossy(); - let home = home.trim_end_matches('/'); - if home.is_empty() { - return Cow::Borrowed(path); - } - if path == home { - return Cow::Owned("~".to_string()); - } - match path.strip_prefix(home) { - Some(rest) if rest.starts_with('/') => Cow::Owned(format!("~{rest}")), - _ => Cow::Borrowed(path), - } + // The shared comparison: HOME with a USERPROFILE fallback, separators + // normalized, case folded — a Windows pane whose cwd spells itself + // `C:/Users/…` shortens under a `C:\Users\…` home too (#544). + crate::ui::path_display::abbreviate_home(path) } /// The separator a path spells itself with. A path carrying a single `\` is