mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(tabs): stop a directory-named tab repeating itself on hover (#740)
`tab_title_tooltip` compared the raw string the label was built from
against the label. Those are two spellings of one name as soon as a home
is known — `/Users/x/repo` against `~/repo` — so the "nothing was
dropped, stay quiet" guard never fired, and what it then returned was
`abbreviate_home` of that raw string: the label, exactly. Every tab named
after a directory inside the home grew a tooltip saying what the row
already said, which is every tab this change exists to fix. The Osc rung
had the same hole for a title that was a bare path; only the shell
integration's `user@host:` head kept it from showing.
Abbreviate first, compare after. The decision moves to `tooltip_of`
beside `label_of`, so the two read the same `TabView` through the same
spelling and can be pinned without a window.
Tests. `a_tab_named_after_a_directory_says_nothing_more_on_hover_unless
_it_was_cut` fails on the old guard with `Some("~/repo")` where `None`
was due.
And the wiring the last commit added had nothing standing on it: four
tests of `label_of` over hand-built views pass just as well with
`tab_label` put back the way it was. `the_strip_names_a_titleless_pane
_exactly_as_the_switcher_does` builds a real tab — a quiet pane seeded
with a cwd and no title, which is what OSC 7 without OSC 0 leaves — runs
it through `Tab::label_view`, `tab_label` and `tab_title_tooltip`, and
holds the answer against what `label_of` renders from the machine tree's
reading of that same pane. Reverting `tab_label` fails it with "tty7"
against "/work/repo", which is the disagreement in one line.
`RemoteTerminal::seed_cwd` widens to `pub(crate)` for it — the same call
the spawn path makes, not a test-only door.
This commit is contained in:
@@ -705,7 +705,7 @@ impl RemoteTerminal {
|
||||
/// here wins: it describes where the shell actually landed, which is not
|
||||
/// always where we asked (a missing directory sends the daemon home, an
|
||||
/// rc file may `cd` on its own).
|
||||
fn seed_cwd(&self, cwd: Option<PathBuf>) {
|
||||
pub(crate) fn seed_cwd(&self, cwd: Option<PathBuf>) {
|
||||
let Some(cwd) = cwd else { return };
|
||||
if let Ok(mut guard) = self.cwd.lock() {
|
||||
guard.get_or_insert(cwd);
|
||||
|
||||
+142
-17
@@ -191,6 +191,37 @@ pub(crate) fn label_of(
|
||||
}
|
||||
}
|
||||
|
||||
/// What a row can add on hover: the name behind the one [`label_of`] cut down,
|
||||
/// or `None` when it cut nothing and the tooltip would only repeat the row.
|
||||
///
|
||||
/// The comparison has to happen on the *same* spelling, which is the whole
|
||||
/// trick here. `label_of` abbreviates a path under the home before it elides
|
||||
/// it, and this returns the abbreviated form too, so a raw `/Users/x/repo`
|
||||
/// measured against a label of `~/repo` looks like a difference that isn't
|
||||
/// one — and every tab named after a directory inside the home would hang a
|
||||
/// tooltip saying exactly what it already says. Abbreviate first, compare
|
||||
/// after.
|
||||
fn tooltip_of(
|
||||
view: &crate::ui::machine_mirror::TabView,
|
||||
index: usize,
|
||||
home: Option<&std::path::Path>,
|
||||
) -> Option<SharedString> {
|
||||
use crate::ui::machine_mirror::TabLabel;
|
||||
|
||||
// The other rungs are never shortened: a given name and a process name are
|
||||
// printed whole, and an agent's is a word.
|
||||
let raw = match view.label() {
|
||||
TabLabel::Osc(title) => title,
|
||||
TabLabel::Cwd(cwd) => cwd,
|
||||
_ => return None,
|
||||
};
|
||||
let full = abbreviate_home(raw.trim(), home);
|
||||
if full.trim().is_empty() || full.as_ref() == label_of(view, index, home).as_str() {
|
||||
return None;
|
||||
}
|
||||
Some(SharedString::from(full.into_owned()))
|
||||
}
|
||||
|
||||
/// Width of `text` shaped in `font` at `size`, in pixels.
|
||||
///
|
||||
/// The window's text system caches shaped runs, so measuring the same labels
|
||||
@@ -1284,7 +1315,7 @@ impl Tty7App {
|
||||
/// It has to unshorten whatever the label was *made of*, which is why it
|
||||
/// reads the same [`TabView`](crate::ui::machine_mirror::TabView) the label
|
||||
/// did: a tab named after its directory wants that directory spelled out,
|
||||
/// not the title it never had.
|
||||
/// not the title it never had. See [`tooltip_of`].
|
||||
pub(crate) fn tab_title_tooltip(
|
||||
&self,
|
||||
tab: &Tab,
|
||||
@@ -1292,23 +1323,8 @@ impl Tty7App {
|
||||
window: Option<&Window>,
|
||||
cx: &App,
|
||||
) -> Option<SharedString> {
|
||||
use crate::ui::machine_mirror::TabLabel;
|
||||
|
||||
let (view, home) = tab.label_view(window, cx);
|
||||
// The other rungs are never shortened: a given name and a process name
|
||||
// are printed whole, and an agent's is a word.
|
||||
let raw = match view.label() {
|
||||
TabLabel::Osc(title) => title,
|
||||
TabLabel::Cwd(cwd) => cwd,
|
||||
_ => return None,
|
||||
};
|
||||
let raw = raw.trim();
|
||||
if raw.is_empty() || raw == label_of(&view, index, home.as_deref()) {
|
||||
return None;
|
||||
}
|
||||
Some(SharedString::from(
|
||||
abbreviate_home(raw, home.as_deref()).into_owned(),
|
||||
))
|
||||
tooltip_of(&view, index, home.as_deref())
|
||||
}
|
||||
|
||||
/// What this window puts on a tab of its own — the same ladder, through the
|
||||
@@ -2624,6 +2640,115 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A tooltip exists to say what the row had to leave out. One that repeats
|
||||
/// the row is worse than none, and the label and the raw string it came
|
||||
/// from are not comparable until both have been abbreviated: `~/repo` and
|
||||
/// `/Users/x/repo` are the same name spelled two ways, and reading them as
|
||||
/// a difference hung a tooltip on every tab named after a directory under
|
||||
/// the home — which, after this change, is most of them.
|
||||
#[test]
|
||||
fn a_tab_named_after_a_directory_says_nothing_more_on_hover_unless_it_was_cut() {
|
||||
let mut tab = strip_tab();
|
||||
tab.cwd = Some("/Users/x/repo".into());
|
||||
|
||||
assert_eq!(label_of(&tab, 0, Some(home())), "~/repo");
|
||||
assert_eq!(
|
||||
tooltip_of(&tab, 0, Some(home())),
|
||||
None,
|
||||
"the row is already showing the whole directory"
|
||||
);
|
||||
|
||||
// Cut down to its last three segments, so the head is worth having.
|
||||
tab.cwd = Some("/Users/x/repo/crates/tty7-core/src".into());
|
||||
assert_eq!(label_of(&tab, 0, Some(home())), "…/crates/tty7-core/src");
|
||||
assert_eq!(
|
||||
tooltip_of(&tab, 0, Some(home())).as_deref(),
|
||||
Some("~/repo/crates/tty7-core/src")
|
||||
);
|
||||
|
||||
// The same holds for a title that happens to be a path — the rung this
|
||||
// guard was already getting wrong before a directory could reach it.
|
||||
let mut titled = strip_tab();
|
||||
titled.osc_title = Some("/Users/x/repo".into());
|
||||
assert_eq!(tooltip_of(&titled, 0, Some(home())), None);
|
||||
|
||||
// A shell integration's `user@host:` head is not in the label, so it
|
||||
// is still worth spelling out.
|
||||
titled.osc_title = Some("me@box:/Users/x/repo".into());
|
||||
assert_eq!(
|
||||
tooltip_of(&titled, 0, Some(home())).as_deref(),
|
||||
Some("me@box:/Users/x/repo")
|
||||
);
|
||||
}
|
||||
|
||||
/// The one test that fails if any of the wiring is put back: a real tab,
|
||||
/// built the way the window builds one, named through `tab_label` — and
|
||||
/// checked against what the switcher renders from the machine tree's view
|
||||
/// of that very same pane. Before this change the strip said "tty7" and
|
||||
/// the switcher said the directory (#740).
|
||||
#[gpui::test]
|
||||
fn the_strip_names_a_titleless_pane_exactly_as_the_switcher_does(cx: &mut TestAppContext) {
|
||||
use crate::ui::pane::{Pane, PaneSlot};
|
||||
|
||||
let (app, mut vcx) = crate::ui::app::test_window::harness(cx);
|
||||
let _stream = app.update_in(&mut vcx, |app, window, cx| {
|
||||
let (view, stream) = crate::terminal::view::quiet_test_pane(1, window, cx);
|
||||
// A pane that has reported where it is over OSC 7 and has never
|
||||
// titled itself — every shell tty7 ships integration for except
|
||||
// PowerShell.
|
||||
view.read(cx)
|
||||
.terminal
|
||||
.seed_cwd(Some(std::path::PathBuf::from("/work/repo")));
|
||||
app.tabs
|
||||
.push(crate::ui::app::Tab::new(Pane::leaf(PaneSlot::Ready(view))));
|
||||
app.active = app.tabs.len() - 1;
|
||||
stream
|
||||
});
|
||||
vcx.background_executor.run_until_parked();
|
||||
|
||||
app.update_in(&mut vcx, |app, window, cx| {
|
||||
let index = app.active;
|
||||
let tab = &app.tabs[index];
|
||||
let (view, home) = tab.label_view(Some(window), cx);
|
||||
assert_eq!(view.osc_title, None, "the pane never titled itself");
|
||||
assert_eq!(view.cwd.as_deref(), Some("/work/repo"));
|
||||
|
||||
let strip = app.tab_label(tab, index, Some(window), cx);
|
||||
assert_eq!(strip, "/work/repo");
|
||||
assert_ne!(
|
||||
strip,
|
||||
crate::terminal::view::DEFAULT_TITLE,
|
||||
"and is not named after the app any more"
|
||||
);
|
||||
|
||||
// The machine tree's reading of the same pane, which is all the
|
||||
// switcher ever has: no title was seen, the cwd is the one above,
|
||||
// and `title` is the foreground process name.
|
||||
let from_tree = crate::ui::machine_mirror::TabView {
|
||||
id: tab.tree_id.get(),
|
||||
name: None,
|
||||
title: "zsh".into(),
|
||||
osc_title: None,
|
||||
cwd: Some("/work/repo".into()),
|
||||
agent: None,
|
||||
status: None,
|
||||
live: true,
|
||||
panes: 1,
|
||||
};
|
||||
assert_eq!(
|
||||
strip,
|
||||
label_of(&from_tree, index, home.as_deref()),
|
||||
"the two columns name the same tab the same way"
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
app.tab_title_tooltip(tab, index, Some(window), cx),
|
||||
None,
|
||||
"and the row is showing the whole path, so it stays quiet"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_pane_with_nothing_to_say_falls_back_the_way_it_always_did() {
|
||||
// No title and no directory: the placeholder, exactly as before.
|
||||
|
||||
Reference in New Issue
Block a user