mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-11-06 00:15:06 +00:00
fix(tabs): a title that only swaps its status mark no longer repaints the window (#1114)
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. Each one rebuilt the whole window for a tab that reads the same, because TabView::label() strips the mark before drawing it. Two places now read "same title" the way label() does (trimmed, status mark stripped, via tab_view::same_title_ignoring_status_mark): - settle_title treats a frame that only changes the mark as a Revert, so it queues no 400 ms wait and ends in no cx.notify() on the pane. - on_layout_delta asks the machine mirror, before applying a PaneFacts delta, whether the pane's record differs from the mirror's by anything but such a title, and skips apply_layout_delta (whose only effect for PaneFacts is the notify of the root view) when it does not. The mirror is still updated and sync_window still runs. A title whose words change lands as before, so a ticking counter in a title repaints as often as it did. Co-authored-by: kalpak <307643502+kalpakprod@users.noreply.github.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<TabView> {
|
||||
#[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() {
|
||||
|
||||
+37
-2
@@ -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());
|
||||
|
||||
@@ -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::<WorkspaceId>() 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();
|
||||
|
||||
+12
-5
@@ -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::<WorkspaceId>().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"
|
||||
|
||||
Reference in New Issue
Block a user