From 7da2a58fd5107a293e9fd599771540264adbbeda Mon Sep 17 00:00:00 2001 From: bdeshi Date: Fri, 31 Jul 2026 03:57:52 +0600 Subject: [PATCH] generate sized derivatives ahead of the request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pass over the content at startup writes three widths per picture into a cache outside the site root, named by the source's content hash and the width (ADR-0042). Idempotent by construction: a rerun stats and skips, an edited picture takes a new name, and nothing stale can be served under an old one. Restarting the evidence site made 0 derivatives the second time, as it should. Ahead of the request rather than during it, because resampling is felt and there is no page cache yet to hide it. Outside the site root, because the engine reads that directory and must not leave generated files in somebody's content git — a lost cache costs one startup pass and no correctness. Markup now carries the original as src, the derivatives as srcset closed by the original at its own width, and width/height from the original — which retires most of the latent row about the output floor; only a gallery's alt is still empty, and a filename cannot supply that. Two things the work itself decided: `Fragment.Items` became `Fragment.Pictures`, ADR-0037's own revisit trigger. Items had one consumer, so widening it beat adding a second list beside it. "A browser can show it" and "we can resample it" are different questions, and conflating them nearly deleted content: an SVG has no decoder here, so a single predicate would have dropped SVGs from galleries silently. Undecodable and unsupported pictures are now rendered as they are, without a size or a srcset. --- cmd/khosra/main.go | 28 ++- docs/content-model.md | 21 ++- docs/decisions.md | 25 +++ docs/state.md | 22 +-- docs/theme-contract.md | 16 +- go.mod | 2 + go.sum | 2 + internal/content/content.go | 7 + internal/ext/shortcodes/images.go | 215 ++++++++++++++++++++++ internal/ext/shortcodes/images_test.go | 120 ++++++++++++ internal/ext/shortcodes/shortcodes.go | 38 ++-- internal/render/render.go | 22 ++- internal/render/templates/shortcodes.html | 11 +- internal/web/asset_test.go | 2 +- internal/web/discover_test.go | 2 +- internal/web/slug_test.go | 4 +- internal/web/web.go | 8 +- internal/web/web_test.go | 16 +- 18 files changed, 502 insertions(+), 59 deletions(-) create mode 100644 internal/ext/shortcodes/images.go create mode 100644 internal/ext/shortcodes/images_test.go diff --git a/cmd/khosra/main.go b/cmd/khosra/main.go index 645893c..e019afa 100644 --- a/cmd/khosra/main.go +++ b/cmd/khosra/main.go @@ -8,9 +8,11 @@ import ( "log/slog" "net/http" "os" + "path/filepath" "strings" "khosra/internal/content" + "khosra/internal/ext/shortcodes" "khosra/internal/render" "khosra/internal/web" ) @@ -19,6 +21,7 @@ func main() { site := flag.String("site", os.Getenv("KHOSRA_SITE"), "path to the site root (or KHOSRA_SITE)") addr := flag.String("addr", "localhost:8080", "address to listen on") base := flag.String("base", "", "canonical site origin, overriding site.yaml (e.g. https://khosra.example)") + cache := flag.String("cache", defaultCache(), "directory for generated files; never inside the site root") flag.Parse() if *site == "" { @@ -44,12 +47,33 @@ func main() { fatal("cannot prepare the theme", err) } - slog.Info("serving", "site", *site, "bundles", len(bundles), "addr", *addr) - if err := http.ListenAndServe(*addr, web.Handler(content.NewSite(bundles), renderer, fsys, settings)); err != nil { + // Derivatives are made before the first request rather than during one (ADR-0042). A failure here is not + // fatal: pages still serve the author's originals, which is the whole point of the fallback. + made, err := shortcodes.Derive(fsys, *cache) + if err != nil { + slog.Error("some derivatives were not made", "cache", *cache, "err", err) + } + derivedFS, err := content.OpenSite(*cache) + if err != nil { + slog.Error("generated files will not be served", "cache", *cache, "err", err) + } + + slog.Info("serving", "site", *site, "bundles", len(bundles), "derivatives", made, "addr", *addr) + if err := http.ListenAndServe(*addr, web.Handler(content.NewSite(bundles), renderer, fsys, derivedFS, settings)); err != nil { fatal("server stopped", err) } } +// defaultCache is where generated files go when nothing says otherwise: the user's cache directory, never +// the site root, because the engine reads that and must not litter somebody's content git (ADR-0042). +func defaultCache() string { + dir, err := os.UserCacheDir() + if err != nil { + return "" + } + return filepath.Join(dir, "khosra", "derived") +} + // fatal reports a startup failure and exits. Startup failure is fatal and loud; request-time failure // degrades instead (conventions.md). func fatal(msg string, err error) { diff --git a/docs/content-model.md b/docs/content-model.md index 89b2fdb..dfbbc7f 100644 --- a/docs/content-model.md +++ b/docs/content-model.md @@ -333,12 +333,23 @@ Sharing one fragment between bundles is deliberately not possible yet: it needs parts, which is a decision about the disk contract rather than a missing feature. Transclusion of another bundle's *body* is Arc 4 and needs a cycle guard of its own. -## Images `[spec]` +## Images -Optimisation and sizing are a Stage plus emitted derivative files, content-addressed by source hash -and target width so rebuilds are idempotent and cheap. Emit width/height into the markup to prevent -layout shift. Never mutate the author's original. Prefer stdlib decoders; a small dependency only -with an ADR. +Sized derivatives are generated by a pass over the content **before the first request**, into a cache +directory outside the site root (`-cache`, defaulting under the user cache dir). Each is named by its +source's content hash plus the target width, so the pass is idempotent, an edited picture yields a new name, +and nothing stale is ever served (ADR-0042). The engine never writes into the site root and never touches an +original. + +Three widths — 480, 960, 1440 — and never an upscale: a picture already narrower than a target gets no +derivative for it. Markup carries the original as `src`, the derivatives as `srcset` closed by the original at +its own width, and `width`/`height` from the original so a page reserves the box before any bytes arrive. + +JPEG, PNG, GIF and WebP are decoded and resampled (ADR-0040). SVG needs no resizing and AVIF has no decoder +available, so both are rendered as they are, with no size and no `srcset` — a picture the engine cannot +optimise is still the author's picture, and is never dropped from a gallery. + +`/derived/` is engine-owned, like `/tags/` and `/robots.txt`: nothing an author writes is addressed there. ## Time-dependent presentation `[spec]` diff --git a/docs/decisions.md b/docs/decisions.md index 8367057..2839cbd 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -550,3 +550,28 @@ going there: if they land in core, `EXT_LOC` measures nothing and invariant 9 be the whole reason there are two ceilings. Revisit if: core approaches 2800. That is the question "what here is not core?" and the answer is a leaf, not a third raise. + +## ADR-0042 — Derivatives are generated ahead of the request, into a cache outside the site root +Date: 2026-07-30 · Status: accepted (replaces `Fragment.Items` with `Fragment.Pictures`, ADR-0037's own +revisit trigger) +Decision: sized image derivatives are produced by a pass over the content at startup, not during a request, +and written to a cache directory outside the site root (`-cache`, defaulting under `os.UserCacheDir`). Each is +named by the source's content hash and the target width, so the pass is idempotent and a changed source yields +a different name. The engine never writes into the site root and never touches an original. Core serves that +cache as a directory of opaque names; every image decision — widths, naming, dimensions, which files are +images — lives in `internal/ext/shortcodes`, the package whose shortcodes need it. A fragment now receives +`Pictures`, each carrying `Src`, `Srcset`, `Width` and `Height`, replacing the bare `Items` list. +Why: resampling on the request path would make the first view of a page take seconds, and there is no page +cache yet to hide it. Writing derivatives into the site root would put generated files in somebody's content +git — the engine reads that directory and must not litter it, and derived state is disposable by definition +(ADR-0010). Content-addressed names mean a rebuild rewrites nothing, and a lost cache costs one startup pass +rather than any correctness. `Items` had exactly one consumer, so widening it in place beat adding a second +list beside it. +Consequence: cheap — no request pays for resampling, the cache can be deleted at any time, and `width`/`height` +in the markup end the layout-shift problem the output floor named. Expensive — a new image needs a restart +until change detection lands (queue 21), the cache is a second directory to think about when deploying, and +AVIF passes through unresized since nothing can decode it. A feature still cannot serve a route of its own, so +core carries a generic "serve this directory of derived files" — which is the seam to revisit when a second +feature wants output of its own. +Revisit if: startup time becomes noticeable on a large site — then the pass wants a manifest and a change +check rather than a stat per candidate. diff --git a/docs/state.md b/docs/state.md index 9f81835..0380bff 100644 --- a/docs/state.md +++ b/docs/state.md @@ -1,6 +1,6 @@ # State -**Verified against:** `5e49094` on 2026-07-30 — update this line every change. +**Verified against:** `f4ba695` on 2026-07-30 — update this line every change. If this file disagrees with the code, the code is right and this file is a bug. ## Inventory @@ -15,20 +15,20 @@ If this file disagrees with the code, the code is right and this file is a bug. | `internal/render/render.go` | goldmark with the typographer, per-kind template sets with site override, the `Partial`/`Origin` seams features render and resolve through, `Page`/`List`/`Sequence`/`head` | 409 | | `internal/render/chrome.go` | the engine's own words: phrase table, month names, digits, and the `t`/`num`/`day` template funcs (ADR-0034) | 105 | | `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` | 315 | +| `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 picture inspection (ADR-0042) | 534 | | `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 | | `internal/web/discover.go` | `/robots.txt` and `/sitemap.xml`, absolute and only with a declared base (ADR-0039) | 74 | | `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`), wiring, startup — the only place things are assembled | 60 | -| `*_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, 404 | 2050 | +| `cmd/khosra/main.go` | flags (`-site`, `-addr`, `-base`, `-cache`), wiring, startup including the derivative pass — the only place things are assembled | 88 | +| `*_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, 404 | 2170 | 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 section-narrowed, sequence navigation and a series archive on any nested bundle, `static/` verbatim, a directory bundle's own files under its -URL, plus `/robots.txt` and `/sitemap.xml`. +URL, generated derivatives under `/derived/`, 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 `-site` (ADR-0011). `site.yaml` declares `base` and `title`; with a base, canonical, hreflang and OpenGraph @@ -38,7 +38,7 @@ Frontmatter the parser lifts today: `title`, `date`, `tags`, `aliases`, `order`, `content-model.md`'s table — including `draft` and `type` — lands in `Extra` unread, so that table is the accepted format, not a list of what runs. -Dependencies: three, all allowlisted — `goldmark`, `golang.org/x/text`, `gopkg.in/yaml.v3`. +Dependencies: four, all allowlisted — `goldmark`, `golang.org/x/text`, `golang.org/x/image`, `gopkg.in/yaml.v3`. ## Counters — the earn-it authority @@ -51,13 +51,13 @@ this change*. | Routing cases | 8 | **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 / output formats | 2 | **2** — due | Two template sets exist (bundle, listing); the View layer is Arc 2's third item | -| Effects | 0 | **2** | Effect runner + trigger wiring (change / schedule / demand) | +| 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 | 2 | **3** | Extension registry (`extensions.md`). The wire file arrived with the first feature rather than the registry — `cmd/khosra/wire.go`, one line, no struct | | Interface implementations | — | **2** | The interface itself | -| Non-stdlib dependencies | 3 direct | budget in `scripts/budgets.env` | — | +| Non-stdlib dependencies | 4 direct | budget in `scripts/budgets.env` | — | -Allowlist, all three imported: `goldmark` (markdown), `golang.org/x/text` (NFC, ADR-0015), -`gopkg.in/yaml.v3` (frontmatter, ADR-0020). +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). ## Latent items — known, deliberately unfixed @@ -70,7 +70,7 @@ with a stated reason. A list nothing drains is a graveyard of known defects. | No mechanical check that the counters are *correct* | Accepted at the Arc 1 boundary: the coupling gate makes forgetting them impossible, which is the real failure mode, and checking the values needs code to count | The first page-level transform (queue 15), now that the transform counter means something narrower | | No mechanical gate on the untrusted boundary (ADR-0003) | Scheduled to Arc 3: nothing untrusted is read yet. Half of it is now mechanical — `verify.sh` rejects `WithUnsafe`, so authored Markdown cannot become HTML — but there is still no check that a *future* untrusted source stays out of shortcode and template evaluation | The comment path — a test that untrusted input reaches no shortcode or template evaluation | | `date` stays in `Extra` after being lifted onto `Bundle.Date`, unlike `title`, `aliases`, `tags` and `order`, which are deleted | Spotted while adding `order`; the theme contract says `Extra` holds what the parser does not name, so one of the two is wrong. Harmless today — a template reading `.Extra.date` gets the raw YAML value | Whatever next reads `Extra` generically: feeds (queue 14) or `check` (17) | -| The reference theme's images carry no `width`/`height`, and a gallery's carry no `alt` — below the output floor `conventions.md` states | Nothing can supply them yet: dimensions need the image read, and a filename is not alt text. An empty `alt` is at least honest about a picture nothing describes | Image derivatives (queue 13) compute dimensions; structured gallery items with captions land with them (ADR-0037's revisit note) | +| A gallery's images carry no `alt` | `width`/`height` now come from the original (ADR-0042), so only alt text is missing, and a filename does not supply one. An empty `alt` is honest for a picture the page has already introduced | Captions per gallery entry — a sidecar or a frontmatter list — if the reference theme ever needs them | | Sequence resolution rescans the index on every bundle request — two passes over every key, each doing a `Lookup` | No cache exists anywhere yet, and a site of this size resolves in microseconds. Measuring first is the rule (queue 16) | The page cache (queue 16), which is the thing that makes the cost visible | ## Open questions diff --git a/docs/theme-contract.md b/docs/theme-contract.md index 7f557f6..64416b0 100644 --- a/docs/theme-contract.md +++ b/docs/theme-contract.md @@ -84,12 +84,20 @@ Every fragment receives the same two fields (ADR-0037): | Field | Contents | |---|---| | `.Args` | the call's `key="value"` pairs, exactly as the author wrote them | -| `.Items` | a list the *engine* gathered, empty unless that shortcode gathers one | +| `.Pictures` | images the *engine* gathered: one for a figure, many for a gallery, none when the call names nothing it recognises (ADR-0042) | + +Each picture carries: + +| Field | Contents | +|---|---| +| `.Src` | the author's own file, relative to the bundle — always usable on its own | +| `.Srcset` | the generated widths, closed by the original at its own width; empty when nothing was worth generating, or when the format has no decoder | +| `.Width`, `.Height` | the original's intrinsic size, for reserving the box; zero when the file could not be read | | Shortcode | Template | Receives | |---|---|---| | `{{< figure src="…" alt="…" caption="…" >}}` | `figure` | `.Args.src`, `.Args.alt`, `.Args.caption` | -| `{{< gallery >}}` | `gallery` | `.Items` — the picture filenames beside the bundle, in filename order | +| `{{< gallery >}}` | `gallery` | `.Pictures` — every picture beside the bundle, in filename order | `{{< include file="…" >}}` has **no fragment**: an included file is content, so it renders as Markdown in place and a theme has nothing to style about it (ADR-0038). @@ -99,8 +107,8 @@ engine found. Arguments are escaped by `html/template` like any other data, in w template puts them — which is what keeps an author's text out of the markup. A call whose template is missing renders nothing and logs; it never fails the page. -A gallery entry is a bare filename, relative to the bundle, so a template writes it straight into `src`. It -carries no alt text or dimensions, because nothing in a filename supplies either. +A gallery entry has no alt text: nothing in a filename supplies one, and inventing it would be worse than an +empty `alt` on a picture the page has already introduced. A figure's alt is the author's, from `.Args.alt`. The set is overlaid the same way as the page kinds: a site's `templates/shortcodes.html` is parsed after the embedded one, so redefining `figure` replaces it and any fragment left alone is inherited. Argument diff --git a/go.mod b/go.mod index 6d5b8e5..8feb564 100644 --- a/go.mod +++ b/go.mod @@ -8,3 +8,5 @@ require ( ) require github.com/yuin/goldmark v1.8.5 + +require golang.org/x/image v0.44.0 diff --git a/go.sum b/go.sum index 93aac37..9db6365 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ github.com/yuin/goldmark v1.8.5 h1:r6N5afV5qj/5S4UTch8agZHJ8UxNCMwX7WjkkJam2NA= github.com/yuin/goldmark v1.8.5/go.mod h1:ip/1k0VRfGynBgxOz0yCqHrbZXhcjxyuS66Brc7iBKg= +golang.org/x/image v0.44.0 h1:+tDekMZED9+LrtB3G5xzRggpVh9CARjZqROla3R3R+I= +golang.org/x/image v0.44.0/go.mod h1:V8K3KE9KKKE+pLpQDOeN18w9oacNSvy1tDOirTu4xtY= golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/internal/content/content.go b/internal/content/content.go index ab11aa9..78ecdb0 100644 --- a/internal/content/content.go +++ b/internal/content/content.go @@ -380,6 +380,13 @@ func TagURL(section, slug, lang string, page int) string { return PageURL(key, lang, page) } +// DerivedPrefix is where generated files are served from. Reserved like any other engine-owned path, and +// deliberately not under content: nothing an author writes is addressed there (ADR-0042). +const DerivedPrefix = "/derived/" + +// DerivedURL is the address of one generated file. +func DerivedURL(name string) string { return DerivedPrefix + name } + // TagsSegment is reserved at the top level and inside every section, so no bundle may be slugged with it. const TagsSegment = "tags" diff --git a/internal/ext/shortcodes/images.go b/internal/ext/shortcodes/images.go new file mode 100644 index 0000000..00328f7 --- /dev/null +++ b/internal/ext/shortcodes/images.go @@ -0,0 +1,215 @@ +package shortcodes + +import ( + "bytes" + "crypto/sha256" + "encoding/hex" + "fmt" + "image" + _ "image/gif" + "image/jpeg" + "image/png" + "io/fs" + "log/slog" + "os" + "path" + "path/filepath" + "strconv" + "strings" + + "golang.org/x/image/draw" + _ "golang.org/x/image/webp" + + "khosra/internal/content" + "khosra/internal/render" +) + +// widths are the derivative sizes offered to a browser. Three, spanning phone to desktop: enough for srcset +// to matter and few enough that the startup pass stays quick. They become a setting when someone wants a +// different set, not before. +var widths = []int{480, 960, 1440} + +// Derive writes every missing derivative for the pictures under content/, returning how many it made. +// +// Ahead of the request, never during one: resampling takes long enough to be felt, and there is no page cache +// to hide it (ADR-0042). Idempotent, because a derivative is named after its source's content — so a rerun +// stats and skips, and an edited picture simply has a different name. Originals are only ever read. +func Derive(siteFS fs.FS, cacheDir string) (int, error) { + if siteFS == nil || cacheDir == "" { + return 0, nil + } + if err := os.MkdirAll(cacheDir, 0o755); err != nil { + return 0, fmt.Errorf("derivative cache %s: %w", cacheDir, err) + } + made := 0 + err := fs.WalkDir(siteFS, "content", func(p string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() || !derivable(p) { + return err + } + data, err := fs.ReadFile(siteFS, p) + if err != nil { + slog.Error("skipping unreadable picture", "path", p, "err", err) + return nil + } + n, err := derive(data, p, cacheDir) + if err != nil { + // One unreadable picture must not stop a site from starting (ADR-0029). + slog.Error("skipping picture", "path", p, "err", err) + return nil + } + made += n + return nil + }) + if err != nil { + return made, fmt.Errorf("walk for pictures: %w", err) + } + return made, nil +} + +// derive writes the derivatives one picture is missing. +func derive(data []byte, name, cacheDir string) (int, error) { + src, _, err := image.Decode(bytes.NewReader(data)) + if err != nil { + return 0, err + } + made := 0 + for _, w := range widths { + if src.Bounds().Dx() <= w { + // Never upscale: a bigger file that looks worse is not a derivative worth having. + continue + } + file := filepath.Join(cacheDir, derivedName(data, name, w)) + if _, err := os.Stat(file); err == nil { + continue + } + if err := write(file, scale(src, w), name); err != nil { + return made, err + } + made++ + } + return made, nil +} + +// scale resamples to a target width, keeping the aspect ratio. +// +// CatmullRom because the alternative in the standard library is nearest neighbour, which is visibly wrong on +// photographs — the reason ADR-0040 accepted a dependency at all. +func scale(src image.Image, width int) image.Image { + b := src.Bounds() + height := b.Dy() * width / b.Dx() + out := image.NewRGBA(image.Rect(0, 0, width, height)) + draw.CatmullRom.Scale(out, out.Bounds(), src, b, draw.Over, nil) + return out +} + +// write encodes an image beside its siblings in the cache, atomically. +// +// Through a temporary file and a rename, so a derivative is either absent or complete: a half-written one +// would be served as a broken image and, being content-named, never regenerated. +func write(file string, img image.Image, source string) error { + tmp, err := os.CreateTemp(filepath.Dir(file), ".khosra-*") + if err != nil { + return err + } + defer os.Remove(tmp.Name()) + if err := encode(tmp, img, source); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + return os.Rename(tmp.Name(), file) +} + +// encode writes PNG for sources that may carry transparency and JPEG for the rest, which is also what +// derivedName spells in the extension. +func encode(w *os.File, img image.Image, source string) error { + if lossless(source) { + return png.Encode(w, img) + } + return jpeg.Encode(w, img, &jpeg.Options{Quality: 82}) +} + +// Picture describes one image for a fragment: where to fetch it, what a browser may choose instead, and the +// intrinsic size, so a page reserves the right box before any bytes arrive. +// +// Src stays the original: a derivative is an optimisation, and a browser that ignores srcset still gets the +// picture the author put there (ADR-0042). +func picture(origin render.Origin, file string) (render.Picture, bool) { + if origin.Files == nil || !showable(file) { + return render.Picture{}, false + } + // The author's file is the picture, whatever the engine can make of it. An SVG has no decoder here and an + // AVIF has none anywhere, so both are rendered as they are, without a size or a srcset — dropping them + // would delete content because the engine cannot optimise it. + p := render.Picture{Src: file} + if !derivable(file) { + return p, true + } + name := path.Join(origin.Dir, file) + data, err := fs.ReadFile(origin.Files, name) + if err != nil { + slog.Error("picture unreadable", "path", name, "err", err) + return p, true + } + cfg, _, err := image.DecodeConfig(bytes.NewReader(data)) + if err != nil { + slog.Warn("picture kept but not sized: cannot decode", "path", name, "err", err) + return p, true + } + p.Width, p.Height = cfg.Width, cfg.Height + var sources []string + for _, w := range widths { + if cfg.Width <= w { + continue + } + sources = append(sources, content.DerivedURL(derivedName(data, name, w))+" "+strconv.Itoa(w)+"w") + } + if len(sources) > 0 { + // The original closes the set at its own width, so a wide viewport still has the best file to pick. + sources = append(sources, file+" "+strconv.Itoa(cfg.Width)+"w") + p.Srcset = strings.Join(sources, ", ") + } + return p, true +} + +// derivedName is the cache filename for one source at one width: the source's content hash, so an edit +// changes the name and nothing stale is ever served, plus the width and the encoding's extension. +func derivedName(data []byte, source string, width int) string { + sum := sha256.Sum256(data) + ext := ".jpg" + if lossless(source) { + ext = ".png" + } + return hex.EncodeToString(sum[:8]) + "-w" + strconv.Itoa(width) + ext +} + +// lossless reports whether a source may carry transparency, which decides the derivative's encoding. +func lossless(source string) bool { + switch strings.ToLower(path.Ext(source)) { + case ".png", ".gif", ".webp": + return true + } + return false +} + +// showable reports whether a browser can display the file: what belongs in a gallery, whether or not this +// engine can do anything clever with it. +func showable(name string) bool { + switch strings.ToLower(path.Ext(name)) { + case ".jpg", ".jpeg", ".png", ".gif", ".webp", ".svg", ".avif": + return true + } + return false +} + +// derivable reports whether the file can be decoded and resampled here. SVG needs no resizing and AVIF has no +// decoder available, so neither gets derivatives (ADR-0040). +func derivable(name string) bool { + switch strings.ToLower(path.Ext(name)) { + case ".jpg", ".jpeg", ".png", ".gif", ".webp": + return true + } + return false +} diff --git a/internal/ext/shortcodes/images_test.go b/internal/ext/shortcodes/images_test.go new file mode 100644 index 0000000..48c8ba4 --- /dev/null +++ b/internal/ext/shortcodes/images_test.go @@ -0,0 +1,120 @@ +package shortcodes + +import ( + "bytes" + "image" + "image/color" + "image/jpeg" + "image/png" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + "testing/fstest" +) + +// wide builds a real encoded picture of a given width, so the tests exercise decoding rather than a stub. +func wide(t *testing.T, width int, asPNG bool) []byte { + t.Helper() + img := image.NewRGBA(image.Rect(0, 0, width, width/2)) + for x := range width { + for y := range width / 2 { + img.Set(x, y, color.RGBA{uint8(x % 256), uint8(y % 256), 128, 255}) + } + } + var out bytes.Buffer + var err error + if asPNG { + err = png.Encode(&out, img) + } else { + err = jpeg.Encode(&out, img, nil) + } + if err != nil { + t.Fatal(err) + } + return out.Bytes() +} + +func TestDeriveMakesEachMissingWidthAndIsIdempotent(t *testing.T) { + cache := t.TempDir() + big := wide(t, 2000, false) + fsys := fstest.MapFS{ + "content/art/set/index.md": {Data: []byte("---\ntitle: Set\n---\n")}, + "content/art/set/big.jpg": {Data: big}, + "content/art/set/small.png": {Data: wide(t, 300, true)}, // narrower than every target + "content/art/set/notes.md": {Data: []byte("not a picture")}, + } + made, err := Derive(fsys, cache) + if err != nil { + t.Fatal(err) + } + if made != len(widths) { + t.Errorf("made %d derivatives, want %d — one per width below the original's", made, len(widths)) + } + // Never upscale: nothing is made for a picture already narrower than the targets. + entries, err := os.ReadDir(cache) + if err != nil { + t.Fatal(err) + } + if len(entries) != len(widths) { + t.Errorf("cache holds %d files, want %d", len(entries), len(widths)) + } + // Each is a real image of the width it claims. + for _, e := range entries { + data, err := os.ReadFile(filepath.Join(cache, e.Name())) + if err != nil { + t.Fatal(err) + } + cfg, _, err := image.DecodeConfig(bytes.NewReader(data)) + if err != nil { + t.Fatalf("%s is not a decodable image: %v", e.Name(), err) + } + if !strings.Contains(e.Name(), "-w"+strconv.Itoa(cfg.Width)) { + t.Errorf("%s decodes to width %d, which its name does not claim", e.Name(), cfg.Width) + } + if cfg.Height != cfg.Width/2 { + t.Errorf("%s is %dx%d — the aspect ratio was not kept", e.Name(), cfg.Width, cfg.Height) + } + } + // A second pass writes nothing: the name is the source's content, so there is nothing new to make. + again, err := Derive(fsys, cache) + if err != nil { + t.Fatal(err) + } + if again != 0 { + t.Errorf("second pass made %d, want 0 — the pass must be idempotent (ADR-0042)", again) + } +} + +func TestAnEditedPictureGetsADifferentName(t *testing.T) { + // Content-addressed, so a stale derivative can never be served under a name that now means something else. + first := derivedName(wide(t, 1000, false), "content/a/x.jpg", 480) + second := derivedName(wide(t, 1200, false), "content/a/x.jpg", 480) + if first == second { + t.Error("two different pictures must not share a derivative name") + } + if derivedName([]byte("same"), "a.png", 480) == derivedName([]byte("same"), "a.png", 960) { + t.Error("two widths of one picture must not share a name either") + } + if !strings.HasSuffix(derivedName([]byte("x"), "a.png", 480), ".png") { + t.Error("a source that may carry transparency keeps a lossless derivative") + } +} + +func TestOriginalsAreNeverTouched(t *testing.T) { + cache := t.TempDir() + big := wide(t, 2000, false) + original := append([]byte(nil), big...) + fsys := fstest.MapFS{"content/art/set/big.jpg": {Data: big}} + if _, err := Derive(fsys, cache); err != nil { + t.Fatal(err) + } + after, err := fsys.ReadFile("content/art/set/big.jpg") + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(original, after) { + t.Error("the author's own file was modified") + } +} diff --git a/internal/ext/shortcodes/shortcodes.go b/internal/ext/shortcodes/shortcodes.go index 470e370..a4fd834 100644 --- a/internal/ext/shortcodes/shortcodes.go +++ b/internal/ext/shortcodes/shortcodes.go @@ -6,7 +6,6 @@ import ( "io/fs" "log/slog" "path" - "slices" "sort" "strings" @@ -141,8 +140,8 @@ type node struct { ast.BaseBlock name string args map[string]string - // items are what the feature gathered at parse time, when it still knew which bundle this is. - items []string + // pictures are what the feature gathered at parse time, when it still knew which bundle this is. + pictures []render.Picture // content is output the feature produced itself, written instead of a theme fragment. An included file // is content, not decoration, so it has no template (ADR-0038). content []byte @@ -172,7 +171,13 @@ func (blocks) Open(parent ast.Node, reader text.Reader, pc parser.Context) (ast. case "gallery": // Reading the filesystem happens here, where the parse context says which bundle this is; the // renderer never gets one, so anything gathered has to be gathered now. - n.items = images(pc) + n.pictures = gallery(pc) + case "figure": + if origin, ok := render.OriginFrom(pc); ok { + if p, isPicture := picture(origin, args["src"]); isPicture { + n.pictures = []render.Picture{p} + } + } case "include": // Filled in by the transformer, which runs once this parse is complete. n.isContent = true @@ -180,16 +185,12 @@ func (blocks) Open(parent ast.Node, reader text.Reader, pc parser.Context) (ast. return n, parser.NoChildren } -// pictures are the extensions a gallery treats as an image. A file the browser cannot show is not a -// gallery entry, and guessing by content would mean reading every file in the directory. -var pictures = []string{".avif", ".gif", ".jpeg", ".jpg", ".png", ".svg", ".webp"} - -// images lists the pictures sitting beside the bundle being rendered, sorted by filename. +// gallery lists the pictures sitting beside the bundle being rendered, sorted by filename. // // Sorted because the sparse numeric-prefix convention orders entries without putting numbers in URLs // (ADR-0016), and because a directory read has no order worth relying on. A renderer without a site root // gathers nothing rather than guessing. -func images(pc parser.Context) []string { +func gallery(pc parser.Context) []render.Picture { origin, ok := render.OriginFrom(pc) if !ok || origin.Files == nil { return nil @@ -199,16 +200,19 @@ func images(pc parser.Context) []string { slog.Error("gallery cannot read its bundle directory", "dir", origin.Dir, "err", err) return nil } - var found []string + var names []string for _, e := range entries { - if e.IsDir() { - continue + if !e.IsDir() && showable(e.Name()) { + names = append(names, e.Name()) } - if slices.Contains(pictures, strings.ToLower(path.Ext(e.Name()))) { - found = append(found, e.Name()) + } + sort.Strings(names) + var found []render.Picture + for _, name := range names { + if p, ok := picture(origin, name); ok { + found = append(found, p) } } - sort.Strings(found) return found } @@ -247,7 +251,7 @@ func (f fragments) render(w util.BufWriter, source []byte, n ast.Node, entering } return ast.WalkContinue, nil } - out, err := f.partial(call.name, render.Fragment{Args: call.args, Items: call.items}) + out, err := f.partial(call.name, render.Fragment{Args: call.args, Pictures: call.pictures}) if err != nil { slog.Error("skipping shortcode", "name", call.name, "err", err) return ast.WalkContinue, nil diff --git a/internal/render/render.go b/internal/render/render.go index ad135bb..feb5331 100644 --- a/internal/render/render.go +++ b/internal/render/render.go @@ -127,13 +127,27 @@ type Renderer struct { // because markup belongs to the theme and a feature must not write any (ADR-0036). type Partial func(name string, data Fragment) ([]byte, error) -// Fragment is what a fragment template receives (ADR-0037). +// Fragment is what a fragment template receives (ADR-0037, widened by ADR-0042). type Fragment struct { // Args are the call's key="value" pairs, exactly as written. Escaping is the template's. Args map[string]string - // Items is a list the feature gathered rather than the author wrote — the filenames a gallery found. - // Kept apart from Args so a supplied value can never be mistaken for an authored one. - Items []string + // Pictures are what the engine gathered rather than the author wrote: one for a figure, many for a + // gallery, none when the call names nothing a picture. Kept apart from Args so a supplied value can never + // be mistaken for an authored one. + Pictures []Picture +} + +// Picture is one image a fragment can render (ADR-0042). +type Picture struct { + // Src is the author's own file, relative to the bundle. A browser that ignores Srcset still gets the + // picture that was put there. + Src string + // Srcset offers the derivatives, closed by the original at its own width; empty when the picture is + // already small enough that no derivative was worth making. + Srcset string + // Width and Height are the original's intrinsic size, so a page can reserve the box before the bytes + // arrive. Zero when the file could not be read. + Width, Height int } // Origin tells a feature which bundle is being rendered, so a path in a call can resolve relative to it. diff --git a/internal/render/templates/shortcodes.html b/internal/render/templates/shortcodes.html index 4dbeb5d..ba856cf 100644 --- a/internal/render/templates/shortcodes.html +++ b/internal/render/templates/shortcodes.html @@ -1,6 +1,11 @@ {{define "figure" -}}
+{{- range .Pictures}} +{{$.Args.alt}} +{{- end}} +{{- if not .Pictures}} {{.Args.alt}} +{{- end}} {{- if .Args.caption}}
{{.Args.caption}}
{{- end}} @@ -8,10 +13,10 @@ {{- end}} {{define "gallery" -}} -{{if .Items -}} +{{if .Pictures -}} {{- end}} diff --git a/internal/web/asset_test.go b/internal/web/asset_test.go index 4e1941b..89eb383 100644 --- a/internal/web/asset_test.go +++ b/internal/web/asset_test.go @@ -38,7 +38,7 @@ func assetHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - return Handler(content.NewSite(bundles), r, fsys, content.Settings{}) + return Handler(content.NewSite(bundles), r, fsys, nil, content.Settings{}) } func TestABundlesOwnFilesAreServed(t *testing.T) { diff --git a/internal/web/discover_test.go b/internal/web/discover_test.go index 426725a..328dad2 100644 --- a/internal/web/discover_test.go +++ b/internal/web/discover_test.go @@ -29,7 +29,7 @@ func crawlerHandler(t *testing.T, settings content.Settings, extra fstest.MapFS) if err != nil { t.Fatal(err) } - return Handler(content.NewSite(bundles), r, fsys, settings) + return Handler(content.NewSite(bundles), r, fsys, nil, settings) } func TestSitemapListsEveryVariantAbsolutely(t *testing.T) { diff --git a/internal/web/slug_test.go b/internal/web/slug_test.go index e02f3d6..6cbbb54 100644 --- a/internal/web/slug_test.go +++ b/internal/web/slug_test.go @@ -21,7 +21,7 @@ func slugHandler(t *testing.T, fsys fstest.MapFS) http.Handler { if err != nil { t.Fatal(err) } - return Handler(content.NewSite(bundles), r, fsys, content.Settings{}) + return Handler(content.NewSite(bundles), r, fsys, nil, content.Settings{}) } func TestASlugRenamesTheAddressInEveryLanguage(t *testing.T) { @@ -82,7 +82,7 @@ func TestListingsAndSitemapsUseTheSluggedAddress(t *testing.T) { if err != nil { t.Fatal(err) } - h := Handler(content.NewSite(bundles), r, fsys, settings) + h := Handler(content.NewSite(bundles), r, fsys, nil, settings) rec := httptest.NewRecorder() h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/posts/", nil)) diff --git a/internal/web/web.go b/internal/web/web.go index b72c13d..1a817ce 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -15,7 +15,7 @@ import ( // Handler serves a site. // // One mux entry, because URL shape is the resolver's business rather than the mux's: see resolve. -func Handler(site *content.Site, r *render.Renderer, siteFS fs.FS, settings content.Settings) http.Handler { +func Handler(site *content.Site, r *render.Renderer, siteFS, derivedFS fs.FS, settings content.Settings) http.Handler { mux := http.NewServeMux() mux.HandleFunc("GET /", func(w http.ResponseWriter, req *http.Request) { serve(w, req, site, r, siteFS) @@ -33,6 +33,12 @@ func Handler(site *content.Site, r *render.Renderer, siteFS fs.FS, settings cont mux.Handle("GET /static/", http.StripPrefix("/static/", serveStatic(sub))) } } + // Generated files, served as opaque names. Core knows only that a directory of them exists: which files + // are there, and what they are derived from, is the feature's business (ADR-0042). + if derivedFS != nil { + mux.Handle("GET "+content.DerivedPrefix, + http.StripPrefix(content.DerivedPrefix, serveStatic(derivedFS))) + } return mux } diff --git a/internal/web/web_test.go b/internal/web/web_test.go index e63f90d..4b990fe 100644 --- a/internal/web/web_test.go +++ b/internal/web/web_test.go @@ -28,7 +28,7 @@ func testHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - return Handler(content.NewSite(bundles), r, fsys, content.Settings{}) + return Handler(content.NewSite(bundles), r, fsys, nil, content.Settings{}) } func TestServeBundleAtItsPermalink(t *testing.T) { @@ -76,7 +76,7 @@ func multilingualHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - return Handler(content.NewSite(bundles), r, fsys, content.Settings{}) + return Handler(content.NewSite(bundles), r, fsys, nil, content.Settings{}) } func TestPrefixedLanguageServesThatVariant(t *testing.T) { @@ -128,7 +128,7 @@ func aliasHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - return Handler(content.NewSite(bundles), r, fsys, content.Settings{}) + return Handler(content.NewSite(bundles), r, fsys, nil, content.Settings{}) } func TestAliasRedirectsToCanonical(t *testing.T) { @@ -175,7 +175,7 @@ func listingHandler(t *testing.T, n int) http.Handler { if err != nil { t.Fatal(err) } - return Handler(content.NewSite(bundles), r, fsys, content.Settings{}) + return Handler(content.NewSite(bundles), r, fsys, nil, content.Settings{}) } func TestSectionIndexListsNewestFirst(t *testing.T) { @@ -243,7 +243,7 @@ func TestStaticFilesAreServedAndDirectoriesAreNot(t *testing.T) { if err != nil { t.Fatal(err) } - h := Handler(content.NewSite(bundles), r, fsys, content.Settings{}) + h := Handler(content.NewSite(bundles), r, fsys, nil, content.Settings{}) for path, want := range map[string]int{ "/static/style.css": http.StatusOK, "/static/img/logo.svg": http.StatusOK, @@ -284,7 +284,7 @@ func TestAStaticPathThatEscapesTheRootIs404(t *testing.T) { if err != nil { t.Fatal(err) } - h := Handler(content.NewSite(nil), r, fsys, content.Settings{}) + h := Handler(content.NewSite(nil), r, fsys, nil, content.Settings{}) for path, want := range map[string]int{ "/static/ok.css": http.StatusOK, "/static/escape.txt": http.StatusNotFound, @@ -317,7 +317,7 @@ func seriesHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - return Handler(content.NewSite(bundles), r, nil, content.Settings{}) + return Handler(content.NewSite(bundles), r, nil, nil, content.Settings{}) } func TestSequenceNavigationLinksNeighbours(t *testing.T) { @@ -407,7 +407,7 @@ func tagHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - return Handler(content.NewSite(bundles), r, nil, content.Settings{}) + return Handler(content.NewSite(bundles), r, nil, nil, content.Settings{}) } func TestGlobalTagListingSpansSectionsGroupedByOne(t *testing.T) {