From f852629ba109985f61f3e301f0e33e18e4987a71 Mon Sep 17 00:00:00 2001 From: l0ng-ai Date: Fri, 25 Sep 2026 13:49:28 +0800 Subject: [PATCH 1/5] fix(procinfo): read proc_listallpids' answer as a pid count, not bytes (#947) libproc's proc_listallpids takes its buffer size in bytes but answers with the number of pids, and the macOS process table read that number as bytes twice: once sizing the buffer (count / 4 + 64 slots) and once reading back how much was filled (count / 4 again). On a Mac with ~700 processes only the first ~60 pids reached the table. The kernel lists its newest processes first, so what survived was whatever started most recently. A `go run` server launched a moment ago was in; the pane's shell, started long before, usually was not, and a walk from a root missing from the table returns nothing: no processes, no ports, and a probe state of Ok. A freshly built parent/child chain, which is what a quick check of the probe builds, is exactly the case that happened to work. The listing now goes through list_all_pids, which takes the call as a closure so the size arithmetic is tested off a Mac against an emulation of libproc's convention. Claude-Session: https://claude.ai/code/session_01JRqYZ9E153WpSHGS2AW3BM --- crates/tty7-core/src/daemon/procinfo.rs | 91 ++++++++++++++++++++----- 1 file changed, 74 insertions(+), 17 deletions(-) diff --git a/crates/tty7-core/src/daemon/procinfo.rs b/crates/tty7-core/src/daemon/procinfo.rs index 217a530b..e55581fb 100644 --- a/crates/tty7-core/src/daemon/procinfo.rs +++ b/crates/tty7-core/src/daemon/procinfo.rs @@ -225,23 +225,15 @@ fn walk(table: &HashMap, shell_pid: u32, fg_pgid: Option) -> Vec< #[cfg(target_os = "macos")] fn process_table() -> HashMap { let mut table = HashMap::new(); - let bytes = unsafe { libc::proc_listallpids(std::ptr::null_mut(), 0) }; - if bytes <= 0 { - return table; - } - let cap = (bytes as usize / std::mem::size_of::()) + 64; - let mut pids = vec![0 as libc::c_int; cap]; - let written = unsafe { - libc::proc_listallpids( - pids.as_mut_ptr() as *mut libc::c_void, - (cap * std::mem::size_of::()) as libc::c_int, - ) - }; - if written <= 0 { - return table; - } - let n = written as usize / std::mem::size_of::(); - for &pid in pids.iter().take(n.min(cap)) { + let pids = list_all_pids(|buf| { + let ptr = match buf.is_empty() { + true => std::ptr::null_mut(), + false => buf.as_mut_ptr() as *mut libc::c_void, + }; + let bytes = std::mem::size_of_val(buf) as libc::c_int; + unsafe { libc::proc_listallpids(ptr, bytes) } + }); + for &pid in &pids { if pid <= 0 { continue; } @@ -276,6 +268,37 @@ fn process_table() -> HashMap { table } +/// Every pid on the machine, by way of `list`, which has the calling +/// convention of libproc's `proc_listallpids`: handed a buffer, it fills as +/// much of it as it can, and it answers with a *count of pids* — not a byte +/// count, although the size it is given is in bytes. An empty buffer asks +/// only for the count, padded by the kernel for processes born in between. +/// +/// Reading that count as bytes was #731. It divided the answer by four twice +/// over, once sizing the buffer and once reading back how much was filled, so +/// only about one pid in sixteen made it into the table — and since the kernel +/// lists its newest processes first, the ones kept were whatever had started +/// most recently. A `go run` server just launched was in; the pane's shell, +/// started long before, was not, and a walk from a root the table has never +/// heard of comes back empty: no processes, no ports, and nothing to say why. +/// A process tree built moments earlier, which is what a quick check of the +/// probe builds, is exactly the case that happened to work. +#[cfg_attr(not(target_os = "macos"), allow(dead_code))] +fn list_all_pids(mut list: impl FnMut(&mut [i32]) -> i32) -> Vec { + let count = list(&mut []); + if count <= 0 { + return Vec::new(); + } + let mut pids = vec![0; count as usize + 64]; + let written = list(&mut pids); + if written <= 0 { + return Vec::new(); + } + pids.truncate(written as usize); + pids.retain(|&pid| pid > 0); + pids +} + #[cfg(target_os = "macos")] fn cstr_field(buf: &[libc::c_char]) -> String { let bytes: Vec = buf @@ -968,6 +991,40 @@ mod tests { ); } + /// `proc_listallpids` as libproc implements it: the kernel's newest + /// processes first, a padded estimate when asked with no buffer, and + /// always a count of pids back, never bytes. + fn fake_listallpids(live: &[i32]) -> impl FnMut(&mut [i32]) -> i32 + '_ { + move |buf: &mut [i32]| { + if buf.is_empty() { + return live.len() as i32 + 20; + } + let n = live.len().min(buf.len()); + buf[..n].copy_from_slice(&live[..n]); + n as i32 + } + } + + /// #731 on macOS: a busy Mac has hundreds of processes, and the pane's + /// shell is an old one. Every pid has to reach the table, or the walk + /// starts from a root it cannot find and the pane shows no processes and + /// no ports at all. + #[test] + fn every_pid_is_listed_not_the_newest_sixteenth() { + // Newest first, the way the kernel lists them: the `go run` server + // just started, and the shell it runs under from an hour ago. + let live: Vec = (1..=700).rev().collect(); + let got = list_all_pids(fake_listallpids(&live)); + assert_eq!(got.len(), 700, "the whole machine, not the newest few"); + assert!(got.contains(&1), "the oldest process is still there"); + } + + #[test] + fn a_pid_list_that_cannot_be_read_is_empty() { + assert!(list_all_pids(|_| -1).is_empty()); + assert!(list_all_pids(|_| 0).is_empty()); + } + #[test] fn a_pathological_tree_still_stops() { let mut table: HashMap = [(100, row(1, "zsh"))].into_iter().collect(); From 3a52e34b02acf5b51bd2daa8e0cec23e67985b00 Mon Sep 17 00:00:00 2001 From: l0ng-ai Date: Fri, 25 Sep 2026 13:50:39 +0800 Subject: [PATCH 2/5] fix(linux): wrap the confirmation dialog's text instead of clipping it (#948) Linux has no native prompt, so every window.prompt fell through to gpui's fallback renderer, which sets the message and the detail as unbreakable single lines in a fixed-width box with overflow hidden. The quit-and-stop warning was cut off mid-sentence. Install a prompt builder on platforms without a native dialog that lays the same prompt out as a card whose text wraps, in the app's own surfaces, with Return taking answer 0 and Escape the cancel answer. Claude-Session: https://claude.ai/code/session_01JRqYZ9E153WpSHGS2AW3BM --- src/main.rs | 1 + src/ui/mod.rs | 1 + src/ui/prompt.rs | 180 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+) create mode 100644 src/ui/prompt.rs diff --git a/src/main.rs b/src/main.rs index 8a9500df..76c9559d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -705,6 +705,7 @@ fn main() { .detach(); gpui_component::init(cx); register_bundled_fonts(cx); + crate::ui::prompt::install(cx); cx.activate(true); #[cfg(target_os = "macos")] set_dock_icon_for_bare_binary(); diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 4ec0d0af..df167e08 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -27,6 +27,7 @@ pub mod pending_pane; pub mod perf; pub mod prefill; pub mod presets; +pub mod prompt; pub mod remote_connect; pub mod remote_workspace; pub mod reorder; diff --git a/src/ui/prompt.rs b/src/ui/prompt.rs new file mode 100644 index 00000000..80c6459e --- /dev/null +++ b/src/ui/prompt.rs @@ -0,0 +1,180 @@ +//! The confirmation dialog on platforms with no native one. +//! +//! macOS and Windows answer `window.prompt` with NSAlert and TaskDialog. Linux +//! has neither, so gpui falls back to a renderer of its own — and that one sets +//! the message and the detail as single unbreakable lines inside a fixed-width +//! box that clips its overflow. Any sentence longer than the box is cut off +//! mid-word, which for the quit-and-stop warning is exactly the part that says +//! what will be lost (#920). This is the same dialog laid out as a card that +//! wraps its text, in the app's own surfaces, with Return and Escape answering +//! the way they do on the native dialogs. + +use gpui::{ + App, Context, EventEmitter, FocusHandle, Focusable, PromptButton, PromptHandle, PromptLevel, + PromptResponse, RenderablePromptHandle, Window, div, prelude::*, px, relative, +}; +use gpui_component::button::{Button, ButtonVariants as _}; +use gpui_component::{ActiveTheme as _, Sizable as _, h_flex, v_flex}; + +/// Route every `window.prompt` through [`TextPrompt`] where the platform has +/// no dialog of its own to show. +pub(crate) fn install(cx: &mut App) { + if cfg!(any(target_os = "macos", target_os = "windows")) { + return; + } + cx.set_prompt_builder(build); +} + +fn build( + _level: PromptLevel, + message: &str, + detail: Option<&str>, + answers: &[PromptButton], + handle: PromptHandle, + window: &mut Window, + cx: &mut App, +) -> RenderablePromptHandle { + let prompt = cx.new(|cx| TextPrompt::new(message, detail, answers, cx)); + handle.with_view(prompt, window, cx) +} + +pub(crate) struct TextPrompt { + message: String, + detail: Option, + answers: Vec, + focus: FocusHandle, +} + +impl TextPrompt { + fn new( + message: &str, + detail: Option<&str>, + answers: &[PromptButton], + cx: &mut Context, + ) -> Self { + Self { + message: message.to_string(), + detail: detail.map(str::to_string), + answers: answers.to_vec(), + focus: cx.focus_handle(), + } + } + + /// The answer a key picks. Return is answer 0 — `confirm_answers` puts the + /// action there for exactly this — and Escape is the one marked cancel; + /// a prompt with no cancel answer has nothing Escape can safely mean. + fn answer_for_key(&self, key: &str) -> Option { + match key { + "enter" if !self.answers.is_empty() => Some(0), + "escape" => self.answers.iter().position(PromptButton::is_cancel), + _ => None, + } + } +} + +impl EventEmitter for TextPrompt {} + +impl Focusable for TextPrompt { + fn focus_handle(&self, _: &App) -> FocusHandle { + self.focus.clone() + } +} + +impl Render for TextPrompt { + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + let muted = cx.theme().muted_foreground; + // Answer 0 goes rightmost, where the native dialogs put it and where + // `confirm_answers` expects it to land. + let buttons = self.answers.iter().enumerate().rev().map(|(ix, answer)| { + Button::new(("prompt-answer", ix)) + .label(answer.label().clone()) + .small() + .when(ix == 0 && !answer.is_cancel(), |b| b.primary()) + .on_click(cx.listener(move |_, _, _, cx| { + cx.emit(PromptResponse(ix)); + cx.stop_propagation(); + })) + }); + + let card = v_flex() + .occlude() + .w(px(420.)) + .max_w(relative(0.9)) + .gap_3() + .p_5() + .map(|card| crate::ui::theme::floating_surface(card, cx)) + .child( + div() + .text_sm() + .font_weight(gpui::FontWeight::SEMIBOLD) + .child(self.message.clone()), + ) + .children( + self.detail + .clone() + .map(|detail| div().text_sm().text_color(muted).child(detail)), + ) + .child( + h_flex() + .pt_2() + .justify_end() + .flex_wrap() + .gap_2() + .children(buttons), + ); + + div() + .id("text-prompt") + .track_focus(&self.focus) + .size_full() + .cursor_default() + .bg(crate::ui::presets::scrim_fill(cx)) + .on_key_down(cx.listener(|this, ev: &gpui::KeyDownEvent, _, cx| { + if let Some(ix) = this.answer_for_key(&ev.keystroke.key) { + cx.emit(PromptResponse(ix)); + cx.stop_propagation(); + } + })) + .flex() + .items_center() + .justify_center() + .child(card) + } +} + +#[cfg(test)] +mod tests { + use super::TextPrompt; + use gpui::{AppContext as _, PromptButton, TestAppContext}; + + fn prompt(cx: &mut TestAppContext, answers: &[PromptButton]) -> gpui::Entity { + cx.update(|cx| { + cx.new(|cx| { + TextPrompt::new( + "Quit and stop tty7 server?", + Some("a detail long enough to need more than one line"), + answers, + cx, + ) + }) + }) + } + + #[gpui::test] + fn return_confirms_and_escape_takes_the_cancel_answer(cx: &mut TestAppContext) { + let p = prompt(cx, &crate::ui::confirm_answers("Quit", "Cancel")); + p.read_with(cx, |p, _| { + assert_eq!(p.answer_for_key("enter"), Some(0)); + assert_eq!(p.answer_for_key("escape"), Some(1)); + assert_eq!(p.answer_for_key("a"), None); + }); + } + + #[gpui::test] + fn escape_does_nothing_without_a_cancel_answer(cx: &mut TestAppContext) { + let p = prompt(cx, &[PromptButton::ok("OK"), PromptButton::new("Retry")]); + p.read_with(cx, |p, _| { + assert_eq!(p.answer_for_key("escape"), None); + }); + } +} From 575d2cd68e3adaabe5bb89e73edfb2b3aaff3e6f Mon Sep 17 00:00:00 2001 From: LiuSir <32128085+CodeHourra@users.noreply.github.com> Date: Fri, 25 Sep 2026 13:52:36 +0800 Subject: [PATCH 3/5] fix(agents): drop the previous session id when the foreground agent changes (#957) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 同一 pane 里前台从 Claude 换成 omp，或别的 agent 的 hook 写进这个 TTY 时，旧的 session id 会留在新 agent 上，Fork 就会拿错 id。 Co-authored-by: stevelliu Co-authored-by: Cursor --- crates/tty7-core/src/daemon/pane.rs | 45 ++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/crates/tty7-core/src/daemon/pane.rs b/crates/tty7-core/src/daemon/pane.rs index 8c9650a8..2a9281f3 100644 --- a/crates/tty7-core/src/daemon/pane.rs +++ b/crates/tty7-core/src/daemon/pane.rs @@ -3057,6 +3057,14 @@ fn apply_agent_signals( let before = st.agent_session.clone(); for event in &events { + // A hook names the agent that emitted it. Once this pane has a + // foreground agent, a different one's session id must not land here: + // omp forking with a Claude UUID is "session not found". + if let (Some(current), Some(from)) = (st.agent, event.agent) + && current != from + { + continue; + } if st.agent.is_none() && event.agent.is_some() { st.agent = event.agent; notify(st, DaemonMsg::Agent(st.agent)); @@ -3183,7 +3191,9 @@ fn apply_agent( stamp_launch_argv(st, argv); return; } - if agent.is_none() && st.agent_session.is_some() { + // The session belongs to whoever was in the foreground. Switching from + // Claude to omp (or back to the shell) must not keep the previous id. + if st.agent_session.is_some() { st.agent_session = None; notify(st, DaemonMsg::AgentStatus(None)); } @@ -5378,6 +5388,39 @@ mod tests { assert!(matches!(rx.try_recv(), Ok(DaemonMsg::Agent(None)))); } + #[test] + fn a_foreign_agent_hook_does_not_replace_the_foreground_session() { + use crate::core::cli_agent::CLIAgent; + + let mut st = test_state(true); + let mut sniffer = OscSniffer::new(); + let omp = concat!( + "\x1b]777;notify;tty7://cli-agent;", + r#"{"v":1,"agent":"omp","event":"session-start","session_id":"01a0d21f-2f79-72e0-bb16-d6d908aaa6e0"}"#, + "\x07", + ); + apply_signals(&mut st, sniffer.feed(omp.as_bytes())); + assert_eq!(st.agent, Some(CLIAgent::OhMyPi)); + + let claude = concat!( + "\x1b]777;notify;tty7://cli-agent;", + r#"{"v":1,"agent":"claude","event":"session-start","session_id":"e18a20ff-4c8c-4b94-867b-dd4b79032a6c"}"#, + "\x07", + ); + apply_signals(&mut st, sniffer.feed(claude.as_bytes())); + assert_eq!( + st.agent_session.as_ref().unwrap().session_id.as_deref(), + Some("01a0d21f-2f79-72e0-bb16-d6d908aaa6e0"), + "a Claude hook on an omp pane must not steal the session id" + ); + + apply_agent(&mut st, Some((CLIAgent::Claude, vec!["claude".into()]))); + assert!( + st.agent_session.is_none(), + "switching the foreground agent drops the previous session" + ); + } + #[test] fn opaque_notifications_only_fall_back_when_no_rich_state() { use crate::core::cli_agent::{AgentSessionState, AgentStatus, CLIAgent}; From 455e74dc2ca5be078df52a6f3555bfbe20caee19 Mon Sep 17 00:00:00 2001 From: l0ng-ai Date: Fri, 25 Sep 2026 13:53:44 +0800 Subject: [PATCH 4/5] feat(scm): filter changed files and show them as a tree (#950) * feat(scm): filter changed files and show them as a tree * test(scm): key the tree test's settle on the panel's own root --- CHANGELOG.md | 17 ++ assets/icons/list-flat.svg | 1 + assets/icons/list-tree.svg | 1 + crates/tty7-core/src/core/config.rs | 7 + src/ui/assets.rs | 4 + src/ui/i18n/en.rs | 4 + src/ui/i18n/ja.rs | 4 + src/ui/i18n/mod.rs | 4 + src/ui/i18n/zh.rs | 4 + src/ui/right_panel.rs | 14 ++ src/ui/scm/actions.rs | 10 + src/ui/scm/mod.rs | 1 + src/ui/scm/panel.rs | 279 +++++++++++++++++++++++++- src/ui/scm/state.rs | 16 ++ src/ui/scm/tree.rs | 298 ++++++++++++++++++++++++++++ 15 files changed, 660 insertions(+), 4 deletions(-) create mode 100644 assets/icons/list-flat.svg create mode 100644 assets/icons/list-tree.svg create mode 100644 src/ui/scm/tree.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index f77640ff..09ae4e40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,23 @@ All notable changes to tty7 are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +- **The source control panel's changed files can be filtered and shown as a + tree** (#473). A filter field sits above the list: every word typed has to + appear somewhere in a file's path, in any order and any case, and the group + headers' counts and their stage/unstage/discard-all buttons follow what the + filter leaves on screen. The tile at its end switches the list to a + directory tree — directories before files, a chain of directories that hold + nothing but the next one compacted into a single row (`crates/core/src`), + each directory row foldable and counting the files beneath it. A filter + opens every folded directory while it is set. The choice between list and + tree is remembered in the config. Both are built from the status the host + already sends, so a remote repository gets them unchanged. Searching the + diff text itself is not part of this. + ## [26.9.3] - 2026-09-23 ### Added diff --git a/assets/icons/list-flat.svg b/assets/icons/list-flat.svg new file mode 100644 index 00000000..b6249b13 --- /dev/null +++ b/assets/icons/list-flat.svg @@ -0,0 +1 @@ + diff --git a/assets/icons/list-tree.svg b/assets/icons/list-tree.svg new file mode 100644 index 00000000..04086a1a --- /dev/null +++ b/assets/icons/list-tree.svg @@ -0,0 +1 @@ + diff --git a/crates/tty7-core/src/core/config.rs b/crates/tty7-core/src/core/config.rs index 01fb7bfc..07bc28b0 100644 --- a/crates/tty7-core/src/core/config.rs +++ b/crates/tty7-core/src/core/config.rs @@ -246,6 +246,12 @@ pub struct Config { /// impression than one they asked for. #[serde(default)] pub scm_graph_expanded: bool, + /// Whether the source control panel lists changed files as a directory + /// tree rather than one flat run per group. Off by default: the flat list + /// is what a handful of changes reads best as, and the tree is for the + /// agent run that touched sixty files across a dozen modules. + #[serde(default)] + pub scm_changes_tree: bool, /// Whether a file opens in the code panel with soft wrap on. Not a /// setting anyone picks up front: it is whatever the status bar's Wrap /// toggle (or `ToggleDocumentWrap`) was last left at, so the next file @@ -676,6 +682,7 @@ impl Default for Config { document_layout: DocumentLayout::default(), document_ratio: default_document_ratio(), scm_graph_expanded: false, + scm_changes_tree: false, editor_soft_wrap: false, editor_markdown_preview: false, sidebar_grouping: SidebarGrouping::Repo, diff --git a/src/ui/assets.rs b/src/ui/assets.rs index 2bf9e9ab..a35f0329 100644 --- a/src/ui/assets.rs +++ b/src/ui/assets.rs @@ -47,6 +47,8 @@ fn agent_icon(path: &str) -> Option<&'static [u8]> { "icons/machine-local.svg" => include_bytes!("../../assets/icons/machine-local.svg"), "icons/machine-remote.svg" => include_bytes!("../../assets/icons/machine-remote.svg"), "icons/refresh.svg" => include_bytes!("../../assets/icons/refresh.svg"), + "icons/list-tree.svg" => include_bytes!("../../assets/icons/list-tree.svg"), + "icons/list-flat.svg" => include_bytes!("../../assets/icons/list-flat.svg"), "icons/agents/claude.svg" => include_bytes!("../../assets/icons/agents/claude.svg"), "icons/agents/codex.svg" => include_bytes!("../../assets/icons/agents/codex.svg"), "icons/agents/traecli.svg" => include_bytes!("../../assets/icons/agents/traecli.svg"), @@ -114,6 +116,8 @@ mod tests { "icons/git-branch.svg", "icons/git-sync.svg", "icons/git-commit.svg", + "icons/list-tree.svg", + "icons/list-flat.svg", ] { assert!( Assets.load(path).unwrap().is_some(), diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index d0e8621a..6013f4e6 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -1216,6 +1216,10 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::ScmShowMore => "Show more", L10nKey::ScmShowLess => "Show less", L10nKey::ScmCommitNotFound => "This commit is not in this repository.", + L10nKey::ScmFilterChanges => "Filter changed files…", + L10nKey::ScmViewAsTree => "View as Tree", + L10nKey::ScmViewAsList => "View as List", + L10nKey::ScmNoMatchingChanges => "No changed files match the filter.", L10nKey::ScmTooManyChanges => "Showing the first {shown} of {total} changes.", L10nKey::ScmOpenChanges => "Open Changes", L10nKey::ScmDiscardAllConfirm => { diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 56470abf..028e45a5 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -1290,6 +1290,10 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::ScmShowMore => "続きを表示", L10nKey::ScmShowLess => "折りたたむ", L10nKey::ScmCommitNotFound => "このリポジトリにそのコミットはありません。", + L10nKey::ScmFilterChanges => "変更ファイルを絞り込む…", + L10nKey::ScmViewAsTree => "ツリーで表示", + L10nKey::ScmViewAsList => "リストで表示", + L10nKey::ScmNoMatchingChanges => "条件に一致する変更ファイルはありません。", L10nKey::ScmTooManyChanges => { "変更が多いため、{total} 件のうち先頭 {shown} 件のみ表示しています。" } diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index c3b97bb1..b8b60db2 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -913,6 +913,10 @@ l10n_keys! { ScmShowLess, ScmCommitNotFound, ScmTooManyChanges, + ScmFilterChanges, + ScmViewAsTree, + ScmViewAsList, + ScmNoMatchingChanges, ScmOpenChanges, ScmDiscardAllConfirm, ScmAmendConfirm, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 82d96fb2..f9c59eb0 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -1156,6 +1156,10 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::ScmShowMore => "展开", L10nKey::ScmShowLess => "收起", L10nKey::ScmCommitNotFound => "本仓库中没有这个提交。", + L10nKey::ScmFilterChanges => "筛选变更文件…", + L10nKey::ScmViewAsTree => "以树形显示", + L10nKey::ScmViewAsList => "以列表显示", + L10nKey::ScmNoMatchingChanges => "没有匹配筛选的变更文件。", L10nKey::ScmTooManyChanges => "改动过多，仅显示前 {shown} 项（共 {total} 项）。", L10nKey::ScmOpenChanges => "查看改动", L10nKey::ScmDiscardAllConfirm => { diff --git a/src/ui/right_panel.rs b/src/ui/right_panel.rs index 994f83c0..d96dbb32 100644 --- a/src/ui/right_panel.rs +++ b/src/ui/right_panel.rs @@ -729,6 +729,18 @@ impl Tty7App { input: &gpui::Entity, cx: &mut Context, ) -> AnyElement { + self.panel_search_with(input, None, cx) + } + + /// [`Self::panel_search`] with a tile at its far end, inset the way the + /// branch row's tile is so the two stack in one column. + pub(crate) fn panel_search_with( + &self, + input: &gpui::Entity, + trailing: Option, + cx: &mut Context, + ) -> AnyElement { + let has_trailing = trailing.is_some(); h_flex() .flex_none() .items_center() @@ -753,6 +765,8 @@ impl Tty7App { // still reads as one line of chrome. .child(Input::new(input).appearance(false).xsmall().cleanable(true)), ) + .when(has_trailing, |row| row.pr(px(tile_trailing_inset_sm()))) + .children(trailing) .into_any_element() } diff --git a/src/ui/scm/actions.rs b/src/ui/scm/actions.rs index 3d5ea2e1..bdede8b6 100644 --- a/src/ui/scm/actions.rs +++ b/src/ui/scm/actions.rs @@ -71,6 +71,16 @@ impl Tty7App { cx.notify(); } + /// Switch the changed-file list between flat and tree, and remember it. + /// + /// Global like `diff_view`: someone who reads changes by directory reads + /// every repository's that way. + pub(crate) fn scm_toggle_changes_tree(&mut self, cx: &mut Context) { + let next = !cx.global::().scm_changes_tree; + self.update_config(cx, |cfg| cfg.scm_changes_tree = next); + cx.notify(); + } + /// Flip the diff overlay between side-by-side and unified. /// /// Global rather than per-overlay, matching `diffEditor.renderSideBySide`: diff --git a/src/ui/scm/mod.rs b/src/ui/scm/mod.rs index 50f4f4e9..39529ce4 100644 --- a/src/ui/scm/mod.rs +++ b/src/ui/scm/mod.rs @@ -14,6 +14,7 @@ pub(crate) mod path; pub(crate) mod state; #[allow(dead_code)] pub(crate) mod status; +pub(crate) mod tree; pub(crate) use actions::ScmIntent; pub(crate) use state::{GraphState, ScmPanelState}; diff --git a/src/ui/scm/panel.rs b/src/ui/scm/panel.rs index 63013637..1bd1c430 100644 --- a/src/ui/scm/panel.rs +++ b/src/ui/scm/panel.rs @@ -39,6 +39,7 @@ use crate::ui::scm::ScmIntent; use crate::ui::scm::path::split_display_path; use crate::ui::scm::state::{RepoKey, ScmGroup}; use crate::ui::scm::status::{status_color, status_glyph}; +use crate::ui::scm::tree::{TreeRow, normalize_query, path_matches, tree_rows}; /// Shared compact pitch for working-tree and commit-detail file rows. pub(super) const ROW_H: f32 = 26.; @@ -52,6 +53,10 @@ pub(super) const ROW_H: f32 = 26.; /// different numbers. const BADGE_W: f32 = 14.; +/// One level of the tree view, the file tree's step: the Files panel and this +/// one draw the same directories, and the same depth should read the same. +const TREE_INDENT: f32 = 14.; + /// The key context the message box installs, and the one `ScmCommit` is /// bound inside. The two are the same string on purpose: a binding whose /// context nothing attaches is a binding that never fires. @@ -236,7 +241,12 @@ impl Tty7App { Some(t(L10nKey::PanelNoChangesHint)), cx, ), - None => self.scm_groups(&repo, &status, cx), + None => { + // Pinned with the rest, so the filter stays in reach however + // far down the list has been scrolled. + pinned.push(self.scm_filter_row(window, cx)); + self.scm_groups(&repo, &status, cx) + } }; let history = self.render_graph_section(&repo, window, cx); self.scm_shell_full(title, pinned, body, history) @@ -1291,24 +1301,63 @@ impl Tty7App { status: &Arc, cx: &mut Context, ) -> AnyElement { + let query = self.scm_filter_query(cx); + let tree = cx.global::().scm_changes_tree; + let mut matched = 0; let mut list = v_flex().px(px(CONTENT_INSET - ROW_INSET)).py(px(2.)); for group in ScmGroup::ORDER { + // Filtered before anything else reads it, so the header's count + // and its stage/discard-all act on what is on screen — a + // "discard all" that also reached the files the filter hid would + // be the one surprise this panel cannot take back. let entries: Vec<&StatusEntry> = status .entries .iter() .filter(|e| in_group(e, group)) + .filter(|e| { + query + .as_deref() + .is_none_or(|q| path_matches(e.path.as_str(), q)) + }) .collect(); if entries.is_empty() { continue; } + matched += entries.len(); let collapsed = self.scm.group_collapsed(group, entries.len()); list = list.child(self.scm_group_header(repo, group, &entries, collapsed, cx)); if collapsed { continue; } let shown = entries.len().min(MAX_RENDERED_FILES); - for entry in entries.iter().take(shown) { - list = list.child(self.scm_file_row(repo, group, entry, cx)); + if tree { + // A filter opens every directory: a match folded away under + // a closed row is a match the filter failed to show. + let filtering = query.is_some(); + let folded = &self.scm.folded_dirs; + let rows = tree_rows(entries[..shown].iter().map(|e| e.path.as_str()), |key| { + !filtering && folded.contains(&(group, key.to_string())) + }); + for row in rows { + list = list.child(match row { + TreeRow::Dir { + key, + label, + depth, + files, + collapsed, + } => self.scm_dir_row( + group, key, label, depth, files, collapsed, !filtering, cx, + ), + TreeRow::File { index, depth } => { + self.scm_file_row(repo, group, entries[index], Some(depth), cx) + } + }); + } + } else { + for entry in entries.iter().take(shown) { + list = list.child(self.scm_file_row(repo, group, entry, None, cx)); + } } if entries.len() > shown { list = list.child(self.scm_note( @@ -1317,6 +1366,9 @@ impl Tty7App { )); } } + if query.is_some() && matched == 0 { + list = list.child(self.scm_note(t(L10nKey::ScmNoMatchingChanges).to_string(), cx)); + } if status.truncated { list = list.child(self.scm_note( t_fmt( @@ -1332,6 +1384,142 @@ impl Tty7App { list.into_any_element() } + /// The filter over the changed files, with the list/tree switch at its + /// end. + /// + /// A standing field, like the Files panel's, rather than one behind a + /// tile like the history's: the list is what this panel is for, and a + /// run that touched sixty files is exactly when nobody wants to go + /// looking for how to narrow it. + fn scm_filter_row(&mut self, window: &mut Window, cx: &mut Context) -> AnyElement { + let input = match self.scm.filter.clone() { + Some(input) => input, + None => { + let input = cx.new(|cx| { + InputState::new(window, cx).placeholder(t(L10nKey::ScmFilterChanges)) + }); + // An `InputState` is its own entity; without this the list + // would never hear about the text typed into it. + self.scm.filter_sub = + Some( + cx.subscribe_in(&input, window, |_this, _input, ev, _window, cx| { + if matches!(ev, gpui_component::input::InputEvent::Change) { + cx.notify(); + } + }), + ); + self.scm.filter = Some(input.clone()); + input + } + }; + let tree = cx.global::().scm_changes_tree; + // The tile shows where it goes, not where you are — the rows under it + // already say which view this is. + let (glyph, tip) = match tree { + true => ("icons/list-flat.svg", t(L10nKey::ScmViewAsList)), + false => ("icons/list-tree.svg", t(L10nKey::ScmViewAsTree)), + }; + let toggle = crate::ui::tab_strip::chrome_tile_sized( + Button::new("scm-changes-view").icon(Icon::empty().path(glyph)), + crate::ui::app::TILE_SIZE_SM, + crate::ui::app::TILE_GLYPH_SM, + false, + cx, + ) + .rounded_md() + .tooltip(tip) + .on_click(cx.listener(|this, _, _window, cx| this.scm_toggle_changes_tree(cx))) + .into_any_element(); + self.panel_search_with(&input, Some(toggle), cx) + } + + /// The filter box's text, if it has any. + fn scm_filter_query(&self, cx: &gpui::App) -> Option { + normalize_query(&self.scm.filter.as_ref()?.read(cx).value()) + } + + /// A directory in the tree view: a chevron in the status letters' + /// column, the (possibly compacted) name, and how many changed files + /// are under it. + #[allow(clippy::too_many_arguments)] + fn scm_dir_row( + &self, + group: ScmGroup, + key: String, + label: String, + depth: usize, + files: usize, + collapsed: bool, + foldable: bool, + cx: &mut Context, + ) -> AnyElement { + let sf = cx.global::().sidebar; + let mono = cx.theme().mono_font_family.clone(); + let muted = cx.theme().muted_foreground; + let id = SharedString::from(format!("scm-dir-{group:?}-{key}")); + let selector = id.clone(); + let toggle = cx.listener({ + let key = key.clone(); + move |this, _, _window, cx| { + this.scm.toggle_dir(group, key.clone()); + cx.notify(); + } + }); + h_flex() + .id(id) + .debug_selector(move || selector.to_string()) + .items_center() + .gap(px(8.)) + .min_h(rems(ROW_H / 16.)) + .w_full() + .min_w_0() + .px(px(ROW_INSET)) + .pl(px(ROW_INSET + depth as f32 * TREE_INDENT)) + .rounded(px(5.)) + .when(foldable, |row| { + row.cursor_pointer() + .hover(|s| s.bg(gpui::rgb(sf.hover))) + .on_click(toggle) + }) + .tooltip(move |window, cx| { + gpui_component::tooltip::Tooltip::new(key.clone()).build(window, cx) + }) + .child( + div() + .flex_none() + .w(px(BADGE_W)) + .flex() + .justify_center() + .text_color(muted) + .child( + Icon::new(if collapsed { + IconName::ChevronRight + } else { + IconName::ChevronDown + }) + .xsmall(), + ), + ) + .child( + div() + .flex_1() + .min_w_0() + .truncate() + .text_size(rems(crate::ui::right_panel::TEXT)) + .text_color(gpui::rgb(sf.text_resting)) + .child(label), + ) + .child( + div() + .flex_none() + .text_size(rems(META_MONO)) + .font_family(mono) + .text_color(muted) + .child(files.to_string()), + ) + .into_any_element() + } + /// "…and 40 more", and the note about a status git had to truncate. /// /// Secondary: it is prose about the list rather than a row of it, and the @@ -1413,11 +1601,15 @@ impl Tty7App { .into_any_element() } + /// One changed file. `depth` is its level in the tree view, and `None` in + /// the flat list — where the row carries its directory beside the name, + /// since there is no directory row above it to say it. fn scm_file_row( &self, repo: &RepoKey, group: ScmGroup, entry: &StatusEntry, + depth: Option, cx: &mut Context, ) -> AnyElement { let sf = cx.global::().sidebar; @@ -1438,8 +1630,10 @@ impl Tty7App { cx, ); + let selector = id.clone(); h_flex() .id(id.clone()) + .debug_selector(move || selector.to_string()) .group(id) .relative() .items_center() @@ -1448,6 +1642,9 @@ impl Tty7App { .w_full() .min_w_0() .px(px(ROW_INSET)) + .when_some(depth, |row, depth| { + row.pl(px(ROW_INSET + depth as f32 * TREE_INDENT)) + }) .py(px(3.)) .rounded(px(5.)) .cursor_pointer() @@ -1503,7 +1700,7 @@ impl Tty7App { // Cap the secondary directory while the filename fills the rest. // A rem cap also gives short paths their intrinsic width inside // the context-menu wrapper's flex layout. - .when(!dir.is_empty(), |this| { + .when(!dir.is_empty() && depth.is_none(), |this| { this.child( div() .flex_none() @@ -3012,6 +3209,80 @@ mod render_idle_gpui_tests { assert_eq!(draws_while_idle(&mut vcx), 0); let _ = std::fs::remove_dir_all(&root); } + + /// The tree view and the filter, end to end over a real repository: the + /// rows that come out are the ones the pure helpers promise, drawn. + #[gpui::test] + fn the_changes_list_folds_into_a_tree_and_filters_by_path(cx: &mut TestAppContext) { + let _serial = serial(); + crate::core::config::pin_test_config_dir(); + let root = scratch("tree"); + git(&root, &["init", "--quiet"]); + std::fs::create_dir_all(root.join("src/ui")).unwrap(); + std::fs::create_dir_all(root.join("docs")).unwrap(); + std::fs::write(root.join("src/ui/a.rs"), "a\n").unwrap(); + std::fs::write(root.join("src/ui/b.rs"), "b\n").unwrap(); + std::fs::write(root.join("docs/readme.md"), "r\n").unwrap(); + + // Keyed off the root the panel resolved, not off `root`: on Windows + // `scratch` hands back a `\\?\` verbatim path that never compares + // equal to git's spelling of the same directory — see the note on + // `a_settled_source_control_panel_reaches_render_idle`. + let (app, mut vcx, _pane) = scm_panel_on(cx, &root, |app, cx| { + app.scm.repo.as_ref().is_some_and(|r| { + crate::terminal::git_data::status_of(cx, r.host, &r.root) + .is_some_and(|s| s.untracked().count() == 3) + }) + }); + let drawn = |vcx: &mut VisualTestContext, what: &'static str| { + app.update_in(vcx, |_, _, cx| cx.notify()); + vcx.run_until_parked(); + vcx.debug_bounds(what).is_some() + }; + let a = "scm-row-Untracked-src/ui/a.rs"; + let readme = "scm-row-Untracked-docs/readme.md"; + let src_ui = "scm-dir-Untracked-src/ui"; + + // The flat list is the default, and it has no directory rows. + assert!(drawn(&mut vcx, a)); + assert!(!drawn(&mut vcx, src_ui)); + + // `src` holds nothing but `ui`, so the two share one row. + app.update(&mut vcx, |app, cx| app.scm_toggle_changes_tree(cx)); + assert!(vcx.update(|_, cx| cx.global::().scm_changes_tree)); + assert!(drawn(&mut vcx, src_ui)); + assert!(!drawn(&mut vcx, "scm-dir-Untracked-src")); + assert!(drawn(&mut vcx, "scm-dir-Untracked-docs")); + assert!(drawn(&mut vcx, a)); + + // Folding the directory keeps its row and hides what is under it. + app.update(&mut vcx, |app, cx| { + app.scm.toggle_dir(ScmGroup::Untracked, "src/ui".into()); + cx.notify(); + }); + assert!(drawn(&mut vcx, src_ui)); + assert!(!drawn(&mut vcx, a)); + assert!(drawn(&mut vcx, readme)); + + // A filter drops what does not match — and opens the folded + // directory the match is in. + app.update_in(&mut vcx, |app, window, cx| { + let input = app.scm.filter.clone().expect("the filter row was drawn"); + input.update(cx, |state, cx| state.set_value("UI A", window, cx)); + }); + assert!(drawn(&mut vcx, a)); + assert!(!drawn(&mut vcx, "scm-row-Untracked-src/ui/b.rs")); + assert!(!drawn(&mut vcx, readme)); + assert!(!drawn(&mut vcx, "scm-dir-Untracked-docs")); + + // The same filter narrows the flat list too. + app.update(&mut vcx, |app, cx| app.scm_toggle_changes_tree(cx)); + assert!(drawn(&mut vcx, a)); + assert!(!drawn(&mut vcx, readme)); + assert!(!drawn(&mut vcx, src_ui)); + + let _ = std::fs::remove_dir_all(&root); + } } /// The action strip is the one place in the panel where a hover reveal sits diff --git a/src/ui/scm/state.rs b/src/ui/scm/state.rs index a201385f..ef44bef5 100644 --- a/src/ui/scm/state.rs +++ b/src/ui/scm/state.rs @@ -96,6 +96,14 @@ pub(crate) struct ScmPanelState { /// opening one by hand has to outlast the next file landing in it. pub(crate) collapsed: HashSet, pub(crate) toggled: HashSet, + /// Directories folded shut in the tree view, per group — the same + /// directory can be open under Changes and shut under Staged. Keyed by + /// the full repo-relative path of the (possibly compacted) row. + pub(crate) folded_dirs: HashSet<(ScmGroup, String)>, + /// The filter over the changed files. Created on first render like + /// `commit_input`, with the subscription that turns typing into a repaint. + pub(crate) filter: Option>, + pub(crate) filter_sub: Option, /// Working directory → the repository root containing it, or `None` when /// there is none, with when the answer was given. The root is what every /// write runs from and what every cache is keyed by, so it is resolved @@ -165,6 +173,14 @@ impl ScmPanelState { } } + /// Fold a tree directory shut, or open it again. + pub(crate) fn toggle_dir(&mut self, group: ScmGroup, key: String) { + let entry = (group, key); + if !self.folded_dirs.remove(&entry) { + self.folded_dirs.insert(entry); + } + } + pub(crate) fn set_group_collapsed(&mut self, group: ScmGroup, collapsed: bool) { self.toggled.insert(group); if collapsed { diff --git a/src/ui/scm/tree.rs b/src/ui/scm/tree.rs new file mode 100644 index 00000000..c0ed93b4 --- /dev/null +++ b/src/ui/scm/tree.rs @@ -0,0 +1,298 @@ +//! The changed-file list as a tree, and the filter both views share. +//! +//! Pure, and over plain paths rather than `StatusEntry`: the panel hands in +//! one group's paths and gets back rows that point into that same slice, so +//! the file rows it draws are the ones the flat list would have drawn — same +//! letter, same click, same context menu — only indented under a directory. +//! +//! Everything here comes out of the status the host already sent. A remote +//! repository gets the same tree without a single extra read of its disk. + +use std::collections::BTreeMap; + +/// One line of the tree view. +#[derive(Clone, PartialEq, Eq, Debug)] +pub(crate) enum TreeRow { + /// A directory, possibly several compacted into one: `src/ui/scm` when + /// `src` and `ui` hold nothing but the next directory down. `key` is the + /// full repo-relative path, which is what the fold state is keyed by; + /// `label` is what the row reads, relative to its parent row. + Dir { + key: String, + label: String, + depth: usize, + /// Every file underneath, at any depth — what a folded row still + /// owes the reader. + files: usize, + collapsed: bool, + }, + /// A file, by its position in the paths the tree was built from. + File { index: usize, depth: usize }, +} + +#[derive(Default)] +struct Node<'a> { + dirs: BTreeMap<&'a str, Node<'a>>, + files: Vec<(&'a str, usize)>, +} + +impl Node<'_> { + fn file_count(&self) -> usize { + self.files.len() + self.dirs.values().map(Node::file_count).sum::() + } +} + +/// Group `paths` by directory, directories before files at every level and +/// each run in byte order — the order git itself lists paths in. +/// +/// A directory whose only content is one more directory is folded into it, +/// the way VS Code's compact folders do: an agent's change three levels down +/// `crates/tty7-core/src` should cost one row to reach, not three. +/// +/// `collapsed` is asked once per directory row with its full path. A folded +/// directory still gets its own row; only what is under it is left out. +pub(crate) fn tree_rows<'a>( + paths: impl IntoIterator, + collapsed: impl Fn(&str) -> bool, +) -> Vec { + let mut root = Node::default(); + for (index, path) in paths.into_iter().enumerate() { + // An untracked directory arrives as `dir/`. It is a leaf here, the + // same as it is a single row in the flat list. + let trimmed = path.strip_suffix('/').unwrap_or(path); + let (dir, name) = match trimmed.rsplit_once('/') { + Some((dir, name)) => (Some(dir), name), + None => (None, trimmed), + }; + let mut node = &mut root; + for part in dir.into_iter().flat_map(|d| d.split('/')) { + node = node.dirs.entry(part).or_default(); + } + node.files.push((name, index)); + } + let mut rows = Vec::new(); + emit(&root, "", 0, &collapsed, &mut rows); + rows +} + +fn emit( + node: &Node<'_>, + prefix: &str, + depth: usize, + collapsed: &impl Fn(&str) -> bool, + rows: &mut Vec, +) { + for (name, child) in &node.dirs { + let mut label = (*name).to_string(); + let mut child = child; + while child.files.is_empty() && child.dirs.len() == 1 { + let (next, grandchild) = child.dirs.iter().next().expect("one entry"); + label.push('/'); + label.push_str(next); + child = grandchild; + } + let key = match prefix { + "" => label.clone(), + _ => format!("{prefix}/{label}"), + }; + let folded = collapsed(&key); + rows.push(TreeRow::Dir { + key: key.clone(), + label, + depth, + files: child.file_count(), + collapsed: folded, + }); + if !folded { + emit(child, &key, depth + 1, collapsed, rows); + } + } + let mut files = node.files.clone(); + files.sort_by(|a, b| a.0.cmp(b.0)); + rows.extend( + files + .into_iter() + .map(|(_, index)| TreeRow::File { index, depth }), + ); +} + +/// The filter box's text, normalised once: lowercased, trimmed, and `None` +/// when there is nothing left to filter by. +pub(crate) fn normalize_query(text: &str) -> Option { + let q = text.trim().to_lowercase(); + (!q.is_empty()).then_some(q) +} + +/// Whether a changed path survives the filter. +/// +/// Every whitespace-separated word has to appear somewhere in the path, in +/// any order: `scm panel` finds `src/ui/scm/panel.rs`, and so does `panel +/// scm`. `query` is what [`normalize_query`] returned. +pub(crate) fn path_matches(path: &str, query: &str) -> bool { + let hay = path.to_lowercase(); + query.split_whitespace().all(|word| hay.contains(word)) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn open(paths: &[&str]) -> Vec { + tree_rows(paths.iter().copied(), |_| false) + } + + fn dir(key: &str, label: &str, depth: usize, files: usize) -> TreeRow { + TreeRow::Dir { + key: key.into(), + label: label.into(), + depth, + files, + collapsed: false, + } + } + + fn file(index: usize, depth: usize) -> TreeRow { + TreeRow::File { index, depth } + } + + #[test] + fn files_are_grouped_under_their_directories() { + let rows = open(&["src/a.rs", "README.md", "src/b.rs", "docs/x.md"]); + assert_eq!( + rows, + vec![ + dir("docs", "docs", 0, 1), + file(3, 1), + dir("src", "src", 0, 2), + file(0, 1), + file(2, 1), + // Files after directories, at the root as everywhere else. + file(1, 0), + ] + ); + } + + #[test] + fn a_directory_holding_only_a_directory_is_compacted() { + let rows = open(&[ + "crates/core/src/git/status.rs", + "crates/core/src/git/diff.rs", + "crates/core/Cargo.toml", + ]); + assert_eq!( + rows, + vec![ + // `crates` holds only `core`, so they share a row; `core` + // holds a file, so the chain stops there. + dir("crates/core", "crates/core", 0, 3), + dir("crates/core/src/git", "src/git", 1, 2), + file(1, 2), + file(0, 2), + file(2, 1), + ] + ); + } + + #[test] + fn a_directory_with_two_children_is_not_compacted() { + let rows = open(&["a/b/x.rs", "a/c/y.rs"]); + assert_eq!( + rows, + vec![ + dir("a", "a", 0, 2), + dir("a/b", "b", 1, 1), + file(0, 2), + dir("a/c", "c", 1, 1), + file(1, 2), + ] + ); + } + + #[test] + fn a_folded_directory_keeps_its_row_and_its_count() { + let rows = tree_rows(["src/ui/a.rs", "src/ui/b.rs", "src/main.rs"], |key| { + key == "src/ui" + }); + assert_eq!( + rows, + vec![ + dir("src", "src", 0, 3), + TreeRow::Dir { + key: "src/ui".into(), + label: "ui".into(), + depth: 1, + files: 2, + collapsed: true, + }, + file(2, 1), + ] + ); + } + + #[test] + fn folding_is_asked_about_the_compacted_path() { + let asked = std::cell::RefCell::new(Vec::new()); + tree_rows(["a/b/c/x.rs"], |key| { + asked.borrow_mut().push(key.to_string()); + false + }); + assert_eq!(asked.into_inner(), vec!["a/b/c".to_string()]); + } + + #[test] + fn an_untracked_directory_is_a_leaf() { + let rows = open(&["build/", "src/new/"]); + assert_eq!(rows, vec![dir("src", "src", 0, 1), file(1, 1), file(0, 0)]); + } + + #[test] + fn every_path_lands_in_exactly_one_file_row() { + let paths = [ + "z.rs", + "a/b/c.rs", + "a/b/d.rs", + "a/e.rs", + "f/g/h/i.rs", + "f/j.rs", + "a/b/", + ]; + let mut seen: Vec = open(&paths) + .into_iter() + .filter_map(|row| match row { + TreeRow::File { index, .. } => Some(index), + TreeRow::Dir { .. } => None, + }) + .collect(); + seen.sort_unstable(); + assert_eq!(seen, (0..paths.len()).collect::>()); + } + + #[test] + fn an_empty_list_is_an_empty_tree() { + assert!(open(&[]).is_empty()); + } + + #[test] + fn the_filter_needs_every_word_in_any_order_and_any_case() { + let q = normalize_query(" Scm PANEL ").unwrap(); + assert!(path_matches("src/ui/scm/panel.rs", &q)); + assert!(path_matches("src/ui/Panel/SCM.rs", &q)); + assert!(!path_matches("src/ui/scm/state.rs", &q)); + let q = normalize_query("panel scm").unwrap(); + assert!(path_matches("src/ui/scm/panel.rs", &q)); + } + + #[test] + fn a_blank_filter_is_no_filter() { + assert_eq!(normalize_query(""), None); + assert_eq!(normalize_query(" "), None); + assert_eq!(normalize_query(" a "), Some("a".into())); + } + + #[test] + fn the_filter_reads_non_ascii_paths() { + let q = normalize_query("设计").unwrap(); + assert!(path_matches("文档/设计/方案.md", &q)); + assert!(!path_matches("文档/实现.md", &q)); + } +} From 23948bcddaef05dad3336ea30f2114ab1dbb9823 Mon Sep 17 00:00:00 2001 From: l0ng-ai Date: Fri, 25 Sep 2026 16:28:49 +0800 Subject: [PATCH 5/5] fix(render): draw fallback Nerd Font icons at the text's size (#949) * fix(render): scale fallback-font icons up to fill their cell A lone Private Use Area glyph supplied by a fallback face is drawn at the primary's font size on the fallback's own metrics, so Nerd Font icons came out about two thirds of the cell. Grow such a glyph, aspect ratio kept, until it fills the width of its cells or the height of the row (at most 2x), and centre it there. Overflow is still shrunk as before, text from a fallback face keeps its metrics, and the primary's own icons are untouched. Closes #866 * fix(render): fit fallback icons to their cells, borrowing a same-colour blank Checked in the running app with Symbols Nerd Font Mono behind Menlo: its icons ink a full em (1.6 cells), so growth alone never fired. What made them small was the one-cell budget an icon gets when the space after it paints a background, which is every icon in a coloured prompt segment. Fit a fallback icon to its cells and one em of height instead, letting it take the blank after it when that blank paints no background or the icon's own, and only when that makes it bigger. Leave the Powerline separators to the existing rule. * docs(render): name icon_fit in the fit_scale test's comment --- CHANGELOG.md | 17 ++ src/terminal/element.rs | 390 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 379 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09ae4e40..fdd34533 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 already sends, so a remote repository gets them unchanged. Searching the diff text itself is not part of this. +### Fixed + +- **Nerd Font icons from a fallback font come out at the text's size** (#866). + With a Nerd Font icon face such as Symbols Nerd Font Mono behind a primary + that lacks the icons, an icon followed by a space on the same background — + every icon in a coloured Powerline or p10k segment — was held to one cell + and shrunk to about two thirds of the text's height. Icons with a plain + space after them were already drawn full size, so one prompt had icons in + two sizes. A lone Private Use Area glyph supplied by a fallback face is now + fitted, aspect ratio kept, to its cells and one em of height, taking the + blank after it when that blank paints no background or the icon's own and + when doing so makes it bigger, and centred in the row; a face whose icons + ink less than a cell grows (at most 2×). The Powerline separators, CJK, + emoji, other text and the primary font's own icons are drawn as before. + Not on Linux, where the text system reports a glyph's advance box rather + than its ink. + ## [26.9.3] - 2026-09-23 ### Added diff --git a/src/terminal/element.rs b/src/terminal/element.rs index 448aedc1..7424b570 100644 --- a/src/terminal/element.rs +++ b/src/terminal/element.rs @@ -884,8 +884,8 @@ fn segment_row(row: &[RenderCell]) -> Vec { thread_local! { static CHAR_STRINGS: RefCell> = RefCell::new(HashMap::new()); - /// Measured ink extents and the font size they were measured at. - static INK_EXTENTS: RefCell<(Pixels, HashMap<(gpui::FontId, char), Option>)> = + /// Measured ink bounds and the font size they were measured at. + static INK_BOUNDS: RefCell<(Pixels, HashMap<(gpui::FontId, char), Option>>)> = RefCell::new((px(0.), HashMap::new())); } @@ -1089,6 +1089,9 @@ fn seg_budget(solo: bool, measured: bool, cells: usize, room: bool, cell_width: } /// How much to shrink a segment so its glyph stops inside its budget. +/// +/// Never more than 1. A fallback icon is not held to this at all: +/// [`icon_fit`] sizes it, up or down. fn fit_scale(ink: Pixels, budget: Pixels) -> f32 { if ink <= budget || ink <= px(0.) { 1. @@ -1097,22 +1100,157 @@ fn fit_scale(ink: Pixels, budget: Pixels) -> f32 { } } -/// Where a segment's ink ends, measured from the left edge of its first cell. +/// Whether the platform's `typographic_bounds` is the glyph's ink. +/// +/// CoreText and DirectWrite answer with the ink box. cosmic-text answers with +/// the advance box pinned to the baseline, which says nothing about where an +/// icon's ink sits; sizing and centring on that would misplace it, so the +/// Linux build keeps the shrink-only rule for icons too. +const INK_BOUNDS_ARE_INK: bool = cfg!(any(target_os = "macos", target_os = "windows")); + +/// The most an icon is ever enlarged by. +/// +/// The cap keeps a glyph that is small by design — a dot, a bullet — from +/// being blown up to a cell-sized blob. +const MAX_ICON_GROWTH: f32 = 2.; + +/// Whether a character is an icon: a code point in one of Unicode's Private +/// Use Areas, where Nerd Fonts, Font Awesome, Codicons and Material Design +/// icons all live. +/// +/// Nothing in these ranges is text, so sizing one to its cells cannot change +/// how prose reads — the reason the rule is keyed on the code point and not +/// on the face: a CJK or emoji fallback keeps its own metrics. +/// +/// The Powerline separators (`U+E0B0`–`U+E0D7`) are not icons. They are +/// shapes meant to abut the cells around them, the solid ones are painted +/// natively, and the thin ones already reach the row's full height, so +/// fitting them to an em and centring them would only pull them away from +/// the segments they join. +fn is_icon(c: char) -> bool { + matches!( + c as u32, + 0xE000..=0xE0AF | 0xE0D8..=0xF8FF | 0xF_0000..=0xF_FFFD | 0x10_0000..=0x10_FFFD + ) +} + +/// Whether the cell after an icon can lend it room, the way kitty lets a +/// Private Use Area glyph followed by a space take two cells. +/// +/// Everything [`has_room_after`] lends, and one thing more: a blank that +/// paints the *same* background as the icon's own cell. That is the space +/// after every icon in a coloured prompt segment, and backgrounds are painted +/// before any glyph, so the icon lands on its own colour. Held back to one +/// cell there, a Nerd Font icon is shrunk to about two thirds of the text's +/// height, which is #866. +fn lends_to_icon(row: &[RenderCell], start: usize, cells: usize) -> bool { + if has_room_after(row, start, cells) { + return true; + } + let (Some(icon), Some(next)) = (row.get(start), row.get(start + cells)) else { + return false; + }; + is_blank(next) + && !next.selected + && icon.draw_bg + && next.draw_bg + && next.bg == icon.bg + && next.match_hit == icon.match_hit + && next.match_current == icon.match_current + && !GlyphStyle::of(next).draws_on_blanks() +} + +/// How to size an icon a fallback face supplied, and how many cells it spans. +/// +/// A fallback face is drawn at the primary's font size on its own metrics, so +/// what an icon looks like beside the text depends on how the two faces' +/// cells compare. Symbols Nerd Font Mono inks a full em per icon, which is +/// 1.6 cells of a 0.6em primary; a face with a narrower cell than the +/// primary's inks less than one. Neither is the icon at the text's size. +/// +/// So fit the ink, aspect ratio kept, to the cells it is given and one em of +/// height — the size the icon was drawn at in its own face, and never taller +/// than the text's line. That may shrink or grow it, growth capped at +/// [`MAX_ICON_GROWTH`] and ignored under 2%, which is not worth a reshape. +/// When `lend` says the next cell is free the icon may take it too, but only +/// if that lets it come out bigger: a narrow glyph that already fits one cell +/// keeps to it and stays beside what follows. +fn icon_fit( + ink: Bounds, + cell_width: Pixels, + cells: usize, + lend: bool, + height: Pixels, +) -> (f32, usize) { + let (w, h) = (ink.size.width.as_f32(), ink.size.height.as_f32()); + if !(w > 0. && h > 0.) { + return (1., cells); + } + let fit = |cells: usize| { + let scale = (cell_width.as_f32() * cells as f32 / w) + .min(height.as_f32() / h) + .min(MAX_ICON_GROWTH); + if (1. ..=1.02).contains(&scale) { + 1. + } else { + scale + } + }; + let own = fit(cells); + if lend && fit(cells + 1) > own { + (fit(cells + 1), cells + 1) + } else { + (own, cells) + } +} + +/// Where to paint a fitted icon, relative to its cell's top-left corner, so +/// its ink sits centred in `room` — vertically always, and horizontally only +/// when `centre_x`. An icon spilling into a borrowed cell stays where its +/// face put it instead, as it would unfitted, so the gap before the text +/// after it is the one its face drew. +/// +/// `ink` is the glyph's bounds at its native size (y up from the baseline); +/// `ascent` and `descent` are the line's as shaped at the fitted size, which +/// is what gpui centres the baseline in the row with. +fn icon_offset( + ink: Bounds, + scale: f32, + room: gpui::Size, + centre_x: bool, + ascent: Pixels, + descent: Pixels, +) -> Point { + let baseline = (room.height - ascent - descent) / 2. + ascent; + let ink_top = baseline - (ink.origin.y + ink.size.height) * scale; + let x = if centre_x { + (room.width - ink.size.width * scale) / 2. - ink.origin.x * scale + } else { + // Nudged only as far as it takes to keep the ink inside the room. + px(0.) + .min(room.width - (ink.origin.x + ink.size.width) * scale) + .max(-ink.origin.x * scale) + }; + point(x, (room.height - ink.size.height * scale) / 2. - ink_top) +} + +/// The ink bounds of a segment's first glyph, relative to its pen position, +/// y up from the baseline. /// /// Advance is the wrong yardstick: Apple Color Emoji advances 1.31em but only /// inks 1.25em, so going by advance shrinks glyphs that would have fit. Ask /// for the glyph's own bounds instead, cached per (face, char) because the /// lookup is a font query a screenful of CJK would otherwise repeat on every /// cell of every frame. -fn ink_extent( +fn ink_bounds( cx: &App, shaped: &gpui::ShapedLine, text: &str, font_size: Pixels, -) -> Option { +) -> Option> { let font_id = shaped.runs.first()?.font_id; let c = text.chars().next()?; - INK_EXTENTS.with(|cache| { + INK_BOUNDS.with(|cache| { let hit = { let mut cache = cache.borrow_mut(); if cache.0 != font_size { @@ -1127,13 +1265,12 @@ fn ink_extent( if let Some(extent) = hit { return extent; } - let extent = cx + let bounds = cx .text_system() .typographic_bounds(font_id, font_size, c) - .ok() - .map(|bounds| bounds.origin.x + bounds.size.width); - cache.borrow_mut().1.insert((font_id, c), extent); - extent + .ok(); + cache.borrow_mut().1.insert((font_id, c), bounds); + bounds }) } @@ -1234,7 +1371,7 @@ fn shape_piece( .shape_line(piece, font_size, run_buf, force_width) } -/// Whether [`ink_extent`]'s answer speaks for the whole segment. +/// Whether [`ink_bounds`]'s answer speaks for the whole segment. /// /// It measures the segment's first character in the run's first face. That is /// all of a one-character segment and only part of anything longer: a @@ -1284,6 +1421,9 @@ fn paint_glyphs( build_font(bold_font.unwrap_or(base_font), true, true), ]; + // Resolved only once an icon asks, so a pane without one pays nothing. + let mut primary_ids: [Option; 4] = [None; 4]; + let run_buf = &mut [TextRun { len: 0, font: base_font.clone(), @@ -1435,17 +1575,40 @@ fn paint_glyphs( .shape_line(text.clone(), font_size, run_buf, force_width); // Measured before the budget is set, because how far a solo glyph // may reach turns on whether its ink is known at all. - let ink = ink_extent(cx, &shaped, &text, font_size); + let bounds = ink_bounds(cx, &shaped, &text, font_size); + let ink = bounds.map(|b| b.origin.x + b.size.width); + let measured = ink_covers_segment(ink, &text); let budget = seg_budget( solo, - ink_covers_segment(ink, &text), + measured, cells, has_room_after(row_cells, start, cells), geom.cell_width, ); - if let Some(ink) = ink { - let scale = fit_scale(ink, budget); - if scale < 1. { + let mut origin = point(x, y); + let mut clip_width = budget; + if let (Some(ink), Some(bounds)) = (ink, bounds) { + // Only a lone icon that a fallback face supplied is fitted: + // the primary's own icons were drawn for its cell already. + let icon = INK_BOUNDS_ARE_INK + && measured + && text.chars().next().is_some_and(is_icon) + && shaped.runs.first().is_some_and(|r| { + r.font_id + != *primary_ids[face_ix].get_or_insert_with(|| { + window.text_system().resolve_font(&faces[face_ix]) + }) + }); + let (scale, room) = if icon { + let lend = lends_to_icon(row_cells, start, cells); + let height = font_size.min(geom.line_height); + let (scale, spans) = icon_fit(bounds, geom.cell_width, cells, lend, height); + let room = size(geom.cell_width * spans as f32, geom.line_height); + (scale, Some((room, spans == cells))) + } else { + (fit_scale(ink, budget), None) + }; + if scale != 1. { shaped = window.text_system().shape_line( text.clone(), font_size * scale, @@ -1453,18 +1616,16 @@ fn paint_glyphs( force_width, ); } + if let Some((room, centre_x)) = room { + origin += + icon_offset(bounds, scale, room, centre_x, shaped.ascent, shaped.descent); + clip_width = room.width; + } } - let clip = Bounds::new(point(x, y), size(budget, geom.line_height)); + let clip = Bounds::new(point(x, y), size(clip_width, geom.line_height)); window.with_content_mask(Some(ContentMask { bounds: clip }), |window| { - _ = shaped.paint( - point(x, y), - geom.line_height, - TextAlign::Left, - None, - window, - cx, - ); + _ = shaped.paint(origin, geom.line_height, TextAlign::Left, None, window, cx); }); } } @@ -2675,11 +2836,12 @@ mod tests { } #[test] - fn fit_scale_only_shrinks_glyphs_that_overflow_their_budget() { + fn fit_scale_shrinks_what_overflows_and_never_enlarges() { let budget = px(15.); assert_eq!(fit_scale(px(19.2), budget), 15. / 19.2); // Whatever already fits keeps its own size, including a glyph that - // lands exactly on the edge and a run that measured as empty. + // lands exactly on the edge and a run that measured as empty. Making + // a small glyph bigger is `icon_fit`'s rule, not this one's. assert_eq!(fit_scale(px(15.), budget), 1.); assert_eq!(fit_scale(px(12.), budget), 1.); assert_eq!(fit_scale(px(0.), budget), 1.); @@ -4452,4 +4614,176 @@ mod tests { Some(&['\u{FE0F}'][..]), ); } + + fn ink(x: f32, y: f32, w: f32, h: f32) -> Bounds { + Bounds::new(point(px(x), px(y)), size(px(w), px(h))) + } + + /// Menlo at 22px: a 13.2px cell in a 30.8px row (line height 1.4). + const CELL: f32 = 13.2; + const EM: f32 = 22.; + + fn fit(ink: Bounds, cells: usize, lend: bool) -> (f32, usize) { + icon_fit(ink, px(CELL), cells, lend, px(EM)) + } + + #[test] + fn only_private_use_code_points_count_as_icons() { + for c in [ + '\u{e000}', // first of the BMP Private Use Area + '\u{e0a0}', // Powerline branch + '\u{e0d8}', // first past the Powerline separators + '\u{f179}', // Font Awesome apple + '\u{f8ff}', // last of the BMP Private Use Area + '\u{f0001}', // Material Design icon, Supplementary PUA-A + '\u{10fffd}', // end of Supplementary PUA-B + ] { + assert!(is_icon(c), "U+{:04X}", c as u32); + } + // Text a fallback face supplies keeps its own metrics: CJK, emoji, + // Latin, and the drawing characters painted natively anyway. + for c in [ + '中', + 'あ', + '한', + '\u{1F600}', + '\u{2764}', + 'M', + '─', + '█', + '\u{f900}', + ] { + assert!(!is_icon(c), "U+{:04X}", c as u32); + } + // Nor do the Powerline separators, which are shapes, not icons. + for c in ['\u{e0b0}', '\u{e0b1}', '\u{e0b3}', '\u{e0d7}'] { + assert!(!is_icon(c), "U+{:04X}", c as u32); + } + } + + #[test] + fn a_nerd_font_icon_before_a_space_takes_two_cells_at_full_size() { + // Symbols Nerd Font Mono's apple, as CoreText measures it at 22px: a + // full em tall and 1.4 cells wide. Held to one cell it is shrunk to + // 0.71 of the text's size, which is what #866 saw; with the space + // after it lent, it keeps its whole em. + let apple = ink(1.76, -4.43, 18.48, 22.03); + let (held, cells) = fit(apple, 1, false); + assert_eq!(cells, 1); + assert!((apple.size.width.as_f32() * held - CELL).abs() < 1e-3); + let (lent, cells) = fit(apple, 1, true); + assert_eq!(cells, 2); + assert!((apple.size.height.as_f32() * lent - EM).abs() < 1e-3); + assert!(apple.size.width.as_f32() * lent <= 2. * CELL); + } + + #[test] + fn a_narrow_icon_keeps_to_its_own_cell_even_when_the_next_is_free() { + // The branch symbol is an em tall and 0.4em wide: one cell already + // holds it at full height, so borrowing would only push it off + // towards the text after it. + let branch = ink(6.52, -4.4, 8.96, 22.); + assert_eq!(fit(branch, 1, true), (1., 1)); + } + + #[test] + fn a_small_icon_from_a_narrow_face_grows_to_fill_its_cell() { + // A face with a narrower cell than the primary inks well under it. + let icon = ink(0.5, -1., 8.8, 9.); + let (scale, cells) = fit(icon, 1, false); + assert_eq!(cells, 1); + assert!(scale > 1.); + assert!((icon.size.width.as_f32() * scale - CELL).abs() < 1e-3); + // The same icon two cells wide may use both. + let (two, cells) = fit(icon, 2, false); + assert_eq!(cells, 2); + assert!(two > scale); + } + + #[test] + fn a_fitted_icon_is_centred_in_its_room() { + let icon = ink(0.4, -0.6, 5.3, 6.); + let room = size(px(CELL), px(30.8)); + let (scale, _) = fit(icon, 1, false); + let (ascent, descent) = (px(20.4 * scale), px(5.2 * scale)); + let at = icon_offset(icon, scale, room, true, ascent, descent); + // Spilling into a borrowed cell, it stays where its face put it, + // unless that would take its ink past either edge of the room. + assert_eq!( + icon_offset(icon, scale, room, false, ascent, descent).x, + px(0.) + ); + let lent = size(px(2. * CELL), px(30.8)); + let overhang = ink(-1., 0., 20., 20.); + assert_eq!( + icon_offset(overhang, 1., lent, false, ascent, descent).x, + px(1.) + ); + let pushed = ink(8., 0., 20., 20.); + let x = icon_offset(pushed, 1., lent, false, ascent, descent).x; + assert!((x.as_f32() - (2. * CELL - 28.)).abs() < 1e-4, "{x:?}"); + let left = at.x.as_f32() + icon.origin.x.as_f32() * scale; + let right = left + icon.size.width.as_f32() * scale; + assert!((left - (CELL - right)).abs() < 1e-3, "{left}..{right}"); + let baseline = at.y + (room.height - ascent - descent) / 2. + ascent; + let top = (baseline - (icon.origin.y + icon.size.height) * scale).as_f32(); + let bottom = (baseline - icon.origin.y * scale).as_f32(); + assert!((top - (30.8 - bottom)).abs() < 1e-3, "{top}..{bottom}"); + } + + #[test] + fn icon_fit_leaves_what_is_already_the_right_size_alone() { + // Within 2% of the cell: not worth shaping the glyph again. + assert_eq!(fit(ink(0., 0., 13., 13.), 1, false), (1., 1)); + // Nothing measured, nothing to scale by. + assert_eq!(fit(ink(0., 0., 0., 0.), 1, true), (1., 1)); + // A dot is small by design and grows no more than the cap. + assert_eq!(fit(ink(1., 2., 2., 2.), 1, false).0, MAX_ICON_GROWTH); + } + + fn icon_row(next: RenderCell) -> Vec { + let icon = RenderCell { + c: '\u{f179}', + ..RenderCell::default() + }; + vec![icon, next] + } + + #[test] + fn a_blank_in_the_same_colour_lends_an_icon_its_cell() { + let blank = RenderCell::default(); + assert!(lends_to_icon(&icon_row(blank.clone()), 0, 1)); + // Whatever `has_room_after` lends, the end of the row included. + assert!(lends_to_icon(&icon_row(blank.clone())[..1], 0, 1)); + // A prompt segment: icon and the space after it on one background. + let bg = Hsla::blue(); + let mut row = icon_row(RenderCell { + bg, + draw_bg: true, + ..RenderCell::default() + }); + row[0].bg = bg; + row[0].draw_bg = true; + assert!(lends_to_icon(&row, 0, 1)); + // The next segment's colour, a glyph or a selection do not. + row[1].bg = Hsla::red(); + assert!(!lends_to_icon(&row, 0, 1)); + let letter = RenderCell { + c: 'x', + ..RenderCell::default() + }; + assert!(!lends_to_icon(&icon_row(letter), 0, 1)); + let selected = RenderCell { + selected: true, + ..RenderCell::default() + }; + assert!(!lends_to_icon(&icon_row(selected), 0, 1)); + // Nor does a coloured blank after an icon on the default background. + let coloured = RenderCell { + bg, + draw_bg: true, + ..blank + }; + assert!(!lends_to_icon(&icon_row(coloured), 0, 1)); + } }