diff --git a/cmd/khosra/wire.go b/cmd/khosra/wire.go index 67cdb73..51351fd 100644 --- a/cmd/khosra/wire.go +++ b/cmd/khosra/wire.go @@ -4,7 +4,6 @@ import ( "github.com/yuin/goldmark" "khosra/internal/ext/shortcodes" - "khosra/internal/ext/widows" "khosra/internal/render" ) @@ -16,6 +15,5 @@ import ( func extenders(partial render.Partial) []goldmark.Extender { return []goldmark.Extender{ shortcodes.New(partial), - widows.New(), } } diff --git a/docs/content-model.md b/docs/content-model.md index 3e0a5d4..b7eabb9 100644 --- a/docs/content-model.md +++ b/docs/content-model.md @@ -290,10 +290,9 @@ functions in `theme-contract.md`. A Bengali page reads `পৃষ্ঠা ২ / **Machine-readable output never localises.** A `datetime` attribute, a URL, or anything a parser reads stays ASCII in every locale. -**Widows are prevented**: the last two words of a paragraph or heading are joined by a non-breaking space, -so a single word never falls alone onto its own line. It works over the parsed tree, which is what keeps it -out of code spans — a pass over rendered HTML could not tell prose from an escaped one. A block ending in a -code span, link or emphasis is left alone, because the last "word" is then a construct rather than a word. +**Line breaking is not the engine's.** Widows and orphans are a layout problem the browser solves with +`text-wrap: pretty` and `text-wrap: balance`, and the engine has no business inserting characters into an +author's prose to influence it (ADR-0045). The reference theme's stylesheet does it. ## Shortcodes diff --git a/docs/decisions.md b/docs/decisions.md index c875719..d8ce429 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -614,3 +614,23 @@ never evicted, which is correct for a single-author site and wrong for an unboun Revisit if: a page render exceeds a few milliseconds after this, or output stops being a pure function of content — a comment stream, a per-visitor fragment. Then the parked validity model is the right shape, and its five axes will have consumers instead of guesses. + +## ADR-0045 — Widow prevention is the browser's job; the feature is deleted +Date: 2026-07-31 · Status: accepted (removes `internal/ext/widows`, shipped two commits earlier) +Decision: delete the widow-prevention feature. Line breaking belongs to whatever is laying out the text, so the +reference theme sets `text-wrap: pretty` on body copy and `text-wrap: balance` on headings, and the engine stops +touching the text. A layer test goes in `CLAUDE.md` so the question is asked before the next feature: content on +disk, data the browser needs, markup, or presentation — a presentation problem the browser can solve is not the +engine's. +Why: it was built at the wrong layer, and it contradicted two rules already recorded here. `theme-contract.md` +says the engine decides nothing about how something looks, and ADR-0034 says authored body text is the author's +— yet this inserted U+00A0 into that text. The practical harm follows from the layer error: the engine cannot +see the line box, so joining the last two words is a guess that can overflow a narrow viewport, and a reader +copying the paragraph gets a non-breaking space in their clipboard. CSS knows the line box and needs no bytes in +the content. +Consequence: cheap — 108 lines of engine deleted, one CSS declaration gained, and authored text is untouched +again. Expensive — `text-wrap: pretty` is unimplemented in some browsers, so those readers get ordinary wrapping; +that is a smaller cost than editing prose, and it improves on its own as browsers ship it. The typographer stays, +because turning `--` into an en dash is a text transformation no stylesheet can express — the distinction the +layer test is meant to draw. +Revisit if: nothing. If widows matter more than this, the answer is a better stylesheet. diff --git a/docs/extensions.md b/docs/extensions.md index 7e28bcb..58a68a5 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -65,7 +65,7 @@ wearing a disguise. |---|---|---| | `PhaseLoad` | raw bytes + frontmatter | translation fallback. *Not* includes: they turned out to be parse-phase, because splicing another file's parsed nodes into a page is invalid rather than merely awkward (ADR-0038) | | `PhaseParse` | the parsed Markdown tree | shortcodes, transclusion, image derivatives | -| `PhaseMarkup` | rendered HTML fragments, code spans skipped | nothing yet. Everything expected here turned out to belong earlier or later: smart quotes and dashes are a Markdown parser option, chrome localisation is a template function (ADR-0034), and widows are a tree transform — over rendered HTML none of them could tell prose from an escaped code span | +| `PhaseMarkup` | rendered HTML fragments, code spans skipped | nothing, and possibly nothing ever. Everything expected here belonged somewhere else: smart quotes and dashes are a Markdown parser option, chrome localisation is a template function (ADR-0034), and widows turned out to be CSS (ADR-0045). A phase with no inhabitants is worth noticing before it is built | | `PhasePage` | the assembled page object | OpenGraph, JSON-LD, related posts, series nav | | `PhaseOutput` | the final byte stream | minification, dithering, gemtext conversion | diff --git a/docs/state.md b/docs/state.md index 06cdc4c..b69d359 100644 --- a/docs/state.md +++ b/docs/state.md @@ -17,7 +17,6 @@ If this file disagrees with the code, the code is right and this file is a bug. | `internal/render/templates/` | reference theme: `base.html`, `page.html`, `list.html`, `shortcodes.html`, `theme.css` (ADR-0026) | — | | `internal/ext/shortcodes/` | first feature: `{{< name key="value" >}}` block parser and node renderer, rendering through a theme fragment (ADR-0036). `figure`, `gallery`, `include`, plus the derivative pass and remembered picture inspection (ADR-0042, ADR-0044) | 564 | | `internal/ext/check/` | third feature: validates a site root — what the engine worked around, broken internal links, missing titles and alt text, mixed series ordering | 216 | -| `internal/ext/widows/` | second feature: joins the last two words of a paragraph or heading with a non-breaking space, over the tree so code spans are safe | 108 | | `cmd/khosra/wire.go` | the only list of enabled features (`extensions.md`) | 20 | | `internal/web/resolve.go` | URL → (key, lang, page, tag) or a canonical redirect: language prefix, `/en/…` fork guard, pagination, tags, trailing slash | 112 | | `internal/web/asset.go` | files inside a bundle's own directory, looked up through the owning bundle so visibility can only ever inherit (ADR-0024) | 58 | @@ -26,7 +25,7 @@ If this file disagrees with the code, the code is right and this file is a bug. | `internal/web/web.go` | handler: resolve, look up with fallback, section and tag listings, sequence, `/static/` (misses and refusals alike answer 404), degrade on failure | 152 | | `cmd/khosra/main.go` | flags (`-site`, `-addr`, `-base`, `-cache`), wiring, startup including the derivative pass — the only place things are assembled | 92 | | `cmd/khosra/check.go` | the `check` subcommand: parse, print, exit code. What counts as a finding lives in the feature | 45 | -| `*_test.go` | table-driven, one file per source file; symlink escape (content and static), canonical paths, language fallback, aliases, pagination, tags, sequences, chrome, typography, shortcode escaping, galleries, includes, partials, widows, site settings, absolute URLs, robots, sitemap, slug routes, bundle assets, derivatives, feeds, 404, plus benchmarks for the render path and the checker | 2604 | +| `*_test.go` | table-driven, one file per source file; symlink escape (content and static), canonical paths, language fallback, aliases, pagination, tags, sequences, chrome, typography, shortcode escaping, galleries, includes, partials, site settings, absolute URLs, robots, sitemap, slug routes, bundle assets, derivatives, feeds, 404, plus benchmarks for the render path and the checker | 2608 | Serves a bundle at `/{section}/{slug}/` — the slug derived, or declared in frontmatter without moving the key (ADR-0035) — a paginated listing per section, tag listings global and @@ -34,7 +33,7 @@ section-narrowed, sequence navigation and a series archive on any nested bundle, URL, generated derivatives under `/derived/`, Atom feeds per site, section and tag, plus `/robots.txt` and `/sitemap.xml`. Chrome text, dates and digits render in English or Bengali; authored text is untouched but for typographic -smoothing and widow prevention (ADR-0034). This repo holds engine source only — the site root is external and passed with +smoothing (ADR-0034); line breaking is left to CSS (ADR-0045). This repo holds engine source only — the site root is external and passed with `khosra check` validates a site root and exits non-zero on anything that makes it wrong. `-site` (ADR-0011). `site.yaml` declares `base` and `title`; with a base, canonical, hreflang and OpenGraph URLs go absolute (ADR-0039). @@ -52,21 +51,20 @@ this change*. | Counter | Now | Extraction due at | What it buys | |---|---|---|---| -| Render transforms — **page-level only** | 0 | **3** | Stage pipeline (ordered `func(ctx,*Page) error`). Parse-phase work does *not* count and must not: goldmark's extender list is already an ordered pipeline for it, so typography, shortcodes and widows compose there (`cmd/khosra/wire.go`) and a second pipeline beside it would be pure duplication. This counts transforms over the assembled page, which nothing hosts yet — OpenGraph and JSON-LD (queue 15) are the first candidates | +| Render transforms — **page-level only** | 0 | **3** | Stage pipeline (ordered `func(ctx,*Page) error`). Parse-phase work does *not* count and must not: goldmark's extender list is already an ordered pipeline for it, so typography and shortcodes compose there (`cmd/khosra/wire.go`) and a second pipeline beside it would be pure duplication. This counts transforms over the assembled page, which nothing hosts yet — OpenGraph and JSON-LD (queue 15) are the first candidates | | Routing cases | 9 | **2** — done | Resolver at `internal/web/resolve.go`: bundle, language prefix, pagination, tag, section-narrowed tag | | Collection pages | 4 | **1** — done | Query primitive: `content.Query{Section, Tag, Lang}` + `Site.Run`. The fourth — a series archive — resolves through `Site.Sequence` instead: membership is structural and the sort ascends, so it shares the index but not the Query | | Views — **per-bundle selection only** | 0 | **2** | The View layer `architecture.md` describes: `view:` in frontmatter choosing a presentation, resolved through the cascade. Nothing selects a view yet. *Output formats* are counted separately and are not it: HTML, sitemap XML and Atom are three functions with nothing to share — an interface over them would have one member and no leverage | | Effects | 1 | **2** | Effect runner + trigger wiring (change / schedule / demand). The first is the derivative pass (ADR-0042), called straight from `cmd` at startup — one call needs no runner, and startup is the only change signal until queue 21 | -| Extensions | 3 | **3** — due | Extension registry (`extensions.md`). Three features exist, so the threshold is reached: see the note below the table before building one | Extension registry (`extensions.md`). The wire file arrived with the first feature rather than the registry — `cmd/khosra/wire.go`, one line, no struct | +| Extensions | 2 | **3** | Extension registry (`extensions.md`). Was briefly 3; deleting the widows feature (ADR-0045) put it back to 2, which is the counter doing its job — a threshold reached by a feature that should not have existed was not a threshold | | Interface implementations | — | **2** | The interface itself | | Non-stdlib dependencies | 4 direct | budget in `scripts/budgets.env` | — | -**Extensions counter is due, and the answer is probably still no.** Three features exist -(`shortcodes`, `widows`, `check`) but they plug in three different ways: two are goldmark extenders listed in -`extenders()`, and `check` is a function `cmd` calls. A registry would have to abstract over "thing that -extends Markdown" and "thing that validates content", which share nothing but the word *feature*. What the -counter is really detecting is that `wire.go` lists only one kind. Revisit when a *fourth* feature wants a -third way in — or when one wants a route, which is the seam ADR-0042 already named. +**When the Extensions counter comes due, look at how the features plug in.** The two that exist attach in +different ways — `shortcodes` is a goldmark extender in `extenders()`, `check` is a function `cmd` calls — so a +registry would have to abstract over "extends Markdown" and "validates content", which share nothing but the +word *feature*. Revisit when a third wants a *third* way in, or when one wants a route (the seam ADR-0042 +named). Allowlist, all four imported: `goldmark` (markdown), `golang.org/x/text` (NFC, ADR-0015), `gopkg.in/yaml.v3` (frontmatter, ADR-0020), `golang.org/x/image` (resampling and WebP, ADR-0040). diff --git a/docs/theme-contract.md b/docs/theme-contract.md index d8564a8..dda11d1 100644 --- a/docs/theme-contract.md +++ b/docs/theme-contract.md @@ -125,6 +125,13 @@ the document head, which is what a reader looks for: Only with a declared `base`: without one there is no feed to point at. +## Typography is the theme's + +The engine transforms text where no stylesheet could — smart quotes, dashes, ellipses — and stops there. How +text *lays out* is the theme's, done in CSS: the reference stylesheet sets `text-wrap: pretty` on body copy and +`text-wrap: balance` on headings, which is where widow and orphan control belongs (ADR-0045). The engine will +not insert characters into an author's prose to influence line breaking. + ## The stability rule Fields and names are **added, never renamed or removed**. Absence is always legal: a template reading a diff --git a/internal/ext/widows/doc.go b/internal/ext/widows/doc.go deleted file mode 100644 index b8a64b8..0000000 --- a/internal/ext/widows/doc.go +++ /dev/null @@ -1,7 +0,0 @@ -// Package widows keeps the last word of a paragraph or heading from falling alone onto its own line. -// -// Contributes: a Markdown AST transformer (PhaseParse). -// Cascade keys: none. -// Contract fields: none — it changes one space in the text, and nothing a theme reads. -// Not doing: hyphenation, orphans, balancing headings across lines — those are the theme's typography. -package widows diff --git a/internal/ext/widows/widows.go b/internal/ext/widows/widows.go deleted file mode 100644 index bd0fb17..0000000 --- a/internal/ext/widows/widows.go +++ /dev/null @@ -1,101 +0,0 @@ -package widows - -import ( - "bytes" - - "github.com/yuin/goldmark" - "github.com/yuin/goldmark/ast" - "github.com/yuin/goldmark/parser" - "github.com/yuin/goldmark/text" - "github.com/yuin/goldmark/util" -) - -// nbsp is the space that will not break. Written as bytes because it replaces bytes in the text. -var nbsp = []byte(" ") - -// New returns the Markdown extension. -func New() goldmark.Extender { return extension{} } - -type extension struct{} - -func (extension) Extend(md goldmark.Markdown) { - md.Parser().AddOptions(parser.WithASTTransformers(util.Prioritized(transformer{}, 200))) -} - -// transformer joins the last two words of every paragraph and heading. -// -// It works on the parsed tree rather than on rendered HTML, which is the whole reason this waited for the -// shortcode machinery: a pass over HTML cannot tell prose from an escaped code span, and would happily edit -// the inside of one (content-model.md). -type transformer struct{} - -func (transformer) Transform(doc *ast.Document, reader text.Reader, pc parser.Context) { - source := reader.Source() - err := ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) { - if !entering { - return ast.WalkContinue, nil - } - switch n.Kind() { - case ast.KindParagraph, ast.KindHeading: - join(n, source) - } - return ast.WalkContinue, nil - }) - if err != nil { - // Walk only fails if this function does, and it does not. - return - } -} - -// join replaces the last space of a block's final line with a non-breaking one. -// -// Only plain text qualifies. A block ending in a code span, a link or emphasis is left alone: the last -// "word" is then a construct rather than a word, and joining it to the one before would mean reaching inside -// markup for a typographic nicety. -func join(block ast.Node, source []byte) { - run := finalLine(block) - if len(run) == 0 { - return - } - var line []byte - for _, t := range run { - line = append(line, t.Segment.Value(source)...) - } - i := bytes.LastIndexByte(line, ' ') - if i < 0 || i == len(line)-1 { - // One word, or a trailing space with nothing after it: nothing to keep together. - return - } - // A String node carries its own bytes, so the text can differ from the source. Editing the segment is - // impossible — a segment is an offset into bytes every other node shares. - joined := make([]byte, 0, len(line)+len(nbsp)) - joined = append(joined, line[:i]...) - joined = append(joined, nbsp...) - joined = append(joined, line[i+1:]...) - block.ReplaceChild(block, run[0], ast.NewString(joined)) - for _, t := range run[1:] { - block.RemoveChild(block, t) - } -} - -// finalLine is the block's last line as the consecutive text nodes that make it up. -// -// Several nodes, not one: another extension may have split the run. The typographer splits it wherever it -// looks for a substitution, so a paragraph ending "hand." arrives as two text nodes and the last of them -// holds no space at all — which is exactly how the first version of this passed its own tests and did -// nothing in the assembled engine. -func finalLine(block ast.Node) []*ast.Text { - var run []*ast.Text - last := block.LastChild() - for n := last; n != nil; n = n.PreviousSibling() { - t, isText := n.(*ast.Text) - if !isText { - break - } - if n != last && (t.SoftLineBreak() || t.HardLineBreak()) { - break // the final line starts after this node - } - run = append([]*ast.Text{t}, run...) - } - return run -} diff --git a/internal/ext/widows/widows_test.go b/internal/ext/widows/widows_test.go deleted file mode 100644 index 5afd430..0000000 --- a/internal/ext/widows/widows_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package widows - -import ( - "strings" - "testing" - - "github.com/yuin/goldmark" - gmext "github.com/yuin/goldmark/extension" -) - -const nb = "\u00a0" - -func convert(t *testing.T, markdown string) string { - t.Helper() - md := goldmark.New(goldmark.WithExtensions(New())) - var out strings.Builder - if err := md.Convert([]byte(markdown), &out); err != nil { - t.Fatal(err) - } - return out.String() -} - -func TestTheLastTwoWordsStayTogether(t *testing.T) { - got := convert(t, "The rain did not stop for nine days.\n") - if !strings.Contains(got, "nine"+nb+"days.") { - t.Errorf("expected a non-breaking space before the last word:\n%q", got) - } - if strings.Count(got, nb) != 1 { - t.Errorf("exactly one space should change, got %d:\n%q", strings.Count(got, nb), got) - } -} - -func TestHeadingsGetItToo(t *testing.T) { - got := convert(t, "## The Long Monsoon\n") - if !strings.Contains(got, "Long"+nb+"Monsoon") { - t.Errorf("a heading widow is the ugliest one:\n%q", got) - } -} - -func TestNothingToJoinIsLeftAlone(t *testing.T) { - for _, markdown := range []string{"Word\n", "\n"} { - got := convert(t, markdown) - if strings.Contains(got, nb) { - t.Errorf("%q should be untouched, got %q", markdown, got) - } - } -} - -func TestABlockEndingInMarkupIsLeftAlone(t *testing.T) { - // The last "word" is a construct, not a word. Reaching inside markup for a typographic nicety is how a - // transform starts corrupting content. - for _, markdown := range []string{ - "Run it with `khosra -site`\n", - "Read more [in the archive](/posts/)\n", - "It was *raining*\n", - } { - got := convert(t, markdown) - if strings.Contains(got, nb) { - t.Errorf("%q ends in markup and should be untouched, got %q", markdown, got) - } - } -} - -func TestCodeSpansAreNeverEdited(t *testing.T) { - got := convert(t, "Run `go test ./...` and then read the output.\n") - if strings.Contains(got, "go"+nb+"test") || strings.Contains(got, "test"+nb) { - t.Errorf("a code span must survive byte for byte:\n%q", got) - } - if !strings.Contains(got, "the"+nb+"output.") { - t.Errorf("the prose after it should still be joined:\n%q", got) - } -} - -func TestTextIsStillEscaped(t *testing.T) { - // The joined text becomes a String node, which is a different render path — it must escape like any - // other text, or this transform would be an injection route. - got := convert(t, "Compare a < b and c > d.\n") - if strings.Contains(got, "a < b") || !strings.Contains(got, "<") { - t.Errorf("a rewritten run must stay escaped:\n%q", got) - } -} - -func TestItStillWorksBesideTheTypographer(t *testing.T) { - // The bug this exists to catch: the typographer splits a text run wherever it looks for a substitution, - // so a paragraph ending "hand." arrives as two text nodes and the last holds no space. A version that - // only inspected the last child passed every test above and did nothing in the real engine. - md := goldmark.New(goldmark.WithExtensions(gmext.Typographer, New())) - var out strings.Builder - if err := md.Convert([]byte("Built by hand.\n"), &out); err != nil { - t.Fatal(err) - } - if !strings.Contains(out.String(), "by"+nb+"hand.") { - t.Errorf("widows must survive being combined with other extensions:\n%q", out.String()) - } -} diff --git a/internal/render/templates/theme.css b/internal/render/templates/theme.css index c6a7851..8182e35 100644 --- a/internal/render/templates/theme.css +++ b/internal/render/templates/theme.css @@ -1,7 +1,9 @@ /* Reference theme: legibility only, no design opinions (ADR-0026). */ html { font-family: Georgia, serif; line-height: 1.6; color: #1a1a1a; background: #fdfdfb; } main { max-width: 34rem; margin: 3rem auto; padding: 0 1rem; } -h1, h2, h3 { line-height: 1.25; font-weight: 600; } +h1, h2, h3 { line-height: 1.25; font-weight: 600; text-wrap: balance; } +/* Line breaking belongs to the browser, which knows the line box no engine-side guess can see (ADR-0045). */ +p, li, figcaption { text-wrap: pretty; } a { color: #1a4d7a; } img { max-width: 100%; height: auto; } pre, code { font-family: ui-monospace, monospace; font-size: 0.9em; }