diff --git a/crates/tty7-core/src/core/config.rs b/crates/tty7-core/src/core/config.rs index 457ae18e..17f9e532 100644 --- a/crates/tty7-core/src/core/config.rs +++ b/crates/tty7-core/src/core/config.rs @@ -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, #[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, diff --git a/src/ui/diff_overlay.rs b/src/ui/diff_overlay.rs index 4a3e595c..61e746e7 100644 --- a/src/ui/diff_overlay.rs +++ b/src/ui/diff_overlay.rs @@ -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)), ), ), ) diff --git a/src/ui/path_display.rs b/src/ui/path_display.rs index 5a76cf6b..99ffb1e9 100644 --- a/src/ui/path_display.rs +++ b/src/ui/path_display.rs @@ -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") + ); + } } diff --git a/src/ui/right_panel.rs b/src/ui/right_panel.rs index 4b3b5462..bf77398f 100644 --- a/src/ui/right_panel.rs +++ b/src/ui/right_panel.rs @@ -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, 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") - ); - } } diff --git a/src/ui/tab_sidebar.rs b/src/ui/tab_sidebar.rs index 53b6231a..173b0c19 100644 --- a/src/ui/tab_sidebar.rs +++ b/src/ui/tab_sidebar.rs @@ -164,6 +164,13 @@ impl Tty7App { let keys: Rc>> = 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 = match query.is_empty() { + true => cx.global::().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 = §ions[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>> = 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 = 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 = visible.clone(); let row_slots: Rc>>> = 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>>> = rows.into_iter().map(Some).collect(); let rows: Vec = 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, cx: &mut Context) { + 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> { let grouping = cx.global::().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) -> String { + key.as_ref() + .map(|p| p.to_string_lossy().into_owned()) + .unwrap_or_default() +} + #[derive(Debug, PartialEq)] struct Section { key: Option, @@ -1491,6 +1559,148 @@ pub(crate) fn diff_click_cwd(cfg: &Config, target: Option) -> Option { 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::().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::().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();