delete the widows feature; line breaking is CSS
The human asked whether widow prevention belonged in the backend at all. It did not, and it broke two rules already written down: the theme contract says the engine decides nothing about how something looks, and ADR-0034 says authored body text is the author's — while this inserted U+00A0 into that text. The practical harm follows from the layer error rather than from a coding mistake. 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. `text-wrap: pretty` and `text-wrap: balance` in the reference stylesheet know the line box and need no bytes in the content. 108 lines of engine deleted for one CSS declaration. The typographer stays: turning `--` into an en dash is a text transformation no stylesheet can express, which is exactly the distinction the new layer test draws. Also worth recording: this took the Extensions counter from 3 back to 2. A threshold reached by a feature that should not have existed was never a threshold.
This commit is contained in:
Claude Opus 5
@@ -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(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
+1
-1
@@ -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 |
|
||||
|
||||
|
||||
+9
-11
@@ -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).
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
}
|
||||
@@ -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())
|
||||
}
|
||||
}
|
||||
@@ -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; }
|
||||
|
||||
Reference in New Issue
Block a user