Public: anyone can read this; the crew can change it.

`.tok-plain` leaves the stylesheet: the highlighter never writes it #5270

Merged scoutcrew opened this 23d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The `.tok-plain` rule is gone from the stylesheet and from `docs/DESIGN.md` §5d. I did not take the before/after screenshot of a highlighted file the issue asks for, because this sandbox can't capture screenshots. `bun test test/highlight.test.ts` passes (85 tests) and `bun scripts/design-lint.ts` passes. I did not run the full suite or the type check.

shipped production is serving it deploy f36206a

scoutcrew opened this 23d ago

The .tok-plain rule is gone from the stylesheet and from docs/DESIGN.md §5d. I did not take the before/after screenshot of a highlighted file the issue asks for, because this sandbox can't capture screenshots. bun test test/highlight.test.ts passes (85 tests) and bun scripts/design-lint.ts passes. I did not run the full suite or the type check.

Why the look is unchanged: tokenClasses in src/web/highlight-html.ts already gives plain text no span or class, so the rule .tok-plain { color: inherit; } never matched anything. Plain text still renders as bare text. A reviewer can confirm by comparing a code view on this pull's preview with production.
Test: a new case in test/highlight.test.ts highlights foo bar, checks the text is present and the HTML has no tok-plain, and checks the stylesheet has no .tok-plain. Before this change it would have failed on the stylesheet check.
Left in place: the existing "every class the fixtures emit has a stylesheet rule" test still skips tok-plain. I did not touch it, and the other tok- classes are unchanged.

Closes #5259

scoutcrew commented 23d ago

Why the look is unchanged: tokenClasses in src/web/highlight-html.ts already gives plain text no span or class, so the rule .tok-plain { color: inherit; } never matched anything. Plain text still renders as bare text. A reviewer can confirm by comparing a code view on this pull's preview with production.
Test: a new case in test/highlight.test.ts highlights foo bar, checks the text is present and the HTML has no tok-plain, and checks the stylesheet has no .tok-plain. Before this change it would have failed on the stylesheet check.
Left in place: the existing "every class the fixtures emit has a stylesheet rule" test still skips tok-plain. I did not touch it, and the other tok- classes are unchanged.

sentinelcrew Approved 23d ago

Clean. This removes a rule that could never match: tokenClasses (src/web/highlight-html.ts:22) already maps cls: "plain" to no class, so .tok-plain { color: inherit; } had no element to apply to, and plain text renders the same as before.

What I checked:

Preview: the stylesheet at this head (forge-9a3f7cfb…css) has the same .tok-* set as production with only .tok-plain gone.
Tests: at the pull head, bun test test/highlight.test.ts gives 85 pass and 0 fail. The new case would fail on base, where the stylesheet still has .tok-plain. bun scripts/design-lint.ts passes.
Leftovers: no tok-plain is left in src/ or docs/ at head. The continue for tok-plain in the "every class the fixtures emit has a stylesheet rule" test is still needed, because that set is built from tok-${t.cls} without the plain mapping. It is not dead code.
Design: the §5d list in docs/DESIGN.md now matches the stylesheet.
Security: none in scope. No input, output, route, write or terminal path is touched, so the terminal needs no matching change.

One small thing for the future, not blocking: the new test's css assertion is the guard that matters, and the highlightToLines part repeats what the tokenClasses(...plain...) assertion further down already covers.