feat(model): a house style for the README panel - #47
Merged
Conversation
Preprocessor (badges/HTML/emoji removal, link unwrapping) + custom glamour theme in keepkit palette for panel [3]. Design approved in brainstorm, hardened by plan review (URL hiding moved from style config to preprocessor after the review's spike showed autolink deletion). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A README is written for a browser. Panel [3] handed glamour whatever the repo shipped: rows of badge images, a <picture> logo wrapper, full hrefs bloating paragraphs in a narrow panel, and emoji a terminal font renders as tofu. cleanReadmeMarkdown is a pure pass that removes them. No image can ever render in a TTY, so images go whole in every form and no badge-vs-logo heuristic is needed; panel [3] links are not clickable, so [text](url) is unwrapped to its text. Autolinks and bare URLs stay — the URL is their content. Code is never rewritten, which is what the segmentation is for: a fenced block and an inline span are exactly how a README shows the markup these rules delete. Fences are split out first — an unterminated one protects to EOF, since version.getReadme truncates at readmeMaxBytes and the cut can land mid-fence — and inline spans are masked inside a cleanable segment rather than segmented out, because the block-level rules (a multi-line comment, a <picture> body) need the segment to stay one string and a span containing "<!--" has to survive too. Two gates keep the rules from eating content. The HTML tag strip matches an allowlist of the elements READMEs actually use, not a generic identifier shape: by shape there is no telling <kbd> from Vec<String>, and an unknown name left as written is the honest degradation. Shortcut references ([label]) are unwrapped only for labels a definition actually declared, collected document-wide because definitions collect at the bottom while their uses sit at the top; ungated, the rule would eat a task list's [x]. The per-line tidy-up is gated on the line having changed, so an untouched "- - -" is never mistaken for an emptied bullet and a hard line break keeps its trailing spaces. Its indent comes from the original line: a leading pictograph that removal turns into a space would otherwise become an indent the author never wrote, and four of them read as an indented code block. Shortcodes go only when the name is in goldmark-emoji's GitHub dictionary — the one glamour already pulls in, so promoting it to a direct dependency adds nothing to go.sum. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The standard glamour style is another app's design: an H1 on a bright plate — the loudest thing on the screen — headings and links in colors that appear nowhere else in keepkit, and a document margin eating 2-4 columns of an already narrow panel. keepkitStyle clones the standard config and re-accents it from internal/ui. Cloning rather than building from scratch is deliberate: StyleConfig has dozens of fields — chroma tokens, table separators, list indents — and inheriting them means a glamour upgrade that adds one cannot leave the panel with a hole in it. The globals it clones hold pointers that styles.DefaultStyles aliases, so every override assigns a fresh pointer; writing through a cloned one would restyle glamour for the whole process. TestKeepkitStyleLeavesGlobalsUntouched compares a JSON snapshot rather than a saved struct, because a struct copy aliases the very pointers that bug corrupts and would pass on it. Only the peach heading accents and the structural fixes reach the light variant: #E8E8E8 body text and #555555 rules are chosen against a dark panel and would be unreadable on white, so there the standard light colors stay. The theme is asserted at the struct level — tests have no TTY, so the color profile is Ascii and glamour strips every color it would emit. With production no longer choosing a style *name*, readmeStyleName degenerated to its test branch and is gone; renderReadme branches on testReadmeStyle directly and the seam keeps its one job, forcing the constructor to fail. The fallback now returns the preprocessed text, or a failed render would be the one path still showing badge and href noise. render_test.go's "content that renders to nothing" gets its teeth back along the way. It used to t.Skipf on a fixture that might or might not render empty; a badge-only README now cleans down to whitespace and renderReadme returns "" before glamour is built, so the placeholder path is a guarantee and the test fails instead of skipping. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CLAUDE.md and ARCHITECTURE.md: the two new model files, and the panel [3] rendering clause, which still described a fixed WithStandardStyle over merely sanitized text. The empty-render placeholder note changed with it — that case used to be a curiosity worth one sentence and is now a guarantee for any badge-only README. README.md: the two user-facing panel descriptions say a README is cleaned up before rendering, and goldmark-emoji joins the Stack, which is supposed to match go.mod's direct deps. Plan moved to docs/plans/completed/ with the deviations recorded — the tag allowlist, span masking, the per-line tidy-up landing a task early, and the shortcut-reference rule. One thing found while checking a real README is left out on purpose: a centered <h1 align="center">name</h1> keeps its text but loses its heading status, because the tag strip is text-preserving by design. Turning <h1>-<h6> into markdown headings is a rule nobody approved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A review of the finished branch found nine confirmed bugs in the README preprocessor. Build, vet, race tests and golangci-lint were all green on every one of them, which is the more useful finding: the suite tested the rules, not the inputs that break them. Each fix lands with a test row. 1. rcLineContent was quadratic — 8.4 s of frozen TUI. The pattern is ^-anchored, so ReplaceAllString could only ever rewrite the head, yet it copied the whole remaining line every pass. One line of "🚀 " plus 262144 "- " markers — 512 KiB, exactly readmeMaxBytes — inside a synchronous Update(), driven by untrusted remote markdown. It slices now, and a perf test guards the shape. 2. A URL containing ")" leaked its tail on screen. The flat "[^)]*" destination stopped at the inner paren of a shields.io badge (…/badge/a-(b)-blue.svg) or a Wikipedia link (…/wiki/Foo_(bar)) and left the rest as text — the exact noise the pass removes. mdDest now allows one level of nesting, which fixes the card's converter with it. 3. A paragraph line shaped like a definition was deleted. "[note]: this matters" sitting mid-paragraph vanished. Deleting a line of someone's README is the most destructive thing here, so it now takes two guards: a single-token destination with at most a quoted title, and CommonMark's rule that a definition cannot interrupt a paragraph. 4. The reference form [text][ref] was ungated while the shortcut form three lines below it was carefully gated for the identical reason: "the value arr[i][j] is used" came out as "the value arri is used". 5. A label declared inside an HTML comment gated the unwrapping. The label set was collected before the HTML rules ran, so a commented-out "[x]: url" ate a task list's "[x]" — the bracket the gate exists to protect. Collection moved after stripping; the pass is two phases now. 6. An HTML attribute containing ">" ended the tag early: <img alt="a > b" src="x.png"> rendered as ` b" src="x.png">`. The attribute pattern matches quoted values whole. 7. An orphaned setext underline outlived its dropped title, leaving a visible row of equals signs. Only the "=" form is dropped — a "-" run is equally a thematic break and renders as a rule either way. 8. CRLF fed straight to cleanReadmeMarkdown disabled the entire pass: a "```\r" closer never matches, so the first fence protected to EOF. Production was safe because cleanTerminalOutput drops \r first — but every unit test calls the function directly, i.e. every test took the broken path. Normalized on entry, like markdownToLines. 9. The hard-line-break claim was false. Both the comment and CLAUDE.md said the change-gate preserved two trailing spaces; it dropped them whenever the line changed. rcTidyLine carries them over explicitly. Documented rather than fixed: two code spans separated by nothing but a removed construct come back adjacent (`x``y` → `x``y`), which CommonMark reads as one span. It needs zero whitespace on both sides, and any fix invents a separator the author did not write. Also closed the test gaps the review named: paren URLs, CRLF, the ungated reference form, definition-in-a-paragraph, the ten-badge fold, the stray-backtick end-to-end case, and a table-cell autolink row that actually kills the WithInlineTableLinks mutation — verified by deleting the option and watching the test fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Panel
[3]rendered every README the way its repo shipped it: rows of badge images, a<picture>logo wrapper, full hrefs bloating paragraphs in a narrow panel, emoji as tofu, and glamour's standard style on top — an H1 on a bright plate, in colors that appear nowhere else in keepkit.Two independent layers, composed in
renderReadme:cleanReadmeMarkdown(internal/model/readme_clean.go) — a pure preprocessor betweencleanTerminalOutputand glamour. Removes images, HTML, emoji and shortcodes; unwraps[text](url)to its text. No image can ever render in a TTY and panel[3]links are not clickable, so neither needs a heuristic. Autolinks and bare URLs stay — the URL is their content.keepkitStyle(internal/model/readme_style.go) — glamour's standard config cloned and re-accented frominternal/ui's palette. No H1 plate, no document margin.What is load-bearing
version.getReadmetruncates atreadmeMaxBytesand the cut can land mid-fence); inline spans are masked, not segmented, because the block-level rules need the segment to stay one string and a span containing<!--has to survive.<kbd>fromVec<String>; an unknown name is left as written.[x].keepkitStyleclones hold pointersstyles.DefaultStylesaliases; writing through one would restyle glamour for the whole process.TestKeepkitStyleLeavesGlobalsUntouchedcompares a JSON snapshot — a struct copy aliases the very pointers the bug corrupts and would pass on it.readmeStyleNameis gone: with production no longer choosing a style name, it degenerated to its test branch.testReadmeStylekeeps its one job — forcing the constructor to fail.Dependencies
goldmark-emojipromoted from indirect to direct (glamour already pulls it in;go.sumunchanged). It supplies the GitHub shortcode dictionary, so keepkit and the renderer agree on what a shortcode is.Testing
internal/model/readme_clean_test.go(tables + negatives: autolink,Vec<String>,a < b,12:30:45,[x],- - -, hard line break, list indentation),readme_style_test.go(struct-level theme + end-to-end content), updatedreadme_test.goandrender_test.go.Preflight green:
go build,go vet ./...,go test -race ./...,golangci-lint run, plus the cross-compile step.Not in this PR
A centered
<h1 align="center">name</h1>— very common in badge-heavy READMEs — keeps its text but loses its heading status: the tag strip is text-preserving by design. Turning<h1>–<h6>elements into markdown headings is a separate rule and was not part of the approved plan.A mailto autolink still renders as
team@example.com mailto:team@example.com. Pre-existing glamour behaviour, and the plan's decision log rejectedLink.Formatblanking because it deletes plain autolinks outright.demo/hero.gifshows panel[3]and will visibly change — re-recording is a follow-up.🤖 Generated with Claude Code