Branch review round two: escaped identifiers, quoted braces in leafRules, cleanup status; red then green open
On branch fiesta-1-1
Fixes for Astra's second branch review (fiesta-1-1-branch-review2.md): three commits on ef382e6.
| Commit | Finding | |---|---| | 1ab0dfc | Important: escaped identifiers still hide rules | | dbd18a6 | Moderate: leafRules cuts a rule at a quoted brace | | 2ed4142 | Minor: a failed cleanup still reports success |
Nothing under css/, js/, i18n/, icons/ or releases/ changed, and internal/build/strip.go is untouched. Gzip size added: 0 bytes.
1. A backslash outside a string is refused (1ab0dfc)
The scanner (cssSpan, check/markuphelp_test.go) now refuses any backslash outside a string: "line N: a backslash outside a string is not read here; write the character it escapes plainly". It no longer tries to follow escape forms. A backslash inside a string is read as before. The structure readers behind the gate (cssItems, matchingBrace, cssDeclarations) no longer skip a backslash outside a string, since none reaches them.
No stylesheet is affected. grep -c '\\' over every CSS file in releases/1.0.0, releases/1.1.0, check/testdata/seed and css/ (26 files) finds 0 backslashes in each, inside strings or out.
Red, on the old scanner. Astra's two inputs read with no error, as one rule:
input: [rst-snippet] { --rst-a: \url(/*); } [rst-page] { padding-inline: 3rem; } [rst-snippet] { --rst-b: \url(*/ ); }
scanner: "[rst-snippet] { --rst-a: \\url( ); }" err=<nil>
declarationsOf: 1 records err=<nil> parseCSSRules: err=<nil>, rule ["[rst-snippet]"]
input: [rst-snippet] { --rst-a: u\72 l(/*); } [rst-page] { padding-inline: 3rem; } [rst-snippet] { --rst-b: u\72 l(*/ ); }
scanner: "[rst-snippet] { --rst-a: u\\72 l( ); }" err=<nil>
declarationsOf: 1 records err=<nil> parseCSSRules: err=<nil>, rule ["[rst-snippet]"]Both are now rejection controls in three tests. All three fail on the old scanner:
- TestTheReaderRefusesWhatItCannotReadAsABrowserDoes:
declarationsOf(...): <nil>, want an error naming the backslashfor both inputs, and fori\"j,\"and\/*. - TestTheNewRuleGateHasNoKnownHoles: "appending … \url(/*) … to 1.0.0's tokens.css passes the gate; it changes 1.0 pages", and the same for
u\72 l(. A reader refusal now counts as the gate refusing. The four older backslash cases are matched on the reader's message. - TestTheRuleReaderSeesEveryRuleABrowserApplies:
parseCSSRules(...): <nil>, want an error naming the backslashfor both inputs and the two older backslash cases.
Green: all three pass. The gate logs "refused: css/tokens.css: line 3707: a backslash outside a string is not read here; write the character it escapes plainly" for all six backslash cases. Plain go test ./check also passes.
Mutations (each failed, then reverted):
- E1: skip a backslash and its next byte again, instead of refusing it.
- E2: let a name run swallow an escape. This reopens
u\72 l(. - E3: no escape handling inside a string. This fails on
"\"/*".
2. leafRules reads with the shared scanner (dbd18a6)
readLeafRules (check/css_test.go) walks the CSS with cssSpan and treats a brace as structure only outside a string. leafRules(t, css) is its form that fails the test. The print-shape check uses the error form.
Red, on the regex: TestLeafRulesReadABraceInAStringAsText
leafRules(...) = [{"[rst-snippet]" " --rst-x: \""} {"content: '" "'; color: red "} {"[rst-b]" " cursor: pointer "}]
interactiveCSSRules(...) finds ["[rst-b]"], want ["[rst-snippet]" "[rst-b]"]Green: the test passes. On all 26 stylesheets (css/, the seed, releases/1.0.0 and releases/1.1.0), the new reader returns exactly the rules the regex did; a throwaway comparison checked this.
Mutations (each failed, then reverted):
- L1: step one byte at a time instead of one span.
- L2: do not close the block at a
}. This adds a bogus[rst-b]rule from the@mediaclose.
Also green:
go test ./check;-tags browser: TestTheFloorChangesNothingButSizeOnADesktop (calls leafRules), TestBothSpellingsComputeTheSameStyles, TestTheConsoleDegradesTheWayItSaysItDoesWithoutHas;- TestEveryTestRunsInExactlyOneRole (721 tests);
make gofmt staticcheck.
3. A failed cleanup fails site-links-published (2ed4142)
The EXIT trap now keeps the status it was given. When rm -rf fails, it prints "could not remove" and turns a pass into exit 1.
Each run below has a stub rm that exits 1 first on PATH, with TMPDIR in scratch. The shell's status is in make's "Error N":
| Case | Old recipe | New recipe | |---|---|---| | link check passes, rm fails | PASS, exit 0, tree left | PASS, "could not remove", Error 1 | | go stubbed to exit 3, rm fails | Error 3 | Error 3 | | SITE_REF=no-such-ref, rm fails | Error 1 | Error 1 | | INT sent to the recipe's shell, rm fails | n/a | Error 130 | | nothing stubbed | n/a | PASS, exit 0, TMPDIR empty |
Hub
amadan ci status rastrillo/fiesta -branch fiesta-1-1: CI passed on 2ed4142. gofmt, no-rastrillo, size, history and all 48 suite shards passed. The size step reports fiesta.css 23,823, fiesta-plain.css 23,104 and fiesta-signal.css 23,217 bytes gzipped, the same as on ef382e6.