From 2ee9e7780c1043087c762200608c98413a6bbb4e Mon Sep 17 00:00:00 2001 From: stanlyzoolo <51911715+stanlyzoolo@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:57:36 +0300 Subject: [PATCH 1/4] docs: add changelog markdown rendering implementation plan Co-Authored-By: Claude Fable 5 --- docs/plans/20260727-changelog-markdown.md | 268 ++++++++++++++++++++++ 1 file changed, 268 insertions(+) create mode 100644 docs/plans/20260727-changelog-markdown.md diff --git a/docs/plans/20260727-changelog-markdown.md b/docs/plans/20260727-changelog-markdown.md new file mode 100644 index 0000000..e728ca1 --- /dev/null +++ b/docs/plans/20260727-changelog-markdown.md @@ -0,0 +1,268 @@ +# 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 diff --git a/internal/model/textutil.go b/internal/model/textutil.go index 887f287..2045672 100644 --- a/internal/model/textutil.go +++ b/internal/model/textutil.go @@ -145,43 +145,274 @@ 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. Each allows CommonMark's up-to-3-space +// indent; the list patterns require whitespace after the marker, so a leading +// "#123" stays an issue reference and "**Breaking**" stays a paragraph. +var ( + mdFenceRe = regexp.MustCompile("^ {0,3}(?:```|~~~)") + mdHeadingRe = regexp.MustCompile(`^ {0,3}#{1,6}[ \t]+(.*)$`) + mdThematicRe = regexp.MustCompile(`^ {0,3}(?:-[ \t]*){3,}$|^ {0,3}(?:\*[ \t]*){3,}$|^ {0,3}(?:_[ \t]*){3,}$`) + 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 { + mdEmitWrapped(emit, mdInline(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 mdThematicRe.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 + } + + text := mdInline(strings.TrimSpace(line)) + if strings.TrimSpace(text) == "" { + emitBlank() + continue + } + mdEmitWrapped(emit, text, "", "", width, mdBody) + } + + for len(out) > 0 && out[len(out)-1].text == "" { + out = out[:len(out)-1] + } + return out +} + +// 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 { + 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 { - blankCount = 0 - sb.WriteString(line + "\n") + 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 From b000f5cb66a69a1da8ce367ead8426ea4e818576 Mon Sep 17 00:00:00 2001 From: stanlyzoolo <51911715+stanlyzoolo@users.noreply.github.com> Date: Tue, 28 Jul 2026 08:10:51 +0300 Subject: [PATCH 3/4] fix(model): three converter edges found reviewing the PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. The fence pattern enforced CommonMark's 3-space indent limit, where 4+ spaces means an indented code block. This converter does not implement those at all, so the limit only mis-read a fence nested under a list item — "1. " aligns its content at column 4 — and the language tag leaked out as a body line reading "go" while the sample lost its verbatim treatment. The fence now takes any indent. 2. A setext underline survived as a literal row of equals signs: "---" already collapsed as a thematic break, "===" matched nothing. Both are now rule lines that drop to a blank. Full setext support (retagging the paragraph above as a heading) stays out of scope — the text and the layout are right, only the emphasis is lost. 3. A heading whose markup collapsed to nothing (## with only an image) emitted an empty line still tagged mdHeading, bypassing the blank collapse the body path went through. Harmless in today's renderer, which checks text before kind, but it is a line that is not a heading wearing the tag. Heading and paragraph now share mdEmitInline, so the blank can only arrive through emitBlank — and the duplicated guard is gone with it. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 2 +- internal/model/textutil.go | 55 +++++++++++++++++++++++---------- internal/model/textutil_test.go | 31 +++++++++++++++++++ 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e9b1c9a..2c0132b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -89,7 +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; `---`/`***`/`___` are thematic breaks that collapse to a blank line and are **never** list items (that is the stock separator above "Full Changelog"); 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. +- **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. - **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/internal/model/textutil.go b/internal/model/textutil.go index 2045672..e75cb6e 100644 --- a/internal/model/textutil.go +++ b/internal/model/textutil.go @@ -217,16 +217,27 @@ type mdLine struct { kind mdLineKind } -// Block-level markdown patterns. Each allows CommonMark's up-to-3-space -// indent; the list patterns require whitespace after the marker, so a leading -// "#123" stays an issue reference and "**Breaking**" stays a paragraph. +// 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 ( - mdFenceRe = regexp.MustCompile("^ {0,3}(?:```|~~~)") - mdHeadingRe = regexp.MustCompile(`^ {0,3}#{1,6}[ \t]+(.*)$`) - mdThematicRe = regexp.MustCompile(`^ {0,3}(?:-[ \t]*){3,}$|^ {0,3}(?:\*[ \t]*){3,}$|^ {0,3}(?:_[ \t]*){3,}$`) - mdBulletRe = regexp.MustCompile(`^([ \t]*)[-*+](?:[ \t]+(.*))?$`) - mdOrderedRe = regexp.MustCompile(`^([ \t]*)(\d{1,9})[.)](?:[ \t]+(.*))?$`) - mdQuoteRe = regexp.MustCompile(`^ {0,3}>[ \t]?`) + // 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 ( @@ -308,12 +319,12 @@ func markdownToLines(s string, width int) []mdLine { } if m := mdHeadingRe.FindStringSubmatch(line); m != nil { - mdEmitWrapped(emit, mdInline(m[1]), "", "", width, mdHeading) + 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 mdThematicRe.MatchString(line) { + if mdRuleRe.MatchString(line) { emitBlank() continue } @@ -326,12 +337,7 @@ func markdownToLines(s string, width int) []mdLine { continue } - text := mdInline(strings.TrimSpace(line)) - if strings.TrimSpace(text) == "" { - emitBlank() - continue - } - mdEmitWrapped(emit, text, "", "", width, mdBody) + mdEmitInline(emit, emitBlank, line, width, mdBody) } for len(out) > 0 && out[len(out)-1].text == "" { @@ -340,6 +346,21 @@ func markdownToLines(s string, width int) []mdLine { 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 diff --git a/internal/model/textutil_test.go b/internal/model/textutil_test.go index d756d2c..3afd2e6 100644 --- a/internal/model/textutil_test.go +++ b/internal/model/textutil_test.go @@ -193,6 +193,37 @@ func TestMarkdownToLines(t *testing.T) { 80, []string{"B|text", "B|", "B|more"}, }, + { + // A fence nested under a list item sits past CommonMark's 3-space + // limit ("1. " content aligns at column 4). Enforcing the limit + // leaked the language tag as a body line reading "go" and dropped + // the sample's verbatim treatment. + "fence indented under a list item still opens", + "1. run this:\n\n ```go\n x := 1\n ```\n2. done", + 80, + []string{"B|1. run this:", "B|", "B| x := 1", "B|2. done"}, + }, + { + "setext underline does not survive as a row of equals", + "Highlights\n==========\nbody", + 80, + []string{"B|Highlights", "B|", "B|body"}, + }, + { + // A heading whose only content is an image converts to nothing; the + // blank must come from the collapse, never as an empty line still + // tagged mdHeading. + "heading that collapses to nothing yields a body blank", + "a\n## ![](https://img/x.svg)\n\nb", + 80, + []string{"B|a", "B|", "B|b"}, + }, + { + "a body that is only an empty heading converts to nothing", + "## ![](https://img/x.svg)", + 80, + nil, + }, { "unclosed fence runs to the end", "text\n~~~\ncode line", From ec9a408a37d81ae771d0d7ec4d2c6a256b6b7797 Mon Sep 17 00:00:00 2001 From: stanlyzoolo <51911715+stanlyzoolo@users.noreply.github.com> Date: Tue, 28 Jul 2026 08:11:20 +0300 Subject: [PATCH 4/4] perf(model): memoize the release-notes conversion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The whole card is rebuilt on every spinner frame — the spinner.TickMsg handler calls SetContent(renderCard()) about twelve times a second for as long as a [r] refresh or a [u] update runs — so the release body went through the converter's regexes twelve times a second to animate one glyph. Measured on a 49 KB body: 3.3 ms and 1 MB of garbage per pass, against 15 ns for a cache hit. A typical 1.2 KB body costs 140 µs per pass, which is invisible on its own; the point is that it repeats on a timer and the new converter is ~10x the old string-trimming one. changelogRenderCache is readmeRenderCache's shape, one entry keyed by (body, width) — the only two inputs markdownToLines has. 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, which is what the ~50 tests building Model{} literals get without touching any of them. Co-Authored-By: Claude Opus 5 --- ARCHITECTURE.md | 2 +- CLAUDE.md | 2 +- internal/model/model.go | 51 +++++++++++++++++++---------------- internal/model/render.go | 31 ++++++++++++++++++++- internal/model/render_test.go | 38 ++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 26 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 0c744e5..be0ad98 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -70,7 +70,7 @@ 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`, `markdownToLines` — the card's release-notes markdown → pre-wrapped tagged lines, …) | | `browser.go` | Opening URLs per `GOOS` | diff --git a/CLAUDE.md b/CLAUDE.md index 2c0132b..600b903 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -89,7 +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. +- **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/internal/model/model.go b/internal/model/model.go index e4f094e..b3d5fec 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -360,9 +360,13 @@ type Model struct { readmeLoading map[string]bool // readmeRender memoizes the last glamour render (see readme.go). readmeRender readmeRenderCache - helpSearch textinput.Model - helpMatches []int - helpMatchIdx int + // changelogRender memoizes the last release-notes markdown conversion + // (see render.go). A pointer, so the value-receiver card renderers can + // fill it; nil is a working cache-less mode for Model{} literals. + changelogRender *changelogRenderCache + helpSearch textinput.Model + helpMatches []int + helpMatchIdx int // helpEntries indexes the navigable entries (flag/subcommand line plus its // description block) of the current help text, in wrapped display-line @@ -436,26 +440,27 @@ func New(meta []loader.ToolMeta) Model { sp.Style = lipgloss.NewStyle().Foreground(ui.ColorPrimary) m := Model{ - tools: loader.ToolsFromMeta(meta), - versions: make(map[string]VersionInfo), - repoStatus: make(map[string]string), - repoCards: make(map[string]version.RepoCard), - changelogData: make(map[string]changelogMsg), - helpCache: make(map[string][2]string), - readmeData: make(map[string]readmeMsg), - readmeLoading: make(map[string]bool), - search: ti, - noteInput: ni, - tagsInput: tgi, - helpSearch: hsi, - trackInput: tri, - nameInput: nmi, - runInput: rni, - lastRun: make(map[string]string), - tokenInput: tki, - spinner: sp, - meta: meta, - helpNavIdx: -1, + tools: loader.ToolsFromMeta(meta), + versions: make(map[string]VersionInfo), + repoStatus: make(map[string]string), + repoCards: make(map[string]version.RepoCard), + changelogData: make(map[string]changelogMsg), + helpCache: make(map[string][2]string), + readmeData: make(map[string]readmeMsg), + readmeLoading: make(map[string]bool), + changelogRender: &changelogRenderCache{}, + search: ti, + noteInput: ni, + tagsInput: tgi, + helpSearch: hsi, + trackInput: tri, + nameInput: nmi, + runInput: rni, + lastRun: make(map[string]string), + tokenInput: tki, + spinner: sp, + meta: meta, + helpNavIdx: -1, // The README is the default source of [3]: it is the only one that // exists for a tracked-but-not-installed tool, which is exactly the // state where docs matter most. diff --git a/internal/model/render.go b/internal/model/render.go index 5bae488..8aaef00 100644 --- a/internal/model/render.go +++ b/internal/model/render.go @@ -1358,6 +1358,35 @@ func (m Model) sectionDivider(label string) string { lipgloss.NewStyle().Foreground(ui.ColorBorder).Render(strings.Repeat("─", dashes)) + "\n\n" } +// changelogRenderCache memoizes the last markdown conversion. The whole card +// is rebuilt on every spinner frame while a refresh or an update runs (see the +// spinner.TickMsg handler), so the body of a large release note would go +// through the converter's regexes ~12 times a second to animate one glyph. +// Same shape as readmeRenderCache, one entry keyed by (body, width) — the two +// inputs markdownToLines has. +// +// It hangs off Model as a POINTER so a value receiver can still fill it, and +// its method tolerates a nil receiver: most tests build Model{} literals and +// must keep working without New(). +type changelogRenderCache struct { + body string + width int + out []mdLine + ok bool +} + +func (c *changelogRenderCache) lines(body string, width int) []mdLine { + if c == nil { + return markdownToLines(body, width) + } + if c.ok && c.width == width && c.body == body { + return c.out + } + out := markdownToLines(body, width) + *c = changelogRenderCache{body: body, width: width, out: out, ok: true} + return out +} + func (m Model) renderChangelogBlock(msg changelogMsg) string { if msg.err != nil { return ui.InfoStyle.Render("changelog unavailable: "+msg.err.Error()) + "\n" @@ -1373,7 +1402,7 @@ func (m Model) renderChangelogBlock(msg changelogMsg) string { // keep ANSI out of the rune-based wrap math. An empty result covers both // an empty body and one the converter consumed whole (all comments, all // separators). - lines := markdownToLines(msg.body, max(m.briefW-2, 10)) + lines := m.changelogRender.lines(msg.body, max(m.briefW-2, 10)) if len(lines) == 0 { sb.WriteString(ui.InfoStyle.Render("no release notes available.") + "\n") return sb.String() diff --git a/internal/model/render_test.go b/internal/model/render_test.go index 9e10b0d..b363573 100644 --- a/internal/model/render_test.go +++ b/internal/model/render_test.go @@ -3936,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") + } +}