diff --git a/README.md b/README.md index 4c25a4d..bb7f2a0 100644 --- a/README.md +++ b/README.md @@ -23,10 +23,11 @@ the questions you actually have: issue word). A collapsed parent counts the problems hidden below it (`·1`) and shows `●` if a staged pin is hidden. - **Words, not symbols.** Every state is a short word (`unpushed`, `conflict`, - `missing`, `off pin`, `changed`), so nothing depends on color alone. + `missing`, `off pin`, `changed`, `off target`), so nothing depends on color + alone. - **Live.** Follows changes you make in other shells within about two - seconds. It never fetches on its own: the header shows when you last did, - `f` fetches everything. + seconds. It never fetches on its own: the detail pane shows when a + repository was last fetched, `f` fetches everything. - **Fix it from the same screen.** Restore a repository to exactly what git records (`x`), or switch it to another branch (`b`). Both ask first if work would be lost. @@ -58,7 +59,8 @@ sub --mock # built-in sample data, no repository needed ### Find what needs attention Problems stand out: `unpushed` in red, `off pin` and `changed` in yellow, -everything else muted. Select a submodule to see the details on the right: +`off target` (the pin is not on the branch it should be, usually `main`) in +blue, everything else muted. Select a submodule to see the details on the right: every issue in words, the resolved ref and why, where the pin, the tip and your checkout are in the history, and a preview of uncommitted changes. diff --git a/docs/decisions.md b/docs/decisions.md index 88a8099..215894a 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -220,7 +220,7 @@ shown in a palette in the detail pane, next to what they act on; global keys instead, so small terminals need no extra UI (no overlay or action menu). Keys never depend on what is visible. -## Pin resolves to the closest branch +## Pin resolves to the closest branch (superseded by D26) A pin on a pushed feature branch showed as diverged from `origin/main`, because `branch = main` in `.gitmodules` won before any distance was looked @@ -424,3 +424,60 @@ happened is what decides between a merge and a rebase. Cost: one Only one fork is drawn; with two diverged commits (rare) the old line with the diverged gap remains, since nodes on three lanes would need the merge bases between all of them. + +## D26 — The ref is the target branch; a pin off it is `off target` + +**Context.** The main use is checking that a superproject points at the right +commits of its submodules, ideally the tips of `main`. D1's "closest branch" +(see "Pin resolves to the closest branch") answered *where the pin is*, not +*where it should be*: a pin on a feature branch showed that branch, `in sync` +and quiet, because `unpushed` (D6) is satisfied by any remote branch. The +closest branch also changed with the branch checked out in the submodule, so +two machines could show different refs for the same superproject commit. +**Decision.** Supersedes "Pin resolves to the closest branch". The ref is the +target: the `branch =` of `.gitmodules` (no fallback to the default branch if +it does not exist locally), else the remote's default branch (`HEAD`, `main`, +`master`). Without a target the checked-out branch's upstream, a tag or the +SHA are shown for orientation, but cannot make a pin off target. A pushed pin +with commits its target lacks (`Ahead > 0`) is `off target`: a new issue with +the new level `LevelInfo` (blue), listed last. The other branches that contain +the pin are named in its text. +**Alternatives.** Severity by the superproject's branch (strict on `main`, +quiet elsewhere): rejected, the severity should not depend on the branch. +Matching the superproject's branch name in submodules: rejected, feature +branches need not be named alike. Warn level: not now; behind the target is +quiet and `off target` is information, since a feature branch of the +superproject legitimately pins feature commits. +**Consequences.** `RefFromContains` and the 16-branch scan are gone; +`Pin.Branches` replaces them. `off target` is counted in the header and below +collapsed rows (blue `·k`) and is part of "problems only". The issue word +needs 10 columns, so the detail pane's label column is 11 wide. + +**Amendment (with D26): target and ref are separate.** Checking a real +repository, a stale `branch = marco-dev` in `.gitmodules` made every pin +"↑52↓12 from marco-dev" although the pin was on `main`, one commit behind. The +target (`PinInfo.Target`) now only decides `off target`; the ref shown is the +target if the pin is on it, else the branch containing the pin that is closest +to it, with its real distance. The closest branch is the one with the fewest +commits after the pin; ties go to the upstream of the branch the submodule is +on, then alphabetically. The default branch is no favorite: if +`.gitmodules` targets another branch, `main` is just another branch. The +tie-break must be fixed, or the same superproject commit would show +different branches on different machines. The text of the issue names both +(`pin not on origin/marco-dev, on origin/fix/x +1`), so a stale declaration +is visible. `RefFromContains` is back; `RefSource.IsTarget` is gone. + +**Amendment (with D26): the target on the spine.** An `off target` row only +said that the pin is elsewhere, not how far from the target. The target's tip +is now a node (`target`) next to the pin, the tip of the shown ref and the +checkout, so a feature pin reads as a distance from main. The shown ref +contains the pin, so only the target can fork from it, and the fork code +needs no new case. Cost: `log -1`, one `rev-list --left-right --count` and, +if diverged, one `merge-base` per off-target submodule. + +**Amendment (with D26): labels.** The target's tip is labelled with its short +branch name (`marco-dev`) instead of the word `target`: it is usually short, +and the name is what people know. Long names are cut at 16 columns, so the +label column cannot grow without bound. In a label every role word has its +own style, and `you` is bold in the default color: it was lost between the +muted words, and it is the one the user looks for. diff --git a/docs/requirements.md b/docs/requirements.md index 37a3cfc..68219ff 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -48,8 +48,14 @@ commit hashes. It doesn't answer the questions users actually have: muted `in sync`, or a blue `staged` if their pin change is staged. With issues, a staged pin adds a blue `●` after the word. See [state-model.md](state-model.md) for the full rules. -- **F11** Each submodule shows its pin as a ref (`origin/main`, a tag, or a - short SHA as a fallback) and the distance to that ref (`↑n`, `↓n`, `↑n↓m`). +- **F11** Each submodule shows its pin as a ref and the distance to it + (`↑n`, `↓n`, `↑n↓m`). The ref is the **target** branch (`branch =` of + `.gitmodules`, else the remote's default branch) if the pin is on it, else + the branch the pin is on that it is closest to; only without any a tag or + a short SHA. +- **F11a** A pin that is pushed but not on its target branch (for example + only on a feature branch) is `off target`, an information (blue) naming + the branch it is on. It does not depend on the superproject's branch. - **F12** A pin that is not reachable from any remote ref ("unpushed") is the most prominent state. - **F13** States load asynchronously per submodule; a submodule shows a diff --git a/docs/state-model.md b/docs/state-model.md index f6ee0c5..9c6647c 100644 --- a/docs/state-model.md +++ b/docs/state-model.md @@ -11,12 +11,13 @@ them. |---|---| | **Pinned** | The gitlink recorded in the superproject index (differs from HEAD → `PinStaged`) | | **Checked out** | The submodule's `HEAD` | -| **Tip** | The tip of the ref the pin was resolved to, e.g. `origin/main` | +| **Tip** | The tip of the ref the pin was resolved to, normally the target branch, e.g. `origin/main` | This gives two independent questions: - **Is the checkout on the pin?** → `off pin` if not. -- **Where is the pin relative to its ref?** → `↑n` / `↓n` / `↑n↓m`. +- **Where is the pin relative to its ref?** → `↑n` / `↓n` / `↑n↓m`. A pin + that is not on its target branch (but on another) is `off target`. ## Raw status dimensions @@ -32,7 +33,8 @@ in several at once. | `Pin` | `Ref`, `Source`, `Ahead`, `Behind`, `Pushed` | | `Pinned`, `CheckedOut`, `Tip` | Commits (SHA, subject, author, time) | | `Drift` | Checkout compared with the pin: `Ahead` = commits only the checkout has, `Behind` = commits only the pin has | -| `Bases` | `Tip`, `Checkout`, `Committed`: `merge-base ` for each commit that is diverged from the pin (ahead and behind); nil otherwise or without a common ancestor | +| `TargetTip`, `TargetDrift` | If the pin is not on its target (`off target`): the target's tip and how it compares with the pin, like `Drift` (`Ahead` = commits only the target has); nil/zero otherwise | +| `Bases` | `Tip`, `Checkout`, `Committed`, `Target`: `merge-base ` for each commit that is diverged from the pin (ahead and behind); nil otherwise or without a common ancestor | | `Committed`, `CommittedDrift` | With a staged pin: the pin in the superproject's HEAD (nil if it is not on disk or the submodule was only added) and how it compares with the staged pin, like `Drift` | | `Changes` | Changed files of the worktree (nil if clean), see below | | `FetchedAt` | Last fetch of the submodule (`FETCH_HEAD` mtime); zero if never | @@ -75,14 +77,19 @@ truth for what counts as a problem. | `missing` | Not initialized; directory missing | Warn | | `off pin` | Checkout differs from pin | Warn | | `changed` | Uncommitted changes; untracked files (worded "untracked repository" if all of them are git repositories) | Warn | +| `off target` | Pin is pushed, but not on the target branch (`Pin.Target` is set and differs from `Pin.Ref`, see [Resolving a pin to a ref](#resolving-a-pin-to-a-ref)); the text names the target and the branch the pin is on (`pin not on origin/marco-dev, on origin/main +1`, `+n` for further branches). Not for the superproject | Info | Rules: - If the submodule is **missing**, that is its only issue (nothing else can be determined). - Issues are returned **most severe first**: unpushed, conflict, off pin, - changed. -- **Not issues** (quiet): in sync, pin behind/ahead/diverged but pushed, + changed, off target. +- An unpushed pin is not also `off target`: it has the worse issue. +- `off target` does not depend on the superproject's branch: a feature + branch of the superproject that pins feature-branch commits shows it too. + It is information, not a warning (D26). +- **Not issues** (quiet): in sync, pin behind its target (or another ref), staged pin, detached HEAD on the pin. They are shown as information only. A staged pin is also visible in the tree (`staged` or a `●` marker), see [ux-guide.md](ux-guide.md#tree-row), but is never counted as an issue. @@ -94,14 +101,15 @@ Rules: | Level | Meaning | Presentation | |---|---|---| | `LevelQuiet` | Nothing to do | Whole row muted, no word | +| `LevelInfo` | Good to know | Blue word | | `LevelWarn` | Look at it | Yellow word | | `LevelDanger` | Act now | Red bold word | ### Hidden issues `Submodule.HiddenIssues()` counts the issues of **all** loaded descendants -per level (`Hidden{Warn, Danger}`). A collapsed submodule shows them as `·k` -per level, red first, then yellow, so a danger can never hide inside a +per level (`Hidden{Danger, Warn, Info}`). A collapsed submodule shows them as `·k` +per level, red first, then yellow, then blue, so a danger can never hide inside a collapsed subtree and a warning is never mistaken for one. A descendant whose load failed (`error`) counts as one danger. @@ -115,8 +123,9 @@ is never `CheckoutDiffers` (`CheckoutDetached` for a detached HEAD), "HEAD not pushed to any remote" (Danger, like a submodule's pin). - **Ref** is the upstream of the current branch (`RefFromCurrentBranch`), so - ahead/behind agrees with the header. Closest-branch resolution is only the - fallback without an upstream (detached, or no upstream configured). + ahead/behind agrees with the header. The target (below) is only the + fallback without an upstream (detached, or no upstream configured). The + superproject has no `off target` issue. - **Changes** ignore submodules (`status --ignore-submodules=all`): an off-pin or staged submodule is shown by its own row, not twice. A leftover directory of a submodule the branch no longer has is an untracked @@ -125,38 +134,43 @@ is never `CheckoutDiffers` (`CheckoutDetached` for a detached HEAD), ## Resolving a pin to a ref -The pin is displayed as a ref, not a hash. A commit can be on several -branches, on none, or deep in the history. The git provider shows the -**closest** branch and records which rule proposed it in `PinInfo.Source`. +Two refs are involved (D26): -Preferred refs, in this order (each only if it exists): +- The **target** is the branch the pin is supposed to be on, the same for + everyone: the `branch =` of `.gitmodules` (`.` is the superproject's + current branch), else the remote's default branch (`/HEAD`, else + `/main`, else `/master`). A declared branch that does not + exist locally gives **no** target (the default branch would be a wrong + answer). The remote is `origin`, else the first remote. `PinInfo.Target`. +- The **ref** is what is displayed, and what `Tip`, `Ahead` and `Behind` + refer to. `PinInfo.Ref`, with the rule that found it in `PinInfo.Source`. -| Preference | Ref | `RefSource` | -|---|---|---| -| a | `branch = …` in `.gitmodules` | `RefFromGitmodules` | -| b | Upstream of the branch checked out in the submodule | `RefFromCurrentBranch` | -| c | Remote default branch (`/HEAD`) | `RefFromDefaultBranch` | - -Resolution: +Rules for the ref, in order: | # | Rule | `RefSource` | |---|---|---| -| 1 | Pin pushed: the branch containing the pin with the fewest commits after it. Candidates are the preferred refs that contain the pin plus any remote branch containing it (`for-each-ref --contains`); a tie goes to the preferred ref | preferred ref's, else `RefFromContains` | -| 2 | Pin unpushed: the preferred ref with the fewest commits on either side (`↑n↓m`), ties by preference | preferred ref's | -| 3 | A tag (`git describe`) | `RefFromTag` | -| 4 | Short SHA | `RefFromSHA` | +| 1 | The pin is on the target (a remote branch containing it): the target | `RefFromGitmodules` / `RefFromDefaultBranch` | +| 2 | The pin is on other remote branches: the one closest to the pin, fewest commits after it (`rev-list --count ..`). A tie goes to the upstream of the branch checked out in the submodule, then to the first name in alphabetical order. No branch is a favorite, the default branch included. At most 32 branches are compared (oldest tip first) | `RefFromContains` | +| 3 | The pin is on no remote branch (unpushed): the target, for the distance | the target's | +| 4 | There is no target: upstream of the branch checked out in the submodule | `RefFromCurrentBranch` | +| 5 | A tag (`git describe`) | `RefFromTag` | +| 6 | Short SHA; no tip, no distance | `RefFromSHA` | -So a `.gitmodules` branch is shown when it is as close as any other branch, -but a pin that sits on a feature branch shows that branch rather than -"diverged from main". The trade-off: a pin on main that a younger branch -also contains shows the younger branch if its tip is fewer commits away. +Rule 2 is why a pin on a feature branch shows that branch with its real +distance (`origin/feat/x ↓1`) instead of "diverged from main". `Ref` differs +from `Target` exactly then, and the issue `off target` says so. It does not +matter which branch the target is: if `.gitmodules` declares `marco-dev`, +`main` is just another branch. + +`Pin.Branches` lists the other remote branches that contain the pin besides +`Ref` (oldest tip first, at most 8). Distances: `git rev-list --left-right --count ...` gives -`Ahead` (left) and `Behind` (right). +`Ahead` (left: commits the pin has that the ref lacks) and `Behind` (right). `Pushed` is true if the pin is reachable from any remote ref. A pin that is -ahead of its ref can still be pushed (e.g. on another branch); only -`Pushed == false` is dangerous. +not on its target is `off target` if it is pushed (on another branch), and +`unpushed` if it is not. ## Git commands @@ -172,21 +186,17 @@ are local; `` is the index gitlink. | `Changes` line counts | `diff HEAD --numstat -z -M --ignore-submodules=dirty`, only if a tracked file changed; failure leaves the counts at 0 | | `FileChange.Modified` | `stat` of each changed file that still exists | | `Pushed` | `for-each-ref --contains refs/remotes` is not empty | -| `Pin.Ref` | rules above; the remote is `origin`, else the first remote | +| `Pin.Ref`, `Pin.Target`, `Pin.Branches` | rules above; `for-each-ref --contains refs/remotes` (sorted by commit date), `rev-list --count ..` per candidate | | `Ahead`, `Behind` | `rev-list --left-right --count ...` | | `Drift` | `rev-list --left-right --count ...`, only if the checkout differs from the pin | -| `Bases` | `merge-base ` / `` / ``, only when diverged; then `log -1` of it | +| `TargetTip`, `TargetDrift` | `log -1` of the target and `rev-list --left-right --count ...`, only if the ref is not the target | +| `Bases` | `merge-base ` / `` / `` / ``, only when diverged; then `log -1` of it | | `Committed`, `CommittedDrift` | `ls-tree HEAD` gitlink, if it differs from the pin and exists locally; `rev-list --left-right --count ...` | Details: -- Preference a: `branch = .` means the superproject's current branch. The - ref counts only if `/` exists locally. -- Preference b uses the checked-out branch's upstream (`@{upstream}`). -- Rule 1 counts `rev-list --count ..`. Besides the preferred refs - it compares at most 16 containing branches (oldest tip first). -- Rule 2 counts `rev-list --count ...`. -- Rule 4 shows the short SHA; there is no tip and no distance. +- Rule 4 uses the checked-out branch's upstream (`@{upstream}`). +- Rule 6 shows the short SHA; there is no tip and no distance. - `--ignore-submodules=dirty`: changes inside a nested submodule do not make its parent `changed` (D7), but a moved nested pin does. - Renames are detected only for staged moves (`git mv`); an unstaged move @@ -204,7 +214,8 @@ Details: `Status.Spine()` arranges the pin, the tip and the checkout as one line of history, newest on top, for the detail pane. Each node is a commit with its -roles (`pin`, `tip`, `you`; `HEAD` for the superproject). It uses `Pin.Ahead` / +roles (`pin`, `tip`, `you`; `HEAD` for the superproject; `target` for the +target branch's tip if the pin is not on it). It uses `Pin.Ahead` / `Pin.Behind` (pin ↔ tip) and `Drift` (checkout ↔ pin). - Roles on the same commit share a node (`pin · tip · you`); @@ -214,8 +225,8 @@ roles (`pin`, `tip`, `you`; `HEAD` for the superproject). It uses `Pin.Ahead` / *behind* below it; the `Link` between two nodes counts the commits separating them. The count is the distance, so a tip 4 commits ahead of the pin shows `4 commits`, and an adjacent one `1 commit`. -- A commit that has **diverged** from the pin (tip, checkout or committed - pin) is drawn as a **fork** if it is the only one and its merge base is +- A commit that has **diverged** from the pin (target, tip, checkout or + committed pin) is drawn as a **fork** if it is the only one and its merge base is known (`Status.Bases`): it is the first node (`SpineNode.Fork`), with the number of commits down to the base as its `Below`; the main lane follows (pin and the linear nodes), and a `RoleBase` node where the two histories diff --git a/docs/ux-guide.md b/docs/ux-guide.md index 6ed28f4..ca02b68 100644 --- a/docs/ux-guide.md +++ b/docs/ux-guide.md @@ -82,7 +82,7 @@ For the superproject (header selected), the `pin` and `you` roles become 1. Directory (muted) and name (bold); `fetched ` of the submodule, muted, right-aligned in the name's line (left out if the pane is too narrow, or the submodule is missing) -2. One line per issue: word (colored, 10 columns) + explanation; +2. One line per issue: word (colored, 11 columns) + explanation; `in sync nothing to do` if there are none and no pin is staged; `staged` and `detached HEAD on pin` as information 3. `Changes` (only with uncommitted changes), see @@ -107,7 +107,8 @@ you ● 48005ac Merge branch 'feat/ethercat… position, not by comparing SHAs. The commits between two nodes are not listed, only counted (`┊ 4 commits`; `1 commit` if adjacent). - The label column holds the roles (`pin`, `tip`, `you`; `HEAD` for the - superproject), joined by ` · ` if they sit on one commit + superproject; the target's tip is labelled with the name of its branch, e.g. + `marco-dev`, without the remote and cut at 16 columns; `target` if unknown), joined by ` · ` if they sit on one commit (`pin · tip`, `pin · you`). If all of them sit on one commit it says `in sync` instead, which also keeps the commit column in line with the other values; the column only grows for a longer label. @@ -116,8 +117,25 @@ you ● 48005ac Merge branch 'feat/ethercat… visible as a distance between two nodes. The committed pin is left out if it is not on disk. If the staged pin is *older* than the committed one it sits below it, and the gap above it says `(staged pin moves back)` (blue). -- The checkout (`you`) is yellow (label and SHA) when it is on its own node, - i.e. off pin. Everything else is the default text color, the gap muted. +- If the pin is `off target`, the target branch's tip is on the spine, + labelled with the name of its branch (`marco-dev`, without the remote, cut + at 16 columns), so the distance from main is visible: below the pin if the pin is + on top of the target, a fork if both have commits of their own. `tip` is + then the tip of the branch the pin is on (the `Ref`): + + ``` + marco-dev ● d0bbbd7 updated submodules + ┊ 12 commits + tip ● ┊ a523224 Remove stale entry + ┊ ┊ 1 commit + pin · you ● ┊ 5a7df75 Merge branch 'ethercat… + ┊ ┊ 52 commits + base ●─╯ 6707725 updated submodule + ``` +- The role words are muted, each word on its own: `you` is bold in the + default text color, so the checkout stands out in `pin · you`. On its own + node, i.e. off pin, it is yellow (word and SHA); `staged` is blue. Commit + subjects use the default text color, the gap is muted. - A commit that has **diverged** from the pin gets its own lane to the right of the main one. The two lanes join at a `base` node, the commit where the histories split; the counts say how many commits each side has since: @@ -198,7 +216,7 @@ branch and state stay at the left. If the line is too narrow it is cut with `…` at the right end, so the most important counts stay. Issue counts are ordered by severity (unpushed, conflict, missing, off pin, -changed), each in its level color. Zero counts are omitted. `n loading` +changed, off target), each in its level color. Zero counts are omitted. `n loading` counts only submodules whose status is still being loaded; ones whose load failed are counted separately as `n error` (red, before the issue counts). A detached superproject shows `(detached)` as its branch. @@ -224,7 +242,7 @@ colors in components. |---|---|---| | Danger | ANSI `1` red, bold | `unpushed`, `conflict`, `error`, `↑n` of an unpushed pin | | Warn | ANSI `3` yellow | `missing`, `off pin`, `changed`, differing checkout SHA | -| Info | ANSI `4` blue | `staged`, the `●` staged marker | +| Info | ANSI `4` blue | `staged`, the `●` staged marker, `off target` | | Primary | ANSI `6` cyan | branch names, commit hashes, spinner | | Muted | ANSI `8` grey | quiet rows, directory headers, arrows, labels | | Text | terminal default | normal text | @@ -248,6 +266,7 @@ colors in components. | `missing` | not initialized / directory missing | | `off pin` | checkout differs from pin | | `changed` | uncommitted changes / untracked files | +| `off target` | pin not on ``, on `` (+n more) | | `staged` | pin change staged, not committed | | `in sync` | nothing to do | diff --git a/internal/submodule/git/provider.go b/internal/submodule/git/provider.go index 68ea130..9766efa 100644 --- a/internal/submodule/git/provider.go +++ b/internal/submodule/git/provider.go @@ -20,9 +20,13 @@ import ( "submarine/internal/submodule" ) +// maxPinBranches limits Pin.Branches, the other remote branches containing the +// pin. +const maxPinBranches = 8 + // maxContainsCandidates limits how many remote branches containing the pin -// are compared besides the preferred refs when resolving a pushed pin. -const maxContainsCandidates = 16 +// are compared when the pin is not on its target. +const maxContainsCandidates = 32 // Provider reads a superproject and its submodules with git. type Provider struct { @@ -296,7 +300,7 @@ func (p *Provider) rootStatus(ctx context.Context) (*submodule.Status, error) { } // the root is compared with its upstream, so that the header and the - // detail pane agree; only without one the closest branch is looked for + // detail pane agree; only without one the target is looked for var chosen *candidate if branch != "" { if up, ok, err := try(ctx, in.dir, "rev-parse", "--abbrev-ref", "--symbolic-full-name", branch+"@{upstream}"); err != nil { @@ -340,10 +344,15 @@ func pinState(ctx context.Context, in info, st *submodule.Status, pin, head, bra var source submodule.RefSource if chosen != nil { ref, source = chosen.ref, chosen.source - } else if ref, source, err = resolve(ctx, in, pin, branch, containing); err != nil { + } else if ref, source, st.Pin.Target, err = resolve(ctx, in, pin, branch, containing); err != nil { return err } st.Pin.Ref, st.Pin.Source = ref, source + for _, b := range containing { + if b != ref && len(st.Pin.Branches) < maxPinBranches { + st.Pin.Branches = append(st.Pin.Branches, b) + } + } // commits if st.Pinned, err = commit(ctx, in.dir, pin); err != nil { @@ -386,134 +395,162 @@ func pinState(ctx context.Context, in info, st *submodule.Status, pin, head, bra return err } } + + // the pin is on another branch than its target: where the target is + if target := st.Pin.Target; target != "" && target != ref { + if st.TargetTip, err = commit(ctx, in.dir, target); err != nil { + return err + } + out, err = run(ctx, in.dir, "rev-list", "--left-right", "--count", target+"..."+pin) + if err != nil { + return err + } + if st.TargetDrift.Ahead, st.TargetDrift.Behind, err = parseLeftRight(out); err != nil { + return err + } + if st.TargetDrift.Ahead > 0 && st.TargetDrift.Behind > 0 { + if st.Bases.Target, err = mergeBase(ctx, in.dir, pin, target); err != nil { + return err + } + } + } return nil } -// resolve finds the ref that describes the pin, following the order in -// docs/state-model.md: the closest branch wins, the preferred refs (see -// preferredRefs) break ties. containing lists the remote branches containing -// the pin, oldest tip first. -func resolve(ctx context.Context, in info, pin, branch string, containing []string) (string, submodule.RefSource, error) { - preferred, err := preferredRefs(ctx, in, branch) +// resolve finds the ref the pin is compared with, following the order in +// docs/state-model.md: the target (see targetRef) if the pin is on it, else the +// branch containing the pin that is closest to it, else the target anyway, +// the upstream of the checked-out branch, a tag, the commit itself. It also +// returns the target ("" if there is none). containing lists the remote +// branches containing the pin, oldest tip first. +func resolve(ctx context.Context, in info, pin, branch string, containing []string) (string, submodule.RefSource, string, error) { + target, source, hasTarget, err := targetRef(ctx, in) if err != nil { - return "", 0, err + return "", 0, "", err + } + if !hasTarget { + target = "" + } + if hasTarget && slices.Contains(containing, target) { + return target, source, target, nil } - - // 1. pushed: the branch containing the pin with the fewest commits after it if len(containing) > 0 { - var cands []candidate - for _, c := range preferred { - if _, ok, err := try(ctx, in.dir, "merge-base", "--is-ancestor", pin, c.ref); err != nil { - return "", 0, err - } else if ok { - cands = append(cands, c) - } - } - for _, r := range containing[:min(len(containing), maxContainsCandidates)] { - if !slices.ContainsFunc(cands, func(c candidate) bool { return c.ref == r }) { - cands = append(cands, candidate{r, submodule.RefFromContains}) - } - } - if c, ok, err := closest(ctx, in.dir, pin+"..", cands); err != nil { - return "", 0, err - } else if ok { - return c.ref, c.source, nil - } + ref, err := closestContaining(ctx, in, pin, branch, containing) + return ref, submodule.RefFromContains, target, err + } + if hasTarget { + return target, source, target, nil } - // 2. unpushed: the preferred ref with the fewest commits on either side - if c, ok, err := closest(ctx, in.dir, pin+"...", preferred); err != nil { - return "", 0, err + // the checked-out branch depends on the machine, so it is no target + if up, ok, err := upstream(ctx, in.dir, branch); err != nil { + return "", 0, "", err } else if ok { - return c.ref, c.source, nil + return up, submodule.RefFromCurrentBranch, "", nil } - - // 3. nearest tag if tag, ok, err := try(ctx, in.dir, "describe", "--tags", "--abbrev=0", pin); err != nil { - return "", 0, err + return "", 0, "", err } else if ok && tag != "" { - return tag, submodule.RefFromTag, nil + return tag, submodule.RefFromTag, "", nil } - - // 4. the commit itself - return "", submodule.RefFromSHA, nil + return "", submodule.RefFromSHA, "", nil } -// candidate is a ref the pin may resolve to and the rule that proposed it. +// upstream returns the upstream of the local branch, if there is one. +func upstream(ctx context.Context, dir, branch string) (string, bool, error) { + if branch == "" { + return "", false, nil + } + return try(ctx, dir, "rev-parse", "--abbrev-ref", "--symbolic-full-name", branch+"@{upstream}") +} + +// closestContaining picks the branch the pin is closest to among the remote +// branches containing it: the fewest commits after the pin. A tie goes to the +// upstream of the checked-out branch, then to the first name in alphabetical +// order, so that the answer is the same on every machine. No branch, +// including the default branch, is preferred otherwise. +func closestContaining(ctx context.Context, in info, pin, branch string, containing []string) (string, error) { + var best []string + bestN := -1 + for _, c := range containing[:min(len(containing), maxContainsCandidates)] { + out, err := run(ctx, in.dir, "rev-list", "--count", pin+".."+c) + if err != nil { + return "", err + } + var n int + if _, err := fmt.Sscan(out, &n); err != nil { + continue + } + switch { + case bestN < 0 || n < bestN: + best, bestN = []string{c}, n + case n == bestN: + best = append(best, c) + } + } + if len(best) == 0 { + return containing[0], nil + } + if up, ok, err := upstream(ctx, in.dir, branch); err != nil { + return "", err + } else if ok && slices.Contains(best, up) { + return up, nil + } + slices.Sort(best) + return best[0], nil +} + +// targetRef returns the branch the pin should be on: the `branch =` of +// .gitmodules (`.` is the superproject's current branch), else the remote's +// default branch, else main or master. A declared branch that does not exist +// locally is not replaced by the default: the answer would be wrong. +func targetRef(ctx context.Context, in info) (string, submodule.RefSource, bool, error) { + remote, err := remoteName(ctx, in.dir) + if err != nil || remote == "" { + return "", 0, false, err + } + exists := func(ref string) (bool, error) { + _, ok, err := try(ctx, in.dir, "rev-parse", "--verify", "-q", "refs/remotes/"+ref) + return ok, err + } + + if b := in.branch; b != "" { + if b == "." { + if b, _, err = try(ctx, in.superDir, "symbolic-ref", "--short", "-q", "HEAD"); err != nil { + return "", 0, false, err + } + } + if b == "" { + return "", 0, false, nil + } + ok, err := exists(remote + "/" + b) + if err != nil || !ok { + return "", 0, false, err + } + return remote + "/" + b, submodule.RefFromGitmodules, true, nil + } + + if def, ok, err := try(ctx, in.dir, "symbolic-ref", "--short", "-q", "refs/remotes/"+remote+"/HEAD"); err != nil { + return "", 0, false, err + } else if ok { + return def, submodule.RefFromDefaultBranch, true, nil + } + for _, name := range []string{"main", "master"} { + if ok, err := exists(remote + "/" + name); err != nil { + return "", 0, false, err + } else if ok { + return remote + "/" + name, submodule.RefFromDefaultBranch, true, nil + } + } + return "", 0, false, nil +} + +// candidate is a ref and the rule that proposed it. type candidate struct { ref string source submodule.RefSource } -// preferredRefs returns the refs the user most likely means, in order of -// preference and without duplicates: the .gitmodules branch, the upstream of -// the checked-out branch, the remote default branch. Each must exist. -func preferredRefs(ctx context.Context, in info, branch string) ([]candidate, error) { - remote, err := remoteName(ctx, in.dir) - if err != nil { - return nil, err - } - var refs []candidate - add := func(ref string, source submodule.RefSource) { - if ref != "" && !slices.ContainsFunc(refs, func(c candidate) bool { return c.ref == ref }) { - refs = append(refs, candidate{ref, source}) - } - } - - // branch from .gitmodules - if b := in.branch; b != "" && remote != "" { - if b == "." { - b, _, err = try(ctx, in.superDir, "symbolic-ref", "--short", "-q", "HEAD") - if err != nil { - return nil, err - } - } - if b != "" { - if _, ok, err := try(ctx, in.dir, "rev-parse", "--verify", "-q", "refs/remotes/"+remote+"/"+b); err != nil { - return nil, err - } else if ok { - add(remote+"/"+b, submodule.RefFromGitmodules) - } - } - } - - // upstream of the checked-out branch - if branch != "" { - if up, ok, err := try(ctx, in.dir, "rev-parse", "--abbrev-ref", "--symbolic-full-name", branch+"@{upstream}"); err != nil { - return nil, err - } else if ok { - add(up, submodule.RefFromCurrentBranch) - } - } - - // remote default branch - if remote != "" { - if def, ok, err := try(ctx, in.dir, "symbolic-ref", "--short", "-q", "refs/remotes/"+remote+"/HEAD"); err != nil { - return nil, err - } else if ok { - add(def, submodule.RefFromDefaultBranch) - } - } - return refs, nil -} - -// closest returns the candidate with the fewest commits in the range -// `` (from is e.g. ".."); the first one wins a tie. -func closest(ctx context.Context, dir, from string, cands []candidate) (candidate, bool, error) { - best, bestN := candidate{}, -1 - for _, c := range cands { - out, err := run(ctx, dir, "rev-list", "--count", from+c.ref) - if err != nil { - return candidate{}, false, err - } - var n int - if _, err := fmt.Sscan(out, &n); err == nil && (bestN < 0 || n < bestN) { - best, bestN = c, n - } - } - return best, bestN >= 0, nil -} - // remoteName returns "origin" if it exists, else the first remote, else "". func remoteName(ctx context.Context, dir string) (string, error) { out, err := run(ctx, dir, "remote") diff --git a/internal/submodule/git/provider_test.go b/internal/submodule/git/provider_test.go index 4c9081c..6936e68 100644 --- a/internal/submodule/git/provider_test.go +++ b/internal/submodule/git/provider_test.go @@ -90,26 +90,29 @@ func TestStatus(t *testing.T) { path string issues []submodule.IssueKind ref string + target string // the target if the pin is not on it; else the ref is the target source submodule.RefSource ahead int behind int staged bool checkout submodule.Checkout drift submodule.Distance // checkout compared with the pin + branches []string // other remote branches containing the pin + toTarget submodule.Distance // target tip compared with the pin; zero if the pin is on it }{ {path: "src/core", issues: []submodule.IssueKind{submodule.IssueOffPin}, ref: "origin/main", source: submodule.RefFromDefaultBranch, checkout: submodule.CheckoutDiffers, drift: submodule.Distance{Behind: 1}}, - {path: "src/ui", ref: "origin/main", source: submodule.RefFromCurrentBranch, behind: 2}, - {path: "src/net", issues: []submodule.IssueKind{submodule.IssueUnpushed}, ref: "origin/main", source: submodule.RefFromCurrentBranch, ahead: 1}, + {path: "src/ui", ref: "origin/main", source: submodule.RefFromDefaultBranch, behind: 2}, + {path: "src/net", issues: []submodule.IssueKind{submodule.IssueUnpushed}, ref: "origin/main", source: submodule.RefFromDefaultBranch, ahead: 1}, {path: "libs/store", ref: "origin/develop", source: submodule.RefFromGitmodules}, - {path: "libs/log", ref: "origin/main", source: submodule.RefFromCurrentBranch}, + {path: "libs/log", ref: "origin/main", source: submodule.RefFromDefaultBranch, branches: []string{"origin/archive"}}, {path: "libs/log/modules/nested", issues: []submodule.IssueKind{submodule.IssueChanged}, ref: "origin/main", source: submodule.RefFromDefaultBranch, checkout: submodule.CheckoutDetached}, - {path: "libs/feature", ref: "origin/feat/search", source: submodule.RefFromContains, behind: 1, checkout: submodule.CheckoutDetached}, - {path: "libs/topic", ref: "origin/feat/search", source: submodule.RefFromContains, checkout: submodule.CheckoutDetached}, - {path: "libs/conflict", issues: []submodule.IssueKind{submodule.IssueConflict}, ref: "origin/main", source: submodule.RefFromCurrentBranch, staged: true}, - {path: "libs/staged", ref: "origin/main", source: submodule.RefFromCurrentBranch, staged: true}, + {path: "libs/feature", issues: []submodule.IssueKind{submodule.IssueOffTarget}, ref: "origin/feat/search", source: submodule.RefFromContains, target: "origin/main", behind: 1, checkout: submodule.CheckoutDetached, toTarget: submodule.Distance{Behind: 1}}, + {path: "libs/topic", issues: []submodule.IssueKind{submodule.IssueOffTarget}, ref: "origin/feat/search", source: submodule.RefFromContains, target: "origin/main", checkout: submodule.CheckoutDetached, toTarget: submodule.Distance{Behind: 2}}, + {path: "libs/conflict", issues: []submodule.IssueKind{submodule.IssueConflict}, ref: "origin/main", source: submodule.RefFromDefaultBranch, staged: true}, + {path: "libs/staged", ref: "origin/main", source: submodule.RefFromDefaultBranch, staged: true}, {path: "libs/missing", issues: []submodule.IssueKind{submodule.IssueMissing}}, - {path: "vendor", issues: []submodule.IssueKind{submodule.IssueChanged}, ref: "origin/main", source: submodule.RefFromCurrentBranch, behind: 2}, - {path: "libs/work", issues: []submodule.IssueKind{submodule.IssueChanged}, ref: "origin/main", source: submodule.RefFromCurrentBranch}, + {path: "vendor", issues: []submodule.IssueKind{submodule.IssueChanged}, ref: "origin/main", source: submodule.RefFromDefaultBranch, behind: 2}, + {path: "libs/work", issues: []submodule.IssueKind{submodule.IssueChanged}, ref: "origin/main", source: submodule.RefFromDefaultBranch}, } for _, tt := range tests { st, err := p.Status(context.Background(), tt.path) @@ -130,6 +133,13 @@ func TestStatus(t *testing.T) { if st.Pin.Ref != tt.ref || st.Pin.Source != tt.source { t.Errorf("%s: ref %q (%v), want %q (%v)", tt.path, st.Pin.Ref, st.Pin.Source, tt.ref, tt.source) } + want := tt.target + if want == "" && (tt.source == submodule.RefFromGitmodules || tt.source == submodule.RefFromDefaultBranch) { + want = tt.ref + } + if st.Pin.Target != want { + t.Errorf("%s: target %q, want %q", tt.path, st.Pin.Target, want) + } if st.Pin.Ahead != tt.ahead || st.Pin.Behind != tt.behind { t.Errorf("%s: ↑%d↓%d, want ↑%d↓%d", tt.path, st.Pin.Ahead, st.Pin.Behind, tt.ahead, tt.behind) } @@ -142,6 +152,12 @@ func TestStatus(t *testing.T) { if tt.path == "libs/staged" && (st.CommittedDrift.Ahead != 0 || st.CommittedDrift.Behind == 0) { t.Errorf("%s: CommittedDrift %+v, want the committed pin behind the staged one", tt.path, st.CommittedDrift) } + if !slices.Equal(st.Pin.Branches, tt.branches) { + t.Errorf("%s: other branches %v, want %v", tt.path, st.Pin.Branches, tt.branches) + } + if st.TargetDrift != tt.toTarget || (st.TargetTip != nil) != (tt.toTarget != submodule.Distance{}) { + t.Errorf("%s: TargetDrift %+v with tip %+v, want %+v", tt.path, st.TargetDrift, st.TargetTip, tt.toTarget) + } if st.Drift != tt.drift { t.Errorf("%s: drift %+v, want %+v", tt.path, st.Drift, tt.drift) } @@ -334,6 +350,22 @@ func TestBasesOfDivergedCommits(t *testing.T) { t.Errorf("src/core: Bases.Checkout = %+v", b) } + // libs/feature: its pin is on a feature branch, and main moved on + pushUpstream(t, root, "feature") + if err := p.Fetch(ctx, "libs/feature"); err != nil { + t.Fatal(err) + } + st, err = p.Status(ctx, "libs/feature") + if err != nil { + t.Fatal(err) + } + if st.TargetDrift != (submodule.Distance{Ahead: 1, Behind: 1}) || st.Bases.Target == nil { + t.Errorf("libs/feature: TargetDrift %+v, Bases.Target %+v, want a diverged target with a base", st.TargetDrift, st.Bases.Target) + } + if sp := st.Spine(); len(sp) == 0 || !sp[0].Fork || sp[0].Roles[0] != submodule.RoleTarget { + t.Errorf("libs/feature: spine = %+v, want a fork headed by the target", sp) + } + // in sync and linear: no bases if st, err = p.Status(ctx, "src/ui"); err != nil { t.Fatal(err) @@ -341,3 +373,59 @@ func TestBasesOfDivergedCommits(t *testing.T) { t.Errorf("src/ui: Bases = %+v, want none", st.Bases) } } + +// A `branch =` that does not exist locally is not replaced by the default +// branch: judging the pin against the wrong branch would be worse than not +// judging it. +func TestDeclaredBranchMissing(t *testing.T) { + p, root := ownFixture(t) + git(t, filepath.Join(root, "super"), "config", "-f", ".gitmodules", "submodule.libs/store.branch", "gone") + if _, err := p.Tree(context.Background()); err != nil { + t.Fatal(err) + } + st, err := p.Status(context.Background(), "libs/store") + if err != nil { + t.Fatal(err) + } + if st.Pin.Target != "" { + t.Errorf("target %q, want none", st.Pin.Target) + } + for _, is := range st.Issues() { + if is.Kind == submodule.IssueOffTarget { + t.Errorf("issue %+v without a target", is) + } + } +} + +// Two branches contain the pin equally close: the branch the submodule is on +// wins, else the first name; the default branch is no favorite. +func TestContainingBranchTie(t *testing.T) { + p, root := ownFixture(t) + super := filepath.Join(root, "super") + // the target is a branch that does not exist, so none applies + git(t, super, "config", "-f", ".gitmodules", "submodule.libs/log.branch", "gone") + if _, err := p.Tree(context.Background()); err != nil { + t.Fatal(err) + } + ref := func() (string, submodule.RefSource, []string) { + st, err := p.Status(context.Background(), "libs/log") + if err != nil { + t.Fatal(err) + } + return st.Pin.Ref, st.Pin.Source, st.Pin.Branches + } + + // on branch main (upstream origin/main) in the fixture + if r, src, _ := ref(); r != "origin/main" || src != submodule.RefFromContains { + t.Errorf("on a branch: ref %q (%v), want origin/main", r, src) + } + + git(t, filepath.Join(super, "libs", "log"), "checkout", "-q", "--detach") + r, src, others := ref() + if r != "origin/archive" || src != submodule.RefFromContains { + t.Errorf("detached: ref %q (%v), want origin/archive", r, src) + } + if !slices.Equal(others, []string{"origin/main"}) { + t.Errorf("detached: other branches %v, want [origin/main]", others) + } +} diff --git a/internal/submodule/mock/mock.go b/internal/submodule/mock/mock.go index 4a87b8c..eb3064a 100644 --- a/internal/submodule/mock/mock.go +++ b/internal/submodule/mock/mock.go @@ -8,6 +8,7 @@ import ( "fmt" "math/rand/v2" "path" + "strings" "sync" "time" @@ -132,8 +133,12 @@ func commit(sha, subject, author string, ago time.Duration) *submodule.Commit { } func inSync(ref string, c *submodule.Commit) *submodule.Status { + pin := submodule.PinInfo{Ref: ref, Pushed: true} + if strings.HasPrefix(ref, "origin/") { + pin.Target = ref // on its target branch + } return &submodule.Status{ - Pin: submodule.PinInfo{Ref: ref, Pushed: true}, + Pin: pin, Pinned: c, CheckedOut: c, Tip: c, } } @@ -271,13 +276,16 @@ func fixture() []node { Tip: sub3Tip, }}, {relPath: "libs/logging", status: &submodule.Status{ - Pin: submodule.PinInfo{Ref: "origin/main", Behind: 3, Pushed: true}, - Pinned: logPin, - CheckedOut: logPin, - Tip: logTip, + Pin: submodule.PinInfo{Ref: "origin/feat/slog", Source: submodule.RefFromContains, Target: "origin/main", Behind: 3, Pushed: true}, + Pinned: logPin, + CheckedOut: logPin, + Tip: logTip, + TargetTip: commit("0f1e2d3", "Bump dependencies", "renovate", 30*time.Hour), + TargetDrift: submodule.Distance{Ahead: 2, Behind: 4}, + Bases: submodule.Bases{Target: commit("9a8b7c6", "Release 0.8.0", "dana", 14*24*time.Hour)}, }}, {relPath: "libs/http", status: &submodule.Status{ - Pin: submodule.PinInfo{Ref: "origin/develop", Ahead: 1, Behind: 2, Pushed: true, Source: submodule.RefFromContains}, + Pin: submodule.PinInfo{Ref: "origin/develop", Target: "origin/develop", Ahead: 1, Behind: 2, Source: submodule.RefFromGitmodules}, Bases: submodule.Bases{Tip: commit("a9b8c7d", "Release 1.9.0", "frank", 20*24*time.Hour)}, Pinned: httpPin, CheckedOut: httpPin, diff --git a/internal/submodule/model.go b/internal/submodule/model.go index 088023d..8fe90e1 100644 --- a/internal/submodule/model.go +++ b/internal/submodule/model.go @@ -67,10 +67,10 @@ const ( type RefSource int const ( - RefFromGitmodules RefSource = iota - RefFromCurrentBranch - RefFromDefaultBranch - RefFromContains + RefFromGitmodules RefSource = iota // the target, `branch =` in .gitmodules; the pin is on it + RefFromDefaultBranch // the target, the remote's default branch; the pin is on it + RefFromContains // a branch containing the pin, which is not on the target + RefFromCurrentBranch // upstream of the checked-out branch, there being no better RefFromTag RefFromSHA ) @@ -79,12 +79,12 @@ func (r RefSource) String() string { switch r { case RefFromGitmodules: return ".gitmodules branch" - case RefFromCurrentBranch: - return "checked out branch" case RefFromDefaultBranch: return "remote default branch" case RefFromContains: return "branch containing pin" + case RefFromCurrentBranch: + return "checked out branch" case RefFromTag: return "tag" default: @@ -96,9 +96,17 @@ func (r RefSource) String() string { type PinInfo struct { Ref string // e.g. "origin/main"; empty if unresolved Source RefSource + // Target is the branch the pin is supposed to be on (`branch =` of + // .gitmodules, else the remote's default branch); empty if there is none or + // it does not exist locally. Ref is the target if the pin is on it, else + // the branch the pin is on instead. + Target string Ahead int // commits in pin not in ref Behind int // commits in ref not in pin Pushed bool // pin is reachable from some remote ref + // Branches are remote branches other than Ref that contain the pin, oldest + // tip first and limited. + Branches []string } // Commit is a short commit description. @@ -179,7 +187,7 @@ type Distance struct{ Ahead, Behind int } // Bases are the merge bases of the pin with the commits that have diverged // from it, where the histories split. A field is nil if that commit has not // diverged or the base is unknown. -type Bases struct{ Tip, Checkout, Committed *Commit } +type Bases struct{ Tip, Checkout, Committed, Target *Commit } // Status is the full state of one submodule. type Status struct { @@ -198,7 +206,12 @@ type Status struct { // only added. CommittedDrift compares it with Pinned like Drift. Committed *Commit CommittedDrift Distance - Bases Bases - Changes *Changes // nil if the worktree is clean or the details are unknown - FetchedAt time.Time // last fetch of the submodule; zero if never + // TargetTip is the tip of Pin.Target if the pin is not on it (the ref is + // another branch), else nil. TargetDrift compares it with Pinned like + // Drift: Ahead = commits only the target has. + TargetTip *Commit + TargetDrift Distance + Bases Bases + Changes *Changes // nil if the worktree is clean or the details are unknown + FetchedAt time.Time // last fetch of the submodule; zero if never } diff --git a/internal/submodule/spine.go b/internal/submodule/spine.go index 7f772bd..6517fad 100644 --- a/internal/submodule/spine.go +++ b/internal/submodule/spine.go @@ -11,6 +11,7 @@ const ( RoleHead // the superproject's HEAD, which takes the pin's place RoleStaged // the pin in the index, if it differs from RolePin RoleTip // the tip of the ref the pin was resolved to + RoleTarget // the tip of the target branch, if the pin is not on it RoleYou // the commit checked out in the submodule RoleBase // where a diverged commit's history splits from the pin's ) @@ -100,6 +101,9 @@ func (s *Status) Spine() []SpineNode { if s.Tip != nil { place(RoleTip, s.Tip, s.Bases.Tip, s.Pin.Behind, s.Pin.Ahead, true) } + if s.TargetTip != nil { + place(RoleTarget, s.TargetTip, s.Bases.Target, s.TargetDrift.Ahead, s.TargetDrift.Behind, true) + } if s.PinStaged && s.Committed != nil { place(RolePin, s.Committed, s.Bases.Committed, s.CommittedDrift.Ahead, s.CommittedDrift.Behind, false) } diff --git a/internal/submodule/spine_test.go b/internal/submodule/spine_test.go index d0ecbee..d60668a 100644 --- a/internal/submodule/spine_test.go +++ b/internal/submodule/spine_test.go @@ -122,6 +122,20 @@ func TestSpine(t *testing.T) { nodes: []string{"t:tip", "p:pin", "y:you"}, links: []Link{{Above: 2, Below: 1}, {Above: 1, Below: 1}}, }, + { + name: "pin on a branch ahead of its target: the target is below the pin", + st: Status{Pinned: c("p"), CheckedOut: c("p"), Tip: c("t"), Pin: PinInfo{Behind: 1}, + TargetTip: c("m"), TargetDrift: Distance{Behind: 3}}, + nodes: []string{"t:tip", "p:pin,you", "m:target"}, + links: []Link{{Above: 1}, {Above: 3}}, + }, + { + name: "pin on a branch diverged from its target: a fork", + st: Status{Pinned: c("p"), CheckedOut: c("p"), Tip: c("t"), Pin: PinInfo{Behind: 1}, + TargetTip: c("m"), TargetDrift: Distance{Ahead: 12, Behind: 52}, Bases: Bases{Target: c("b")}}, + nodes: []string{"m:target", "t:tip", "p:pin,you", "b:base"}, + links: []Link{{Above: 12}, {Above: 1}, {Above: 52}}, + }, { name: "no tip, checkout not loaded", st: Status{Pinned: c("p")}, @@ -134,7 +148,7 @@ func TestSpine(t *testing.T) { links: []Link{{Above: 2}}, }, } - words := map[Role]string{RolePin: "pin", RoleStaged: "staged", RoleHead: "head", RoleTip: "tip", RoleYou: "you", RoleBase: "base"} + words := map[Role]string{RolePin: "pin", RoleStaged: "staged", RoleHead: "head", RoleTip: "tip", RoleTarget: "target", RoleYou: "you", RoleBase: "base"} for _, tt := range tests { var nodes []string var links []Link diff --git a/internal/submodule/status.go b/internal/submodule/status.go index 4055f69..e67a887 100644 --- a/internal/submodule/status.go +++ b/internal/submodule/status.go @@ -1,10 +1,13 @@ package submodule +import "fmt" + // Level is how much attention an issue deserves. type Level int const ( LevelQuiet Level = iota // nothing to do + LevelInfo // good to know LevelWarn // look at it LevelDanger // act now ) @@ -13,11 +16,12 @@ const ( type IssueKind int const ( - IssueMissing IssueKind = iota // not initialized or directory missing - IssueUnpushed // pin not reachable from any remote ref - IssueConflict // merge conflict in the worktree - IssueOffPin // checkout differs from pin - IssueChanged // uncommitted changes or untracked files + IssueMissing IssueKind = iota // not initialized or directory missing + IssueUnpushed // pin not reachable from any remote ref + IssueConflict // merge conflict in the worktree + IssueOffPin // checkout differs from pin + IssueChanged // uncommitted changes or untracked files + IssueOffTarget // pin pushed, but not on the target branch ) // Issue is one problem of a submodule. Detail distinguishes the states that @@ -61,15 +65,29 @@ func (s *Status) Issues() []Issue { case WorktreeUntracked: out = append(out, Issue{IssueChanged, LevelWarn, s.untrackedText()}) } + // a pin that is pushed, but not on the target (usually main); an + // unpushed pin is not on it either, but has the worse issue + if !s.Root && s.Pin.Pushed && s.Pin.Target != "" && s.Pin.Ref != s.Pin.Target { + out = append(out, Issue{IssueOffTarget, LevelInfo, s.offTargetText()}) + } return out } +// offTargetText says which branch the pin is not on and where it is instead. +func (s *Status) offTargetText() string { + text := fmt.Sprintf("pin not on %s, on %s", s.Pin.Target, s.Pin.Ref) + if n := len(s.Pin.Branches); n > 0 { + text += fmt.Sprintf(" +%d", n) + } + return text +} + // Hidden counts issues below a submodule per level, plus staged pins (which // are not issues). -type Hidden struct{ Warn, Danger, Staged int } +type Hidden struct{ Info, Warn, Danger, Staged int } // Total is the number of issues of all levels; staged pins are not counted. -func (h Hidden) Total() int { return h.Warn + h.Danger } +func (h Hidden) Total() int { return h.Info + h.Warn + h.Danger } // HiddenIssues counts the issues and staged pins of all loaded descendants. A // descendant whose load failed counts as one danger, since its state is @@ -93,6 +111,8 @@ func HiddenIn(mods []*Submodule) Hidden { h.Danger++ case LevelWarn: h.Warn++ + case LevelInfo: + h.Info++ } } }) diff --git a/internal/submodule/status_test.go b/internal/submodule/status_test.go index 09d3fdb..ba9dcb3 100644 --- a/internal/submodule/status_test.go +++ b/internal/submodule/status_test.go @@ -29,6 +29,12 @@ func TestIssues(t *testing.T) { {"untracked is changed", &Status{Worktree: WorktreeUntracked, Pin: pushed}, []IssueKind{IssueChanged}}, {"danger first", &Status{Checkout: CheckoutDiffers, Worktree: WorktreeModified}, []IssueKind{IssueUnpushed, IssueOffPin, IssueChanged}}, {"conflict", &Status{Worktree: WorktreeConflict, Pin: pushed}, []IssueKind{IssueConflict}}, + {"pushed, but not on the target", &Status{Pin: PinInfo{Pushed: true, Ref: "origin/feat", Target: "origin/main", Source: RefFromContains}}, []IssueKind{IssueOffTarget}}, + {"off target comes last", &Status{Checkout: CheckoutDiffers, Pin: PinInfo{Pushed: true, Ref: "origin/feat", Target: "origin/main"}}, []IssueKind{IssueOffPin, IssueOffTarget}}, + {"unpushed is not also off target", &Status{Pin: PinInfo{Ref: "origin/main", Target: "origin/main", Ahead: 1}}, []IssueKind{IssueUnpushed}}, + {"on the target, ahead of nothing else", &Status{Pin: PinInfo{Pushed: true, Ref: "origin/main", Target: "origin/main", Behind: 4}}, nil}, + {"no target, no judgement", &Status{Pin: PinInfo{Pushed: true, Ref: "origin/feat", Source: RefFromContains}}, nil}, + {"the superproject has no target", &Status{Root: true, Pin: PinInfo{Pushed: true, Ref: "origin/feat", Target: "origin/main"}}, nil}, } for _, tt := range tests { if got := kinds(tt.s.Issues()); !slices.Equal(got, tt.want) { @@ -37,6 +43,23 @@ func TestIssues(t *testing.T) { } } +func TestOffTargetText(t *testing.T) { + s := &Status{Pin: PinInfo{Target: "origin/marco-dev", Ref: "origin/fix/x"}} + for _, c := range []struct { + branches []string + want string + }{ + {nil, "pin not on origin/marco-dev, on origin/fix/x"}, + {[]string{"origin/main"}, "pin not on origin/marco-dev, on origin/fix/x +1"}, + {[]string{"origin/main", "origin/y"}, "pin not on origin/marco-dev, on origin/fix/x +2"}, + } { + s.Pin.Branches = c.branches + if got := s.offTargetText(); got != c.want { + t.Errorf("%v: %q, want %q", c.branches, got, c.want) + } + } +} + func TestHiddenIssues(t *testing.T) { ok := &Status{Pin: PinInfo{Pushed: true}} dirty := &Status{Worktree: WorktreeModified, Pin: PinInfo{Pushed: true}, PinStaged: true} diff --git a/internal/tui/detail/detail.go b/internal/tui/detail/detail.go index 007d447..f3d2659 100644 --- a/internal/tui/detail/detail.go +++ b/internal/tui/detail/detail.go @@ -16,7 +16,7 @@ import ( "submarine/internal/tui/theme" ) -const labelWidth = 10 +const labelWidth = 11 type Model struct { theme theme.Theme @@ -136,7 +136,7 @@ func (m Model) lines(s *submodule.Submodule, repo *submodule.Repo) []string { if st.Pin.Ref != "" { out = append(out, label("Ref")+text.Render(st.Pin.Ref)+muted.Render(" · via "+st.Pin.Source.String()), "") } - out = append(out, m.spineLines(st.Spine(), st.Checkout == submodule.CheckoutDiffers)...) + out = append(out, m.spineLines(st.Spine(), st.Checkout == submodule.CheckoutDiffers, st.Pin.Target)...) if s.FetchErr != nil { out = append(out, theme.Fg(t.Warn).Width(labelWidth).Render("fetch")+text.Render("failed: "+s.FetchErr.Error())) } @@ -175,6 +175,9 @@ func (m Model) hiddenParts(h submodule.Hidden) string { if h.Warn > 0 { parts = append(parts, t.LevelStyle(submodule.LevelWarn, lipgloss.NewStyle()).Render(fmt.Sprintf("%d warning(s)", h.Warn))) } + if h.Info > 0 { + parts = append(parts, t.LevelStyle(submodule.LevelInfo, lipgloss.NewStyle()).Render(fmt.Sprintf("%d note(s)", h.Info))) + } if h.Staged > 0 { parts = append(parts, theme.Fg(t.Info).Render(fmt.Sprintf("%d %s", h.Staged, theme.Staged))) } @@ -185,22 +188,37 @@ func (m Model) hiddenParts(h submodule.Hidden) string { // newest on top: a node per commit with its roles in the label column, and // between two nodes how many commits separate them. offPin colors the // checkout's own node. -func (m Model) spineLines(nodes []submodule.SpineNode, offPin bool) []string { +func (m Model) spineLines(nodes []submodule.SpineNode, offPin bool, target string) []string { t := m.theme muted, text := theme.Fg(t.Muted), theme.Fg(t.Text) + // the roles of a node, each word in its own style: the checkout stands + // out from the rest, and a staged pin is blue labels := make([]string, len(nodes)) + labelWidths := make([]int, len(nodes)) width := labelWidth for i, n := range nodes { - words := make([]string, len(n.Roles)) - for j, r := range n.Roles { - words[j] = theme.RoleWord(r) + var plain, styled []string + for _, r := range n.Roles { + word := theme.RoleWord(r, target) + style := muted + switch { + case r == submodule.RoleStaged: + style = theme.Fg(t.Info) + case r == submodule.RoleYou && offPin && len(n.Roles) == 1: + style = theme.Fg(t.Warn) + case r == submodule.RoleYou: + style = text.Bold(true) + } + plain = append(plain, word) + styled = append(styled, style.Render(word)) } - labels[i] = strings.Join(words, " · ") + labels[i] = strings.Join(styled, muted.Render(" · ")) + labelWidths[i] = ansi.StringWidth(strings.Join(plain, " · ")) if n.InSync() { - labels[i] = theme.InSync + labels[i], labelWidths[i] = muted.Render(theme.InSync), len(theme.InSync) } - width = max(width, ansi.StringWidth(labels[i])+1) + width = max(width, labelWidths[i]+1) } // a staged pin below the committed one moves the pin back @@ -224,12 +242,9 @@ func (m Model) spineLines(nodes []submodule.SpineNode, offPin bool) []string { var out []string for i, n := range nodes { - label, sha := muted, theme.Fg(t.Primary) - if hasRole(n, submodule.RoleStaged) { - label = theme.Fg(t.Info) - } + sha := theme.Fg(t.Primary) if offPin && len(n.Roles) == 1 && n.Roles[0] == submodule.RoleYou { - label, sha = theme.Fg(t.Warn), theme.Fg(t.Warn) + sha = theme.Fg(t.Warn) } node := sha.Render(t.Glyphs.Node) var cells string @@ -243,7 +258,7 @@ func (m Model) spineLines(nodes []submodule.SpineNode, offPin bool) []string { default: cells = lanes(node, " ") } - out = append(out, label.Width(width).Render(labels[i])+cells+" "+sha.Render(n.Commit.SHA)+" "+text.Render(n.Commit.Subject)) + out = append(out, labels[i]+strings.Repeat(" ", width-labelWidths[i])+cells+" "+sha.Render(n.Commit.SHA)+" "+text.Render(n.Commit.Subject)) if i < len(nodes)-1 { // the main lane starts below the fork's head, the fork's lane ends at the base main, side := " ", " " diff --git a/internal/tui/detail/spine_test.go b/internal/tui/detail/spine_test.go index 260667f..ef80190 100644 --- a/internal/tui/detail/spine_test.go +++ b/internal/tui/detail/spine_test.go @@ -1,7 +1,9 @@ package detail import ( + "fmt" "slices" + "strings" "testing" "github.com/charmbracelet/x/ansi" @@ -20,16 +22,16 @@ func TestSpineLines(t *testing.T) { Drift: submodule.Distance{Behind: 1}, } var got []string - for _, l := range m.spineLines(st.Spine(), true) { + for _, l := range m.spineLines(st.Spine(), true, "") { got = append(got, ansi.Strip(l)) } g := theme.Default().Glyphs want := []string{ - "tip " + g.Node + " 3f1bf20 Fix deps", - " " + g.Gap + " 4 commits", - "pin " + g.Node + " 9ac01de Add node", - " " + g.Gap + " 1 commit", - "you " + g.Node + " 48005ac Merge branch", + lab("tip") + g.Node + " 3f1bf20 Fix deps", + gap + g.Gap + " 4 commits", + lab("pin") + g.Node + " 9ac01de Add node", + gap + g.Gap + " 1 commit", + lab("you") + g.Node + " 48005ac Merge branch", } if !slices.Equal(got, want) { t.Errorf("got\n%q\nwant\n%q", got, want) @@ -46,7 +48,7 @@ func TestSpineLinesStagedMovesBack(t *testing.T) { CommittedDrift: submodule.Distance{Ahead: 15}, } var got []string - for _, l := range m.spineLines(st.Spine(), false) { + for _, l := range m.spineLines(st.Spine(), false, "") { got = append(got, ansi.Strip(l)) } g := theme.Default().Glyphs @@ -64,11 +66,11 @@ func TestSpineLinesInSync(t *testing.T) { m := New(theme.Default(), nil) c := &submodule.Commit{SHA: "3f1bf20", Subject: "Fix deps"} st := submodule.Status{Pinned: c, CheckedOut: c, Tip: c} - lines := m.spineLines(st.Spine(), false) + lines := m.spineLines(st.Spine(), false, "") if len(lines) != 1 { t.Fatalf("got %d lines, want 1", len(lines)) } - want := "in sync " + theme.Default().Glyphs.Node + " 3f1bf20 Fix deps" + want := lab("in sync") + theme.Default().Glyphs.Node + " 3f1bf20 Fix deps" if got := ansi.Strip(lines[0]); got != want { t.Errorf("got %q, want %q", got, want) } @@ -83,18 +85,43 @@ func TestSpineLinesFork(t *testing.T) { Bases: submodule.Bases{Tip: c("a1b2c3d", "Release 1.4")}, } var got []string - for _, l := range m.spineLines(st.Spine(), false) { + for _, l := range m.spineLines(st.Spine(), false, "") { got = append(got, ansi.Strip(l)) } g := theme.Default().Glyphs want := []string{ - "tip " + " " + g.Node + " e1f2a3b Drop Go 1.21", - " " + " " + g.Gap + " 2 commits", - "pin · you " + g.Node + " " + g.Gap + " c4d5e6f Experimental", - " " + g.Gap + " " + g.Gap + " 1 commit", - "base " + g.Node + g.Rule + g.Join + " a1b2c3d Release 1.4", + lab("tip") + " " + g.Node + " e1f2a3b Drop Go 1.21", + gap + " " + g.Gap + " 2 commits", + lab("pin · you") + g.Node + " " + g.Gap + " c4d5e6f Experimental", + gap + g.Gap + " " + g.Gap + " 1 commit", + lab("base") + g.Node + g.Rule + g.Join + " a1b2c3d Release 1.4", } if !slices.Equal(got, want) { t.Errorf("got\n%q\nwant\n%q", got, want) } } + +// lab pads a label like the label column; gap is the empty label column. +func lab(s string) string { return fmt.Sprintf("%-*s", labelWidth, s) } + +var gap = lab("") + +func TestSpineLinesTargetName(t *testing.T) { + m := New(theme.Default(), nil) + c := func(sha string) *submodule.Commit { return &submodule.Commit{SHA: sha, Subject: "s"} } + st := submodule.Status{ + Pinned: c("p"), CheckedOut: c("p"), Tip: c("t"), Pin: submodule.PinInfo{Behind: 1}, + TargetTip: c("m"), TargetDrift: submodule.Distance{Behind: 3}, + } + for target, want := range map[string]string{ + "origin/marco-dev": "marco-dev", + "origin/feature/a-very-long-branch-name": "feature/a-very-…", + "": "target", + } { + lines := m.spineLines(st.Spine(), false, target) + last := ansi.Strip(lines[len(lines)-1]) + if !strings.HasPrefix(last, want) { + t.Errorf("target %q: last line %q, want label %q", target, last, want) + } + } +} diff --git a/internal/tui/header/header.go b/internal/tui/header/header.go index 3642a07..8cb9d17 100644 --- a/internal/tui/header/header.go +++ b/internal/tui/header/header.go @@ -74,7 +74,7 @@ func View(t theme.Theme, repo *submodule.Repo, fetch Fetch, spinner string, widt } for _, k := range []submodule.IssueKind{ submodule.IssueUnpushed, submodule.IssueConflict, submodule.IssueMissing, - submodule.IssueOffPin, submodule.IssueChanged, + submodule.IssueOffPin, submodule.IssueChanged, submodule.IssueOffTarget, } { if n := perKind[k]; n > 0 { parts = append(parts, t.LevelStyle(level[k], bar).Render(fmt.Sprintf("%d %s", n, theme.IssueWord(k)))) diff --git a/internal/tui/theme/issues.go b/internal/tui/theme/issues.go index 36d58a4..f44956f 100644 --- a/internal/tui/theme/issues.go +++ b/internal/tui/theme/issues.go @@ -2,9 +2,11 @@ package theme import ( "fmt" + "strings" "time" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" "submarine/internal/submodule" ) @@ -27,13 +29,28 @@ func IssueWord(k submodule.IssueKind) string { return "conflict" case submodule.IssueOffPin: return "off pin" + case submodule.IssueOffTarget: + return "off target" default: return "changed" } } -// RoleWord is the label of a commit's role on the detail pane's spine. -func RoleWord(r submodule.Role) string { +// maxTargetWidth limits the branch name that labels the target on the spine. +const maxTargetWidth = 16 + +// ShortRef is a remote ref without its remote, e.g. "marco-dev" for +// "origin/marco-dev", cut to maxTargetWidth with an ellipsis. +func ShortRef(ref string) string { + if _, name, ok := strings.Cut(ref, "/"); ok { + ref = name + } + return ansi.Truncate(ref, maxTargetWidth, "…") +} + +// RoleWord is the label of a commit's role on the detail pane's spine; the +// target is labelled with the name of its branch. +func RoleWord(r submodule.Role, target string) string { switch r { case submodule.RolePin: return "pin" @@ -43,6 +60,11 @@ func RoleWord(r submodule.Role) string { return Staged case submodule.RoleBase: return "base" + case submodule.RoleTarget: + if target == "" { + return "target" + } + return ShortRef(target) case submodule.RoleTip: return "tip" default: @@ -118,6 +140,8 @@ func (t Theme) LevelStyle(l submodule.Level, base lipgloss.Style) lipgloss.Style return base.Foreground(t.Danger).Bold(true) case submodule.LevelWarn: return base.Foreground(t.Warn) + case submodule.LevelInfo: + return base.Foreground(t.Info) default: return base.Foreground(t.Muted) } diff --git a/internal/tui/tree/tree.go b/internal/tui/tree/tree.go index 2c66de1..cd83341 100644 --- a/internal/tui/tree/tree.go +++ b/internal/tui/tree/tree.go @@ -483,7 +483,7 @@ func (m Model) nodeLine(r row, nameColW, refW int) string { for _, c := range []struct { level submodule.Level n int - }{{submodule.LevelDanger, h.Danger}, {submodule.LevelWarn, h.Warn}} { + }{{submodule.LevelDanger, h.Danger}, {submodule.LevelWarn, h.Warn}, {submodule.LevelInfo, h.Info}} { if c.n == 0 { continue } diff --git a/scripts/make-fixture.sh b/scripts/make-fixture.sh index e9e4972..baf7d98 100755 --- a/scripts/make-fixture.sh +++ b/scripts/make-fixture.sh @@ -44,7 +44,7 @@ upstream() { mkdir -p "$root/remotes" "$root/work" for r in core ui net store log feature nested work; do remote "$r"; done -# "feature" has a branch that main doesn't contain +# "feature" has a branch that main doesn't contain (a pin there is off target) g "$root/work/feature" switch -c feat/search commit "$root/work/feature" search "Add search" commit "$root/work/feature" search "Tune search" @@ -72,7 +72,7 @@ g "$w" push origin main g "$root/work/log" submodule add "$root/remotes/nested.git" modules/nested g "$root/work/log" commit -m "Add nested" g "$root/work/log" push origin main -# "archive" ties with main (same tip, sorts first) -> the preferred ref wins +# "archive" contains the pin of log like main (same tip): another branch with it g "$root/work/log" push origin main:archive super="$root/super" @@ -113,7 +113,7 @@ g "$super" add libs/feature g "$super" commit -m "Pin feature search" # libs/topic: .gitmodules says main, but the pin is the tip of feat/search -# -> the closest branch wins over the configured one +# -> off target: the pin is on a feature branch, not on the configured one g "$super/libs/topic" fetch g "$super/libs/topic" checkout origin/feat/search g "$super" add libs/topic