diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1503fad..491df16 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -55,7 +55,7 @@ graph TD | `internal/proc` | `DetachTTY` — run probes without a controlling terminal; `KillGroup` — process-group SIGKILL (plain `Kill` on Windows) | | `internal/ui` | Lip Gloss styles, `PlaceOverlay`, `StripANSI` | | `internal/updater` | Detect the package manager that owns an installed binary and produce an update `Plan{Manager, Argv, Display}` | -| `internal/version` | Detect the installed version locally — `InstalledVersion(t) (ver, present)`; GitHub API with a 24-hour cache; semver comparison (`IsNewer`); keepkit's own release check (`SelfRepo`, `SelfLatest`) | +| `internal/version` | Detect the installed version locally — `InstalledVersion(t) (ver, present)`; GitHub API with a 24-hour cache; semver comparison (`IsNewer`) and the card's version spelling (`DisplayVersion`); keepkit's own release check (`SelfRepo`, `SelfLatest`) | `configdir`, `launcher`, `logx`, `proc`, `ui`, `updater` and `version` sit at the bottom of the import graph: they know nothing about the TUI (`ui`, `updater` and `version` reach only into @@ -169,9 +169,14 @@ Key invariants: name before the change and remap the index afterwards (`indexOfMeta`). - **Search is a transaction.** `/` remembers `searchPrevName`; `enter` commits the selection (focus moves to the card), `esc` rolls the cursor back to the previous tool. +- **The card has one value column.** Every `label: value` line in `[info]` and `[notes]` + pads its label through `cardLabel` to `cardLabelWidth` (the widest label the card can + print), the single definition of that column — `ui.MetaDetailLabelStyle` carries no + `Width`, or the two sections would drift apart again. Wrapped values (`note`, `tags`) + wrap to `inner - cardLabelWidth` and hang under the column via `hangIndent`. - **Card links are indexed, not parsed.** `buildCard()` returns the card text plus a `line → URL` map recorded while writing (line heights vary with wrapping), so a - click on the `repo:` line or the changelog release URL opens the browser. `handleMouse` + click on the `repo:` line (shown as the bare `owner/repo`, linked as the full ref) or the changelog release URL opens the browser. `handleMouse` rebuilds the map per click, which is why it can never describe stale content. - **A click's X picks the panel, `panelRow` decides whether it is on one at all.** The outer margin, the borders and the status bars share the panels' columns; with a diff --git a/CLAUDE.md b/CLAUDE.md index 72ba48a..d5b3642 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,7 @@ CI (`.github/workflows/ci.yml`) runs build / vet / `test -race` / golangci-lint | `internal/proc` | `DetachTTY` — runs tool probe subprocesses without a controlling terminal (`Setsid` on unix, `DETACHED_PROCESS` on Windows); `KillGroup` — process-group SIGKILL (negative pid; plain `Process.Kill` on Windows) for the update streamer's timeout path | | `internal/ui` | Lip Gloss styles and `PlaceOverlay` helper | | `internal/updater` | Detect the package manager that owns an installed binary and produce an update `Plan{Manager, Argv, Display}` (brew → go → cargo → pipx → uv → pnpm → bun → npm chain; `update_cmd` override always wins; on a `LookPath` miss **or an exhausted chain**, a brew-by-name fallback before giving up — see the `[u]` section). Bottom of the import graph like `version`: no TUI knowledge, depends only on `loader` for `Tool`. Pure `detectFromPath`/`brewNamePlanAt`/`managerDirsFrom`/`pnpmShimTarget` cores + OS-facing `Detect`/`brewNamePlan`/`resolveManagerDirs`/`readPnpmShim` wrappers; `testHomeDir` and `testBrewPrefix` seams (the latter a deliberate duplicate of `version`'s — two bottom leaves that may not import each other) | -| `internal/version` | Detect installed version locally — `InstalledVersion(t) (ver string, present bool)`, the two results independent so the card can tell "installed but won't say its version" from "not installed". Sources in order: `--version`/`-V`, then `brewDirVersion` in `brew.go` (reads the version from the `Caskroom/`/`Cellar/` directory names — no brew subprocess — so casks with no version CLI still resolve), then `cargoListVersion`/`cargoVersionFromList` (same idea one ecosystem over: `cargo install --list` names every cargo-installed crate's version without running its binary; gated on the binary existing, and `LookPath("cargo")` short-circuits before any subprocess). A fallback hit suppresses the anomaly log; `testBrewPrefix` seam. Also: fetch latest release, repo card, changelog and README from the GitHub API with a 24h cache; semver comparison (`IsNewer`); keepkit's own self-check (`selfcheck.go`: `SelfRepo`, `SelfLatest`) | +| `internal/version` | Detect installed version locally — `InstalledVersion(t) (ver string, present bool)`, the two results independent so the card can tell "installed but won't say its version" from "not installed". Sources in order: `--version`/`-V`, then `brewDirVersion` in `brew.go` (reads the version from the `Caskroom/`/`Cellar/` directory names — no brew subprocess — so casks with no version CLI still resolve), then `cargoListVersion`/`cargoVersionFromList` (same idea one ecosystem over: `cargo install --list` names every cargo-installed crate's version without running its binary; gated on the binary existing, and `LookPath("cargo")` short-circuits before any subprocess). A fallback hit suppresses the anomaly log; `testBrewPrefix` seam. Also: fetch latest release, repo card, changelog and README from the GitHub API with a 24h cache; semver comparison (`IsNewer`) and the card's version spelling (`DisplayVersion`); keepkit's own self-check (`selfcheck.go`: `SelfRepo`, `SelfLatest`) | The `model` package is split by responsibility (one package, several files): @@ -88,9 +88,10 @@ The model is a three-panel layout with focus cycling via `→/←` between `focu - **Tool-list search (`/` in `focusTools`)** is a commit/rollback transaction over `modeSearch`. `case "/"` captures `m.searchPrevName` (the selected tool's name; empty when the list is empty) before entering the mode, and `filteredMeta()` narrows the list live as the query changes. The predicate is `searchMatches()` (`model.go`): a tool matches when its **name OR its tag** contains the lowercased query (`matchingTag` still iterates the slice — it predates the one-tag invariant and stays correct under it), returning `[]searchMatch{meta, byTagOnly, tag}` so the renderer knows which rows matched only by tag; `filteredMeta()` is a thin projection over it, so all other callers (count, selection, cursor remap) keep seeing plain metas. While searching, the matched name substring renders peach-bold via `highlightNameMatch` (render.go — distinct from `highlightMatch` in textutil.go, which belongs to the help search), tag-only rows show the earning tag as a dim `#` suffix when it fits the row budget without wrapping, and the search status bar shows an `N/M` counter (matches / total tracked) between the query and the hints (a keystroke that changes the query text resets `metaSelected` to 0 — first match highlighted, marker visible during search; pure cursor movement like `left`/`right` keeps a user-moved highlight; that reset repaints `[3]` through **`setHelpContent()`**, not a bare `SetContent(renderHelpContent())`: the readme branch serves `m.helpBase`, which nothing else re-renders, so the cheaper call would leave the *previous* tool's README on screen under the new tool's name — no fetch is fired there, one per keystroke would spend the quota on rows merely typed past). Inside the mode: `↑`/`↓` move the highlight through the filtered list via `selectMeta` (modular wrap, full `j`/`k` parity; **never** forwarded to the textinput, so the query text is untouched — with zero matches they are consumed as no-ops); `enter` commits — exits to `modeNormal`, clears the query, remaps the cursor onto the unfiltered (but still update-grouped) list by name via `indexOfMeta(mt.Name)` — the search filter is gone once the mode is normal, but the update grouping is not, so `indexOfMeta` resolves the **displayed** index — and moves focus to `focusBrief` (no matches → no-op, search stays open); `esc` rolls back — unfiltered list with the cursor restored via `indexOfMeta(m.searchPrevName)` (fallback 0 when that tool was untracked mid-search). Both exits clear `searchPrevName` and go through `selectMeta`, so the help panel is re-synced too (an arrow move may have loaded another tool's help mid-search). `indexOfMeta(name)` lives next to `filteredMeta()` in `model.go`; the status bar echoes the live query plus the `N/M` counter and `[enter] open [↑/↓] move [esc] cancel` hints. - **Central panel actions (`focusBrief`)** operate on the data the card already shows: `o` opens the repo in the browser, `c` opens the changelog/releases page, `r` force-refreshes the tool's data, `s` cycles the status (`loader.NextStatus`: `active → trying → inactive`, unknown values fall back to active), `e` edits the note, `t` edits the tool's single tag. `o`/`c` go through `openURLCmd` (resolved per-`GOOS` by `browserCommand`); a tool with no `GitHub` sets `m.statusMsg` instead of launching. `s`/`e`/`t` mutate `m.meta` via `loader.UpsertMeta`, persist with `loader.SaveMeta`, then refresh the card with `m.briefViewport.SetContent(m.renderCard())`. The tags editor commits through `parseTag` (mode.go): the input is **one** tag — everything past the first comma is dropped, so typing `cli, foo` and loading a legacy `[cli, foo]` list both land on `cli` and the editor can never disagree with `LoadMeta`'s `Tags[:1]` migration about a tag's shape. Spaces inside a tag are kept (`dev tools`); empty input clears it to `nil`, which `omitempty` drops from `meta.yaml`. Everything downstream reads the one tag through `tagOf(mt)` rather than joining the slice. -- **Clickable card lines**: `renderCard()` is a thin wrapper over **`buildCard() (string, map[int]string)`**, which returns the card text plus the index of its clickable lines (0-based content line → URL) — the `repo:` line (`https://` + `t.GitHub`, exactly what `[o]` opens) and the changelog block's leading release URL (`msg.htmlUrl` **verbatim** — note this is the release's own page, unlike `[c]`'s `/releases`), registered only when the block actually starts with it (a loading/failed/URL-less changelog emits no such line, and registering it unconditionally would map onto body text). Indices are recorded *while writing* (`strings.Count(sb.String(), "\n")` immediately before the line), because line heights vary — dividers are 3 lines and about/note/changelog bodies wrap. The wrapper keeps the ~30 `SetContent(renderCard())` call sites unchanged; `handleMouse` is the only consumer of the map and recomputes it per click, so it can never be stale. A **content line is a screen row** here: the viewport *truncates* a line wider than the panel (a release URL regularly is one) rather than soft-wrapping it, which is what lets a click row map straight onto a content-line index — `TestBriefContentLineIsScreenRow` pins that, because a bubbles that wrapped instead would shift every link below an overlong line. No visual styling marks the links (no underline/bold) — deliberate. +- **Clickable card lines**: `renderCard()` is a thin wrapper over **`buildCard() (string, map[int]string)`**, which returns the card text plus the index of its clickable lines (0-based content line → URL) — the `repo:` line (`https://` + `t.GitHub`, exactly what `[o]` opens — the **link is the full ref while the displayed value is the bare `owner/repo`** from `loader.NormalizeRepo`, the host being implied and its 11 cells being what the label column spends; a ref `NormalizeRepo` rejects — an unsupported or spoofed host like `github.com.evil.com/x/y` — renders in full instead, since shortening exactly there would hide the host that makes the link not what it looks like) and the changelog block's leading release URL (`msg.htmlUrl` **verbatim** — note this is the release's own page, unlike `[c]`'s `/releases`), registered only when the block actually starts with it (a loading/failed/URL-less changelog emits no such line, and registering it unconditionally would map onto body text). Indices are recorded *while writing* (`strings.Count(sb.String(), "\n")` immediately before the line), because line heights vary — dividers are 3 lines and about/note/changelog bodies wrap. The wrapper keeps the ~30 `SetContent(renderCard())` call sites unchanged; `handleMouse` is the only consumer of the map and recomputes it per click, so it can never be stale. A **content line is a screen row** here: the viewport *truncates* a line wider than the panel (a release URL regularly is one) rather than soft-wrapping it, which is what lets a click row map straight onto a content-line index — `TestBriefContentLineIsScreenRow` pins that, because a bubbles that wrapped instead would shift every link below an overlong line. No visual styling marks the links (no underline/bold) — deliberate. - **Card changelog body**: the `[changelog]` section renders the release notes through **`markdownToLines(body, max(m.briefW-2, 10))`** (textutil.go), which replaced a `stripMarkdown` + `wrapText` pair that destroyed the markdown instead of respecting it — it ate list markers (`strings.Trim(line, "*_")` took a leading bullet), left `[text](url)` raw, and its `<…>` HTML strip swallowed `` autolinks whole. The converter is **one line pass** over the body carrying two state flags (inside a fenced block, inside an HTML comment) and returns **pre-wrapped** `mdLine{text, kind}` values: wrapping lives *inside* it because hanging indents need the block structure, and because styling must land on whole finished lines — `wrapLine` counts runes and knows nothing about ANSI. `renderChangelogBlock` is then a trivial consumer: `mdHeading` → `ui.ChangelogHeadingStyle` (bold `ColorText`, one emphasis step above the muted body and deliberately **not** a new color, which would compete with the peach `SectionLabelStyle` heading the card's own sections), everything else → `InfoStyle`, and a blank line written as a bare `"\n"` (styling an empty string only emits an empty escape pair). There are **two kinds only** — no `mdCode`: the sole consumer would have no use for it, so everything that makes code special (verbatim, no inline processing, no wrap, a 2-space indent) lives inside the converter, driven by the fence flag. Rules worth remembering: CRLF is normalized **first** (GitHub bodies routinely arrive CRLF from the web form; the old code was accidentally saved by its per-line `TrimSpace`, and here a `\r`-suffixed closing fence would fail to match and swallow the rest of the body as code); a heading and a list marker both **require the space** after the marker, so `#123 fixed …` stays an issue ref and `**Breaking**` stays a paragraph — the old `TrimLeft(line, "#")`/`Trim(line, "*_")` bug class cannot come back; `---`/`***`/`___` **and** a `===` run are rule lines that collapse to a blank and are **never** list items (`---` is the stock separator above "Full Changelog", and a bare `===` left in place put a literal row of equals signs in the card — full setext support, retagging the paragraph above as a heading, is deliberately out: the text and the layout stay right, only the emphasis is lost); the code fence deliberately accepts **any** indent, unlike CommonMark's 3-space limit, because 4+ spaces there means an indented code block, which this converter does not implement — the strict form only mis-read a fence nested under a list item (`1. ` content aligns at column 4), leaking the language tag out as a body line reading `go` and dropping the sample's verbatim treatment; a heading and a paragraph share **`mdEmitInline`**, so a line whose markup collapses to nothing (a badge-only line, a bare `
`, a heading whose only content was an image) can only ever become a blank through `emitBlank` — an empty line still tagged `mdHeading` would sit outside the collapse and hand the next reader of `kind` a line that is not a heading; bullets normalize to `•` (U+2022 — East-Asian **Ambiguous**, the same accepted class as `⏺`/`↑`/`─`, and it never enters the wrap math, which is rune-based) with nesting clamped to 2 levels of 2 spaces and continuations hanging under the first text column; inline order is load-bearing (images → links → **autolinks before the HTML-tag strip** → emphasis), and the underscore emphasis pattern requires a non-word rune outside both delimiters or `update_cmd` would silently lose its underscore. An empty conversion result falls through to the existing `no release notes available.` branch, which therefore now also covers a **non-empty** body the converter consumed whole (all comments, all separators). The leading release-URL line is written before any of this, so `buildCard`'s clickable-line index still finds it exactly where it did. The conversion is memoized in **`m.changelogRender`** (`changelogRenderCache` in render.go, one entry keyed by `(body, width)` — `markdownToLines`'s only two inputs), the same shape as `readmeRenderCache` and for the same reason: the whole card is rebuilt on **every spinner frame** (the `spinner.TickMsg` handler, ~12/s for as long as a `[r]` refresh or a `[u]` update runs), so a large release body would go through the converter's regexes twelve times a second to animate one glyph — measured at 3.3 ms and 1 MB of garbage per pass on a 49 KB body against 15 ns on a hit. It hangs off `Model` as a **pointer**, because the card renderers are value receivers and could not fill a plain field, and its method **tolerates a nil receiver** (a cache-less but correct mode) since most tests build `Model{}` literals and never call `New()`. -- **Card versions**: `renderCard`'s `[info]` section shows `installed:` from `m.versions[name].Installed` — all four states (`installed: \uf412 `, `installed: detecting…`, `installed: ✓ no version`, `installed: ✕ not installed`) in the section's muted `InfoStyle`. The two version-less states split what `InstalledKnown` alone used to collapse, on `InstalledPresent`: a tool that is installed but won't name its version (a ratatui app that ignores `--version`) is a working install and reads `✓` (U+2713) in `ui.OkStyle` — green, deliberately not `UpdateAvailableStyle`, whose orange means "act on this" — while a genuine absence keeps the `✕` (U+2715) in `DangerStyle`, the same red glyph/style pair the `[L]` overlay uses for exhaustion. The section's gate includes `installed != ""`, so a locally installed tool with no `GitHub` ref still renders it. When `hasUpdate(name)` (installed older than latest per `version.IsNewer`) the `latest:` value renders in `UpdateAvailableStyle` with a ` ↑` suffix — the same glyph the tools list appends to the row. The `latest:` line is gated on `card.Latest != ""` (a repo with no release shows no line) and appends the release date as a ` (YYYY-MM-DD)` suffix derived from `card.PublishedAt`. **Both version lines carry the `\uf412` glyph** (nf-oct-tag) before the version — its meaning is "a version", not specifically "a release tag", which is why it widened from `latest:` to `installed:`; the three states with no version to tag (`detecting…`, `✓ no version`, `✕ not installed`) stay bare. Written as a `\u` escape in source and prose alike: the raw glyph is invisible in most editors and diffs and gets silently lost. Versions live in the card only; the 30-column list never shows version strings. +- **Card label column**: every `label: value` line the card prints — `[info]`'s `repo`/`stars`/`installed`/`latest`/`languages`/`maintenance` and `[notes]`'s `status`/`note`/`tags` — pads its label through **`cardLabel`** to the single **`cardLabelWidth`** (`len("maintenance:") + 1`, the widest label the card can print), so every value starts in one column instead of each label pushing its own value to its own indent. That constant is the **only** definition of the column: `ui.MetaDetailLabelStyle` deliberately carries no `Width` anymore — it used to pad the `[notes]` labels to 8 while `[info]` had no padding at all, which is exactly what made the two sections disagree — and the `[info]` labels could not use a style even if it had one, since label and value are a single `Render` call there. A value's leading marker (the `\uf412` version glyph, `✓`/`✕`, the status `●`) occupies the head of the column, so a marked value's *text* starts two cells further while its marker still lands on the column; that is the only intended step in the card. The two wrapped values (`note`, `tags`) wrap to `inner - cardLabelWidth` rather than `inner` — the old full-width wrap ran the first line `cardLabelWidth` cells past the panel edge, where the viewport truncates it (it does not soft-wrap) — and their continuation lines go through **`hangIndent`**, which pushes them under the value column instead of column 0; the indent is inserted *after* styling, between finished lines, so it stays unstyled and no escape sequence is ever split. The column costs 13 of the 28 cells the brief panel has at the 80×24 baseline, which is why the `repo:` value now drops its host (see **Clickable card lines**) — printed in full it was cut mid-name there. The `latest:` line with its date suffix still overflows at that width, as it did before. `TestCardValuesShareOneColumn` walks every `label:` line of a fully-populated card and fails on any that starts its value elsewhere — its completeness check is what keeps the fixture covering every gated line — and `TestCardNoteWrapsUnderItsLabel` pins the wrap width and the hanging indent. +- **Card versions**: `renderCard`'s `[info]` section shows `installed:` from `m.versions[name].Installed` — all four states (`installed: \uf412 `, `installed: detecting…`, `installed: ✓ no version`, `installed: ✕ not installed`) in the section's muted `InfoStyle`, each label padded to the shared value column by `cardLabel` (see **Card label column**). The two version-less states split what `InstalledKnown` alone used to collapse, on `InstalledPresent`: a tool that is installed but won't name its version (a ratatui app that ignores `--version`) is a working install and reads `✓` (U+2713) in `ui.OkStyle` — green, deliberately not `UpdateAvailableStyle`, whose orange means "act on this" — while a genuine absence keeps the `✕` (U+2715) in `DangerStyle`, the same red glyph/style pair the `[L]` overlay uses for exhaustion. The section's gate includes `installed != ""`, so a locally installed tool with no `GitHub` ref still renders it. When `hasUpdate(name)` (installed older than latest per `version.IsNewer`) the `latest:` value renders in `UpdateAvailableStyle` with a ` ↑` suffix — the same glyph the tools list appends to the row. The `latest:` line is gated on `card.Latest != ""` (a repo with no release shows no line) and appends the release date as a ` (YYYY-MM-DD)` suffix derived from `card.PublishedAt`. **Both version lines carry the `\uf412` glyph** (nf-oct-tag) before the version — its meaning is "a version", not specifically "a release tag", which is why it widened from `latest:` to `installed:`; the three states with no version to tag (`detecting…`, `✓ no version`, `✕ not installed`) stay bare. Written as a `\u` escape in source and prose alike: the raw glyph is invisible in most editors and diffs and gets silently lost. **Both versions are printed through `version.DisplayVersion`**, which puts a `v` in front of a bare version number: a tool's `--version` prints `1.10.2` where its release is tagged `v1.10.2`, and the two lines sat one letter apart for the same binary. It edits nothing else — `canonSemver` decides only *whether* the string is a version number (`nightly`, `cli-2.0` pass through untouched), and its own output is deliberately not what is displayed, since it drops zero-padding (`2024.01.15` → `v2024.1.15`), a 4th segment and build metadata, and the card must show the version the tool actually reported. Versions live in the card only; the 30-column list never shows version strings. - **Panel titles**: all three panels inset a title into their top border (`┌─ [1] Tools ─…─┐`) via the shared `insetPanelTitle` (render.go) — an ANSI-safe splice over the already-rendered frame (`ui`'s `truncateVisible` is unexported; the helper remeasures with `stripANSI` and rebuilds the top line), colored like the border (focus-aware) and dropped whole when the panel is too narrow. The titles are `[1] Tools`, `[2] Brief` and `[3] Readme` / `[3] Help` / `[3] Man` (a `switch m.helpMode`, overridden by `[3] Update` while a live log shows) — they double as the documentation for the digit focus hotkeys, so the status bar carries no digit hints. All title characters are single-width and non-East-Asian-Ambiguous, keeping the border width math stable. - **Refresh (`r` in `focusBrief`)**: `refreshSelectedCmd(t)` force-refreshes the selected tool bypassing the 24h cache TTL — the repo pass (`refreshRemoteCmd` → `version.RefreshRepoData`) + changelog (`refreshChangelogCmd` → `version.RefreshChangelog`) + README (`refreshReadmeCmd` → `version.RefreshReadme`, preceded by a `delete(m.readmeData, name)` so a session-cached 404/rate-limit negative can recover, then a `markReadmeLoading(name)` — the deletion makes `needsReadme` true again for the whole in-flight window, so without the marker leaving and re-entering the tool would spend a second request; `refreshingFor` does *not* cover it, since `remoteMsg` clears that flag as soon as the repo pass lands, which can be well before the README does) + a local installed re-detect (`fetchInstalledCmd`). It emits the same `remoteMsg`/`changelogMsg` as the startup path, so the merge/re-render logic is reused. While the repo pass is in flight `m.refreshingFor` (the tool name) turns the card title into a status line — `refreshing data ` (`bubbles/spinner`, `MiniDot`; the about is hidden) — with no status-bar takeover; the `remoteMsg` handler clears `refreshingFor` on completion, which reverts the title to name+about and halts the `spinner.TickMsg` loop. `refreshingFor` doubles as the double-press guard; a tool with no `GitHub` only re-detects the installed version (`m.statusMsg = "no repo to refresh"`, no spinner). Note the same `case "r"` branches on focus three ways: rename in `focusTools`, refresh in `focusBrief`, README mode in `focusHelp`. - **Update (`u` in `focusBrief`)**: installs a newer release from inside the TUI. `[u]` fires only in `focusBrief` (in `focusTools` `u` stays untrack — same `case "u"`, branched on focus); it requires `hasUpdate(name)` (else `statusMsg`) and reports the shared `updateBusyStatus` (`another update is running`) while `updatingFor != ""` instead of starting a second one (one update at a time, no queue; the same wording `[U]` uses, since the running update's only other sign is a card spinner invisible unless that tool is selected). The key fires `detectUpdateCmd(t, false)` — detection spawns subprocesses (`go version -m`, `cargo install --list`) so it must never run inside `Update()`, same as every other probe. `updater.Detect` runs the brew → go → cargo → pipx → uv → pnpm → bun → npm chain. **Order is load-bearing twice**: brew before go, so a brew-installed Go binary with buildinfo isn't misrouted, and pnpm/bun before npm, because both layouts carry `node_modules` segments the npm step claims on sight — a bun global (`$BUN_INSTALL/bin/` → `install/global/node_modules//…`) really did resolve to `npm install -g `, the wrong manager, installing a duplicate under npm's prefix that the bun copy keeps shadowing on `PATH`. Five steps are **path-convention** based (cargo, pipx, uv, pnpm, bun), and an empty root switches its own step off — enforced **inside `underDir`/`segmentUnder`**, which answer "no match" for an empty dir, deliberately *not* by a `!= ""` guard repeated at each step. The hazard is real: `filepath.Rel("", "bin/exa")` succeeds and yields `bin/exa`, so a **relative** path reads as living under every disabled root, and one forgotten copy of the guard silently claims it. It lived as six hand-copied copies of that one convention until the review that turned it into one definition — the same lesson as `version.applyReleaseOutcome` (`TestPathHelpersRejectEmptyDir`, plus a per-step `empty leaves a relative path undetected` row): `uv` = `segmentUnder(realPath, uvTools)` → `uv tool upgrade `; `pnpm` = `underDir(realPath, pnpmHome)` plus a name from the shim target or the resolved path → `pnpm add -g `; `bun` = `underDir(realPath, bunInstall)` + `npmPackage(realPath)` → `bun add -g `. **`add -g`, not `update -g`**: `update` honours the semver range saved at install time and can silently refuse a major bump, while keepkit promises the version the card shows as `latest:`. A manager's own binary carries no `node_modules` segment and no shim target, so it falls through — `bun upgrade`/`pnpm self-update` are deliberately out of scope. **All five roots** come from **`managerDirsFrom(getenv, home, goos)`** (pure core, `resolveManagerDirs()` wrapper — the `launcher.planFor` idiom), and carrying cargo/pipx there too is what lets `detectFromPath` stop calling `homeDir()`, making its "no I/O, no environment" contract literally true instead of nearly true. cargo and pipx stay home-derived with **no env var of their own** — `$CARGO_HOME`/`$PIPX_HOME` are not consulted, exactly as before the struct existed, since honouring them changes behaviour for anyone who sets them and belongs in its own commit. `/.cargo/bin` and `/.local/pipx/venvs`; `$UV_TOOL_DIR` else `$XDG_DATA_HOME/uv/tools` else `~/.local/share/uv/tools` (macOS included — uv is XDG there too; **Windows env-only**, its layout is unverified and a wrong guess beats no guess only in the wrong direction), `$PNPM_HOME` else `~/Library/pnpm` (darwin) / `$XDG_DATA_HOME/pnpm` else `~/.local/share/pnpm` (linux) / `%LOCALAPPDATA%\pnpm` (windows), `$BUN_INSTALL` else `~/.bun`. An empty field is a *disabled* check, which is what keeps the zero-value `managerDirs{}` backwards compatible. The wrapper then **expands symlinks in every one of the five roots** (`resolveDir` → `filepath.EvalSymlinks`, falling back to the raw path when it does not resolve — a root that does not exist yet is the normal state for an uninstalled manager and must stay a harmless *non-match* rather than becoming an empty, i.e. disabled, field). That expansion is load-bearing, not hygiene: `Detect` compares these roots against an `EvalSymlinks`-**resolved** binary path, so a root carrying any symlink component — a relocated `~/.bun` → `/mnt/big/bun`, a home on a secondary volume, `/home` under autofs, and on macOS `/var` → `/private/var`, which is what made every fixture test here need `EvalSymlinks(t.TempDir())` — silently fails to match. For uv and pnpm's shim/store layouts that only costs the update offer, but a bun global and a legacy pnpm one still carry a plain `node_modules/` segment, so the npm step claims them and offers `npm install -g ` — the exact duplicate-install misdetection this chain exists to prevent, measured on all five layouts and pinned by `TestDetectSymlinkedManagerRoot`. The expansion is a **loop over pointers to all five fields**, so a sixth field added to `managerDirs` but forgotten there would resolve to a root nothing can ever match; `TestResolveManagerDirsExpandsSymlinks` asserts all five at once as the guard. **pnpm needs a second signal** because its globals are not symlinks at all: `$PNPM_HOME/bin/` is a cmd-shim `/bin/sh` script `EvalSymlinks` resolves to itself, whose last line — `# cmd-shim-target=` — is the only machine-readable link to the owning package. `Detect` reads it (best-effort, capped at `pnpmShimMaxBytes` = 8 KiB against real shims of ~1.5 KiB, and **only** when the found path sits under a non-empty `pnpmHome` — reading every binary on `PATH` to look for a comment would be an open per detection) through `readPnpmShim` → the pure `pnpmShimTarget` (last marker wins: cmd-shim writes it as the final line). A file **over** the cap is rejected whole (`LimitReader(cap+1)` + a length check, `getReadme`'s idiom) rather than parsed truncated, and that is a correctness guard, not tidiness: a cut landing inside the marker line hands the parser a *shortened* path, and `…/node_modules/typescript/…` shortened to `…/node_modules/types` yields `types` — a real package on npm, so the chain would offer a confidently wrong `pnpm add -g types` where everything else here degrades honestly (`TestReadPnpmShim`'s oversized row, whose fixture asserts the truncated parse *would* have said `types`), and passes it into the core as `shimTarget` exactly the way `goBuildinfo` rides in; an unreadable file yields `""`, i.e. the check simply has no signal. Two npm-side consequences of the pnpm layout: `npmPackage` **refuses a bare scope** (`…/node_modules/@angular` with nothing after it is a directory holding packages, not one — returning it offered `pnpm add -g @angular`, and the pnpm step feeds this function a *shim target*, i.e. file content nothing validated) and **skips a post-`node_modules` segment starting with `.`** (`.pnpm` is the virtual store, `.bin` the shim dir — never package names) and keeps scanning, and the **npm step refuses any path through `/node_modules/.pnpm/`** — such a path reaching npm means a pnpm layout the pnpm step failed to attribute, and `npm install -g ` there is the duplicate-install failure again, so the chain falls through to `ErrUnknownManager` + the `update_cmd` hint (honest degradation). Detection is convention-based end to end: a future layout change makes its check miss *silently* and degrade to the hint, never to a wrong command. A `update_cmd` in `meta.yaml` always wins and runs via `sh -c` (skips detection entirely). The chain starts from `exec.LookPath(t.Name)`, and a **miss there is not the end**: the tracked name can still be a brew formula whose binaries are named differently — `rust` ships `rustc`/`cargo`, so `LookPath("rust")` misses while `brew upgrade rust` is exactly right — so `brewNamePlan` checks for a `Cellar/`/`Caskroom/` directory before `ErrUnknownManager` is returned. **An exhausted chain takes the same fallback**: a binary can be found and still belong to no known manager — a Homebrew cask app keeps its launcher on `PATH` while the executable lives inside the `.app` bundle (`agterm`), matching neither the Cellar regex nor any later step — so `Detect` retries `brewNamePlan` on `ErrUnknownManager` from `detectFromPath` too (`TestDetectBrewByNameChainExhausted`). Both sites are self-validating (they fire only when such a keg actually exists) and share `version.brewDirVersion`'s traversal guard: a name carrying `/` or `\` can't be a formula name and must not turn the `Join` into a traversal. The `updateDetectedMsg` handler drops a stale result through the shared predicate **`acceptsUpdateDetect(msg)`**: both paths refuse while `updatingFor != ""` and while `m.mode != modeNormal` (detection spawns subprocesses and can answer seconds later — a confirm dialog opening under an editor, a search or an overlay steals the keystroke aimed at it, the mirror of `launchDoneMsg`'s mode gate), and beyond that a *tool* result must still match the selection while keepkit's own has no selection to match. It maps `ErrUnknownManager` to a `statusMsg` hint (`no known updater for — set update_cmd or [o] releases` — no dead-end dialog; the wording branches on **`isSelfUpdate(msg.tool)`**, not on `msg.self`, so the identical failure of the identical binary reads the same from `[u]` on a tracked `keepkit` row as from `[U]` — `msg.self` keeps the one meaning only it has, "no selection to match", inside `acceptsUpdateDetect`), and on success stores `m.updatePlan` plus **`m.updateTarget = msg.tool`** and enters `modeConfirmUpdate`. The target is resolved *there*, not when enter is pressed: a selection that moved while detection ran can no longer retarget the dialog, and keepkit's own update — which has no row, and no selection at all with an empty tracker — needs no second identity. The confirm status bar shows `update : [enter] run [esc] cancel`; `enter` sets `m.updatingFor`/`m.updateLogFor` to that target, resets the log, and fires `startUpdateCmd` (any other key cancels, not just `esc`, and clears `updateTarget` with the plan it named). The `[u] update` hint is prepended to the `focusBrief` bar only when `hasUpdate(selected)`. diff --git a/internal/model/cardlinks_test.go b/internal/model/cardlinks_test.go index 1c1ee17..2884573 100644 --- a/internal/model/cardlinks_test.go +++ b/internal/model/cardlinks_test.go @@ -13,6 +13,7 @@ import ( const ( linkRepo = "github.com/cli/cli" + linkShort = "cli/cli" linkRelURL = "https://github.com/cli/cli/releases/tag/v2.0.0" ) @@ -48,7 +49,7 @@ func TestBuildCardLinks(t *testing.T) { { name: "repo line only while the changelog loads", setup: func(m *Model) { m.changelogLoadingFor = "gh" }, - want: map[string]string{"https://" + linkRepo: "repo: " + linkRepo}, + want: map[string]string{"https://" + linkRepo: cardLabel("repo:") + linkShort}, }, { name: "changelog url linked alongside the repo", @@ -56,7 +57,7 @@ func TestBuildCardLinks(t *testing.T) { m.changelogData["gh"] = changelogMsg{toolName: "gh", htmlUrl: linkRelURL, body: "release notes"} }, want: map[string]string{ - "https://" + linkRepo: "repo: " + linkRepo, + "https://" + linkRepo: cardLabel("repo:") + linkShort, linkRelURL: linkRelURL, }, }, @@ -65,14 +66,14 @@ func TestBuildCardLinks(t *testing.T) { setup: func(m *Model) { m.changelogData["gh"] = changelogMsg{toolName: "gh", htmlUrl: linkRelURL, err: errors.New("boom")} }, - want: map[string]string{"https://" + linkRepo: "repo: " + linkRepo}, + want: map[string]string{"https://" + linkRepo: cardLabel("repo:") + linkShort}, }, { name: "release without an html url", setup: func(m *Model) { m.changelogData["gh"] = changelogMsg{toolName: "gh", body: "release notes"} }, - want: map[string]string{"https://" + linkRepo: "repo: " + linkRepo}, + want: map[string]string{"https://" + linkRepo: cardLabel("repo:") + linkShort}, }, { name: "update marker and wrapped about shift both lines", @@ -85,7 +86,7 @@ func TestBuildCardLinks(t *testing.T) { m.changelogData["gh"] = changelogMsg{toolName: "gh", htmlUrl: linkRelURL, body: "release notes"} }, want: map[string]string{ - "https://" + linkRepo: "repo: " + linkRepo, + "https://" + linkRepo: cardLabel("repo:") + linkShort, linkRelURL: linkRelURL, }, }, @@ -136,6 +137,42 @@ func TestBuildCardLinks(t *testing.T) { } }) + // The repo line shows the bare owner/repo but links the full ref, so the + // two must not be assumed equal — and a ref NormalizeRepo rejects (an + // unsupported or spoofed host) must render in full, or the display would + // hide the host that makes the link not what it looks like. + t.Run("shortened display, full link", func(t *testing.T) { + m := linkedCardModel(t, linkRepo) + content, links := m.buildCard() + for line, url := range links { + if url != "https://"+linkRepo { + continue + } + got := cardLine(content, line) + if !strings.Contains(got, cardLabel("repo:")+linkShort) { + t.Errorf("repo line = %q, want the bare %q", got, linkShort) + } + if strings.Contains(got, "github.com") { + t.Errorf("repo line = %q, want the host dropped from the display", got) + } + } + + spoofed := "github.com.evil.com/cli/cli" + m = linkedCardModel(t, spoofed) + content, links = m.buildCard() + if len(links) != 1 { + t.Fatalf("links = %v, want the repo line linked", links) + } + for line, url := range links { + if url != "https://"+spoofed { + t.Errorf("link = %q, want the raw ref %q", url, "https://"+spoofed) + } + if got := cardLine(content, line); !strings.Contains(got, spoofed) { + t.Errorf("repo line = %q, want the unshortenable ref shown in full", got) + } + } + }) + t.Run("no repo, no links", func(t *testing.T) { m := linkedCardModel(t, "") if _, links := m.buildCard(); len(links) != 0 { diff --git a/internal/model/render.go b/internal/model/render.go index 8aaef00..53978fc 100644 --- a/internal/model/render.go +++ b/internal/model/render.go @@ -1138,6 +1138,32 @@ func scrollColumn(vp viewport.Model, focused bool) string { return strings.Join(rows, "\n") } +// cardLabelWidth is the card's value column: every "label: value" line in the +// card — [info] and [notes] alike — pads its label to it, so the values line +// up in one column instead of each label pushing its own value to a different +// indent. It is the widest label buildCard can print ("maintenance:", 12) plus +// the separating space, so no line ever needs more room than the column gives. +// A label longer than this would still render (cardLabel never truncates), it +// would just push its own value one column past the rest. +const cardLabelWidth = len("maintenance:") + 1 + +// cardLabel pads a card label to cardLabelWidth, separator included — it is +// the single definition of the value column, so a label added to either +// section lands in it for free (ui.MetaDetailLabelStyle deliberately carries +// no Width of its own anymore, or the two would drift). +func cardLabel(label string) string { + return label + strings.Repeat(" ", max(cardLabelWidth-utf8.RuneCountInString(label), 1)) +} + +// hangIndent pushes every line after the first to the value column, so a +// wrapped note or tag reads as one block under its label instead of falling +// back to column 0. Applied to already-styled text: each rendered line carries +// its own escape sequences, so inserting plain spaces between them is safe and +// keeps the indent itself unstyled. +func hangIndent(s string) string { + return strings.ReplaceAll(s, "\n", "\n"+strings.Repeat(" ", cardLabelWidth)) +} + // renderCard is the card text alone — the shape ~30 SetContent call sites want. // The clickable-link index is built by buildCard and only handleMouse needs it. func (m Model) renderCard() string { @@ -1210,15 +1236,27 @@ func (m Model) buildCard() (string, map[int]string) { if hasInfo { sb.WriteString(m.sectionDivider("info")) if t.GitHub != "" { - // Clickable: resolves exactly like the [o] key does. + // The value shows the bare owner/repo: the host is implied, and its + // 11 cells are worth more than the label column costs — at the + // 80-column baseline the panel is 30 wide, where the full ref was + // cut mid-name. NormalizeRepo answering "" (an unsupported or + // spoofed host, e.g. "github.com.evil.com/x/y") falls back to the + // raw value on purpose: shortening there would hide the very host + // that makes the link not what it looks like. + repoText := t.GitHub + if short := loader.NormalizeRepo(t.GitHub); short != "" { + repoText = short + } + // Clickable: resolves exactly like the [o] key does — the full + // t.GitHub, never the shortened display text. links[strings.Count(sb.String(), "\n")] = "https://" + t.GitHub - sb.WriteString(ui.GithubStyle.Render("repo: "+t.GitHub) + "\n") + sb.WriteString(ui.GithubStyle.Render(cardLabel("repo:")+repoText) + "\n") if !hasCard && m.repoStatus[t.Name] == "rate-limited" { sb.WriteString(ui.WarnStyle.Render("rate limited — press [L]") + "\n") } } if hasCard && card.Stars > 0 { - sb.WriteString(ui.InfoStyle.Render(fmt.Sprintf("stars: %s", formatStars(card.Stars))) + "\n") + sb.WriteString(ui.InfoStyle.Render(cardLabel("stars:")+formatStars(card.Stars)) + "\n") } // The version-less states resolve only once the local probe reported // back (InstalledKnown) — before that an empty Installed just means the @@ -1232,17 +1270,17 @@ func (m Model) buildCard() (string, map[int]string) { // glyph is invisible in most editors and diffs and gets silently lost. switch { case installed != "": - sb.WriteString(ui.InfoStyle.Render("installed: \uf412 "+installed) + "\n") + sb.WriteString(ui.InfoStyle.Render(cardLabel("installed:")+"\uf412 "+version.DisplayVersion(installed)) + "\n") case vinfo.InstalledKnown && vinfo.InstalledPresent: - sb.WriteString(ui.InfoStyle.Render("installed: ") + + sb.WriteString(ui.InfoStyle.Render(cardLabel("installed:")) + ui.OkStyle.Render("✓") + ui.InfoStyle.Render(" no version") + "\n") case vinfo.InstalledKnown: - sb.WriteString(ui.InfoStyle.Render("installed: ") + + sb.WriteString(ui.InfoStyle.Render(cardLabel("installed:")) + ui.DangerStyle.Render("✕") + ui.InfoStyle.Render(" not installed") + "\n") default: - sb.WriteString(ui.InfoStyle.Render("installed: detecting…") + "\n") + sb.WriteString(ui.InfoStyle.Render(cardLabel("installed:")+"detecting…") + "\n") } if hasCard { if card.Latest != "" { @@ -1255,20 +1293,20 @@ func (m Model) buildCard() (string, map[int]string) { suffix = " (" + date + ")" } if m.hasUpdate(t.Name) { - sb.WriteString(ui.InfoStyle.Render("latest: ") + - ui.UpdateAvailableStyle.Render(" "+card.Latest+" ↑") + + sb.WriteString(ui.InfoStyle.Render(cardLabel("latest:")) + + ui.UpdateAvailableStyle.Render("\uf412 "+version.DisplayVersion(card.Latest)+" ↑") + ui.InfoStyle.Render(suffix) + "\n") } else { - sb.WriteString(ui.InfoStyle.Render("latest:  "+card.Latest+suffix) + "\n") + sb.WriteString(ui.InfoStyle.Render(cardLabel("latest:")+"\uf412 "+version.DisplayVersion(card.Latest)+suffix) + "\n") } } if len(card.Languages) > 0 { - label := "languages: " + label := cardLabel("languages:") bar := renderLangBar(card.Languages, inner, utf8.RuneCountInString(label)) sb.WriteString(ui.InfoStyle.Render(label) + bar + "\n") } if card.RepoStatus != "" { - sb.WriteString(ui.InfoStyle.Render("maintenance:") + " " + renderRepoStatus(card.RepoStatus) + "\n") + sb.WriteString(ui.InfoStyle.Render(cardLabel("maintenance:")) + renderRepoStatus(card.RepoStatus) + "\n") } } } @@ -1278,28 +1316,37 @@ func (m Model) buildCard() (string, map[int]string) { sb.WriteString(m.sectionDivider("notes")) sym := loader.StatusSymbol[mt.Status] symStyled := ui.StatusStyle(mt.Status).Render(sym + " " + string(mt.Status)) - sb.WriteString(ui.MetaDetailLabelStyle.Render("status:") + " " + symStyled + "\n") + sb.WriteString(ui.MetaDetailLabelStyle.Render(cardLabel("status:")) + symStyled + "\n") + + // Wrapped values are cut to what is left of the panel once the label + // column is spent — wrapping to the full inner width let the first line + // run cardLabelWidth cells past the panel edge, where the viewport + // truncates it (it does not soft-wrap) — and their continuation lines + // hang under the value column instead of falling back to column 0. + valueW := max(inner-cardLabelWidth, 10) if m.mode == modeEditNote { - sb.WriteString(ui.MetaDetailLabelStyle.Render("note:") + " " + m.noteInput.View() + "\n") + sb.WriteString(ui.MetaDetailLabelStyle.Render(cardLabel("note:")) + m.noteInput.View() + "\n") } else { noteText := mt.Note if noteText == "" { noteText = "— (press e to edit)" } - wrapped := wrapText(noteText, inner) - sb.WriteString(ui.MetaDetailLabelStyle.Render("note:") + " " + ui.MetaNoteStyle.Render(wrapped) + "\n") + wrapped := wrapText(noteText, valueW) + sb.WriteString(ui.MetaDetailLabelStyle.Render(cardLabel("note:")) + + hangIndent(ui.MetaNoteStyle.Render(wrapped)) + "\n") } if m.mode == modeEditTags { - sb.WriteString(ui.MetaDetailLabelStyle.Render("tags:") + " " + m.tagsInput.View() + "\n") + sb.WriteString(ui.MetaDetailLabelStyle.Render(cardLabel("tags:")) + m.tagsInput.View() + "\n") } else { tagsText := tagOf(mt) if tagsText == "" { tagsText = "— (press t to edit)" } - wrapped := wrapText(tagsText, inner) - sb.WriteString(ui.MetaDetailLabelStyle.Render("tags:") + " " + ui.MetaTagStyle.Render(wrapped) + "\n") + wrapped := wrapText(tagsText, valueW) + sb.WriteString(ui.MetaDetailLabelStyle.Render(cardLabel("tags:")) + + hangIndent(ui.MetaTagStyle.Render(wrapped)) + "\n") } } diff --git a/internal/model/render_test.go b/internal/model/render_test.go index b363573..c2e220e 100644 --- a/internal/model/render_test.go +++ b/internal/model/render_test.go @@ -1848,8 +1848,8 @@ func TestInstalledMsgCarriesPresence(t *testing.T) { want string notWant string }{ - {"installed but version-less", true, "installed: ✓ no version", "not installed"}, - {"absent", false, "installed: ✕ not installed", "no version"}, + {"installed but version-less", true, cardLabel("installed:") + "✓ no version", "not installed"}, + {"absent", false, cardLabel("installed:") + "✕ not installed", "no version"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2668,6 +2668,102 @@ func TestRenderCardSpinner(t *testing.T) { } } +// TestCardValuesShareOneColumn pins the card's single value column: every +// "label: value" line the card can print — [info] and [notes] alike — starts +// its value at cardLabelWidth. A line added with a hand-written label instead +// of cardLabel fails here rather than shipping a card whose values step in and +// out by a few cells each. The fixture fills every gated line, and the +// completeness check below is what keeps it filling them. +func TestCardValuesShareOneColumn(t *testing.T) { + m := New([]loader.ToolMeta{{ + Name: "gh", + GitHub: "cli/cli", + Status: loader.StatusActive, + Note: strings.Repeat("a wordy note ", 8), + Tags: []string{"git"}, + }}) + updated, _ := m.Update(tea.WindowSizeMsg{Width: 140, Height: 30}) + mm := updated.(Model) + mm.versions["gh"] = VersionInfo{ + Installed: "v1.0.0", Latest: "v2.0.0", InstalledKnown: true, InstalledPresent: true, + } + mm.repoCards["gh"] = version.RepoCard{ + Stars: 42123, + Latest: "v2.0.0", + PublishedAt: "2026-01-02T15:04:05Z", + Languages: map[string]int{"Go": 900, "Shell": 100}, + RepoStatus: "active", + } + + // A label line is "