rastrillo / fiesta Public

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 backslash for both inputs, and for i\"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 backslash for 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 @media close.

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.