From 928e76154fee9c7de876123770341e11c30df236 Mon Sep 17 00:00:00 2001 From: Simon Franken Date: Thu, 22 Oct 2026 12:12:11 +0200 Subject: [PATCH] Add problems-only view (!) and draft the write actions `!` reduces the tree to submodules with issues (or unknown state) plus their ancestors, reusing the filter's reduction. Adds docs/actions.md as a draft spec for restore and stage pin. Co-Authored-By: Claude Sonnet 5.5 --- README.md | 5 +- docs/README.md | 8 +- docs/actions.md | 262 +++++++++++++++++++++++++++++++++ docs/decisions.md | 14 ++ docs/requirements.md | 10 +- docs/ux-guide.md | 28 +++- internal/tui/app.go | 26 +++- internal/tui/tree/filter.go | 56 ++++++- internal/tui/tree/tree.go | 20 ++- internal/tui/tree/tree_test.go | 80 ++++++++++ 10 files changed, 484 insertions(+), 25 deletions(-) create mode 100644 docs/actions.md diff --git a/README.md b/README.md index 67adbf2..aba8a5e 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,7 @@ files. | `space` | Toggle the selected submodule | | `home` `end` / `g` `G`, `pgup` `pgdn` | Jump, page | | `/` | Filter | +| `!` | Problems only | | `c` | Copy path | | `f` / `r` | Fetch / reload | | `?` | Full help | @@ -118,8 +119,8 @@ The mouse works too: click to select, click `▸` to toggle, wheel to move. ## Status -A prototype. Browsing is complete; the actions `u` (check out the pin), -`b` (bump the pin to the tip) and `!` (problems only) are planned. +A prototype. Browsing is complete; the actions `u` (check out the pin) and `b` (bump the pin to the tip) are +planned. ## Documentation diff --git a/docs/README.md b/docs/README.md index cf9af88..ce08103 100644 --- a/docs/README.md +++ b/docs/README.md @@ -14,6 +14,7 @@ bare commit hash. | [state-model.md](state-model.md) | How a submodule's state is determined, summarized into issues, and how pins are resolved to refs | | [ux-guide.md](ux-guide.md) | Layout, attention hierarchy, colors, wording, navigation and key bindings | | [architecture.md](architecture.md) | Package layout, the provider boundary, async loading, testing | +| [actions.md](actions.md) | **Draft:** the write actions restore and stage pin | | [decisions.md](decisions.md) | Decision log with rationale (ADR style) | ## Install @@ -32,10 +33,11 @@ Installs `submarine` and the alias `sub` (linux/macOS, amd64/arm64) to through a git provider (`internal/submodule/git`): `submarine [dir]`. It fetches in the background on startup (`--no-fetch` to skip; `f` to fetch again) and follows changes made in other shells. `submarine --mock` shows -the built-in mock data (`internal/submodule/mock`). The actions `u`, `b`, -`!` only show a "not implemented" message. +the built-in mock data (`internal/submodule/mock`). `!` shows only +submodules with problems. The actions `u` and `b` only show a "not +implemented" message ([drafts of the write actions](actions.md)). Next steps, roughly in order: 1. Actions: checkout pin (`submodule update`), bump pin to tip, init. -2. "Problems only" (`!`) view; it can reuse the tree's filter. +2. (done) "Problems only" (`!`) view, reusing the tree's filter. diff --git a/docs/actions.md b/docs/actions.md new file mode 100644 index 0000000..d2364d8 --- /dev/null +++ b/docs/actions.md @@ -0,0 +1,262 @@ +# Actions (draft) + +**Status: draft, not implemented.** The spec for the first two write actions. +Open questions are listed [at the end](#open-questions). Once they are +answered, the decisions go into [decisions.md](decisions.md), and +[requirements.md](requirements.md) and [ux-guide.md](ux-guide.md) are +updated. + +## Principle + +Offer as few actions as possible, but enough to work with submodules. +submarine shows what differs from what git records. The actions resolve such +a difference in one of two directions: + +| Action | Direction | Key (proposed) | +|---|---|---| +| [Restore](#restore) | The worktree follows the recorded state | `u` | +| [Stage pin](#stage-pin) | The recorded state follows the worktree | `s` | + +There is nothing else for now. Committing, pushing, switching branches and +bumping a pin to the tip are done outside submarine. A bump is "check out +the tip in the submodule, then `s`". + +## Restore + +Makes the selected repository, and everything below it, exactly what git +records. It works on any submodule and on the superproject (header +selected). Restoring the superproject gives the exact project that the +current root commit specifies. + +### Target + +| Selected | Target commit | Branch | +|---|---|---| +| Superproject | `HEAD` | stays on the current branch | +| Submodule | The pin in the parent's **index** (what the UI shows as *Pinned*) | see [Branches](#branches) | +| Nested submodule, during the recursion | The gitlink in the restored parent's commit | as for a submodule | + +Restoring the superproject resets its index too, so **staged pins are +discarded**: afterwards every pin is the one in `HEAD`. Restoring a submodule +does not touch the parent's index, so a staged pin of that submodule stays +and is the target. (See open question 2.) + +### Postconditions + +After a successful restore of repository R with target T: + +1. **R's HEAD is T.** The index and the worktree equal T's tree. No + modified, added, removed or conflicted files remain. +2. **No in-progress operation** remains in R: merge, rebase, cherry-pick, + revert. +3. **No untracked files or directories** remain in R. This includes leftover + directories of submodules that T no longer has (*untracked repository*). + Ignored files are kept (see open question 1). +4. **Every submodule that T records is present and initialized**, even if + it was never initialized, is inactive (`submodule..active`) or has + `update = none / merge / rebase` configured. Its URL comes from T's + `.gitmodules` (synced into the config). +5. **Each of those submodules is restored the same way**, with its gitlink + in T as its target, recursively at any depth. + +So the subtree shows `in sync` everywhere, except for states restore does +not change: `unpushed`, behind/ahead of the ref, and `detached HEAD on pin`. + +The recursion follows **T**, not the tree loaded in the UI. A submodule that +is only in T (for example, after switching branches) is initialized. A +submodule that is only on disk is deleted as an untracked directory. + +### Branches + +- **Superproject:** stays on its branch. The branch is not moved, since the + target is its own `HEAD`. +- **Submodule on the pin:** if HEAD is a branch that points at the target, + the branch stays checked out. +- **Submodule off the pin:** HEAD is detached at the target. The branch that + was checked out is **not moved**, so its commits stay on it. + +Restore never moves, creates or deletes a branch. + +### What can be lost + +Restore discards work. The things it can lose: + +| Lost | Where it comes from | +|---|---| +| Changed files | modified, added, removed, renamed files (`changed`) | +| Untracked files | `changed` (untracked) | +| Conflicts and in-progress operations | `conflict`; merge, rebase, cherry-pick, revert state | +| Commits | A detached HEAD off the pin whose commits are on no branch, tag or remote ref (only the reflog still has them) | +| Repositories | An untracked directory that is a repository with its **own** `.git` directory (a clone that was never added as a submodule). Its commits may exist nowhere else. A removed submodule whose git directory is absorbed into `.git/modules` loses only its worktree. Its objects stay | + +Nothing else is lost. Branches, stashes, remote refs, ignored files and +absorbed submodule git directories stay. + +### Confirmation + +- **Nothing would be lost** (only `off pin`, `missing`, a staged pin of the + root): restore runs at once, without a question. +- **Something would be lost:** a confirmation line opens above the footer, + like the filter line. It says what is discarded, summed over the subtree: + + ``` + Restore src/app? Discards 7 changed files in 3 submodules, 2 detached commits, 1 repository. y restore · n cancel + ``` + + `y` runs it. `n`, `esc` or any other key cancels. +- **The state is not fully known** (a status in the subtree is still + loading or failed to load): the line says so (`… · 2 submodules not + loaded`) and needs the same `y`. Restore does not wait for the loading. +- Lost repositories and commits are shown in the Danger color, files in + Warn. + +The summary only covers the submodules that are loaded. Children of a +submodule that is not initialized are unknown. They are created fresh, so +nothing of them can be lost. + +### Network + +A restore may need objects that are not on disk: + +- a submodule that was never cloned (`not initialized`, no git directory in + `.git/modules`); +- a target commit that was never fetched. + +Proposal: **restore may clone and fetch, but only for objects that are +missing**, and never prompts (`GIT_TERMINAL_PROMPT=0`, as for `Fetch`). +This changes the rule "the provider uses the network only in `Fetch`" +(AGENTS.md, N8). The alternative is to fail with "pinned commit … not +fetched" and leave cloning to the user (open question 3). + +### Running, progress and errors + +- The selected row (or the header) shows a spinner and `restoring` in the + status column until the restore ends. The header shows `restoring src/app` + where the fetch progress usually is. +- Only one write action runs at a time. Pressing `u` or `s` meanwhile + flashes `wait for the restore of src/app to finish`. Navigation and + everything else keep working. +- A running background fetch does not block a restore. +- Done: flash `restored src/app`. The watcher refreshes the affected rows. + The app also reloads the subtree, because the structure can have changed. +- Failure: the restore stops at the first repository that fails. The footer + flashes `restore failed in src/app/lib: `, and the detail + pane of the selected node shows the same message until the next restore. + Restore is idempotent, so it can be run again after fixing the cause. + +### Implementation note + +For each repository, top-down (`T` = target): + +```sh +git merge --abort / rebase --abort / cherry-pick --abort / revert --abort # whichever is in progress +git checkout --detach --force T # submodule off the pin; superproject: git reset --hard HEAD +git reset --hard T +git clean -ffd # -ff: also untracked nested repositories +git submodule sync +git submodule init # all submodules of T, including inactive ones +# for each gitlink in T: make sure the repo exists (clone/absorb), then recurse +``` + +The repository can be walked by submarine itself, not by `git submodule +update --recursive`, so progress, the loss check and errors are per +repository. `git submodule update --init --checkout --force` is an option for +the clone step. The loss check runs right before the restore, against the +current state, not the displayed one. + +## Stage pin + +Records the commit checked out in a submodule as its new pin in the +parent's index. It is the same as `git add ` in the parent, but only +when it is safe. + +### Available when + +All of: + +- the selected node is a submodule, not the superproject; +- it is `off pin` (`Checkout == CheckoutDiffers`): HEAD differs from the + pin in the parent's index; +- its worktree is **clean** (`Worktree == WorktreeClean`). Uncommitted + changes, untracked files and conflicts count as dirty, the same rule as + the `changed` issue. A moved nested pin counts as a change too, so nested + submodules are staged bottom-up: first inside the submodule, then the + submodule itself (after a commit in it, outside submarine). + +Changes *inside* nested submodules don't count. Like the `changed` issue, +staging only looks at the submodule's own worktree. + +### Effect + +- The parent's index records the submodule's HEAD as the gitlink of its + path. Nothing else changes: no worktree, no other index entry, no commit. +- The submodule then shows `staged` (or `●` next to an issue), and the + `off pin` issue is gone. +- If the checkout equals the pin in the parent's `HEAD`, staging removes + the staged pin instead (the index equals `HEAD` again). +- If the checked-out commit is not pushed, it is staged anyway. The row + then shows `unpushed` (Danger), which is the warning. +- There is no confirmation: nothing is lost. +- Undo: there is no unstage action. `git restore --staged ` in the + parent undoes it. Restoring the superproject also discards every staged + pin, together with everything else. + +### Implementation note + +```sh +# in the submodule +git rev-parse HEAD # must still be the SHA shown in the UI, else abort +# in the parent +git update-index --cacheinfo 160000,, +``` + +`update-index` stages exactly the SHA that was checked, independent of the +worktree. Flash: `staged pin of src/app` on success; git's message on +failure. + +## Keys and palette + +Both are submodule keys (`keyMap.Submodule`), shown in the detail pane's +palette, or in the footer, **only while they apply** to the selection: + +| Key | Shown when | Pressed when not applicable (flash) | +|---|---|---| +| `u` restore | The node or a loaded descendant has an issue restore fixes (`changed`, `conflict`, `missing`, `off pin`), or the root has a staged pin | `nothing to restore in src/app` | +| `s` stage pin | [Available](#available-when) | `src/app is on its pin` / `src/app has uncommitted changes; commit or restore them first` / `the superproject has no pin` | + +Full help (`?`) always lists both. + +## Testing + +- Provider: tests on fixtures from `scripts/make-fixture.sh`, which builds + every state. For restore: changed, untracked, untracked repository + (absorbed and standalone), conflict, rebase in progress, off pin on a + branch, off pin detached with unreachable commits, not initialized, + inactive, `update = none`, a submodule added and one removed between two + root commits, nested at depth ≥ 2. Assert the postconditions with + `git status --porcelain --ignored=no` and `git submodule status + --recursive`, and that branches are unchanged. For stage pin: index entry, + refusal on a dirty worktree, refusal when HEAD moved. +- Loss summary: unit tests on `submodule` (no UI), like `Status.Issues()`. +- UI: run it on a fixture in tmux (see + [architecture.md](architecture.md#testing)). Check the confirmation line, + the spinner, the flash messages and the palette. + +## Open questions + +1. **Ignored files.** Should restore also delete ignored files (`git clean + -x`)? That is the literal "no additional files", but it deletes `.env`, + `node_modules`, build output and IDE settings in every submodule. + *Proposal: keep ignored files.* +2. **Target of a submodule restore with a staged pin.** The staged pin + (index, what the UI shows as *Pinned*, what `git submodule update` does) + or the committed one (parent's `HEAD`, which also unstages)? + *Proposal: the index. Unstaging happens only through a superproject + restore.* +3. **Network.** May restore clone uninitialized submodules and fetch missing + pins (non-interactive), or should it fail and ask for `f` / a manual + clone? *Proposal: allow it, only for missing objects.* +4. **Keys.** `u` restore, `s` stage pin? `R` would sit next to `r` reload, + which is a dangerous mix-up for a destructive action. +5. **Confirmation without loss.** Is "no question when nothing is lost" OK, + or should restore always ask? diff --git a/docs/decisions.md b/docs/decisions.md index 5da68b9..2ef781a 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -282,3 +282,17 @@ problem is visible while a submodule is selected. Without an upstream the usual resolution applies. - The root is not in `Walk`, so counts, fetch and loading logic stay submodule-only; the app tracks it by the path `""`. + +## Problems only reuses the filter's tree reduction + +`!` shows only submodules with issues. Instead of a second view, the tree's +filter got a second criterion: a submodule matches if its path contains the +term *and*, with problems only, it has an issue. Ancestors of matches stay as +context, expanded, so a danger deep in the tree is reachable without +unfolding anything. Unlike the filter text it is a mode that outlives the +filter line, so it has its own notice line above the footer: a short list +must not look like a healthy repository (D4). Loading and failed submodules +count as problems because their state is unknown (D7 counts `error` as danger +too); the cost is that rows vanish as statuses arrive, so the selection is +refitted after each status. Staged pins are not shown by it, since they are +not issues. diff --git a/docs/requirements.md b/docs/requirements.md index e74293e..9015387 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -120,13 +120,21 @@ commit hashes. It doesn't answer the questions users actually have: submodule (none if nothing matches); closing the filter keeps the selection (the superproject if none is left). See [Filter](ux-guide.md#filter). +- **F55** `!` toggles "problems only": the tree shows only submodules with + an issue (**F10**) or an unknown state (loading, `error`), plus their + ancestors, expanded. It combines with the filter (**F54**) and stays when + the filter closes. A line above the footer says `problems only`, so a short + list is not mistaken for a healthy repository. See + [Problems only](ux-guide.md#problems-only). + ### Actions (planned, not implemented) | Key | Action | |---|---| | `u` | Check out the pinned commit (`git submodule update`) | | `b` | Bump the pin to the tip of its ref | -| `!` | Show only submodules with issues | + +See [actions.md](actions.md) for the draft of the write actions. ## Usage diff --git a/docs/ux-guide.md b/docs/ux-guide.md index 40fac47..576d48b 100644 --- a/docs/ux-guide.md +++ b/docs/ux-guide.md @@ -220,7 +220,8 @@ colors in components. | `r` | Reload everything (e.g. after editing files) | | `c` | Copy the absolute directory of the selected submodule to the clipboard (OSC 52, so it also works over SSH; needs terminal support, in tmux `set -g set-clipboard on`). The footer confirms with `copied `. Without a local provider (mock) the path relative to the superproject is copied | | `/` | Open the filter, see [Filter](#filter) | -| `u`, `b`, `!` | Planned actions (see requirements) | +| `!` | Toggle problems only, see [Problems only](#problems-only) | +| `u`, `b` | Planned actions (see requirements) | "First child" always means the first child **as displayed** (children are grouped by directory). @@ -250,6 +251,29 @@ match vanish. If nothing matches the tree says `no submodule matches`. - The filter line shows the keys that apply on its right. - The header counts are not filtered. +### Problems only + +`!` reduces the tree to what needs attention. It uses the filter's rules +(matches with their ancestors, shown expanded, see [Filter](#filter)), with +"match" meaning *has an issue* (`Status.Issues()`) instead of *path contains +the term*. + +- A submodule that is still loading or failed to load (`error`) counts as a + problem, since its state is unknown (as in the hidden counts, D7). Healthy + submodules vanish when their status arrives. +- A staged pin is not an issue and does not show the row. +- It combines with the filter (both must match). Unlike the filter it is a + mode, not an input: it stays until `!` is pressed again, also after `esc` + closed the filter. +- A line above the footer shows `problems only` (yellow) and `! show all`. + While the filter line is open, its hint starts with `problems only (!)` + instead. +- With no problem the tree says `no problems`. +- The selection follows the filter's rule: if it stops being visible (also + when a status changes), the next visible submodule is selected, else the + previous one. The superproject stays selectable. +- The header counts are not filtered. + ### Key reference Every key in the UI, where it is shown, and whether it works. Update this @@ -272,7 +296,7 @@ where the palette can't be shown, see [Key bar and palette](#key-bar-and-palette | `f` | Fetch all repositories | Global | Help | yes | | `r` | Reload everything | Global | Help | yes | | `/` | Filter | Global | Help | yes | -| `!` | Problems only | Global | Help | no (flashes "not implemented") | +| `!` | Problems only (toggle) | Global | Help | yes | | `?` | Toggle full help | Global | Bar, Help | yes | | `q`, `ctrl+c` | Quit | Global | Bar, Help | yes | diff --git a/internal/tui/app.go b/internal/tui/app.go index d2603ef..7205cd3 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -199,6 +199,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } else { s.Status, s.Err = msg.status, nil } + m.tree.Refit() } if m.loads.done(msg.path) { return m, m.loadStatus(msg.path) @@ -386,7 +387,10 @@ func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m.filtering = true m.layout() return m, m.filter.Focus() - case key.Matches(msg, k.Update, k.Bump, k.Issues): + case key.Matches(msg, k.Issues): + m.tree.SetProblemsOnly(!m.tree.ProblemsOnly()) + m.layout() + case key.Matches(msg, k.Update, k.Bump): cmd := m.footer.Flash("“" + msg.String() + "” is not implemented in the prototype") m.layout() return m, cmd @@ -438,6 +442,9 @@ func (m Model) filterView() string { if !m.filter.Focused() { hint = "/ edit • esc close" } + if m.tree.ProblemsOnly() { + hint = "problems only (!) • " + hint + } hint = theme.Fg(m.theme.Muted).Render(hint) m.filter.Width = max(m.width-lipgloss.Width(hint)-6, 1) in := m.filter.View() @@ -445,6 +452,19 @@ func (m Model) filterView() string { return lipgloss.NewStyle().Width(m.width).Padding(0, 1).Render(in + strings.Repeat(" ", gap) + hint) } +// extraLine reports whether a line above the footer is shown: the filter, or +// the notice that only problems are shown. +func (m Model) extraLine() bool { return m.filtering || m.tree.ProblemsOnly() } + +// problemsView is the line above the footer while only problems are shown, so +// a short list is not mistaken for a healthy repository. +func (m Model) problemsView() string { + left := theme.Fg(m.theme.Warn).Render("problems only") + right := theme.Fg(m.theme.Muted).Render("! show all") + gap := max(m.width-2-lipgloss.Width(left)-lipgloss.Width(right), 1) + return lipgloss.NewStyle().Width(m.width).Padding(0, 1).Render(left + strings.Repeat(" ", gap) + right) +} + // copyPath copies the absolute directory of the selected submodule to the // clipboard, or its path in the superproject if the provider can't tell. func (m Model) copyPath() (tea.Model, tea.Cmd) { @@ -470,7 +490,7 @@ func (m Model) copyPath() (tea.Model, tea.Cmd) { func (m *Model) layout() { m.footer.SetWidth(m.width) bodyHeight := m.height - headerHeight - lipgloss.Height(m.footer.View()) - if m.filtering { + if m.extraLine() { bodyHeight-- } bodyHeight = max(bodyHeight, 0) @@ -508,6 +528,8 @@ func (m Model) View() string { parts := []string{zone.Mark(m.tree.RootZone(), head), body} if m.filtering { parts = append(parts, m.filterView()) + } else if m.tree.ProblemsOnly() { + parts = append(parts, m.problemsView()) } return zone.Scan(lipgloss.JoinVertical(lipgloss.Left, append(parts, m.footer.View())...)) } diff --git a/internal/tui/tree/filter.go b/internal/tui/tree/filter.go index 929109f..1973fd6 100644 --- a/internal/tui/tree/filter.go +++ b/internal/tui/tree/filter.go @@ -8,7 +8,7 @@ import ( // SetFilter shows only the submodules whose path contains term (ignoring // case), plus their ancestors, which are shown expanded. An empty term -// removes the filter. +// removes the filter. It combines with SetProblemsOnly. // // If the selected submodule is filtered out, the next visible one is // selected (the previous one if there is none after it, none if nothing @@ -21,15 +21,46 @@ func (m *Model) SetFilter(term string) { } old := m.cursor m.filter = term - if term == "" { + m.narrow(old) +} + +// SetProblemsOnly shows only the submodules with issues, plus their +// ancestors, which are shown expanded. A submodule whose status is loading or +// failed to load counts as a problem, since its state is unknown. The +// selection behaves as with SetFilter. +func (m *Model) SetProblemsOnly(on bool) { + if on == m.problemsOnly { + return + } + old := m.cursor + m.problemsOnly = on + m.narrow(old) +} + +// ProblemsOnly reports whether only submodules with issues are shown. +func (m Model) ProblemsOnly() bool { return m.problemsOnly } + +// Refit keeps the selection visible after statuses changed, which can make a +// submodule enter or leave the problems-only view. +func (m *Model) Refit() { + if m.problemsOnly { + m.reselect(m.cursor) + m.clampOffset() + } +} + +// narrow fixes the selection after the filter or the problems-only view +// changed. +func (m *Model) narrow(old *submodule.Submodule) { + if m.narrowed() { + m.reselect(old) + } else { for p := parentOf(old); p != nil; p = p.Parent { m.expanded[p.Path] = true } if m.cursor == nil { m.cursor = m.firstNode() } - } else { - m.reselect(old) } m.clampOffset() } @@ -37,6 +68,9 @@ func (m *Model) SetFilter(term string) { // Filter returns the active filter term in lower case. func (m Model) Filter() string { return m.filter } +// narrowed reports whether the tree is reduced to matches. +func (m Model) narrowed() bool { return m.filter != "" || m.problemsOnly } + func parentOf(s *submodule.Submodule) *submodule.Submodule { if s == nil { return nil @@ -106,10 +140,10 @@ func (m *Model) appendAll(out *[]row, mods []*submodule.Submodule, depth int) { } } -// isExpanded reports whether the children of s are shown. While filtering, +// isExpanded reports whether the children of s are shown. While narrowed, // that is the case if a match is below s, whatever the user expanded. func (m *Model) isExpanded(s *submodule.Submodule) bool { - if m.filter == "" { + if !m.narrowed() { return m.expanded[s.Path] } for _, c := range s.Children { @@ -122,7 +156,7 @@ func (m *Model) isExpanded(s *submodule.Submodule) bool { // filtered returns the submodules of mods that pass the filter. func (m *Model) filtered(mods []*submodule.Submodule) []*submodule.Submodule { - if m.filter == "" { + if !m.narrowed() { return mods } var out []*submodule.Submodule @@ -136,7 +170,7 @@ func (m *Model) filtered(mods []*submodule.Submodule) []*submodule.Submodule { // shown reports whether s matches the filter or has a descendant that does. func (m *Model) shown(s *submodule.Submodule) bool { - if strings.Contains(strings.ToLower(s.Path), m.filter) { + if strings.Contains(strings.ToLower(s.Path), m.filter) && (!m.problemsOnly || hasProblem(s)) { return true } for _, c := range s.Children { @@ -146,3 +180,9 @@ func (m *Model) shown(s *submodule.Submodule) bool { } return false } + +// hasProblem reports whether s has an issue or an unknown state (loading, or +// failed to load, like HiddenIn counts it). +func hasProblem(s *submodule.Submodule) bool { + return s.Status == nil || len(s.Status.Issues()) > 0 +} diff --git a/internal/tui/tree/tree.go b/internal/tui/tree/tree.go index f5d2270..2c66de1 100644 --- a/internal/tui/tree/tree.go +++ b/internal/tui/tree/tree.go @@ -27,11 +27,13 @@ type Model struct { roots []*submodule.Submodule expanded map[string]bool // keyed by Submodule.Path; kept when ancestors collapse filter string // lower-case; empty if not filtering - cursor *submodule.Submodule - offset int // index of the first visible row - width int - height int - spinner string + // problemsOnly hides submodules without issues; it combines with filter. + problemsOnly bool + cursor *submodule.Submodule + offset int // index of the first visible row + width int + height int + spinner string } func New(t theme.Theme) Model { @@ -365,8 +367,12 @@ func (m Model) View() string { lines = append(lines, m.nodeLine(r, nameColW, refW)) } } - if len(rows) == 0 && m.filter != "" { - lines = append(lines, theme.Fg(m.theme.Muted).Width(m.width).Render(" no submodule matches")) + if len(rows) == 0 && m.narrowed() { + none := " no submodule matches" + if m.filter == "" { + none = " no problems" + } + lines = append(lines, theme.Fg(m.theme.Muted).Width(m.width).Render(none)) } for len(lines) < m.height { lines = append(lines, strings.Repeat(" ", m.width)) diff --git a/internal/tui/tree/tree_test.go b/internal/tui/tree/tree_test.go index ac59dde..2ba6fb1 100644 --- a/internal/tui/tree/tree_test.go +++ b/internal/tui/tree/tree_test.go @@ -411,3 +411,83 @@ func TestFilterKeepsRoot(t *testing.T) { t.Error("closing the filter should keep the root") } } + +// healthy gives every submodule of the fixture an issue-free status, then +// marks the given paths as changed. +func healthy(t *testing.T, m *Model, changed ...string) { + t.Helper() + submodule.Walk(m.roots, func(s *submodule.Submodule) { + s.Status = &submodule.Status{Pin: submodule.PinInfo{Pushed: true}} + for _, p := range changed { + if s.Path == p { + s.Status.Worktree = submodule.WorktreeModified + } + } + }) +} + +func TestProblemsOnlyShowsIssuesWithAncestors(t *testing.T) { + m := newTree(t) + healthy(t, m, "d") + m.SetProblemsOnly(true) + want := []string{"src/", "b", "modules/", "c", "d"} + if got := visible(m); !equal(got, want) { + t.Fatalf("rows = %v, want %v", got, want) + } + assertCursor(t, m, "b") // a was selected and is healthy: next available + m.Select("d") + m.SetProblemsOnly(false) + assertCursor(t, m, "d") + if got := visible(m); len(got) != 8 { + t.Fatalf("rows = %v, want all expanded to d", got) + } +} + +func TestProblemsOnlyKeepsUnknownStates(t *testing.T) { + m := newTree(t) + healthy(t, m) + f := m.roots[2] + f.Status, f.Err = nil, os.ErrNotExist // failed to load + m.roots[0].Status = nil // still loading + m.SetProblemsOnly(true) + if got, want := visible(m), []string{"a", "src/", "f"}; !equal(got, want) { + t.Fatalf("rows = %v, want %v", got, want) + } +} + +func TestProblemsOnlyWithoutProblemsKeepsRoot(t *testing.T) { + m := New(theme.Default()) + m.SetRoot(&submodule.Submodule{Status: &submodule.Status{Root: true}}) + m.SetRoots(fixture()) + m.SetSize(80, 20) + healthy(t, &m) + m.SetProblemsOnly(true) + if len(m.rows()) != 0 || !m.RootSelected() { + t.Fatalf("rows %v, root selected %v", visible(&m), m.RootSelected()) + } + m.Down() // must not panic +} + +func TestProblemsOnlyCombinesWithFilter(t *testing.T) { + m := newTree(t) + healthy(t, m, "d", "f") + m.SetProblemsOnly(true) + m.SetFilter("f") + if got, want := visible(m), []string{"src/", "f"}; !equal(got, want) { + t.Fatalf("rows = %v, want %v", got, want) + } + m.SetFilter("") + if !m.ProblemsOnly() || len(visible(m)) != 6 { + t.Fatalf("closing the filter keeps problems only: %v", visible(m)) + } +} + +func TestRefitMovesSelectionWhenProblemDisappears(t *testing.T) { + m := newTree(t) + healthy(t, m, "a", "f") + m.SetProblemsOnly(true) + assertCursor(t, m, "a") + m.roots[0].Status.Worktree = submodule.WorktreeClean + m.Refit() + assertCursor(t, m, "f") +}