Widen the prettier gate to cover CSS and all Markdown, not just top-level docs #12
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sneak/lora.vegas#12
Reference in New Issue
Block a user
Problem
REPO_POLICIES.mdscopes prettier to "JS/CSS/Markdown/HTML".script/fmtandscript/fmt-checkin this repo cover only'*.md'— top-level markdown. Thecanonical scripts use
'**/*.md'.Currently unformatted and ungated:
themes/loravega/static/css/style.css— CSS, squarely in scope.content/_index.mdandarchetypes/default.md— Markdown, in scope, butsee the caveat below.
themes/loravega/layouts/_default/baseof.htmlandthemes/loravega/layouts/index.html— HTML, in scope by the letter of thepolicy, but see "no sensible equivalent" below.
The existing narrow scope was a deliberate choice, documented in the script
comment: "Scope is deliberately limited to top-level docs: site content under
content/is left untouched so rendered output cannot change." That reasoningis sound for one of the three file types and wrong for the other two.
What to change
CSS — add it.
style.cssis inlined verbatim into every page bybaseof.htmlviareadFile. Prettier's CSS formatting is whitespace-only andcannot change rendering. There is no reason it is not covered. Add
'**/*.css'to both scripts.
Markdown under
content/— add it, carefully. The stated fear is thatreformatting changes rendered output. For this content it is worth checking
rather than assuming:
content/_index.mdcontains raw HTML blocks (<div class="settings">,<br>,<span>) andhugo.tomlsetsmarkup.goldmark.renderer.unsafe = true, so the HTML passes through. Prettierdoes reflow markdown and can alter whitespace inside and around inline HTML.
The correct procedure: run
make fmtwith the widened glob, build before andafter, and diff the generated
public/index.html. If the rendered output isbyte-identical, include
content/andarchetypes/. If it differs at all,exclude
content/via.prettierignoreand say so in the script comment —a formatter that can silently change a published page is not worth the
consistency. Report the result either way in the PR.
No sensible Hugo equivalent — do not force this
HTML templates under
themes/loravega/layouts/must stay excluded. Thepolicy says prettier covers HTML, but these are not HTML — they are Go
templates containing
{{ define }},{{ block }},{{ .Content }}, and{{ readFile ... | safeCSS }}. Prettier has no Go-template parser; it willeither fail or reflow the delimiters into something Hugo cannot parse. There is
no supported prettier plugin for Hugo templates in the org's dependency policy
(and adding an unvetted one would need hash-pinning and a
package.json, which#8 explicitly rules out). Add these paths to
.prettierignorewith a commentexplaining why, so the exclusion is a recorded decision and not an oversight.
Definition of done
script/fmtandscript/fmt-checkcover'**/*.md'and'**/*.css'.themes/loravega/layouts/is listed in.prettierignorewith a commentstating that Go templates are not parseable by prettier.
content/has been done andits result is stated in the PR.
content/is either covered (outputidentical) or in
.prettierignorewith a comment (output differed).REPO_POLICIES.md: "If the repo has never been formatted to our standards,run
make fmtand commit the result as a standalone branch/commit/PR beforeany other changes. Formatting diffs can be large and should not be mixed
with functional changes."
make fmt-checkpasses on a clean tree afterwards.make checkpasses andscript/cibuildsucceeds.script/fmtandscript/fmt-checkare updated —they currently describe the old narrow scope.
TODO.mdupdated.Depends on
.prettierignore). Do not start this until #8 has landed, or thisPR will have to create that file itself and conflict.
Ref:
REPO_POLICIES.md— "Use platform-standard formatters…prettierforJS/CSS/Markdown/HTML"; canonical
script/fmtglob'**/*.md'.Implementation plan (dependency on #8 is satisfied —
.prettierrcand.prettierignoreare onmainas of7dea837):content/verdict first, empirically. Buildpublic/from the clean tree inside the pinned-Hugo image (docker buildwithCHECK_EPOCH, then extract/src/publicfrom the image — no bind mounts, so nothing root-owned lands in the checkout). Then run prettier overcontent/andarchetypes/, rebuild, and diff the two rendered trees byte-for-byte.content/_index.mdhas raw<div class="settings">/<br>/<span>blocks andhugo.tomlhasmarkup.goldmark.renderer.unsafe = true, so this is a real risk, not a formality. Identical output →content/andarchetypes/are covered. Any difference at all →content/goes into.prettierignorewith a comment recording why. Result reported either way..prettierignore: addthemes/loravega/layouts/with a comment stating these are Go templates ({{ define }},{{ block }},{{ readFile ... | safeCSS }}), which prettier has no parser for and would mangle. No prettier plugin, nopackage.json— out of scope. Plus thecontent/entry if step 1 says so.script/fmtandscript/fmt-checkto'**/*.md'and'**/*.css', and rewrite both header comments, which currently describe the old top-level-only scope.make checkon its own — an intermediate red commit is what forced #31 to land as a merge commit. So: commit 1 is the pure reformat (prettier applied by hand-invoking the widened scope, but the scripts still at the old narrow glob, somake fmt-checkpasses both before and after it); commit 2 widens the glob and adds the ignore entries, and passes because the tree is already formatted.content/reformat, if it happens, is in commit 1 with the rest of the churn, never mixed with the functional change.TODO.mdupdated in the functional commit.make checkon a clean tree, thenscript/cibuild, confirming the check layer actually executed (#23'sCHECK_EPOCHshould guarantee that, but the output gets inspected rather than assumed).script/bootstrapand.gitea/workflows/deploy.ymlare untouched; the deploy path is unaffected.Implemented in #32.
The
content/verdict: the rendered output DIFFERED, socontent/is excluded. Withcontent/in scope, prettier re-wrapped one list item incontent/_index.md(the Lee's Sandwiches line), and the builtpublic/index.htmlchanged with it — the wrap arrived as a literal newline between7 PM atand the following<a>tag inside the<li>. HTML collapses that newline to a space so the page looks identical, but the published bytes do not match, and per this issue's own rule that is enough to exclude.content/is in.prettierignorewith a comment recording the measurement, and bothscript/fmtandscript/fmt-checkname the exclusion in their header comments. The raw<div class="settings">blocks were untouched in this particular run, but they pass through verbatim undermarkup.goldmark.renderer.unsafe = true, so the risk the issue anticipated is real.archetypes/stays covered — it is a template for new content, not published output, and prettier leaves it unchanged.themes/loravega/layouts/is excluded with its own comment: Go templates, no prettier parser, and covering them would need a plugin and therefore apackage.json. No plugin was added.Both scripts now run
'**/*.md' '**/*.css', sothemes/loravega/static/css/style.cssis formatted and gated for the first time.Two commits, both green on their own, so no merge commit is needed. The reformat lands first, while the scripts still carry the old
'*.md'glob that does not look at CSS; the widening commit then arrives on an already-clean tree. That is the inverse of the ordering that forced #31 to land as a merge commit.Verification.
public/was extracted from the built image (pinned hugo v0.164.0) before the change and again at the final state:index.html,index.xmlandsitemap.xmlare byte-identical, and the only file that differs anywhere in the tree is the verbatim-copiedpublic/css/style.css, which is identical once whitespace is stripped. The minified<style>block inlined intoindex.htmlis unchanged, which is the direct proof that CSS formatting cannot reach the rendered page.make checkpasses at both commits, andscript/cibuildsucceeded with the check layer genuinely executing — its output is present in the build log while thescript/bootstraplayers above it areCACHED.script/bootstrapand.gitea/workflows/deploy.ymlare untouched, so the deploy path is unaffected; nothing was run against the live Cloudflare Pages deploy.