diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bff9e7b..8c95ade3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 stacks, one per screen, alongside the modes it already restores, and puts them back as they stood where the replay begins, so the pushes and pops the replay still carries land on the stack they were written against. + +- **A title that only swaps its status mark no longer repaints the window.** + An agent that animates its title (Claude Code turns a circle one quadrant at + a time) sends a new OSC title every few hundred milliseconds, and each one + rebuilt the whole window for a tab that reads the same: the tab never draws + the mark. The pane's own title now takes the same `strip_status_mark` reading + as the label, so `settle_title` drops such a frame, and a pane-facts delta + that leaves everything a tab shows as it was no longer ends in a + `cx.notify()` on the root view. A title whose words change still lands as + before, so a ticking counter in the title repaints as often as it did. + - **The macOS traffic lights sit on the same line as the tiles beside them.** The buttons AppKit draws are 14 points tall and gpui hangs the top of the frame `y` points below the window's top edge, so the lights' centre falls diff --git a/crates/tty7-core/src/core/tab_view.rs b/crates/tty7-core/src/core/tab_view.rs index bbed34aa..ea8f5f57 100644 --- a/crates/tty7-core/src/core/tab_view.rs +++ b/crates/tty7-core/src/core/tab_view.rs @@ -158,6 +158,17 @@ pub fn strip_status_mark(title: &str) -> &str { } } +/// Whether two titles read the same on a tab: equal once the status mark in +/// front of each is taken off, which is all a spinner frame changes. +/// +/// Claude Code rewrites its title every few hundred milliseconds while it +/// works, one quadrant of the circle at a time, and a tab never draws the mark +/// ([`TabView::label`]). A repaint for such a title would paint the same label +/// again. Trimmed the way `label` trims, so the two agree on what a tab shows. +pub fn same_title_ignoring_status_mark(a: &str, b: &str) -> bool { + strip_status_mark(a.trim()) == strip_status_mark(b.trim()) +} + impl TabView { pub fn label(&self) -> TabLabel<'_> { if let Some(name) = self @@ -228,6 +239,30 @@ pub fn tab_views_of(ws: &Workspace, panes: &[PaneRecord]) -> Vec { #[cfg(test)] mod tests { + /// A spinner frame is the only thing that changes between two of these, so + /// a tab that already shows one has nothing new to paint for the other. + #[test] + fn titles_that_differ_only_by_a_status_mark_read_the_same() { + for (a, b) in [ + ( + "\u{25D0} fixing the switcher", + "\u{25D1} fixing the switcher", + ), + ("\u{2733} fixing the switcher", "fixing the switcher"), + ("\u{280B} building", "\u{2819} building"), + (" \u{25D0} fixing the switcher ", "fixing the switcher"), + ] { + assert!(same_title_ignoring_status_mark(a, b), "on {a:?} / {b:?}"); + } + for (a, b) in [ + ("\u{25D0} fixing the switcher", "\u{25D1} fixing the parser"), + ("vim", "vim \u{2014} main.rs"), + ("", "vim"), + ] { + assert!(!same_title_ignoring_status_mark(a, b), "on {a:?} / {b:?}"); + } + } + /// The marks come off, whichever alphabet the agent picked. #[test] fn a_status_mark_comes_off_the_front_of_a_title() { diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 0f9e08d2..609da591 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -693,7 +693,9 @@ const AGENT_DONE_SETTLE: std::time::Duration = std::time::Duration::from_millis( #[derive(Debug, PartialEq, Eq)] enum TitleSettle { /// The tab already reads this, so whatever was waiting to replace it never - /// has to happen. This is the case a short command lands in. + /// has to happen. This is the case a short command lands in, and the one an + /// agent's spinner lands in: a frame that only changes the status mark in + /// front of the title reads the same on the tab, so it asks for no repaint. Revert, /// Hold it: a wait is already running and adopts the newest title when it /// elapses. Restarting the wait instead would let a program that rewrites @@ -705,7 +707,7 @@ enum TitleSettle { } fn settle_title(showing: &str, waiting: bool, incoming: &str) -> TitleSettle { - if incoming == showing { + if tty7_core::core::tab_view::same_title_ignoring_status_mark(incoming, showing) { return TitleSettle::Revert; } match waiting { @@ -8810,6 +8812,39 @@ mod tests { ); } + #[test] + fn a_spinner_frame_that_only_changes_the_status_mark_asks_for_no_repaint() { + // An agent rewrites its title every few hundred milliseconds while it + // works, and the tab never draws the mark, so none of these may wait + // and repaint the whole window for the same label. + assert_eq!( + settle_title( + "\u{25D0} fixing the switcher", + false, + "\u{25D1} fixing the switcher" + ), + TitleSettle::Revert + ); + assert_eq!( + settle_title( + "\u{25D0} fixing the switcher", + true, + "\u{2733} fixing the switcher" + ), + TitleSettle::Revert, + "and a frame that lands while another title waits drops it, as a revert does" + ); + assert_eq!( + settle_title( + "\u{25D0} fixing the switcher", + false, + "\u{25D1} fixing the parser" + ), + TitleSettle::QueueAndWait, + "a different title still lands" + ); + } + #[test] fn a_notification_title_keeps_at_most_two_segments() { let ws = || Some("tty7".to_string()); diff --git a/src/ui/machine_mirror.rs b/src/ui/machine_mirror.rs index d6fa05d5..e08beb89 100644 --- a/src/ui/machine_mirror.rs +++ b/src/ui/machine_mirror.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use gpui::{App, Global}; use tty7_core::core::group_key::AutoKey; use tty7_core::core::machine::{LayoutDelta, Machine, PaneRecord, Tab, TabId, Workspace}; +use tty7_core::core::tab_view::same_title_ignoring_status_mark; use tty7_core::daemon::control::{ControlRequest, ReplyOk}; use tty7_core::host::HostId; @@ -134,6 +135,21 @@ impl MachineMirrors { cx.refresh_windows(); } + /// Whether `delta` only says again what a pane's tab already shows. + /// + /// Asked before [`Self::apply_delta`], which overwrites the record this + /// compares against. A window has nothing to repaint for such a delta, and + /// repainting anyway is what an agent that rewrites its title every few + /// hundred milliseconds costs: the whole window rebuilt for the same label. + pub fn delta_changes_nothing_shown(cx: &App, host: HostId, delta: &LayoutDelta) -> bool { + let LayoutDelta::PaneFacts { pane } = delta else { + return false; + }; + Self::machine(cx, host) + .and_then(|machine| machine.panes.iter().find(|p| p.id == pane.id)) + .is_some_and(|old| shows_the_same(old, pane)) + } + pub fn apply_delta(cx: &mut App, host: HostId, key: &str, delta: &LayoutDelta) { let Ok(id) = key.parse::() else { return; @@ -286,6 +302,26 @@ impl MachineMirrors { } } +/// Whether `new` would put anything on screen that the mirror's record of the +/// same pane does not already show. +/// +/// The OSC title only has to read the same once its status mark is off, since +/// a tab never draws the mark. Every other field has to match as it stands, +/// compared through the record's own equality so that a field added to it later +/// takes part without anyone remembering to list it here. +fn shows_the_same(old: &PaneRecord, new: &PaneRecord) -> bool { + let titles = match (&old.osc_title, &new.osc_title) { + (Some(a), Some(b)) => same_title_ignoring_status_mark(a, b), + (None, None) => true, + _ => false, + }; + titles + && PaneRecord { + osc_title: new.osc_title.clone(), + ..old.clone() + } == *new +} + fn apply(machine: &mut Machine, workspace: WorkspaceId, delta: &LayoutDelta) -> bool { match delta { LayoutDelta::WorkspaceCreated { workspace: ws } => { @@ -1113,6 +1149,77 @@ mod tests { )); } + /// The question is put to the mirror before the delta lands in it, against + /// the record of the same pane, so a pane the mirror has not met, a host it + /// does not hold and a delta about anything but a pane's facts all have to + /// answer "something may have changed". + #[gpui::test] + fn a_delta_is_only_old_news_against_the_record_the_mirror_holds(cx: &mut gpui::TestAppContext) { + cx.update(|cx| { + let mut known = PaneRecord::new(7); + known.osc_title = Some("\u{25D0} fixing the switcher".into()); + MachineMirrors::install( + cx, + HostId::LOCAL, + Machine { + workspaces: Vec::new(), + panes: vec![known.clone()], + }, + ); + let facts = |pane: PaneRecord| LayoutDelta::PaneFacts { pane }; + let asked = |cx: &App, host, delta: &LayoutDelta| { + MachineMirrors::delta_changes_nothing_shown(cx, host, delta) + }; + + let mut frame = known.clone(); + frame.osc_title = Some("\u{25D1} fixing the switcher".into()); + assert!(asked(cx, HostId::LOCAL, &facts(frame)), "a turned frame"); + + let mut words = known.clone(); + words.osc_title = Some("\u{25D1} fixing the parser".into()); + assert!(!asked(cx, HostId::LOCAL, &facts(words)), "new words"); + + let stranger = PaneRecord::new(8); + assert!( + !asked(cx, HostId::LOCAL, &facts(stranger)), + "a pane the mirror has not met" + ); + assert!( + !asked(cx, HostId(42), &facts(known.clone())), + "a host the mirror does not hold" + ); + assert!( + !asked( + cx, + HostId::LOCAL, + &LayoutDelta::WorkspaceRenamed { name: None } + ), + "a delta that is not about a pane's facts" + ); + }); + } + + #[test] + fn a_spinner_frame_is_not_news_but_any_other_change_is() { + let mut old = PaneRecord::new(7); + old.osc_title = Some("\u{25D0} fixing the switcher".into()); + let mut next = old.clone(); + assert!(shows_the_same(&old, &next), "the same record again"); + next.osc_title = Some("\u{25D1} fixing the switcher".into()); + assert!(shows_the_same(&old, &next), "only the mark moved"); + next.osc_title = Some("\u{25D1} fixing the parser".into()); + assert!(!shows_the_same(&old, &next), "the words changed"); + next.osc_title = None; + assert!(!shows_the_same(&old, &next), "the title was cleared"); + + let mut live = old.clone(); + live.live = true; + assert!(!shows_the_same(&old, &live), "a pane that came alive"); + let mut moved = old.clone(); + moved.cwd = Some("/elsewhere".into()); + assert!(!shows_the_same(&old, &moved), "a pane that moved"); + } + #[test] fn pane_facts_upsert_the_registry_even_for_a_pane_born_elsewhere() { let mut machine = Machine::default(); diff --git a/src/ui/tree_sync.rs b/src/ui/tree_sync.rs index 12b88e45..7f442690 100644 --- a/src/ui/tree_sync.rs +++ b/src/ui/tree_sync.rs @@ -2900,6 +2900,9 @@ fn on_workspace_deleted(cx: &mut App, client_ws: WorkspaceId) { } pub(crate) fn on_layout_delta(cx: &mut App, host: HostId, key: &str, delta: LayoutDelta) { + // Asked while the mirror still holds the pane as it was. + let nothing_shown = + crate::ui::machine_mirror::MachineMirrors::delta_changes_nothing_shown(cx, host, &delta); crate::ui::machine_mirror::MachineMirrors::apply_delta(cx, host, key, &delta); let client_ws = if host.is_local() { key.parse::().ok() @@ -2946,11 +2949,15 @@ pub(crate) fn on_layout_delta(cx: &mut App, host: HostId, key: &str, delta: Layo let Some(handle) = crate::ui::windows::WindowRegistry::window_for(cx, client_ws) else { return; }; - let window_ok = handle - .update(cx, |_, window, cx| { - app.update(cx, |app, cx| app.apply_layout_delta(&delta, window, cx)) - }) - .unwrap_or(true); + // `apply_layout_delta` ends in a `notify()` of the whole window, and for a + // pane-facts delta that is all it does. One that changes nothing a tab + // shows (an agent's spinner turning a quarter) has no use for the repaint. + let window_ok = nothing_shown + || handle + .update(cx, |_, window, cx| { + app.update(cx, |app, cx| app.apply_layout_delta(&delta, window, cx)) + }) + .unwrap_or(true); if !mirror_ok || !window_ok { log::info!( "workspace {client_ws}: delta {delta:?} did not apply cleanly; re-pulling the tree"