feat(ui): fold sidebar groups; stop the diff header clipping its controls (#804)

A sidebar group folds shut when its header is clicked, and stays shut across
launches. A search outranks the fold — a row a live query matches shows
whatever its group says — and so does the active tab's own row, which a fold
must never hide: `spawn_group` seeds a new tab with the group it came from, so
otherwise Cmd-T inside a folded group would draw nothing but a header count
going up by one. Folded rows register no rectangle, so a pane cannot be dropped
into a group that is shut, and the header still counts every row the group has.

Separately, the diff overlay's header stops clipping its own controls. Every
element on it but the commit subject was `flex_shrink_0`, and one of those was
a focused file's path, which is unbounded — in a docked column the two things
you can actually click were pushed off the end and clipped mid-word. The path
and the byline now yield first, ahead of the subject, because each has a second
home elsewhere in the overlay and the subject has none; the path yields
head-first so the filename is the last thing to go. `split_path_leaf` moves to
`path_display` with its tests, which is where a path's display spelling lives.
This commit is contained in:
l0ng-ai
2026-09-08 12:13:03 +08:00
committed by GitHub
parent 644945d137
commit 4c02ad3881
5 changed files with 359 additions and 87 deletions
+17
View File
@@ -212,6 +212,22 @@ pub struct Config {
pub scm_graph_expanded: bool,
#[serde(default, deserialize_with = "de_lenient")]
pub sidebar_grouping: SidebarGrouping,
/// Which sidebar groups are folded shut, by group key: the repo root the
/// group is named after, or the empty string for the scratch group, which
/// has no root of its own and no real key can ever collide with.
///
/// Kept as a list of the folded ones rather than a flag per group because
/// groups come and go with the tabs — a group nobody has opened yet has to
/// start expanded, and an entry for a repo that is no longer around costs
/// one dead path in the file.
///
/// `String`, not `PathBuf`: serde refuses to serialize a non-UTF-8
/// `PathBuf`, and `Config::save` turns that refusal into one `warn!` and
/// a return — so a single repo root with odd bytes in it would silently
/// stop the *whole* config being written from then on. A lossy spelling
/// of such a root at worst folds two of them together.
#[serde(default, deserialize_with = "de_lenient")]
pub sidebar_collapsed_groups: Vec<String>,
#[serde(default = "default_true")]
pub sidebar_diff_preview: bool,
#[serde(default, deserialize_with = "de_lenient")]
@@ -613,6 +629,7 @@ impl Default for Config {
document_ratio: default_document_ratio(),
scm_graph_expanded: false,
sidebar_grouping: SidebarGrouping::Repo,
sidebar_collapsed_groups: Vec::new(),
sidebar_diff_preview: true,
notify_on_command_finish: NotifyMode::Unfocused,
check_for_updates: true,
+34 -6
View File
@@ -689,31 +689,56 @@ impl Tty7App {
})
// The subject takes the slack the spacer below would otherwise
// have, which is why that one is skipped when a label is present:
// two `flex_1` siblings split the line in half and the subject
// two growing siblings split the line in half and the subject
// would truncate with empty space beside it.
//
// `flex_auto` rather than `flex_1` for the shrinking half of that:
// both grow the same, but `flex_1` bases the item at zero, and an
// item based at zero has a scaled shrink factor of zero — it
// absorbs none of a deficit and simply gets nothing, so the
// subject would vanish first however high the others' shrink
// factors were. Based at its content width it yields last, which
// is the order the strip wants.
.when_some(subject.label.as_ref(), |bar, label| {
bar.child(
div()
.flex_1()
.flex_auto()
.min_w_0()
.truncate()
.text_sm()
.child(SharedString::from(label.subject.clone())),
)
// Yields before the subject does, for the same reason the
// path below it does: an author name is unbounded too, and of
// the three things on this strip it is the one nobody reads
// twice.
.child(
div()
.flex_shrink_0()
.min_w_0()
.flex_shrink(999.)
.truncate()
.text_xs()
.text_color(cx.theme().muted_foreground)
.child(label_byline(label, now_unix())),
)
})
// The focused file's path is the only other thing on the strip
// that grows without bound, and it used to refuse to yield any of
// it: a header with a commit label already spends its slack on the
// subject, so the path pushed the view switch and the close tile
// off the end of a docked column and they were clipped away
// mid-word. It shrinks now, ahead of the subject (`999.` against
// the subject's `1.`) because a path has a second home one line
// down in the file list and the subject has none — and it shrinks
// head-first, so the filename is the last thing to go.
.when_some(focused_name(overlay), |bar, name| {
let (head, leaf) = crate::ui::path_display::split_path_leaf(&name);
bar.child(
div().occlude().flex_shrink_0().child(
div().occlude().min_w_0().flex_shrink(999.).child(
h_flex()
.id("diff-overlay-unfocus")
.items_center()
.min_w_0()
.gap_1()
.px_1p5()
.py_0p5()
@@ -734,13 +759,16 @@ impl Tty7App {
.child(
Icon::new(IconName::ChevronLeft)
.small()
.flex_shrink_0()
.text_color(cx.theme().muted_foreground),
)
.child(
div()
h_flex()
.min_w_0()
.text_xs()
.font_family(self.font_family.clone())
.child(name),
.child(div().min_w_0().flex_shrink(999.).truncate().child(head))
.child(div().min_w_0().flex_shrink(1.).truncate().child(leaf)),
),
),
)
+83
View File
@@ -142,6 +142,30 @@ fn abbreviate_under<'a>(path: &'a str, home: &str) -> Cow<'a, str> {
Cow::Owned(format!("~/{}", path[boundary + 1..].replace('\\', "/")))
}
/// Splits a path into the part that may be eaten by truncation and the
/// segment that must survive it.
///
/// A path identifies a thing by its *last* segment, and plain end-truncation
/// eats exactly that: a deep checkout reads "/private/tmp/claude-501…" and
/// tells you nothing. Drawn as two elements — a head that shrinks first and
/// a leaf that shrinks last — the filename stays legible however narrow the
/// row gets, the way a file manager shows a path. `head + leaf` rejoins into
/// the original string, so nothing is invented at either end.
pub(crate) fn split_path_leaf(s: &str) -> (String, String) {
// The larger of the two separator positions, not cfg-gated by platform:
// the Info panel shows remote paths too, so a Windows build describes
// Unix paths and vice versa — and a mixed-spelling path (`C:\Users\dev/
// project`, which agent-reported cwds arrive as) still cuts at its true
// leaf (#544). A Unix filename containing a literal `\` loses a shorter
// leaf; head + leaf still rejoins exactly, so the cost is decorative.
let leaf_at = s.rfind('/').max(s.rfind('\\'));
match leaf_at {
// Keep the separator with the head: "~/a/b/" + "c" rejoins exactly.
Some(i) if i + 1 < s.len() => (s[..=i].to_string(), s[i + 1..].to_string()),
_ => (String::new(), s.to_string()),
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -293,4 +317,63 @@ mod tests {
assert!(matches!(got, Cow::Borrowed(_)));
}
}
#[test]
fn the_head_and_leaf_rejoin_into_the_path_they_came_from() {
for p in [
"~/repo/tty7",
"/private/tmp/claude-501/a-very-long-directory/and-another-level",
"/",
"relative",
"",
"C:\\Users\\dev\\project",
"C:\\Users\\dev/project",
"\\\\server\\share\\dir",
] {
let (head, leaf) = split_path_leaf(p);
assert_eq!(format!("{head}{leaf}"), p, "rejoining {p:?}");
}
}
#[test]
fn the_leaf_is_the_segment_that_names_the_directory() {
let (head, leaf) = split_path_leaf("/a/b/c");
assert_eq!((head.as_str(), leaf.as_str()), ("/a/b/", "c"));
// A trailing slash has no leaf to keep, so the whole thing is head.
let (head, leaf) = split_path_leaf("/a/b/");
assert_eq!((head.as_str(), leaf.as_str()), ("", "/a/b/"));
// Root is one segment with nothing before it.
let (head, leaf) = split_path_leaf("/");
assert_eq!((head.as_str(), leaf.as_str()), ("", "/"));
}
#[test]
fn the_leaf_survives_windows_and_mixed_spellings() {
// Backslash-native, the shape an agent-reported cwd arrives in.
let (head, leaf) = split_path_leaf("C:\\Users\\dev\\project");
assert_eq!(
(head.as_str(), leaf.as_str()),
("C:\\Users\\dev\\", "project")
);
// Mixed separators cut at the *last* one of either kind.
let (head, leaf) = split_path_leaf("C:\\Users\\dev/project");
assert_eq!(
(head.as_str(), leaf.as_str()),
("C:\\Users\\dev/", "project")
);
let (head, leaf) = split_path_leaf("C:/Users/dev\\project");
assert_eq!(
(head.as_str(), leaf.as_str()),
("C:/Users/dev\\", "project")
);
// A drive root has no leaf to keep.
let (head, leaf) = split_path_leaf("C:\\");
assert_eq!((head.as_str(), leaf.as_str()), ("", "C:\\"));
// A UNC path splits at its last component, head keeping the share.
let (head, leaf) = split_path_leaf("\\\\server\\share\\dir");
assert_eq!(
(head.as_str(), leaf.as_str()),
("\\\\server\\share\\", "dir")
);
}
}
+2 -78
View File
@@ -808,7 +808,7 @@ impl Tty7App {
// absorb the shrinking so the leaf survives, the way a file
// manager shows a path.
InfoValue::Path(v) => {
let (head, leaf) = split_path_leaf(&v);
let (head, leaf) = crate::ui::path_display::split_path_leaf(&v);
h_flex()
.flex_1()
.min_w_0()
@@ -1482,23 +1482,6 @@ pub fn reveal_label() -> &'static str {
}
}
/// Splits a path into everything-but-the-last-segment and the last segment,
/// so a row can shrink the first and keep the second.
fn split_path_leaf(s: &str) -> (String, String) {
// The larger of the two separator positions, not cfg-gated by platform:
// the Info panel shows remote paths too, so a Windows build describes
// Unix paths and vice versa — and a mixed-spelling path (`C:\Users\dev/
// project`, which agent-reported cwds arrive as) still cuts at its true
// leaf (#544). A Unix filename containing a literal `\` loses a shorter
// leaf; head + leaf still rejoins exactly, so the cost is decorative.
let leaf_at = s.rfind('/').max(s.rfind('\\'));
match leaf_at {
// Keep the separator with the head: "~/a/b/" + "c" rejoins exactly.
Some(i) if i + 1 < s.len() => (s[..=i].to_string(), s[i + 1..].to_string()),
_ => (String::new(), s.to_string()),
}
}
/// `home` is the home directory of the machine `path` lives on. A remote
/// pane's cwd is measured against *its* host's home, never this machine's
/// (#580) — and against nothing at all while the host has not said.
@@ -1518,7 +1501,7 @@ fn turn_is_jumpable(row: Option<i64>, alt_now: bool) -> bool {
#[cfg(test)]
mod tests {
use super::{InfoRow, InfoValue, split_path_leaf, turn_is_jumpable};
use super::{InfoRow, InfoValue, turn_is_jumpable};
fn diff(added: u32, removed: u32, open: bool) -> InfoRow {
InfoRow {
@@ -1618,63 +1601,4 @@ mod tests {
"there is no sensible clipboard form of two coloured numbers"
);
}
#[test]
fn the_head_and_leaf_rejoin_into_the_path_they_came_from() {
for p in [
"~/repo/tty7",
"/private/tmp/claude-501/a-very-long-directory/and-another-level",
"/",
"relative",
"",
"C:\\Users\\dev\\project",
"C:\\Users\\dev/project",
"\\\\server\\share\\dir",
] {
let (head, leaf) = split_path_leaf(p);
assert_eq!(format!("{head}{leaf}"), p, "rejoining {p:?}");
}
}
#[test]
fn the_leaf_is_the_segment_that_names_the_directory() {
let (head, leaf) = split_path_leaf("/a/b/c");
assert_eq!((head.as_str(), leaf.as_str()), ("/a/b/", "c"));
// A trailing slash has no leaf to keep, so the whole thing is head.
let (head, leaf) = split_path_leaf("/a/b/");
assert_eq!((head.as_str(), leaf.as_str()), ("", "/a/b/"));
// Root is one segment with nothing before it.
let (head, leaf) = split_path_leaf("/");
assert_eq!((head.as_str(), leaf.as_str()), ("", "/"));
}
#[test]
fn the_leaf_survives_windows_and_mixed_spellings() {
// Backslash-native, the shape an agent-reported cwd arrives in.
let (head, leaf) = split_path_leaf("C:\\Users\\dev\\project");
assert_eq!(
(head.as_str(), leaf.as_str()),
("C:\\Users\\dev\\", "project")
);
// Mixed separators cut at the *last* one of either kind.
let (head, leaf) = split_path_leaf("C:\\Users\\dev/project");
assert_eq!(
(head.as_str(), leaf.as_str()),
("C:\\Users\\dev/", "project")
);
let (head, leaf) = split_path_leaf("C:/Users/dev\\project");
assert_eq!(
(head.as_str(), leaf.as_str()),
("C:/Users/dev\\", "project")
);
// A drive root has no leaf to keep.
let (head, leaf) = split_path_leaf("C:\\");
assert_eq!((head.as_str(), leaf.as_str()), ("", "C:\\"));
// A UNC path splits at its last component, head keeping the share.
let (head, leaf) = split_path_leaf("\\\\server\\share\\dir");
assert_eq!(
(head.as_str(), leaf.as_str()),
("\\\\server\\share\\", "dir")
);
}
}
+223 -3
View File
@@ -164,6 +164,13 @@ impl Tty7App {
let keys: Rc<Vec<Option<PathBuf>>> = Rc::new(self.sidebar_group_keys(cx));
let sections = sidebar_sections(&keys);
// A search outranks a fold. Typing something that matches a row inside
// a folded group has to show that row — a box that says nothing
// matches while the match sits behind a chevron is just lying.
let folded_keys: Vec<String> = match query.is_empty() {
true => cx.global::<Config>().sidebar_collapsed_groups.clone(),
false => Vec::new(),
};
// ⌘N runs ActivateTabN, which goes through `activate_visual` — the
// Nth row as the sidebar lays it out, not the Nth tab in `self.tabs`.
@@ -266,8 +273,32 @@ impl Tty7App {
for (group_slot, group_ix) in blocks {
let section = &sections[group_ix];
let group_key = section.key.clone();
// Only a group that draws a header can be folded — there is
// nothing to click otherwise, and the one headerless section (the
// whole sidebar, when grouping is off) must never answer to the
// scratch group's key.
let folded = section.name.is_some() && folded_keys.contains(&collapse_key(&group_key));
let mut rows: Vec<ContextMenu<Stateful<Div>>> = Vec::new();
let visible = visible_by_section[group_ix].clone();
// The header keeps counting every row the group has; folding only
// stops them being drawn. Nothing downstream then registers a
// rectangle for them, which is what keeps a pane from being
// dropped into a group that is shut.
//
// The active tab is the one exception: a fold says "I am done
// with this repo for now", never "hide the tab I am looking at".
// Without it ⌘T inside a folded group — `spawn_group` seeds the
// new tab with the group it came from — draws nothing but a
// header count going up by one, and with the tab bar docked left
// that row is the tab's only representation on screen.
let row_count = visible_by_section[group_ix].len();
let visible: Vec<usize> = match folded {
true => visible_by_section[group_ix]
.iter()
.copied()
.filter(|&i| i == active)
.collect(),
false => visible_by_section[group_ix].clone(),
};
let visible_tabs: Vec<usize> = visible.clone();
let row_slots: Rc<RefCell<Vec<Bounds<Pixels>>>> =
Rc::new(RefCell::new(vec![Bounds::default(); visible.len()]));
@@ -826,7 +857,7 @@ impl Tty7App {
}));
}
if rows.is_empty() {
if row_count == 0 {
continue;
}
@@ -845,7 +876,6 @@ impl Tty7App {
}
None => (0..rows.len()).collect(),
};
let row_count = rows.len();
let mut rows: Vec<Option<ContextMenu<Stateful<Div>>>> =
rows.into_iter().map(Some).collect();
let rows: Vec<AnyElement> = row_display
@@ -894,6 +924,11 @@ impl Tty7App {
.pb_0p5()
.text_size(px(11.))
.text_color(cx.theme().muted_foreground)
.hover(|s| s.text_color(cx.theme().foreground))
.on_click(cx.listener({
let key = group_key.clone();
move |this, _, _window, cx| this.toggle_sidebar_group(&key, cx)
}))
.when_some(group_slot, |header, slot| {
crate::ui::reorder::cursor_grab(header).on_drag(DragGroup, {
let state = self.reorder.clone();
@@ -912,6 +947,15 @@ impl Tty7App {
}
})
})
.child(
div().flex_shrink_0().child(
Icon::new(match folded {
true => IconName::ChevronRight,
false => IconName::ChevronDown,
})
.xsmall(),
),
)
.child(
div()
.flex_shrink(1.)
@@ -1256,6 +1300,21 @@ impl Tty7App {
.then_some(info)
}
/// Fold the sidebar group `key` names, or unfold it if it is already
/// shut. Persisted: a group folded away is a statement about a repo you
/// are done with for now, and it should still be shut tomorrow.
pub(crate) fn toggle_sidebar_group(&mut self, key: &Option<PathBuf>, cx: &mut Context<Self>) {
let id = collapse_key(key);
self.update_config(cx, |cfg| {
match cfg.sidebar_collapsed_groups.iter().position(|p| *p == id) {
Some(at) => {
cfg.sidebar_collapsed_groups.remove(at);
}
None => cfg.sidebar_collapsed_groups.push(id),
}
});
}
fn sidebar_group_keys(&self, cx: &gpui::App) -> Vec<Option<PathBuf>> {
let grouping = cx.global::<Config>().sidebar_grouping;
self.tabs
@@ -1345,6 +1404,15 @@ fn resolved_group(
})
}
/// How a group is named in `Config::sidebar_collapsed_groups`. A keyed group
/// is its repo root; the scratch group has no root, so it is written as the
/// empty string — which no repo root can ever be.
fn collapse_key(key: &Option<PathBuf>) -> String {
key.as_ref()
.map(|p| p.to_string_lossy().into_owned())
.unwrap_or_default()
}
#[derive(Debug, PartialEq)]
struct Section {
key: Option<PathBuf>,
@@ -1491,6 +1559,148 @@ pub(crate) fn diff_click_cwd<T>(cfg: &Config, target: Option<T>) -> Option<T> {
cfg.sidebar_diff_preview.then_some(target).flatten()
}
#[cfg(test)]
mod fold_tests {
use super::*;
use crate::ui::app::test_window::harness_with_tabs;
use gpui::TestAppContext;
/// Bounds a row registered for itself while it was on screen. A folded
/// row leaves the default rectangle behind, and that is what stops a pane
/// being dropped into a group that is shut.
fn drawn(app: &Tty7App, i: usize) -> bool {
app.sidebar_slots.borrow()[i].size.height > px(0.)
}
#[gpui::test]
fn folding_a_group_takes_its_rows_off_the_sidebar(cx: &mut TestAppContext) {
let (app, mut vcx, _streams) = harness_with_tabs(cx, 3);
let alpha = PathBuf::from("/w/alpha");
let beta = PathBuf::from("/w/beta");
app.update(&mut vcx, |app, cx| {
for (i, root) in [(0, &alpha), (1, &alpha), (2, &beta)] {
*app.tabs[i].sidebar_group.borrow_mut() = Some(root.clone());
}
// Active in the group that stays open: the folded group's own
// active row has its own test below, and it would mask this one.
app.active = 2;
cx.notify();
});
vcx.run_until_parked();
app.update(&mut vcx, |app, _| {
assert!(
(0..3).all(|i| drawn(app, i)),
"every row is on screen before anything is folded"
);
});
app.update(&mut vcx, |app, cx| {
app.toggle_sidebar_group(&Some(alpha.clone()), cx)
});
vcx.run_until_parked();
app.update(&mut vcx, |app, cx| {
assert!(
!drawn(app, 0) && !drawn(app, 1),
"the folded group's rows left no rectangle behind"
);
assert!(drawn(app, 2), "the group next to it is untouched");
assert_eq!(
cx.global::<Config>().sidebar_collapsed_groups,
vec!["/w/alpha".to_string()],
"the fold is written where the next launch will read it"
);
});
app.update(&mut vcx, |app, cx| {
app.toggle_sidebar_group(&Some(alpha), cx)
});
vcx.run_until_parked();
app.update(&mut vcx, |app, cx| {
assert!((0..3).all(|i| drawn(app, i)), "unfolding brings them back");
assert!(
cx.global::<Config>().sidebar_collapsed_groups.is_empty(),
"and takes the entry back out rather than piling up"
);
});
}
#[gpui::test]
fn a_search_outranks_a_fold(cx: &mut TestAppContext) {
let (app, mut vcx, _streams) = harness_with_tabs(cx, 2);
let alpha = PathBuf::from("/w/alpha");
app.update(&mut vcx, |app, cx| {
for i in 0..2 {
*app.tabs[i].sidebar_group.borrow_mut() = Some(alpha.clone());
}
app.toggle_sidebar_group(&Some(alpha), cx);
});
vcx.run_until_parked();
// Row 1, not row 0: row 0 is the active tab and a fold never takes
// that one off the screen, so it says nothing about the fold.
app.update(&mut vcx, |app, _| {
assert!(!drawn(app, 1), "folded, so the inactive row is not drawn");
});
// Whatever the row is actually showing — the label is derived from the
// test process's cwd, and this has to be a query that matches it.
app.update_in(&mut vcx, |app, window, cx| {
let label = app.tab_label(&app.tabs[1], 1, Some(window), cx).to_string();
app.sidebar_search.update(cx, |state, cx| {
state.set_value(&label, window, cx);
});
});
vcx.run_until_parked();
app.update(&mut vcx, |app, _| {
assert!(
drawn(app, 1),
"a row a query matches has to show, fold or no fold"
);
});
}
/// A fold means "I am done with this repo for now", never "hide the tab I
/// am looking at". Without this, ⌘T inside a folded group — the new tab
/// inherits the group it was spawned from — draws nothing at all.
#[gpui::test]
fn the_active_row_stays_on_screen_inside_a_folded_group(cx: &mut TestAppContext) {
let (app, mut vcx, _streams) = harness_with_tabs(cx, 2);
let alpha = PathBuf::from("/w/alpha");
app.update(&mut vcx, |app, cx| {
for i in 0..2 {
*app.tabs[i].sidebar_group.borrow_mut() = Some(alpha.clone());
}
app.active = 0;
app.toggle_sidebar_group(&Some(alpha.clone()), cx);
});
vcx.run_until_parked();
app.update(&mut vcx, |app, _| {
assert!(drawn(app, 0), "the active row survives its group folding");
assert!(!drawn(app, 1), "everything else in the group is gone");
});
// And it follows the active tab, rather than being decided once when
// the fold happened.
app.update(&mut vcx, |app, cx| {
app.active = 1;
cx.notify();
});
vcx.run_until_parked();
app.update(&mut vcx, |app, _| {
assert!(drawn(app, 1), "the row that is active now is the one drawn");
assert!(!drawn(app, 0), "and the one that no longer is went away");
});
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -1499,6 +1709,16 @@ mod tests {
PathBuf::from(s)
}
#[test]
fn the_scratch_group_folds_under_a_key_no_repo_can_take() {
assert_eq!(collapse_key(&Some(p("/w/repo"))), "/w/repo");
assert_eq!(
collapse_key(&None),
"",
"scratch has no root, so it is stored as the name that is not one"
);
}
#[test]
fn diff_preview_setting_gates_the_click_target() {
let mut cfg = Config::default();