diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index fba8f6c..be0ad98 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -70,9 +70,9 @@ The `model` package is split across files within a single package: | `model.go` | The `Model` struct, message types, `New`/`Init`/`Update`, selection and filtering helpers (`selectMeta`, `setFocus`, `searchMatches`, `filteredMeta`, `indexOfMeta`, `setHelpContent`) | | `mode.go` | The `inputMode` enum and a handler per input mode | | `commands.go` | All `tea.Cmd` constructors (fetch commands, update streaming) and re-fetch predicates | -| `render.go` | `View`, panel/card/status-bar/gauge/overlay renderers, mouse handling. The two list/card builders return their line index alongside the text: `buildCard` → clickable lines, `buildToolRows` → the tool-index ↔ screen-line maps | +| `render.go` | `View`, panel/card/status-bar/gauge/overlay renderers, mouse handling. The two list/card builders return their line index alongside the text: `buildCard` → clickable lines, `buildToolRows` → the tool-index ↔ screen-line maps. Carries the single-entry `changelogRenderCache` — the card is rebuilt on every spinner frame, so the release-notes conversion must not repeat | | `readme.go` | `renderReadme` — markdown → ANSI via glamour, with a single-entry render cache | -| `textutil.go` | Pure text helpers (`wrapText`, `stripANSI`, `colorizeHelp`, `parseHelpEntries`, …) | +| `textutil.go` | Pure text helpers (`wrapText`, `stripANSI`, `colorizeHelp`, `parseHelpEntries`, `markdownToLines` — the card's release-notes markdown → pre-wrapped tagged lines, …) | | `browser.go` | Opening URLs per `GOOS` | ## Data flow diff --git a/CLAUDE.md b/CLAUDE.md index b2054ce..600b903 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,7 +48,7 @@ The `model` package is split by responsibility (one package, several files): | `commands.go` | Every `tea.Cmd` constructor (`fetchInstalledCmd`, `remoteCmd`, `fetchRateCmd`, `selfCheckCmd`, `changelogCmd`, `fetchHelpCmd`, `readmeCmd`/`fetchReadmeCmd`/`refreshReadmeCmd`, `validateTokenCmd`, `detectUpdateCmd(t, self)` (one command for `[u]` and `[U]` — the flag only tags the message), `startUpdateCmd`, `waitForChunkCmd`, `startLaunchCmd`, `execToolCmd` + the pure per-GOOS `shellCommand`) + fetch predicates (`needsInstalled`, `needsRemote`, `needsReadme`, `refreshSelectedCmd`, `autoFetchCmdsForSelected`) | | `render.go` | `View`, panel/card/status-bar/gauge/overlay renderers, scrollbar, mouse handling | | `readme.go` | `renderReadme` (glamour) + the single-entry `readmeRenderCache`; `readmeStyleName`/`testReadmeStyle` seam | -| `textutil.go` | Pure text/format helpers (`wrapText`/`wrapLine`, `stripANSI`, `colorizeHelp`, `parseHelpEntries`, `findMatches`, `formatStars`, `renderLangBar`, …) | +| `textutil.go` | Pure text/format helpers (`wrapText`/`wrapLine`, `stripANSI`, `colorizeHelp`, `parseHelpEntries`, `markdownToLines`/`mdInline` — the card's release-notes converter, `findMatches`, `formatStars`, `renderLangBar`, …) | | `browser.go` | `openURLCmd` + per-`GOOS` `browserCommand` | ### Data flow @@ -89,6 +89,7 @@ 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. +- **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. - **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`. diff --git a/docs/plans/completed/20260727-changelog-markdown.md b/docs/plans/completed/20260727-changelog-markdown.md new file mode 100644 index 0000000..111abd8 --- /dev/null +++ b/docs/plans/completed/20260727-changelog-markdown.md @@ -0,0 +1,317 @@ +# Structured markdown rendering for the [changelog] card section + +## Overview + +The `[changelog]` section of panel `[2] Brief` destroys the markdown structure of GitHub +release notes instead of respecting it: `stripMarkdown` (`internal/model/textutil.go:148`) +eats list markers (`strings.Trim(line, "*_")` trims a leading `*`/`-` bullet), leaves +`[text](url)` links raw, ignores fenced code blocks, and its `<...>` HTML-strip loop +swallows autolinks (``); `wrapText` then rebuilds lines from `strings.Fields`, +losing every indent. The fix, agreed in a brainstorm session: replace `stripMarkdown` with +a block-based converter that preserves structure (bullets, headings, code) as plain text +in the card's muted aesthetic — headings styled bold-light, everything else `InfoStyle`. +Not glamour (the card must stay muted and the render is synchronous inside `Update()`), +not point fixes (bold headings and hanging indents are unreachable after `wrapText`). + +## Context (from discovery) + +- Files involved: `internal/model/textutil.go` (`stripMarkdown` — single caller is + `renderChangelogBlock`), `internal/model/render.go:1361` (`renderChangelogBlock`), + `internal/ui/styles.go` (new heading style), `internal/model/render_test.go` + (`TestStripMarkdown` at :332 — deleted). Converter and inline-helper tests go into a + **new `internal/model/textutil_test.go`** — `render_test.go` is ~3900 lines and the + package already splits tests by file (`group_test.go`, `readme_test.go`, + `cardlinks_test.go`); the `renderChangelogBlock` styling tests stay in + `render_test.go` next to the other render tables. +- Invariants that must survive untouched: `buildCard()`'s clickable-line map (the release + URL is the block's first line — `TestBuildCardLinks`, `TestMouseBriefLinkClick`) and + "content line = screen row" (`TestBriefContentLineIsScreenRow` — the viewport truncates, + never soft-wraps). +- Patterns to follow: one wrap algorithm per package (`wrapLine`), named styles in + `internal/ui/styles.go` (no inline `lipgloss.NewStyle()` in renderers), styling applied + to whole lines only after wrapping (`wrapLine` counts runes and knows nothing of ANSI). +- `•` (U+2022) is East-Asian Ambiguous — same accepted class as `⏺`/`↑`/`─` (see the + CLAUDE.md marker-glyph note); it never enters wrap math, which is rune-based. + +## Development Approach + +- **testing approach**: Regular (code first, then table tests in the same task) +- complete each task fully before moving to the next +- make small, focused changes +- **CRITICAL: every task MUST include new/updated tests** for code changes in that task + - tests are not optional - they are a required part of the checklist + - tests cover both success and error scenarios +- **CRITICAL: all tests must pass before starting next task** (`go test -race ./...`) +- **CRITICAL: update this plan file when scope changes during implementation** +- maintain backward compatibility: the release-URL line, the clickable-line map and the + empty-body fallback (`no release notes available.`) keep their exact current behavior + +## Testing Strategy + +- **unit tests**: required for every task; table-driven, matching the existing + `TestWrapText`/`TestCleanTerminalOutput` style in `internal/model/render_test.go` +- **e2e tests**: none in this project — the closest equivalents are the invariant tests + (`TestBriefContentLineIsScreenRow`, `TestBuildCardLinks`) which must stay green unmodified +- full check matrix before commit: `preflight` (build / vet / `test -race` / golangci-lint) + +## Progress Tracking + +- mark completed items with `[x]` immediately when done +- add newly discovered tasks with ➕ prefix +- document issues/blockers with ⚠️ prefix +- keep plan in sync with actual work done + +## Solution Overview + +`markdownToLines(s string, width int) []mdLine` replaces `stripMarkdown`: a line-by-line +pass with two state flags (inside fenced code / inside HTML comment) classifies blocks, +strips inline markup, and returns **pre-wrapped** lines each tagged with a kind. Wrapping +happens inside the converter (via the existing `wrapLine`) because hanging indents need +block structure, and styling must land on whole finished lines — the only way to keep +ANSI out of the width math. `renderChangelogBlock` becomes a trivial consumer: iterate, +style `mdHeading` with the new `ui.ChangelogHeadingStyle`, everything else with +`ui.InfoStyle`, join with `\n`. + +## Technical Details + +Types (in `internal/model/textutil.go`): + +```go +type mdLineKind int + +const ( + mdBody mdLineKind = iota // paragraphs, list items, code — everything non-heading + mdHeading // a #-heading line +) + +type mdLine struct { + text string + kind mdLineKind +} + +func markdownToLines(s string, width int) []mdLine +``` + +Two kinds only — no `mdCode`: the sole consumer styles every non-heading line +`InfoStyle`, so a code tag would have zero readers (YAGNI). Everything that makes code +lines special (verbatim, no inline processing, no wrap, 2-space indent) lives entirely +inside the converter, driven by the fence state flag. `markdownToLines` guards +`width <= 0` itself (no wrapping then) — it is a pure function called directly from test +tables, not only through the floored call site. + +Block rules (line pass, two state flags): + +- **Input normalization** — `\r\n`/`\r` → `\n` before the line pass. GitHub release + bodies regularly arrive CRLF (the web form submits it); the old code was accidentally + saved by its per-line `TrimSpace`, and the verbatim code path below would otherwise + carry `\r` straight into the viewport — exactly the class `cleanTerminalOutput` exists + to stop. A `\r`-suffixed closing fence would also fail the fence match and swallow the + rest of the body as code. Precedent: `internal/version/detect.go:162` does + `strings.TrimRight(line, "\r")`. +- **Fenced code** — ``` or `~~~` toggles code mode; fence lines (with their language tag) + are dropped; inside, lines go out verbatim with a 2-space indent and are **never + wrapped** (wrapped shell code is worse than truncated; the viewport truncates overlong + lines already — the accepted behavior `TestBriefContentLineIsScreenRow` pins). An + unclosed fence means code to the end of input — no panic. +- **Headings** — `#{1,6}` **followed by a space** at line start → strip the marker, + `kind=mdHeading`. The space is required: a leading issue ref like `#123 fixed …` stays + literal text — a deliberate improvement over the old `TrimLeft(line, "#")`, which + mangled it to `123 fixed …`. Levels are not distinguished (one emphasis step is enough + in a narrow card). Setext headings — YAGNI. +- **Thematic breaks** — a line of only 3+ `-`/`*`/`_` (`---`, `***`, `___`) → dropped as + a blank line (participates in blank collapse), **never** a list item; `---` is the + stock separator before "**Full Changelog**: …" in real release notes. +- **Lists** — `-`/`*`/`+` or `N.` **followed by a space** after optional indent. No + space → not a list: `*emphasis*` or `**Breaking**` at line start stays body text (the + old `strings.Trim(line, "*_")` bug class must not come back). Marker normalizes to `•` + (numbered items keep their number). Nesting level = leading spaces / 2 (floor; a tab + counts as 4 spaces), clamped to 2 levels, 2 output spaces per level. Wrap with + **hanging indent**: text wraps at `width − indent`, continuations align under the + first text column, not the marker. +- **Blockquotes** — strip the `>` marker, treat as body. +- **HTML comments** — `` cut out, including multi-line (second state flag); + release-note templates are full of them. Unclosed `"}, + {"only a separator", "---\n"}, + {"comment spanning lines", "\n"}, } for _, tt := range tests { - if got := stripMarkdown(tt.in); got != tt.want { - t.Errorf("stripMarkdown(%q) = %q, want %q", tt.in, got, tt.want) - } + t.Run(tt.name, func(t *testing.T) { + got := m.renderChangelogBlock(changelogMsg{toolName: "gh", body: tt.body}) + if want := ui.InfoStyle.Render("no release notes available.") + "\n"; got != want { + t.Errorf("renderChangelogBlock(%q) = %q, want the fallback", tt.body, got) + } + }) + } +} + +// TestRenderChangelogBlockError: the failure branch is untouched by the +// markdown work — it never reaches the converter. +func TestRenderChangelogBlockError(t *testing.T) { + forceColorProfile(t) + m := changelogBlockModel() + + got := m.renderChangelogBlock(changelogMsg{toolName: "gh", err: errors.New("boom"), body: "# ignored"}) + if want := ui.InfoStyle.Render("changelog unavailable: boom") + "\n"; got != want { + t.Errorf("error branch = %q, want %q", got, want) } } +func firstLine(s string) string { + line, _, _ := strings.Cut(s, "\n") + return line +} + func TestRenderRepoStatus(t *testing.T) { tests := []struct { status string @@ -3852,3 +3936,41 @@ func TestRenderStatusBarFocusToolsRunHint(t *testing.T) { t.Errorf("narrow focusTools bar kept right-side cells that should drop first: %q", got) } } + +// TestChangelogRenderCache: the card is rebuilt on every spinner frame while a +// refresh or an update runs, so the converter must not re-parse the same body +// each time. A nil cache stays a working cache-less mode — most tests build +// Model{} literals and never call New(). +func TestChangelogRenderCache(t *testing.T) { + const body = "## What's Changed\n* one\n* two\n" + + var c changelogRenderCache + first := c.lines(body, 60) + again := c.lines(body, 60) + if len(first) == 0 { + t.Fatalf("converter returned nothing for %q", body) + } + if &first[0] != &again[0] { + t.Errorf("same body and width re-converted instead of hitting the cache") + } + + if wider := c.lines(body, 20); &wider[0] == &first[0] { + t.Errorf("a width change was served from the cache") + } + if other := c.lines("## Other\n* x\n", 60); &other[0] == &first[0] { + t.Errorf("a body change was served from the cache") + } + + var nilCache *changelogRenderCache + if got := nilCache.lines(body, 60); !slices.Equal(mdDump(got), mdDump(first)) { + t.Errorf("nil cache = %q, want the plain conversion %q", mdDump(got), mdDump(first)) + } +} + +// TestNewSeedsChangelogCache pins the wiring: without it every card render +// would take the cache-less path and the memo would be dead code. +func TestNewSeedsChangelogCache(t *testing.T) { + if New(nil).changelogRender == nil { + t.Error("New() left changelogRender nil — the memo is never used") + } +} diff --git a/internal/model/textutil.go b/internal/model/textutil.go index 887f287..e75cb6e 100644 --- a/internal/model/textutil.go +++ b/internal/model/textutil.go @@ -145,43 +145,295 @@ func wrapLine(line string, width int) []string { return append(lines, cur.String()) } -func stripMarkdown(s string) string { - var sb strings.Builder - lines := strings.Split(s, "\n") - blankCount := 0 +// Inline markdown patterns, compiled once — mdInline runs per source line of +// every release body the card renders. +// +// The link patterns keep their text class free of the closing delimiter +// ([^\]]* / [^)]*), which is what makes them non-greedy without the RE2 cost +// of an explicit lazy quantifier: two links on one line stay two matches. +// The emphasis patterns require a non-space rune on both inner edges (the +// CommonMark flanking rule in miniature), so "2 * 3 * 4" keeps its asterisks; +// the underscore pair additionally requires a non-word rune outside both +// delimiters, or an identifier like update_cmd would lose its underscore. +var ( + mdImageRe = regexp.MustCompile(`!\[([^\]]*)\]\([^)]*\)`) + mdLinkRe = regexp.MustCompile(`\[([^\]]*)\]\([^)]*\)`) + mdAutolinkRe = regexp.MustCompile(`<((?:https?|ftp|mailto):[^>\s]+)>`) + mdHTMLTagRe = regexp.MustCompile(`<[^<>]*>`) + + mdStrongStarRe = regexp.MustCompile(`\*\*([^\s*](?:[^*]*[^\s*])?)\*\*`) + mdStrongUnderRe = regexp.MustCompile(`__([^\s_](?:[^_]*[^\s_])?)__`) + mdEmStarRe = regexp.MustCompile(`\*([^\s*](?:[^*]*[^\s*])?)\*`) + mdEmUnderRe = regexp.MustCompile(`(^|[^\p{L}\p{N}_])_([^\s_](?:[^_]*[^\s_])?)_([^\p{L}\p{N}_]|$)`) +) - for _, line := range lines { - line = strings.TrimLeft(line, "#") - line = strings.TrimSpace(line) +// mdInline reduces one line of markdown to plain text. Order is load-bearing: +// images before links (otherwise the link pattern eats "[alt](url)" out of an +// image and leaves a stray '!'), autolinks before the HTML-tag strip (which +// would otherwise swallow "" whole — the bug this replaces), and +// emphasis last, after every construct whose delimiters could contain '*'/'_'. +// A malformed construct (unclosed '[', a lone '<') simply matches nothing and +// is left as written. +func mdInline(s string) string { + s = mdImageRe.ReplaceAllString(s, "$1") + s = mdLinkRe.ReplaceAllString(s, "$1") + s = mdAutolinkRe.ReplaceAllString(s, "$1") + s = mdHTMLTagRe.ReplaceAllString(s, "") + s = mdStrongStarRe.ReplaceAllString(s, "$1") + s = mdStrongUnderRe.ReplaceAllString(s, "$1") + s = mdEmStarRe.ReplaceAllString(s, "$1") + // The underscore pattern is the one that matches its own boundaries, and a + // match consumes them: in "_a_ _b_" the separating space goes with the + // first span, leaving the second with no boundary to match against (RE2 has + // no lookaround). A second pass reaches it. Each pass either drops two + // delimiters or changes nothing, so this terminates. + for { + next := mdEmUnderRe.ReplaceAllString(s, "${1}${2}${3}") + if next == s { + break + } + s = next + } + return strings.ReplaceAll(s, "`", "") +} - for _, marker := range []string{"**", "__"} { - line = strings.ReplaceAll(line, marker, "") +// mdLineKind tags a converted line for the styling the consumer applies to it. +// Two kinds only: the card's changelog block paints headings one step brighter +// and everything else muted, so a third tag (code, list, quote) would have no +// reader — what makes those blocks special lives inside the converter. +type mdLineKind int + +const ( + mdBody mdLineKind = iota // paragraphs, list items, code, quotes + mdHeading // a #-heading line +) + +// mdLine is one finished, already-wrapped output line. Wrapping happens inside +// the converter because hanging indents need the block structure, and because +// styling must land on whole finished lines — wrapLine counts runes and knows +// nothing about ANSI. +type mdLine struct { + text string + kind mdLineKind +} + +// Block-level markdown patterns. The list and heading patterns require +// whitespace after the marker, so a leading "#123" stays an issue reference +// and "**Breaking**" stays a paragraph. +var ( + // The fence deliberately accepts ANY indent, unlike CommonMark's 3-space + // limit: 4+ spaces there means an indented code block, which this + // converter does not implement, so the strict form only mis-read a fence + // nested under a list item ("1. " content aligns at column 4) — the + // language tag leaked out as a body line reading "go" and the sample lost + // its verbatim treatment. + mdFenceRe = regexp.MustCompile("^[ \t]*(?:```|~~~)") + mdHeadingRe = regexp.MustCompile(`^ {0,3}#{1,6}[ \t]+(.*)$`) + // A rule line: a thematic break (---, ***, ___) or a setext underline + // (===). Both are dropped to a blank. Full setext support — retagging the + // paragraph above as a heading — is deliberately out of scope; the text + // and the layout stay right, only the emphasis is lost, whereas leaving + // the run in place put a literal row of '=' in the card. + mdRuleRe = regexp.MustCompile(`^ {0,3}(?:-[ \t]*){3,}$|^ {0,3}(?:\*[ \t]*){3,}$|^ {0,3}(?:_[ \t]*){3,}$|^ {0,3}={3,}[ \t]*$`) + mdBulletRe = regexp.MustCompile(`^([ \t]*)[-*+](?:[ \t]+(.*))?$`) + mdOrderedRe = regexp.MustCompile(`^([ \t]*)(\d{1,9})[.)](?:[ \t]+(.*))?$`) + mdQuoteRe = regexp.MustCompile(`^ {0,3}>[ \t]?`) +) + +const ( + mdMaxListLevel = 2 // deeper nesting keeps folding into this indent + mdIndentPerLvl = 2 // output spaces per nesting level + mdCodeIndent = " " +) + +// markdownToLines converts a markdown body (a GitHub release note) into plain +// pre-wrapped lines that keep the structure the old strip-everything pass +// destroyed: list markers, heading emphasis, code verbatim. It is a single +// line pass over the input carrying two state flags — inside a fenced code +// block, inside an HTML comment — and is pure, so the tables test it directly. +// width <= 0 disables wrapping (the call site floors it, but a caller must not +// have to). +func markdownToLines(s string, width int) []mdLine { + if s == "" { + return nil + } + // GitHub release bodies regularly arrive CRLF (the web form submits it). + // The old code was accidentally saved by its per-line TrimSpace; here a + // stray \r would ride the verbatim code path straight into the viewport, + // and a \r-suffixed closing fence would never match, swallowing the rest + // of the body as code. + s = strings.ReplaceAll(s, "\r\n", "\n") + s = strings.ReplaceAll(s, "\r", "\n") + + var out []mdLine + emit := func(text string, kind mdLineKind) { + out = append(out, mdLine{text: text, kind: kind}) + } + // emitBlank collapses runs of blank lines to one and drops leading ones; + // trailing ones are cut after the pass. + emitBlank := func() { + if len(out) > 0 && out[len(out)-1].text != "" { + out = append(out, mdLine{}) } - line = strings.Trim(line, "*_") - line = strings.ReplaceAll(line, "`", "") + } - for strings.Contains(line, "<") && strings.Contains(line, ">") { - start := strings.Index(line, "<") - end := strings.Index(line[start:], ">") - if end < 0 { - break + inCode, inComment := false, false + for _, raw := range strings.Split(s, "\n") { + line := strings.TrimRight(raw, " \t") + + if inCode { + if mdFenceRe.MatchString(line) { + inCode = false + continue } - line = line[:start] + line[start+end+1:] + switch { + case line != "": + emit(mdCodeIndent+line, mdBody) + case len(out) > 0: + // A blank line inside the block is verbatim too, so a sample + // keeps its own spacing instead of going through the collapse + // — but not as the very first output line, which would be a + // leading blank row no one trims. + emit("", mdBody) + } + continue } - line = strings.TrimSpace(line) + // Comments are cut before any classification (a release-note template + // is mostly comments), but never inside code, where they are literal. + line, inComment = mdCutComments(line, inComment) + if strings.TrimSpace(line) == "" { + emitBlank() + continue + } - if line == "" { - blankCount++ - if blankCount <= 1 { - sb.WriteString("\n") - } + if mdFenceRe.MatchString(line) { + inCode = true // the fence line, language tag and all, is dropped + continue + } + + // A blockquote marker is stripped and the remainder re-classified, so + // a list inside a quote stays a list. + for mdQuoteRe.MatchString(line) { + line = mdQuoteRe.ReplaceAllString(line, "") + } + + if m := mdHeadingRe.FindStringSubmatch(line); m != nil { + mdEmitInline(emit, emitBlank, m[1], width, mdHeading) + continue + } + // Checked before the list patterns: "---" is the stock separator above + // "**Full Changelog**: …" and must never read as a bullet. + if mdRuleRe.MatchString(line) { + emitBlank() + continue + } + if m := mdBulletRe.FindStringSubmatch(line); m != nil { + mdEmitListItem(emit, emitBlank, m[1], "• ", m[2], width) + continue + } + if m := mdOrderedRe.FindStringSubmatch(line); m != nil { + mdEmitListItem(emit, emitBlank, m[1], m[2]+". ", m[3], width) + continue + } + + mdEmitInline(emit, emitBlank, line, width, mdBody) + } + + for len(out) > 0 && out[len(out)-1].text == "" { + out = out[:len(out)-1] + } + return out +} + +// mdEmitInline runs the inline pass over one heading or paragraph line and +// emits it wrapped — or a blank when the markup collapsed to nothing, which a +// badge-only line, a bare
and a heading whose only content was an image +// all do. Both callers share it so the blank can only ever arrive through +// emitBlank: an empty line tagged mdHeading would sit outside the collapse and +// hand the next consumer of kind a line that is not a heading. +func mdEmitInline(emit func(string, mdLineKind), emitBlank func(), raw string, width int, kind mdLineKind) { + text := mdInline(strings.TrimSpace(raw)) + if strings.TrimSpace(text) == "" { + emitBlank() + return + } + mdEmitWrapped(emit, text, "", "", width, kind) +} + +// mdEmitListItem writes one list item: the marker normalized (bullets to •, +// numbered items keeping their number), the nesting taken from the source +// indent, and the text wrapped with a hanging indent so continuations align +// under the first text column instead of under the marker. +func mdEmitListItem(emit func(string, mdLineKind), emitBlank func(), indent, marker, text string, width int) { + body := mdInline(strings.TrimSpace(text)) + if strings.TrimSpace(body) == "" { + emitBlank() // a marker with no content is not an item + return + } + pad := strings.Repeat(" ", mdIndentPerLvl*mdListLevel(indent)) + mdEmitWrapped(emit, body, pad+marker, pad+strings.Repeat(" ", utf8.RuneCountInString(marker)), width, mdBody) +} + +// mdListLevel maps a source indent to a nesting level: two spaces per level +// (floor), a tab counting as four, clamped so a deeply nested list cannot walk +// off a 30-column card. +func mdListLevel(indent string) int { + cols := 0 + for _, r := range indent { + if r == '\t' { + cols += 4 } else { - blankCount = 0 - sb.WriteString(line + "\n") + cols++ + } + } + return min(cols/2, mdMaxListLevel) +} + +// mdEmitWrapped wraps text at the remaining width and emits it with first as +// the first line's prefix and hang as every continuation's. Wrapping is +// skipped when nothing is left to wrap into — wrapLine would then degrade to +// one word per line, which is worse than letting the viewport truncate. +func mdEmitWrapped(emit func(string, mdLineKind), text, first, hang string, width int, kind mdLineKind) { + avail := width - utf8.RuneCountInString(first) + if width <= 0 || avail <= 0 { + emit(first+text, kind) + return + } + for i, w := range wrapLine(text, avail) { + if i == 0 { + emit(first+w, kind) + } else { + emit(hang+w, kind) + } + } +} + +// mdCutComments removes HTML comments from one line, carrying the open state +// across lines (release-note templates comment out whole instruction blocks). +// It returns the surviving text and whether a comment is still open — an +// unclosed "") + if i < 0 { + return b.String(), true + } + line = line[i+len("-->"):] + inComment = false + continue + } + i := strings.Index(line, "\nreal text", + 80, + []string{"B|real text"}, + }, + { + "unclosed comment runs to the end", + "kept\n after", + 80, + []string{"B|before after"}, + }, + { + "a body that is only a comment converts to nothing", + "", + 80, + nil, + }, + { + "blank runs collapse and the edges are trimmed", + "\n\na\n\n\n\nb\n\n", + 80, + []string{"B|a", "B|", "B|b"}, + }, + { + // The call site floors the width, but the converter is pure and + // tables call it directly: width 0 must not wrap and must not loop. + "width zero disables wrapping", + "- alpha beta gamma delta epsilon zeta", + 0, + []string{"B|• alpha beta gamma delta epsilon zeta"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := mdDump(markdownToLines(tt.in, tt.width)) + if !slices.Equal(got, tt.want) { + t.Errorf("markdownToLines(%q, %d):\n got %q\nwant %q", tt.in, tt.width, got, tt.want) + } + }) + } +} + +// TestMarkdownToLinesGitHubBody drives the shape GitHub's own release notes +// take: a "What's Changed" heading, "* … by @user in " bullets and the +// "**Full Changelog**" line under a --- separator. +func TestMarkdownToLinesGitHubBody(t *testing.T) { + body := "## What's Changed\r\n" + + "* fix: detect uv tools by [@someone](https://github.com/someone) in https://github.com/o/r/pull/44\r\n" + + "* docs: update README\r\n" + + "\r\n" + + "**Full Changelog**: https://github.com/o/r/compare/v1.0.0...v1.1.0\r\n" + + got := mdDump(markdownToLines(body, 200)) + want := []string{ + "H|What's Changed", + "B|• fix: detect uv tools by @someone in https://github.com/o/r/pull/44", + "B|• docs: update README", + "B|", + "B|Full Changelog: https://github.com/o/r/compare/v1.0.0...v1.1.0", + } + if !slices.Equal(got, want) { + t.Errorf("github release body:\n got %q\nwant %q", got, want) + } +} diff --git a/internal/ui/styles.go b/internal/ui/styles.go index 6d63c4a..ca50a70 100644 --- a/internal/ui/styles.go +++ b/internal/ui/styles.go @@ -88,8 +88,8 @@ var ( // version unknown". Deliberately not UpdateAvailableStyle: orange there // means "act on this", and a working install needs no action. OkStyle = lipgloss.NewStyle(). - Foreground(ColorGreen). - Bold(true) + Foreground(ColorGreen). + Bold(true) // My Tools status colors StatusColorActive = ColorGreen @@ -149,6 +149,14 @@ var ( InfoStyle = lipgloss.NewStyle(). Foreground(ColorMuted) + // ChangelogHeadingStyle marks a markdown heading inside the card's + // [changelog] body: one emphasis step above the muted InfoStyle body, and + // no new color — ColorText keeps it from competing with the peach + // SectionLabelStyle that heads the card's own sections. + ChangelogHeadingStyle = lipgloss.NewStyle(). + Bold(true). + Foreground(ColorText) + // TagHeaderStyle / TagRuleStyle draw the tag-group divider rows in the [1] // tools list: "─ dev ────…". The label is muted (ColorDim), the rule lines // the panel-frame gray (ColorBorder), so group boundaries read geometrically