From 7a3e37ce768c21fc46efeadec61f042890d317ea Mon Sep 17 00:00:00 2001 From: Hewston Fox Date: Tue, 4 Aug 2026 13:15:22 +0300 Subject: [PATCH] fix: Left/h steps up to the nearest visible ancestor when there's nothing to collapse --- CHANGELOG.md | 1 + docs/keys.md | 2 +- docs/usage.md | 12 +++++--- src/controller/mod.rs | 40 +++++++++++++++++++++----- src/tree.rs | 15 ++++++++++ tests/controller.rs | 67 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 125 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 624715d0..94354a2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to this project are documented here. The format is based on ## [Unreleased] ### Fixed +- `←`/`h` on a file, or on a directory that's already collapsed, no longer no-ops. It now walks up to the nearest visible ancestor directory and collapses that instead, so repeated presses climb the tree one level at a time — including under `compact_dirs`, where it correctly skips past every folded intermediate directory to land on the next real row. → [usage](docs/usage.md#the-tree) · [keys](docs/keys.md) - Agent skill: the launch instructions no longer tell agents to pass `--cwd`. herdr resolves the manifest's relative pane command against it, so the launch failed with `plugin_pane_open_failed` — or worse, inside a built plugin checkout, silently ran that checkout's binary. The skill and the `docs/usage.md` snippet now explain that the viewed root follows the *focused herdr pane's* directory, so an agent's own `cd` does not move it. Thanks @AntonyKor (#139) → [agent skill](skills/herdr-file-viewer/SKILL.md) · [usage](docs/usage.md#teach-your-agent) ## [1.15.0] - 2026-08-03 diff --git a/docs/keys.md b/docs/keys.md index a80548f8..49a43d81 100644 --- a/docs/keys.md +++ b/docs/keys.md @@ -14,7 +14,7 @@ is additive and on by default. | `↑` / `k`, `↓` / `j` | Move the tree cursor, or **scroll the content pane** vertically when it is focused | | `Space` / `PageDown`, `PageUp` | Move **one screenful** — page the content pane when it is focused, otherwise jump the tree cursor a page. The step is the focused pane's live height, so it follows a resize and stays a screenful in the narrow single-column layout. `Space` for page-down matches the pager convention (`less`, `more`, `man`, and so `bat`); `less`'s page-up `b` is unavailable here because it is `toggle_baseline`, so remap `page_up` if you want it | | `→` / `l` | Expand the selected directory, or **scroll the content pane right** when it is focused | -| `←` / `h` | Collapse the selected directory, or **scroll the content pane left** when it is focused | +| `←` / `h` | Collapse the selected directory. On a **file**, or a directory that's **already collapsed**, walks up to and collapses the nearest visible ancestor directory instead, so repeated presses climb the tree one level at a time — or **scroll the content pane left** when it is focused | | `H` (Shift+`h`) | Scroll the **tree** pane left (long / deeply-nested rows), inert unless the tree is focused | | `L` (Shift+`l`) | Focus-gated: with the **tree** focused, scroll it right (long / deeply-nested rows); with the **content pane** focused (or zoomed), enter **line-select mode** to select lines and copy either a `file:line` reference or the content itself (see [below](#copy-a-line-reference-or-line-content-l)) | | _line-select mode_ | `j`/`k` (or `↑`/`↓`) move the marker, `Shift`+move (`J`/`K`, Shift+`↑`/`↓`) extends a line selection; **click-drag** with the mouse selects **text** (character-granular); `a` adds an annotation for the selected line/range, `Enter` copies the `path:line` / `path:start-end` **reference**, `y`/`Y` copies the selected **content**, `Esc` exits | diff --git a/docs/usage.md b/docs/usage.md index 1015d7f1..939f366e 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -30,10 +30,14 @@ setting off unless you need it; `.git/` itself always stays hidden. The tree's * looking. Move the cursor with `↑`/`↓` (or `k`/`j`), expand/collapse a directory with `→`/`←` (or `l`/`h`) or -`Enter`. The tree scrolls to keep the selection in view, and sideways for long or deeply-nested -names — reachable by keyboard with `H` / `L` when the tree is focused. A scrollbar appears whenever -there's more than fits. Narrow or widen the tree column with `<` / `>`, or drag the divider; the -starting split, the tree's side, and a column cap are all [configurable](configuration.md). +`Enter`. `←` on a file, or on a directory that's already collapsed, has nothing left to collapse +there — it walks the cursor up to the nearest visible ancestor directory and collapses that instead, +so repeated presses climb the tree one level at a time; under `compact_dirs` this correctly skips +past every folded intermediate directory to land on the next real row. The tree scrolls to keep the +selection in view, and sideways for long or deeply-nested names — reachable by keyboard with `H` / +`L` when the tree is focused. A scrollbar appears whenever there's more than fits. Narrow or widen +the tree column with `<` / `>`, or drag the divider; the starting split, the tree's side, and a +column cap are all [configurable](configuration.md). On a **deeply nested** layout the per-segment tree spends most of a narrow column on indentation, and the file names — the part you came for — are what gets truncated. Set diff --git a/src/controller/mod.rs b/src/controller/mod.rs index 22a1befe..d308c62c 100644 --- a/src/controller/mod.rs +++ b/src/controller/mod.rs @@ -2191,18 +2191,44 @@ impl Controller { } /// Left (←/h): collapse the selected directory when the tree is focused, or scroll the - /// content pane left when it is focused. + /// content pane left when it is focused. When the selection has nothing left to collapse — + /// a **file**, or a directory that's **already collapsed** — this steps up to its nearest + /// *visible* ancestor directory instead, collapses that, and moves the selection onto it — + /// mirroring how collapsing an expanded directory leaves it selected — so Left never strands + /// the cursor on a row that just vanished, and repeated presses walk up the tree one visible + /// level at a time. "Nearest visible" walks the real filesystem ancestry rather than stopping + /// at one level: under `compact_dirs`, a folded chain (`src/main/java`) is a SINGLE row keyed + /// on its deepest directory, so the immediate filesystem parent (`src/main`) has no row of its + /// own — the walk has to keep climbing past every folded intermediate to reach the row that + /// actually represents the next directory up. A no-op once there's no parent row left to + /// collapse onto (the selection is directly under the tree root, which has no visible row of + /// its own). fn collapse(&mut self) -> Effects { if self.focus == Focus::Content { return self.scroll_content_h(-(HSCROLL_STEP as i32)); } - if let Some(node) = self.tree.selected() - && node.kind == NodeKind::Dir - { - self.tree.collapse(&node.path); - return Effects::redraw(); + let Some(node) = self.tree.selected() else { + return Effects::noop(); + }; + match node.kind { + NodeKind::Dir if node.expanded => { + self.tree.collapse(&node.path); + Effects::redraw() + } + NodeKind::Dir | NodeKind::File => { + let mut cur = node.path.as_path(); + loop { + let Some(parent) = cur.parent() else { + return Effects::noop(); + }; + if self.tree.select(parent) { + self.tree.collapse(parent); + return Effects::redraw(); + } + cur = parent; + } + } } - Effects::noop() } /// Activate the selected node (Enter / double-click): a directory toggles expand/collapse; diff --git a/src/tree.rs b/src/tree.rs index 60977d12..04e1652b 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -784,6 +784,21 @@ impl TreeModel { None } + /// Move the cursor to `path`'s visible row, if it currently has one. Unlike [`reveal`], this + /// never expands ancestors or relaxes filters — it only repositions among what's already + /// shown, which is exactly what's needed right after collapsing an ancestor (the ancestor + /// itself stays visible; only its children disappear). Returns `false`, leaving the cursor + /// untouched, when `path` has no visible row (e.g. it's the tree root, which is never a row). + pub fn select(&mut self, path: &Path) -> bool { + match self.visible_nodes().iter().position(|n| n.path == path) { + Some(idx) => { + self.cursor = idx; + true + } + None => false, + } + } + /// Keep the cursor within the (possibly shrunken) visible list after a structural or /// filter change, so indexing by `cursor` can never run past the end. fn clamp_cursor(&mut self) { diff --git a/tests/controller.rs b/tests/controller.rs index f0fab0e2..17def976 100644 --- a/tests/controller.rs +++ b/tests/controller.rs @@ -4106,6 +4106,73 @@ fn view_state_titles_the_tree_with_root_basename_and_branch() { assert!(vs.branch.is_none(), "branch is None outside a git repo"); } +#[test] +fn collapse_on_an_already_collapsed_dir_walks_up_to_the_parent() { + let dir = TempDir::new(); + std::fs::create_dir_all(dir.path().join("a/b")).unwrap(); + std::fs::write(dir.path().join("a/b/file.txt"), "x").unwrap(); + let (mut ctrl, _, _) = controller(dir.path(), false, StubGit::default(), false); + + ctrl.handle(Intent::Expand); // expand "a" — cursor stays on "a" + ctrl.handle(Intent::NavDown); // select "b" (collapsed) + assert_eq!(ctrl.tree().selected().unwrap().path, dir.path().join("a/b")); + + // "b" has nothing left to collapse (it's already collapsed), so Left steps up to "a" + // and collapses that instead of no-op'ing. + let fx = ctrl.handle(Intent::Collapse); + assert!(fx.redraw, "walking up to the parent redraws"); + assert_eq!( + ctrl.tree().selected().unwrap().path, + dir.path().join("a"), + "selection moves onto the parent" + ); + assert!( + !ctrl.tree().selected().unwrap().expanded, + "the parent is collapsed as part of the same keypress" + ); +} + +#[test] +fn collapse_walk_up_climbs_past_every_row_a_compacted_chain_folds_away() { + // `compact_dirs` draws a run of single-child directories as ONE row keyed on the deepest + // directory, so the immediate filesystem parent of that row has no row of its own. The + // walk-up in `collapse()` has to keep climbing — not stop after one `parent()` hop — to + // reach "mid", the nearest directory that is actually a row. + let dir = TempDir::new(); + let deep = dir.path().join("mid/chain/main/java"); + std::fs::create_dir_all(&deep).unwrap(); + std::fs::write(deep.join("App.java"), "x").unwrap(); + // "mid" also holds a file, so — unlike "chain/main/java" below it — "mid" itself is never + // folded into a chain and keeps its own row. + std::fs::write(dir.path().join("mid/other.txt"), "x").unwrap(); + + let (mut ctrl, _, _) = controller(dir.path(), false, StubGit::default(), false); + ctrl.apply_compact_dirs(true); + + ctrl.handle(Intent::Expand); // expand "mid" — cursor stays on "mid" + ctrl.handle(Intent::NavDown); // select the folded "chain/main/java" row (collapsed) + let selected = ctrl.tree().selected().unwrap(); + assert_eq!(selected.path, dir.path().join("mid/chain/main/java")); + assert_eq!(selected.label.as_deref(), Some("chain/main/java")); + + // Left has to climb past "main" and "chain" (neither has a row of its own — both are + // folded into the "chain/main/java" row) to land on "mid". + let fx = ctrl.handle(Intent::Collapse); + assert!( + fx.redraw, + "the walk-up must not silently no-op just because the first parent() hop has no row" + ); + assert_eq!( + ctrl.tree().selected().unwrap().path, + dir.path().join("mid"), + "selection lands on the nearest ancestor that is actually a visible row" + ); + assert!( + !ctrl.tree().selected().unwrap().expanded, + "mid is collapsed" + ); +} + #[test] fn refresh_updates_the_cached_branch_after_an_external_checkout() { // the tree's bottom-border branch is cached on the controller, so it