From f72cf5d21981c3d5cf2eee5f0e0ccec74ec60832 Mon Sep 17 00:00:00 2001 From: Ira Hopkinson Date: Mon, 14 Sep 2026 11:07:52 +1200 Subject: [PATCH 1/4] PT-4313: Compensate every indented paragraph marker in the gutter view Add --para-indent (and --verse-text-start where the indent is hanging) for the 19 paragraph markers whose text-spacing margin had no gutter compensation: ipq imq ipr psi qd p2 iq iq1 iq2 iq3 qm2 qm3 ph ph1 ph2 ph3 tr tr1 tr2. Without it the marker glyph sat at the paragraph's margin edge instead of in the gutter and, for the hanging-indent markers, was painted over the first line of text at every column width. The tr/tr1/tr2 entries are inert for real tables, which render as rather than .para; they are kept so the block stays complete. Add usj-nodes.css.gutter-coverage.test.ts, which derives the expected gutter values from the base text-spacing rules in the same file, so a marker that gains a margin without compensation fails by construction rather than depending on a hand-maintained list. The committed dist is unchanged: usj-nodes.css is not part of the bundle, consumers copy the source file. paranext-core carries the same change on its copies. Co-Authored-By: Claude Fable 5.1 --- packages/platform/src/usj-nodes.css | 45 +++- .../src/usj-nodes.css.gutter-coverage.test.ts | 227 ++++++++++++++++++ 2 files changed, 268 insertions(+), 4 deletions(-) create mode 100644 packages/platform/src/usj-nodes.css.gutter-coverage.test.ts diff --git a/packages/platform/src/usj-nodes.css b/packages/platform/src/usj-nodes.css index 096b6c33..5a8eab52 100644 --- a/packages/platform/src/usj-nodes.css +++ b/packages/platform/src/usj-nodes.css @@ -2831,16 +2831,30 @@ span.read img { } /* Indent compensation — mirrors the inline-start margin values above. + EVERY marker with a text-spacing margin-left above needs an entry here, or its gutter + glyph lands inside the text; usj-nodes.css.gutter-coverage.test.ts derives that list from the + base rules and fails on any gap. + The tr/tr1/tr2 entries mirror their base rules for that invariant but are inert for real + tables: a table row renders as with an inline text-indent of + 0 (ImmutableTableRowNode.createDOM), not as a .para, so the gutter glyph rule never matches + it. They are kept only so the compensation block stays complete for the coverage test. Only active when .text-spacing is present (same condition as the source rules). pi2 and pi3 use the same primary-indent value in both LTR and RTL (10vw and 15vw respectively), so no direction restriction is needed here. */ +.psc-gutter-markers.text-spacing .usfm_p2 { + --para-indent: 2.5vw; +} .psc-gutter-markers.text-spacing .usfm_io, .psc-gutter-markers.text-spacing .usfm_io1, .psc-gutter-markers.text-spacing .usfm_ili, .psc-gutter-markers.text-spacing .usfm_ili1, .psc-gutter-markers.text-spacing .usfm_li, .psc-gutter-markers.text-spacing .usfm_li1, -.psc-gutter-markers.text-spacing .usfm_pi2 { +.psc-gutter-markers.text-spacing .usfm_ph, +.psc-gutter-markers.text-spacing .usfm_ph1, +.psc-gutter-markers.text-spacing .usfm_pi2, +.psc-gutter-markers.text-spacing .usfm_tr, +.psc-gutter-markers.text-spacing .usfm_tr1 { --para-indent: 10vw; } .psc-gutter-markers.text-spacing .usfm_q, @@ -2853,14 +2867,23 @@ span.read img { .psc-gutter-markers.text-spacing .usfm_li2, .psc-gutter-markers.text-spacing .usfm_lim, .psc-gutter-markers.text-spacing .usfm_lim1, -.psc-gutter-markers.text-spacing .usfm_pi3 { +.psc-gutter-markers.text-spacing .usfm_ph2, +.psc-gutter-markers.text-spacing .usfm_pi3, +.psc-gutter-markers.text-spacing .usfm_tr2 { --para-indent: 15vw; } .psc-gutter-markers.text-spacing .usfm_qm, .psc-gutter-markers.text-spacing .usfm_qm1, +.psc-gutter-markers.text-spacing .usfm_qm2, +.psc-gutter-markers.text-spacing .usfm_qm3, +.psc-gutter-markers.text-spacing .usfm_iq, +.psc-gutter-markers.text-spacing .usfm_iq1, +.psc-gutter-markers.text-spacing .usfm_iq2, +.psc-gutter-markers.text-spacing .usfm_iq3, .psc-gutter-markers.text-spacing .usfm_io3, .psc-gutter-markers.text-spacing .usfm_li3, -.psc-gutter-markers.text-spacing .usfm_lim2 { +.psc-gutter-markers.text-spacing .usfm_lim2, +.psc-gutter-markers.text-spacing .usfm_ph3 { --para-indent: 20vw; } .psc-gutter-markers.text-spacing .usfm_io4, @@ -2873,13 +2896,18 @@ span.read img { } .psc-gutter-markers.text-spacing .usfm_ipi, .psc-gutter-markers.text-spacing .usfm_imi, +.psc-gutter-markers.text-spacing .usfm_ipq, +.psc-gutter-markers.text-spacing .usfm_imq, +.psc-gutter-markers.text-spacing .usfm_ipr, .psc-gutter-markers.text-spacing .usfm_pmo, .psc-gutter-markers.text-spacing .usfm_pm, .psc-gutter-markers.text-spacing .usfm_pmc, .psc-gutter-markers.text-spacing .usfm_pmr, .psc-gutter-markers.text-spacing .usfm_pi, .psc-gutter-markers.text-spacing .usfm_pi1, -.psc-gutter-markers.text-spacing .usfm_mi { +.psc-gutter-markers.text-spacing .usfm_psi, +.psc-gutter-markers.text-spacing .usfm_mi, +.psc-gutter-markers.text-spacing .usfm_qd { --para-indent: 5vw; } @@ -2935,6 +2963,15 @@ span.read img { .psc-gutter-markers.text-spacing .usfm_iq3 { --verse-text-start: -5vw; } +.psc-gutter-markers.text-spacing .usfm_ph, +.psc-gutter-markers.text-spacing .usfm_ph1, +.psc-gutter-markers.text-spacing .usfm_ph2, +.psc-gutter-markers.text-spacing .usfm_ph3, +.psc-gutter-markers.text-spacing .usfm_tr, +.psc-gutter-markers.text-spacing .usfm_tr1, +.psc-gutter-markers.text-spacing .usfm_tr2 { + --verse-text-start: -5vw; +} /* ── Active text focus box ──────────────────────────────────────────────────── Applied via .psc-active-focus when viewOptions.hasActiveTextFocusBox is true. diff --git a/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts b/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts new file mode 100644 index 00000000..f0fa403f --- /dev/null +++ b/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts @@ -0,0 +1,227 @@ +// @vitest-environment node +// (jsdom rewrites `import.meta.url` to an http URL, which breaks the stylesheet read; this test +// only reads a file, so the node environment is the correct one anyway.) +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +/** + * In the gutter view (`.psc-gutter-markers`), each paragraph's marker glyph is absolutely + * positioned at `left: calc(-(gutter width) + 0.5em - var(--para-indent))`, so `--para-indent` + * must equal the paragraph's own text-spacing margin or the glyph lands inside the text. Likewise + * the active focus box starts at `var(--verse-text-start)`, which must equal a hanging-indent + * paragraph's negative `text-indent`. + * + * This test derives BOTH expectations from the base text-spacing rules in the same file, so a + * marker that gains a margin or hanging indent without matching gutter compensation fails here by + * construction. A hand-typed expected list cannot do that: it silently encodes whatever gap + * existed when it was written. + * + * Base values follow the USFM stylesheet's LeftMargin / FirstLineIndent (inches x 20 = vw): + * https://github.com/ubsicap/usfm/blob/master/sty/usfm.sty + * + * This stylesheet is mirrored into consumers, so the gutter block is contract: a gap here becomes a + * gap there on the next sync. paranext-core runs the same derivation over its copies in + * `extensions/src/platform-scripture-editor/src/usj-nodes-scss-coverage.test.ts`. + */ + +const css = readFileSync(new URL("./usj-nodes.css", import.meta.url), "utf-8").replace( + /\/\*[\s\S]*?\*\//g, + "", +); + +// The stylesheet is parsed as a flat list of `selector { declarations }` blocks. This regex cannot +// reliably read a rule nested inside another block (an @media query or @keyframes), so a gutter +// rule wrapped in one could be mis-parsed and its markers silently uncovered. `nestingProblems()` +// catches that by brace depth before the coverage checks run. +const RULE_BLOCK = /([^{}]+)\{([^}]+)\}/g; +const blocks = [...css.matchAll(RULE_BLOCK)].map(([, selectors, declarations]) => ({ + // Attribute quotes are normalised so `[dir="ltr"]` and `[dir='ltr']` compare equal: this file + // uses double quotes, the paranext-core SCSS copy single quotes. + selectors: selectors.replace(/\s+/g, " ").replace(/"/g, "'").trim(), + declarations, +})); + +const MARKER_CLASS = /\.usfm_([a-z0-9]+)/g; + +/** Brace nesting depth at a character offset: 1 inside a top-level rule, >1 inside a nested one. */ +function braceDepthAt(index: number): number { + const before = css.slice(0, index); + return (before.match(/\{/g) ?? []).length - (before.match(/\}/g) ?? []).length; +} + +/** + * Reports `property` setters that sit deeper than one block, where a browser would scope them + * away but the flat block parser cannot reliably see them. + */ +function nestingProblems(property: string): string[] { + const setter = new RegExp(`${property}\\s*:`, "g"); + const nested = [...css.matchAll(setter)].filter((match) => braceDepthAt(match.index ?? 0) > 1); + return nested.length === 0 + ? [] + : [ + `${property}: ${nested.length} setter(s) are nested inside an @media/@keyframes block; ` + + `the flat parser below cannot reliably see them. Update the parser.`, + ]; +} + +/** + * Reads the value of one `property: value;` declaration out of a rule's declaration block. + * + * Only a setter of exactly that property counts: the property name must start the block or follow + * a `;` or whitespace, so `margin-left` does not match inside `margin-left-foo` and a `var(--x)` + * read of a custom property does not count as setting it. + * + * @param declarations The text between a rule's `{` and `}`. + * @param property The property name to read, e.g. `margin-left` or `--para-indent`. + * @returns The trimmed value, or `undefined` when the block does not set that property. + */ +function declarationValue(declarations: string, property: string): string | undefined { + const match = new RegExp(`(?:^|;|\\s)${property}\\s*:\\s*([^;]+)`).exec(declarations); + return match ? match[1].trim() : undefined; +} + +/** A block is a gutter compensation rule when its selector carries both scoping classes. */ +function isGutterBlock(selectors: string): boolean { + return selectors.includes("psc-gutter-markers") && selectors.includes("text-spacing"); +} + +/** + * A block is a base text-spacing rule for a paragraph marker when it is scoped by `.text-spacing` + * but is not a gutter rule. RTL blocks are excluded: the gutter variables are direction-agnostic + * (the RTL glyph rule reads the same `--para-indent`), so the LTR/direction-agnostic margin is the + * one the compensation must mirror. + */ +function isNonRtlBaseBlock(selectors: string): boolean { + return ( + selectors.includes("text-spacing") && + !isGutterBlock(selectors) && + !selectors.includes("[dir='rtl']") + ); +} + +/** Maps each `.usfm_` to the value the gutter rules give it for `property`. */ +function getGutterMarkerValues(property: string): Map { + const values = new Map(); + blocks + .filter((block) => isGutterBlock(block.selectors)) + .forEach((block) => { + const value = declarationValue(block.declarations, property); + if (value === undefined) return; + [...block.selectors.matchAll(MARKER_CLASS)].forEach(([, marker]) => + values.set(marker, value), + ); + }); + return values; +} + +/** + * Maps each `.usfm_` to the value the BASE text-spacing rules give it for `property`, + * keeping only values `accept` approves. When a marker has both a direction-agnostic rule + * (`.text-spacing .usfm_x`) and an LTR rule (`.text-spacing[dir='ltr'] .usfm_x`) the LTR value + * wins, matching the cascade for an LTR document. + */ +function getBaseMarkerValues( + property: string, + accept: (value: string) => boolean, +): Map { + const agnostic = new Map(); + const ltr = new Map(); + blocks + .filter((block) => isNonRtlBaseBlock(block.selectors)) + .forEach((block) => { + const value = declarationValue(block.declarations, property); + if (value === undefined || !accept(value)) return; + const target = block.selectors.includes("[dir='ltr']") ? ltr : agnostic; + [...block.selectors.matchAll(MARKER_CLASS)].forEach(([, marker]) => + target.set(marker, value), + ); + }); + return new Map([...agnostic, ...ltr]); +} + +/** A length that moves the box: anything other than a zero (`0`, `0px`, `0vw`, `0in`, ...). */ +function isNonZeroLength(value: string): boolean { + return !/^-?0(\.0+)?[a-z%]*$/.test(value); +} + +/** A negative length — the hanging indent that pulls the first line before the border edge. */ +function isNegativeLength(value: string): boolean { + return value.startsWith("-") && isNonZeroLength(value); +} + +/** Reports each expected marker whose actual `property` value is missing or wrong. */ +function valueMismatches(property: string, expected: Map): string[] { + const actual = getGutterMarkerValues(property); + return [...expected] + .filter(([marker, value]) => actual.get(marker) !== value) + .map( + ([marker, value]) => + `.usfm_${marker}: expected ${property}: ${value}, got ${actual.get(marker) ?? "none"}`, + ); +} + +/** + * Reports gutter markers that set `property` but have no base rule calling for it — the reverse of + * `valueMismatches`, so a stale entry left behind after a marker loses its margin is flagged too. + */ +function unexpectedMarkers(property: string, expected: Map): string[] { + const actual = getGutterMarkerValues(property); + return [...actual] + .filter(([marker]) => !expected.has(marker)) + .map( + ([marker, value]) => + `.usfm_${marker}: sets ${property}: ${value} but no base text-spacing rule calls for it`, + ); +} + +/** + * Reports gutter `property` rules qualified by writing direction (`[dir=…]`). The gutter values are + * identical for LTR and RTL, so a qualifier would leave one direction uncompensated while still + * counting as covered. + */ +function directionQualifiedGutterRules(property: string): string[] { + const setter = new RegExp(`${property}\\s*:`); + return blocks + .filter( + (block) => + isGutterBlock(block.selectors) && + setter.test(block.declarations) && + block.selectors.includes("[dir="), + ) + .map((block) => `${property}: direction-qualified selector "${block.selectors}"`); +} + +// Every base text-spacing margin-left is a paragraph indent the gutter glyph must be pulled back by. +const EXPECTED_PARA_INDENT = getBaseMarkerValues("margin-left", isNonZeroLength); + +// Every base negative text-indent is a hanging indent the focus box must start at. +const EXPECTED_VERSE_TEXT_START = getBaseMarkerValues("text-indent", isNegativeLength); + +describe("usj-nodes.css .psc-gutter-markers.text-spacing coverage", () => { + it("derives a non-empty expectation from the base text-spacing rules", () => { + // If the base parser ever reads nothing, both coverage checks below would pass vacuously. + expect(EXPECTED_PARA_INDENT.size).toBeGreaterThan(40); + expect(EXPECTED_VERSE_TEXT_START.size).toBeGreaterThan(20); + // Spot checks pinning the derivation to known USFM values, so a parser regression that reads + // the wrong property or block shows up as a wrong number rather than a shorter list. + expect(EXPECTED_PARA_INDENT.get("li2")).toBe("15vw"); + expect(EXPECTED_PARA_INDENT.get("qm2")).toBe("20vw"); + expect(EXPECTED_VERSE_TEXT_START.get("li2")).toBe("-7.5vw"); + expect(EXPECTED_VERSE_TEXT_START.get("iq1")).toBe("-15vw"); + expect(EXPECTED_VERSE_TEXT_START.has("p")).toBe(false); + }); + + it("every indented marker sets --para-indent equal to its text-spacing margin", () => { + expect(nestingProblems("--para-indent")).toEqual([]); + expect(directionQualifiedGutterRules("--para-indent")).toEqual([]); + expect(valueMismatches("--para-indent", EXPECTED_PARA_INDENT)).toEqual([]); + expect(unexpectedMarkers("--para-indent", EXPECTED_PARA_INDENT)).toEqual([]); + }); + + it("every hanging-indent marker sets --verse-text-start equal to its text-indent", () => { + expect(nestingProblems("--verse-text-start")).toEqual([]); + expect(directionQualifiedGutterRules("--verse-text-start")).toEqual([]); + expect(valueMismatches("--verse-text-start", EXPECTED_VERSE_TEXT_START)).toEqual([]); + expect(unexpectedMarkers("--verse-text-start", EXPECTED_VERSE_TEXT_START)).toEqual([]); + }); +}); From 9406e9d15566842c8e1cbb066b4f2039bb6f171f Mon Sep 17 00:00:00 2001 From: Ira Hopkinson Date: Tue, 15 Sep 2026 12:21:14 +1200 Subject: [PATCH 2/4] Harden the derived gutter coverage test and drop the table-row entries Review follow-up on #10 (mirrors the paranext-core review of the same change). usj-nodes.css: - Remove the tr/tr1/tr2 gutter entries. A table row is a (ImmutableTableRowNode), never a .para, so the glyph rule cannot match them; the test excludes them from the derivation instead. - Scope the compensation comment to margins set in this file and name what it does not cover (host-injected generateUsjCss output, consumer commentary stylesheets); fix ::before -> ::after. usj-nodes.css.gutter-coverage.test.ts: - Resolve the LTR-over-agnostic cascade before filtering zero values, so a directional zero override can beat an agnostic non-zero margin. - Classify direction per comma-separated selector, not per selector list. - Fail loudly on a margin shorthand or margin-inline-* on a marker rule, on nested margin-left/text-indent setters, and on an LTR/RTL margin mismatch (one --para-indent serves both directions). - Add a small usfm.sty oracle so a base value drifting from the spec is caught rather than compensated. Co-Authored-By: Claude Fable 5.1 --- packages/platform/src/usj-nodes.css | 29 +- .../src/usj-nodes.css.gutter-coverage.test.ts | 265 +++++++++++++----- 2 files changed, 212 insertions(+), 82 deletions(-) diff --git a/packages/platform/src/usj-nodes.css b/packages/platform/src/usj-nodes.css index 5a8eab52..642d2c46 100644 --- a/packages/platform/src/usj-nodes.css +++ b/packages/platform/src/usj-nodes.css @@ -2831,13 +2831,14 @@ span.read img { } /* Indent compensation — mirrors the inline-start margin values above. - EVERY marker with a text-spacing margin-left above needs an entry here, or its gutter - glyph lands inside the text; usj-nodes.css.gutter-coverage.test.ts derives that list from the - base rules and fails on any gap. - The tr/tr1/tr2 entries mirror their base rules for that invariant but are inert for real - tables: a table row renders as with an inline text-indent of - 0 (ImmutableTableRowNode.createDOM), not as a .para, so the gutter glyph rule never matches - it. They are kept only so the compensation block stays complete for the coverage test. + Invariant for THIS stylesheet: every marker its text-spacing rules give a margin-left needs + an entry here with the same value, or the gutter glyph lands inside the text. + usj-nodes.css.gutter-coverage.test.ts derives that requirement from the base rules and fails + on any gap. It says nothing about margins a host injects later in source order (project + StyleInfo CSS from generateUsjCss, or a consumer's commentary stylesheets); those can move a + marker's margin away from the value compensated here and are not covered. + Table rows (tr, tr1, tr2) deliberately have no entry: a real table row is a , not a + .para, so the glyph rule never matches it. Only active when .text-spacing is present (same condition as the source rules). pi2 and pi3 use the same primary-indent value in both LTR and RTL (10vw and 15vw respectively), so no direction restriction is needed here. */ @@ -2852,9 +2853,7 @@ span.read img { .psc-gutter-markers.text-spacing .usfm_li1, .psc-gutter-markers.text-spacing .usfm_ph, .psc-gutter-markers.text-spacing .usfm_ph1, -.psc-gutter-markers.text-spacing .usfm_pi2, -.psc-gutter-markers.text-spacing .usfm_tr, -.psc-gutter-markers.text-spacing .usfm_tr1 { +.psc-gutter-markers.text-spacing .usfm_pi2 { --para-indent: 10vw; } .psc-gutter-markers.text-spacing .usfm_q, @@ -2868,8 +2867,7 @@ span.read img { .psc-gutter-markers.text-spacing .usfm_lim, .psc-gutter-markers.text-spacing .usfm_lim1, .psc-gutter-markers.text-spacing .usfm_ph2, -.psc-gutter-markers.text-spacing .usfm_pi3, -.psc-gutter-markers.text-spacing .usfm_tr2 { +.psc-gutter-markers.text-spacing .usfm_pi3 { --para-indent: 15vw; } .psc-gutter-markers.text-spacing .usfm_qm, @@ -2914,7 +2912,7 @@ span.read img { /* --verse-text-start mirrors the text-indent values for paragraphs with a negative text-indent (hanging indent). These paragraphs render their first line to the inline-start of the element's border edge, so the active focus box - ::before must start at text-indent to align with the text. */ + ::after must start at text-indent to align with the text. */ .psc-gutter-markers.text-spacing .usfm_q, .psc-gutter-markers.text-spacing .usfm_q1 { --verse-text-start: -10vw; @@ -2966,10 +2964,7 @@ span.read img { .psc-gutter-markers.text-spacing .usfm_ph, .psc-gutter-markers.text-spacing .usfm_ph1, .psc-gutter-markers.text-spacing .usfm_ph2, -.psc-gutter-markers.text-spacing .usfm_ph3, -.psc-gutter-markers.text-spacing .usfm_tr, -.psc-gutter-markers.text-spacing .usfm_tr1, -.psc-gutter-markers.text-spacing .usfm_tr2 { +.psc-gutter-markers.text-spacing .usfm_ph3 { --verse-text-start: -5vw; } diff --git a/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts b/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts index f0fa403f..0bf52317 100644 --- a/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts +++ b/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts @@ -14,34 +14,97 @@ import { describe, expect, it } from "vitest"; * This test derives BOTH expectations from the base text-spacing rules in the same file, so a * marker that gains a margin or hanging indent without matching gutter compensation fails here by * construction. A hand-typed expected list cannot do that: it silently encodes whatever gap - * existed when it was written. + * existed when it was written. A small independent oracle from the USFM stylesheet is kept + * alongside, so a base value that drifts from the spec is caught too rather than being "fixed" by + * updating its compensation to match. * - * Base values follow the USFM stylesheet's LeftMargin / FirstLineIndent (inches x 20 = vw): - * https://github.com/ubsicap/usfm/blob/master/sty/usfm.sty + * Scope: the invariant is about margins set in this file. A host that injects project-stylesheet + * CSS (generateUsjCss) or its own commentary stylesheets later in source order can move a marker's + * margin away from the compensated value; that is not covered here. * * This stylesheet is mirrored into consumers, so the gutter block is contract: a gap here becomes a * gap there on the next sync. paranext-core runs the same derivation over its copies in * `extensions/src/platform-scripture-editor/src/usj-nodes-scss-coverage.test.ts`. */ +/** + * Markers whose base rules indent them but which must NOT have gutter compensation. A real table + * row is a `` (`ImmutableTableRowNode`), not a `.para`, so the glyph rule never matches it. + */ +const NOT_COMPENSATED = new Set(["tr", "tr1", "tr2"]); + +// USFM stylesheet oracle (LeftMargin / FirstLineIndent in inches x 20 = vw), independent of this +// file: https://github.com/ubsicap/usfm/blob/master/sty/usfm.sty. One marker per distinct value is +// enough to catch a base rule drifting from the spec; completeness comes from the derivation. +const USFM_LEFT_MARGIN: { [key: string]: string } = { + pi: "5vw", // 0.25" + li1: "10vw", // 0.5" + q1: "15vw", // 0.75" + qm1: "20vw", // 1.0" + li4: "25vw", // 1.25" + lim4: "30vw", // 1.5" +}; +const USFM_FIRST_LINE_INDENT: { [key: string]: string } = { + qm1: "-15vw", // -0.75" + q1: "-10vw", // -0.5" + li1: "-7.5vw", // -0.375" + q3: "-5vw", // -0.25" + q4: "-2.5vw", // -0.125" +}; + +type Direction = "ltr" | "rtl" | "agnostic"; + +/** One `.usfm_` selector from a rule, with the writing direction it is scoped to. */ +interface MarkerSelector { + marker: string; + direction: Direction; +} + +interface Block { + /** The rule's selector list, whitespace-collapsed and attribute quotes normalised to `'`. */ + selectors: string; + /** Each `.usfm_` in the selector list, classified per selector, not per list. */ + markers: MarkerSelector[]; + declarations: string; +} + +const MARKER_CLASS = /\.usfm_([a-z0-9]+)/; + +/** + * Classifies one selector (not a comma list) by the writing direction it is scoped to. Attribute + * quotes are normalised so `[dir="ltr"]` and `[dir='ltr']` compare equal: this file uses double + * quotes, the paranext-core SCSS copy single quotes. + */ +function directionOf(selector: string): Direction { + if (selector.includes("[dir='rtl']")) return "rtl"; + if (selector.includes("[dir='ltr']")) return "ltr"; + return "agnostic"; +} + const css = readFileSync(new URL("./usj-nodes.css", import.meta.url), "utf-8").replace( /\/\*[\s\S]*?\*\//g, "", ); // The stylesheet is parsed as a flat list of `selector { declarations }` blocks. This regex cannot -// reliably read a rule nested inside another block (an @media query or @keyframes), so a gutter +// reliably read a rule nested inside another block (a media query or a keyframes at-rule), so a // rule wrapped in one could be mis-parsed and its markers silently uncovered. `nestingProblems()` -// catches that by brace depth before the coverage checks run. -const RULE_BLOCK = /([^{}]+)\{([^}]+)\}/g; -const blocks = [...css.matchAll(RULE_BLOCK)].map(([, selectors, declarations]) => ({ - // Attribute quotes are normalised so `[dir="ltr"]` and `[dir='ltr']` compare equal: this file - // uses double quotes, the paranext-core SCSS copy single quotes. - selectors: selectors.replace(/\s+/g, " ").replace(/"/g, "'").trim(), - declarations, -})); - -const MARKER_CLASS = /\.usfm_([a-z0-9]+)/g; +// catches that by brace depth before the coverage checks run. Selector lists are split on commas +// and each selector classified on its own, so a group that mixes directions cannot misfile the +// whole group. +const blocks: Block[] = [...css.matchAll(/([^{}]+)\{([^}]+)\}/g)].map( + ([, selectors, declarations]) => { + const normalised = selectors.replace(/\s+/g, " ").replace(/"/g, "'").trim(); + const markers = normalised + .split(",") + .map((selector) => selector.trim()) + .flatMap((selector) => { + const marker = MARKER_CLASS.exec(selector)?.[1]; + return marker ? [{ marker, direction: directionOf(selector) }] : []; + }); + return { selectors: normalised, markers, declarations }; + }, +); /** Brace nesting depth at a character offset: 1 inside a top-level rule, >1 inside a nested one. */ function braceDepthAt(index: number): number { @@ -85,18 +148,9 @@ function isGutterBlock(selectors: string): boolean { return selectors.includes("psc-gutter-markers") && selectors.includes("text-spacing"); } -/** - * A block is a base text-spacing rule for a paragraph marker when it is scoped by `.text-spacing` - * but is not a gutter rule. RTL blocks are excluded: the gutter variables are direction-agnostic - * (the RTL glyph rule reads the same `--para-indent`), so the LTR/direction-agnostic margin is the - * one the compensation must mirror. - */ -function isNonRtlBaseBlock(selectors: string): boolean { - return ( - selectors.includes("text-spacing") && - !isGutterBlock(selectors) && - !selectors.includes("[dir='rtl']") - ); +/** A block is a base text-spacing rule: scoped by `.text-spacing` but not a gutter rule. */ +function isBaseBlock(selectors: string): boolean { + return selectors.includes("text-spacing") && !isGutterBlock(selectors); } /** Maps each `.usfm_` to the value the gutter rules give it for `property`. */ @@ -107,36 +161,32 @@ function getGutterMarkerValues(property: string): Map { .forEach((block) => { const value = declarationValue(block.declarations, property); if (value === undefined) return; - [...block.selectors.matchAll(MARKER_CLASS)].forEach(([, marker]) => - values.set(marker, value), - ); + block.markers.forEach(({ marker }) => values.set(marker, value)); }); return values; } /** - * Maps each `.usfm_` to the value the BASE text-spacing rules give it for `property`, - * keeping only values `accept` approves. When a marker has both a direction-agnostic rule - * (`.text-spacing .usfm_x`) and an LTR rule (`.text-spacing[dir='ltr'] .usfm_x`) the LTR value - * wins, matching the cascade for an LTR document. + * Maps each `.usfm_` to the value the BASE text-spacing rules resolve for `property` in the + * given direction: a `[dir=…]` rule for that direction wins over a direction-agnostic one, matching + * the cascade (the qualified selector is more specific). Every value is kept here, zero included — + * a `margin-left: 0` LTR override must be allowed to beat an agnostic `5vw` before any filtering, + * or the filter would discard the winner and enforce the loser. */ -function getBaseMarkerValues( - property: string, - accept: (value: string) => boolean, -): Map { +function resolveBaseValues(property: string, direction: "ltr" | "rtl"): Map { const agnostic = new Map(); - const ltr = new Map(); + const directed = new Map(); blocks - .filter((block) => isNonRtlBaseBlock(block.selectors)) + .filter((block) => isBaseBlock(block.selectors)) .forEach((block) => { const value = declarationValue(block.declarations, property); - if (value === undefined || !accept(value)) return; - const target = block.selectors.includes("[dir='ltr']") ? ltr : agnostic; - [...block.selectors.matchAll(MARKER_CLASS)].forEach(([, marker]) => - target.set(marker, value), - ); + if (value === undefined) return; + block.markers.forEach((entry) => { + if (entry.direction === "agnostic") agnostic.set(entry.marker, value); + else if (entry.direction === direction) directed.set(entry.marker, value); + }); }); - return new Map([...agnostic, ...ltr]); + return new Map([...agnostic, ...directed]); } /** A length that moves the box: anything other than a zero (`0`, `0px`, `0vw`, `0in`, ...). */ @@ -149,9 +199,30 @@ function isNegativeLength(value: string): boolean { return value.startsWith("-") && isNonZeroLength(value); } +/** The `markers` entries of `values`, as an object, so a whole oracle can be compared in one go. */ +function pick( + values: Map, + markers: string[], +): { [marker: string]: string | undefined } { + return Object.fromEntries(markers.map((marker) => [marker, values.get(marker)])); +} + +/** Keeps the entries `accept` approves, minus the markers that must never be compensated. */ +function needingCompensation( + resolved: Map, + accept: (value: string) => boolean, +): Map { + return new Map( + [...resolved].filter(([marker, value]) => accept(value) && !NOT_COMPENSATED.has(marker)), + ); +} + /** Reports each expected marker whose actual `property` value is missing or wrong. */ -function valueMismatches(property: string, expected: Map): string[] { - const actual = getGutterMarkerValues(property); +function valueMismatches( + actual: Map, + property: string, + expected: Map, +): string[] { return [...expected] .filter(([marker, value]) => actual.get(marker) !== value) .map( @@ -162,10 +233,14 @@ function valueMismatches(property: string, expected: Map): strin /** * Reports gutter markers that set `property` but have no base rule calling for it — the reverse of - * `valueMismatches`, so a stale entry left behind after a marker loses its margin is flagged too. + * `valueMismatches`, so a table-row entry or a stale entry left behind after a marker loses its + * margin is flagged too. */ -function unexpectedMarkers(property: string, expected: Map): string[] { - const actual = getGutterMarkerValues(property); +function unexpectedMarkers( + actual: Map, + property: string, + expected: Map, +): string[] { return [...actual] .filter(([marker]) => !expected.has(marker)) .map( @@ -176,8 +251,8 @@ function unexpectedMarkers(property: string, expected: Map): str /** * Reports gutter `property` rules qualified by writing direction (`[dir=…]`). The gutter values are - * identical for LTR and RTL, so a qualifier would leave one direction uncompensated while still - * counting as covered. + * identical for LTR and RTL by design, so a qualifier would leave one direction uncompensated while + * still counting as covered. */ function directionQualifiedGutterRules(property: string): string[] { const setter = new RegExp(`${property}\\s*:`); @@ -191,37 +266,97 @@ function directionQualifiedGutterRules(property: string): string[] { .map((block) => `${property}: direction-qualified selector "${block.selectors}"`); } -// Every base text-spacing margin-left is a paragraph indent the gutter glyph must be pulled back by. -const EXPECTED_PARA_INDENT = getBaseMarkerValues("margin-left", isNonZeroLength); +/** + * Reports base marker rules that set a margin through a spelling `declarationValue` does not read + * (the `margin` shorthand or a logical `margin-inline-*`). Such a rule would give a marker a + * margin that neither side of the coverage check can see, so it must fail loudly rather than pass + * silently. + */ +function unreadableMarginSpellings(): string[] { + return blocks + .filter((block) => isBaseBlock(block.selectors) && block.markers.length > 0) + .filter((block) => + /(?:^|;|\s)(?:margin|margin-inline(?:-start|-end)?)\s*:/.test(block.declarations), + ) + .map( + (block) => + `"${block.selectors}" sets a margin with a spelling this test does not read; use ` + + `margin-left / margin-right so the gutter compensation can be derived`, + ); +} + +/** + * Reports markers whose RTL inline-start margin differs from their LTR one. The same + * `--para-indent` feeds both the LTR `left` and the RTL `right` glyph calculation, so a marker + * whose two margins disagree cannot be compensated correctly in both directions by one value. + */ +function directionAsymmetries(expected: Map): string[] { + const rtl = resolveBaseValues("margin-right", "rtl"); + return [...expected] + .filter(([marker, ltrValue]) => rtl.get(marker) !== ltrValue) + .map( + ([marker, ltrValue]) => + `.usfm_${marker}: LTR margin-left ${ltrValue} but RTL margin-right ` + + `${rtl.get(marker) ?? "none"}; one --para-indent cannot serve both directions`, + ); +} + +// Every base text-spacing margin-left is a paragraph indent the glyph must be pulled back by. +const EXPECTED_PARA_INDENT = needingCompensation( + resolveBaseValues("margin-left", "ltr"), + isNonZeroLength, +); // Every base negative text-indent is a hanging indent the focus box must start at. -const EXPECTED_VERSE_TEXT_START = getBaseMarkerValues("text-indent", isNegativeLength); +const EXPECTED_VERSE_TEXT_START = needingCompensation( + resolveBaseValues("text-indent", "ltr"), + isNegativeLength, +); + +const ACTUAL_PARA_INDENT = getGutterMarkerValues("--para-indent"); +const ACTUAL_VERSE_TEXT_START = getGutterMarkerValues("--verse-text-start"); describe("usj-nodes.css .psc-gutter-markers.text-spacing coverage", () => { it("derives a non-empty expectation from the base text-spacing rules", () => { // If the base parser ever reads nothing, both coverage checks below would pass vacuously. expect(EXPECTED_PARA_INDENT.size).toBeGreaterThan(40); expect(EXPECTED_VERSE_TEXT_START.size).toBeGreaterThan(20); - // Spot checks pinning the derivation to known USFM values, so a parser regression that reads - // the wrong property or block shows up as a wrong number rather than a shorter list. - expect(EXPECTED_PARA_INDENT.get("li2")).toBe("15vw"); - expect(EXPECTED_PARA_INDENT.get("qm2")).toBe("20vw"); - expect(EXPECTED_VERSE_TEXT_START.get("li2")).toBe("-7.5vw"); - expect(EXPECTED_VERSE_TEXT_START.get("iq1")).toBe("-15vw"); + // The parser must be able to see every margin it is asked to compensate. + expect(unreadableMarginSpellings()).toEqual([]); + expect(nestingProblems("margin-left")).toEqual([]); + expect(nestingProblems("text-indent")).toEqual([]); + }); + + it("base margins and hanging indents match the USFM stylesheet", () => { + // Independent oracle: the derivation alone would accept a base rule that drifted from the spec + // as long as its compensation drifted with it. + expect(pick(EXPECTED_PARA_INDENT, Object.keys(USFM_LEFT_MARGIN))).toEqual(USFM_LEFT_MARGIN); + expect(pick(EXPECTED_VERSE_TEXT_START, Object.keys(USFM_FIRST_LINE_INDENT))).toEqual( + USFM_FIRST_LINE_INDENT, + ); expect(EXPECTED_VERSE_TEXT_START.has("p")).toBe(false); + NOT_COMPENSATED.forEach((marker) => expect(EXPECTED_PARA_INDENT.has(marker)).toBe(false)); }); it("every indented marker sets --para-indent equal to its text-spacing margin", () => { expect(nestingProblems("--para-indent")).toEqual([]); expect(directionQualifiedGutterRules("--para-indent")).toEqual([]); - expect(valueMismatches("--para-indent", EXPECTED_PARA_INDENT)).toEqual([]); - expect(unexpectedMarkers("--para-indent", EXPECTED_PARA_INDENT)).toEqual([]); + // One value serves both directions only if the base margins agree. + expect(directionAsymmetries(EXPECTED_PARA_INDENT)).toEqual([]); + expect(valueMismatches(ACTUAL_PARA_INDENT, "--para-indent", EXPECTED_PARA_INDENT)).toEqual([]); + expect(unexpectedMarkers(ACTUAL_PARA_INDENT, "--para-indent", EXPECTED_PARA_INDENT)).toEqual( + [], + ); }); it("every hanging-indent marker sets --verse-text-start equal to its text-indent", () => { expect(nestingProblems("--verse-text-start")).toEqual([]); expect(directionQualifiedGutterRules("--verse-text-start")).toEqual([]); - expect(valueMismatches("--verse-text-start", EXPECTED_VERSE_TEXT_START)).toEqual([]); - expect(unexpectedMarkers("--verse-text-start", EXPECTED_VERSE_TEXT_START)).toEqual([]); + expect( + valueMismatches(ACTUAL_VERSE_TEXT_START, "--verse-text-start", EXPECTED_VERSE_TEXT_START), + ).toEqual([]); + expect( + unexpectedMarkers(ACTUAL_VERSE_TEXT_START, "--verse-text-start", EXPECTED_VERSE_TEXT_START), + ).toEqual([]); }); }); From 31f5947f6498c6fb6e3e8dbe136241a0ea46837d Mon Sep 17 00:00:00 2001 From: Ira Hopkinson Date: Wed, 16 Sep 2026 07:50:14 +1200 Subject: [PATCH 3/4] Tighten the gutter coverage test after review Review follow-up on #10. usj-nodes.css.gutter-coverage.test.ts: - Derive expected --para-indent from the union of LTR margin-left and RTL margin-right, so a marker indented only in RTL is still required to have an entry; directionAsymmetries reports the one-sided margin. - Run the nesting guard for margin-right as well. - Replace the tautological table-row assertion with one that fails if the exclusion goes stale (the tr markers must still carry a margin). - Call the usfm.sty oracle a spot check; the derivation guarantees base-to-gutter consistency, not base-to-spec. - Explain the `p` assertion, correct the declarationValue doc, trim the header, and put the suite above the parser helpers. usj-nodes.css: - Shorter compensation comment; notes that an "auto" text direction is out of scope (TextDirectionPlugin sets no dir attribute for it, see the KNOWN GAP in ArrowNavigationPlugin). - --verse-text-start comment says what it does today: a fallback the focus box reads through min(), never the winner for any marker here. Co-Authored-By: Claude Fable 5.1 --- packages/platform/src/usj-nodes.css | 29 ++-- .../src/usj-nodes.css.gutter-coverage.test.ts | 160 +++++++++--------- 2 files changed, 95 insertions(+), 94 deletions(-) diff --git a/packages/platform/src/usj-nodes.css b/packages/platform/src/usj-nodes.css index 642d2c46..61c89bc5 100644 --- a/packages/platform/src/usj-nodes.css +++ b/packages/platform/src/usj-nodes.css @@ -2830,18 +2830,14 @@ span.read img { unicode-bidi: isolate; } -/* Indent compensation — mirrors the inline-start margin values above. - Invariant for THIS stylesheet: every marker its text-spacing rules give a margin-left needs - an entry here with the same value, or the gutter glyph lands inside the text. - usj-nodes.css.gutter-coverage.test.ts derives that requirement from the base rules and fails - on any gap. It says nothing about margins a host injects later in source order (project - StyleInfo CSS from generateUsjCss, or a consumer's commentary stylesheets); those can move a - marker's margin away from the value compensated here and are not covered. - Table rows (tr, tr1, tr2) deliberately have no entry: a real table row is a , not a - .para, so the glyph rule never matches it. - Only active when .text-spacing is present (same condition as the source rules). - pi2 and pi3 use the same primary-indent value in both LTR and RTL (10vw and 15vw - respectively), so no direction restriction is needed here. */ +/* Indent compensation: each marker's --para-indent must equal its text-spacing margin-left above + (the same value serves RTL margin-right), or the gutter glyph lands in the text. + usj-nodes.css.gutter-coverage.test.ts enforces this for margins set in THIS file; margins a host + injects later (generateUsjCss, consumer commentary sheets) are not compensated. + Table rows (tr, tr1, tr2) need no entry: a real row is a , not a .para. + Out of scope: an "auto" text direction. TextDirectionPlugin sets no dir attribute for it, so the + [dir]-qualified margins above never apply while this compensation still does; see the KNOWN GAP + note in ArrowNavigationPlugin. */ .psc-gutter-markers.text-spacing .usfm_p2 { --para-indent: 2.5vw; } @@ -2909,10 +2905,11 @@ span.read img { --para-indent: 5vw; } -/* --verse-text-start mirrors the text-indent values for paragraphs with a - negative text-indent (hanging indent). These paragraphs render their first - line to the inline-start of the element's border edge, so the active focus box - ::after must start at text-indent to align with the text. */ +/* --verse-text-start mirrors the negative text-indent (hanging indent) of poetry-style + paragraphs. The active focus box ::after reads it through min() against the glyph column, + so it only wins when a hanging indent reaches further into the gutter than + -(gutter width) - --para-indent. No marker in this file does, so today it is a fallback; + usj-nodes.css.gutter-coverage.test.ts keeps it in step with text-indent regardless. */ .psc-gutter-markers.text-spacing .usfm_q, .psc-gutter-markers.text-spacing .usfm_q1 { --verse-text-start: -10vw; diff --git a/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts b/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts index 0bf52317..87d5fe92 100644 --- a/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts +++ b/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts @@ -7,35 +7,73 @@ import { describe, expect, it } from "vitest"; /** * In the gutter view (`.psc-gutter-markers`), each paragraph's marker glyph is absolutely * positioned at `left: calc(-(gutter width) + 0.5em - var(--para-indent))`, so `--para-indent` - * must equal the paragraph's own text-spacing margin or the glyph lands inside the text. Likewise - * the active focus box starts at `var(--verse-text-start)`, which must equal a hanging-indent - * paragraph's negative `text-indent`. - * - * This test derives BOTH expectations from the base text-spacing rules in the same file, so a - * marker that gains a margin or hanging indent without matching gutter compensation fails here by - * construction. A hand-typed expected list cannot do that: it silently encodes whatever gap - * existed when it was written. A small independent oracle from the USFM stylesheet is kept - * alongside, so a base value that drifts from the spec is caught too rather than being "fixed" by - * updating its compensation to match. - * - * Scope: the invariant is about margins set in this file. A host that injects project-stylesheet - * CSS (generateUsjCss) or its own commentary stylesheets later in source order can move a marker's - * margin away from the compensated value; that is not covered here. + * must equal the paragraph's own text-spacing margin or the glyph lands inside the text. The active + * focus box also reads `--verse-text-start`, a hanging-indent paragraph's negative `text-indent`, + * as a fallback start. Both expectations are derived from the base text-spacing rules in the same + * file, so a marker that gains a margin or hanging indent without matching compensation fails here. * * This stylesheet is mirrored into consumers, so the gutter block is contract: a gap here becomes a * gap there on the next sync. paranext-core runs the same derivation over its copies in * `extensions/src/platform-scripture-editor/src/usj-nodes-scss-coverage.test.ts`. */ +describe("usj-nodes.css .psc-gutter-markers.text-spacing coverage", () => { + it("derives a non-empty expectation from the base text-spacing rules", () => { + // If the base parser ever reads nothing, both coverage checks below would pass vacuously. + expect(EXPECTED_PARA_INDENT.size).toBeGreaterThan(40); + expect(EXPECTED_VERSE_TEXT_START.size).toBeGreaterThan(20); + // The parser must be able to see every margin it is asked to compensate. + expect(unreadableMarginSpellings()).toEqual([]); + expect(nestingProblems("margin-left")).toEqual([]); + expect(nestingProblems("margin-right")).toEqual([]); + expect(nestingProblems("text-indent")).toEqual([]); + }); + + it("spot-checks base margins and hanging indents against the USFM stylesheet", () => { + expect(pick(EXPECTED_PARA_INDENT, Object.keys(USFM_LEFT_MARGIN))).toEqual(USFM_LEFT_MARGIN); + expect(pick(EXPECTED_VERSE_TEXT_START, Object.keys(USFM_FIRST_LINE_INDENT))).toEqual( + USFM_FIRST_LINE_INDENT, + ); + // `p` has a positive first-line indent (2.5vw); only negative ones are hanging indents. + expect(EXPECTED_VERSE_TEXT_START.has("p")).toBe(false); + // The table-row exclusion is only meaningful while those markers still have a base margin. + const ltrMargins = resolveBaseValues("margin-left", "ltr"); + NOT_COMPENSATED.forEach((marker) => expect(ltrMargins.has(marker)).toBe(true)); + }); + + it("every indented marker sets --para-indent equal to its text-spacing margin", () => { + expect(nestingProblems("--para-indent")).toEqual([]); + expect(directionQualifiedGutterRules("--para-indent")).toEqual([]); + // One value serves both directions only if the base margins agree. + expect(directionAsymmetries(EXPECTED_PARA_INDENT)).toEqual([]); + expect(valueMismatches(ACTUAL_PARA_INDENT, "--para-indent", EXPECTED_PARA_INDENT)).toEqual([]); + expect(unexpectedMarkers(ACTUAL_PARA_INDENT, "--para-indent", EXPECTED_PARA_INDENT)).toEqual( + [], + ); + }); + + it("every hanging-indent marker sets --verse-text-start equal to its text-indent", () => { + expect(nestingProblems("--verse-text-start")).toEqual([]); + expect(directionQualifiedGutterRules("--verse-text-start")).toEqual([]); + expect( + valueMismatches(ACTUAL_VERSE_TEXT_START, "--verse-text-start", EXPECTED_VERSE_TEXT_START), + ).toEqual([]); + expect( + unexpectedMarkers(ACTUAL_VERSE_TEXT_START, "--verse-text-start", EXPECTED_VERSE_TEXT_START), + ).toEqual([]); + }); +}); + /** * Markers whose base rules indent them but which must NOT have gutter compensation. A real table * row is a `` (`ImmutableTableRowNode`), not a `.para`, so the glyph rule never matches it. */ const NOT_COMPENSATED = new Set(["tr", "tr1", "tr2"]); -// USFM stylesheet oracle (LeftMargin / FirstLineIndent in inches x 20 = vw), independent of this -// file: https://github.com/ubsicap/usfm/blob/master/sty/usfm.sty. One marker per distinct value is -// enough to catch a base rule drifting from the spec; completeness comes from the derivation. +// Spot check against the USFM stylesheet (LeftMargin / FirstLineIndent in inches x 20 = vw): +// https://github.com/ubsicap/usfm/blob/master/sty/usfm.sty. One marker per distinct value. The +// derivation below guarantees base-to-gutter consistency, not base-to-spec; this catches a drift in +// one of these six, and a re-sync that changes any other marker's base value is not caught here. const USFM_LEFT_MARGIN: { [key: string]: string } = { pi: "5vw", // 0.25" li1: "10vw", // 0.5" @@ -130,9 +168,9 @@ function nestingProblems(property: string): string[] { /** * Reads the value of one `property: value;` declaration out of a rule's declaration block. * - * Only a setter of exactly that property counts: the property name must start the block or follow - * a `;` or whitespace, so `margin-left` does not match inside `margin-left-foo` and a `var(--x)` - * read of a custom property does not count as setting it. + * Only a setter of exactly that property counts. The name must start the block or follow a `;` or + * whitespace, so `margin-left` is not found inside `scroll-margin-left`; and it must be followed by + * `:`, so `margin-left-foo` and a `var(--x)` read of a custom property do not count as setting it. * * @param declarations The text between a rule's `{` and `}`. * @param property The property name to read, e.g. `margin-left` or `--para-indent`. @@ -189,6 +227,18 @@ function resolveBaseValues(property: string, direction: "ltr" | "rtl"): Map { + const ltr = resolveBaseValues("margin-left", "ltr"); + const rtl = resolveBaseValues("margin-right", "rtl"); + return new Map([...rtl, ...ltr]); +} + /** A length that moves the box: anything other than a zero (`0`, `0px`, `0vw`, `0in`, ...). */ function isNonZeroLength(value: string): boolean { return !/^-?0(\.0+)?[a-z%]*$/.test(value); @@ -286,28 +336,27 @@ function unreadableMarginSpellings(): string[] { } /** - * Reports markers whose RTL inline-start margin differs from their LTR one. The same - * `--para-indent` feeds both the LTR `left` and the RTL `right` glyph calculation, so a marker - * whose two margins disagree cannot be compensated correctly in both directions by one value. + * Reports markers whose RTL inline-start margin differs from their LTR one, including a margin set + * in only one direction. The same `--para-indent` feeds both the LTR `left` and the RTL `right` + * glyph calculation, so a marker whose two margins disagree cannot be compensated correctly in both + * directions by one value. */ function directionAsymmetries(expected: Map): string[] { + const ltr = resolveBaseValues("margin-left", "ltr"); const rtl = resolveBaseValues("margin-right", "rtl"); - return [...expected] - .filter(([marker, ltrValue]) => rtl.get(marker) !== ltrValue) + return [...expected.keys()] + .filter((marker) => ltr.get(marker) !== rtl.get(marker)) .map( - ([marker, ltrValue]) => - `.usfm_${marker}: LTR margin-left ${ltrValue} but RTL margin-right ` + + (marker) => + `.usfm_${marker}: LTR margin-left ${ltr.get(marker) ?? "none"} but RTL margin-right ` + `${rtl.get(marker) ?? "none"}; one --para-indent cannot serve both directions`, ); } -// Every base text-spacing margin-left is a paragraph indent the glyph must be pulled back by. -const EXPECTED_PARA_INDENT = needingCompensation( - resolveBaseValues("margin-left", "ltr"), - isNonZeroLength, -); +// Every base inline-start margin is a paragraph indent the glyph must be pulled back by. +const EXPECTED_PARA_INDENT = needingCompensation(resolveInlineStartMargins(), isNonZeroLength); -// Every base negative text-indent is a hanging indent the focus box must start at. +// Every base negative text-indent is a hanging indent the focus box can fall back to. const EXPECTED_VERSE_TEXT_START = needingCompensation( resolveBaseValues("text-indent", "ltr"), isNegativeLength, @@ -315,48 +364,3 @@ const EXPECTED_VERSE_TEXT_START = needingCompensation( const ACTUAL_PARA_INDENT = getGutterMarkerValues("--para-indent"); const ACTUAL_VERSE_TEXT_START = getGutterMarkerValues("--verse-text-start"); - -describe("usj-nodes.css .psc-gutter-markers.text-spacing coverage", () => { - it("derives a non-empty expectation from the base text-spacing rules", () => { - // If the base parser ever reads nothing, both coverage checks below would pass vacuously. - expect(EXPECTED_PARA_INDENT.size).toBeGreaterThan(40); - expect(EXPECTED_VERSE_TEXT_START.size).toBeGreaterThan(20); - // The parser must be able to see every margin it is asked to compensate. - expect(unreadableMarginSpellings()).toEqual([]); - expect(nestingProblems("margin-left")).toEqual([]); - expect(nestingProblems("text-indent")).toEqual([]); - }); - - it("base margins and hanging indents match the USFM stylesheet", () => { - // Independent oracle: the derivation alone would accept a base rule that drifted from the spec - // as long as its compensation drifted with it. - expect(pick(EXPECTED_PARA_INDENT, Object.keys(USFM_LEFT_MARGIN))).toEqual(USFM_LEFT_MARGIN); - expect(pick(EXPECTED_VERSE_TEXT_START, Object.keys(USFM_FIRST_LINE_INDENT))).toEqual( - USFM_FIRST_LINE_INDENT, - ); - expect(EXPECTED_VERSE_TEXT_START.has("p")).toBe(false); - NOT_COMPENSATED.forEach((marker) => expect(EXPECTED_PARA_INDENT.has(marker)).toBe(false)); - }); - - it("every indented marker sets --para-indent equal to its text-spacing margin", () => { - expect(nestingProblems("--para-indent")).toEqual([]); - expect(directionQualifiedGutterRules("--para-indent")).toEqual([]); - // One value serves both directions only if the base margins agree. - expect(directionAsymmetries(EXPECTED_PARA_INDENT)).toEqual([]); - expect(valueMismatches(ACTUAL_PARA_INDENT, "--para-indent", EXPECTED_PARA_INDENT)).toEqual([]); - expect(unexpectedMarkers(ACTUAL_PARA_INDENT, "--para-indent", EXPECTED_PARA_INDENT)).toEqual( - [], - ); - }); - - it("every hanging-indent marker sets --verse-text-start equal to its text-indent", () => { - expect(nestingProblems("--verse-text-start")).toEqual([]); - expect(directionQualifiedGutterRules("--verse-text-start")).toEqual([]); - expect( - valueMismatches(ACTUAL_VERSE_TEXT_START, "--verse-text-start", EXPECTED_VERSE_TEXT_START), - ).toEqual([]); - expect( - unexpectedMarkers(ACTUAL_VERSE_TEXT_START, "--verse-text-start", EXPECTED_VERSE_TEXT_START), - ).toEqual([]); - }); -}); From e34511f15e288afd565c90699d7ef34490fc5348 Mon Sep 17 00:00:00 2001 From: Ira Hopkinson Date: Thu, 17 Sep 2026 12:12:55 +1200 Subject: [PATCH 4/4] Parse the gutter coverage test with postcss and compensate tr1/tr2 Second review round on #10. usj-nodes.css: - Restore the tr1 and tr2 gutter entries. The converter matches the table-row marker as exactly "tr", so the obsolete tr1 and tr2 become plain paragraphs with a 10vw/15vw margin and need compensation; only tr renders as a . - Give var(--para-indent) a 0px fallback in the focus-box min(), so a caret in a table cell (a block root with no .para ancestor) does not invalidate inset-inline-start. - Comment now says host-injected margins override these and leave the compensation stale by the difference, that a marker needs the same margin in both directions, and that under an "auto" text direction RTL content keeps the LTR gutter. usj-nodes.css.gutter-coverage.test.ts: - Parse with postcss instead of a flat regex. Rules are classified per selector, nesting is read from the tree, and declarations come from the parser, which closes the mixed-selector-group, nested-shorthand, wrapped-value and !important blind spots the review probed. - Derive from the union of LTR and RTL margins and compare both sides for every marker, so a one-sided or zero-masked margin is reported. - Marker classes may carry hyphens and capitals; a class the regex cannot read whole throws instead of matching a shorter marker. - Report keyword and calc() values as unreadable rather than deriving from them; treat :dir() as direction-qualified like [dir=]. - A hanging indent on a marker with no margin fails, since it would win the focus-box min() and start the box outside the editor. - Map equality replaces the two hand-written diff helpers; the table-row exclusion is checked against a non-zero margin. postcss is added as a platform devDependency at the version already resolved in the workspace. Co-Authored-By: Claude Fable 5.1 --- packages/platform/package.json | 1 + packages/platform/src/usj-nodes.css | 27 +- .../src/usj-nodes.css.gutter-coverage.test.ts | 395 ++++++++---------- pnpm-lock.yaml | 3 + 4 files changed, 195 insertions(+), 231 deletions(-) diff --git a/packages/platform/package.json b/packages/platform/package.json index 8cd2218f..da88a573 100644 --- a/packages/platform/package.json +++ b/packages/platform/package.json @@ -62,6 +62,7 @@ "quill-delta": "^5.1.0" }, "devDependencies": { + "postcss": "8.5.26", "react": "^18.3.1", "react-dom": "^18.3.1", "shared": "workspace:~", diff --git a/packages/platform/src/usj-nodes.css b/packages/platform/src/usj-nodes.css index 61c89bc5..4b52fc2c 100644 --- a/packages/platform/src/usj-nodes.css +++ b/packages/platform/src/usj-nodes.css @@ -2830,13 +2830,16 @@ span.read img { unicode-bidi: isolate; } -/* Indent compensation: each marker's --para-indent must equal its text-spacing margin-left above - (the same value serves RTL margin-right), or the gutter glyph lands in the text. - usj-nodes.css.gutter-coverage.test.ts enforces this for margins set in THIS file; margins a host - injects later (generateUsjCss, consumer commentary sheets) are not compensated. - Table rows (tr, tr1, tr2) need no entry: a real row is a , not a .para. +/* Indent compensation: each marker's --para-indent must equal its text-spacing margin-left above, + or the gutter glyph lands in the text. The same value serves RTL margin-right, so a marker needs + the same margin in both directions. usj-nodes.css.gutter-coverage.test.ts enforces this for + margins set in THIS file; margins a host injects later (generateUsjCss, consumer commentary + sheets) override these and leave the compensation stale by the difference. + \tr needs no entry: a real row is a , not a .para. The obsolete \tr1 and \tr2 still convert + to plain paragraphs, so they do. Out of scope: an "auto" text direction. TextDirectionPlugin sets no dir attribute for it, so the - [dir]-qualified margins above never apply while this compensation still does; see the KNOWN GAP + [dir]-qualified margins above never apply while this compensation still does, and the + [dir]-qualified gutter rules never match, so RTL content keeps the LTR gutter; see the KNOWN GAP note in ArrowNavigationPlugin. */ .psc-gutter-markers.text-spacing .usfm_p2 { --para-indent: 2.5vw; @@ -2849,7 +2852,8 @@ span.read img { .psc-gutter-markers.text-spacing .usfm_li1, .psc-gutter-markers.text-spacing .usfm_ph, .psc-gutter-markers.text-spacing .usfm_ph1, -.psc-gutter-markers.text-spacing .usfm_pi2 { +.psc-gutter-markers.text-spacing .usfm_pi2, +.psc-gutter-markers.text-spacing .usfm_tr1 { --para-indent: 10vw; } .psc-gutter-markers.text-spacing .usfm_q, @@ -2863,7 +2867,8 @@ span.read img { .psc-gutter-markers.text-spacing .usfm_lim, .psc-gutter-markers.text-spacing .usfm_lim1, .psc-gutter-markers.text-spacing .usfm_ph2, -.psc-gutter-markers.text-spacing .usfm_pi3 { +.psc-gutter-markers.text-spacing .usfm_pi3, +.psc-gutter-markers.text-spacing .usfm_tr2 { --para-indent: 15vw; } .psc-gutter-markers.text-spacing .usfm_qm, @@ -2961,7 +2966,9 @@ span.read img { .psc-gutter-markers.text-spacing .usfm_ph, .psc-gutter-markers.text-spacing .usfm_ph1, .psc-gutter-markers.text-spacing .usfm_ph2, -.psc-gutter-markers.text-spacing .usfm_ph3 { +.psc-gutter-markers.text-spacing .usfm_ph3, +.psc-gutter-markers.text-spacing .usfm_tr1, +.psc-gutter-markers.text-spacing .usfm_tr2 { --verse-text-start: -5vw; } @@ -3004,6 +3011,6 @@ span.read img { .psc-active-focus.psc-gutter-markers .psc-active-text::after { inset-inline-start: min( var(--verse-text-start, 0px), - calc(-1 * var(--psc-gutter-width) - var(--para-indent)) + calc(-1 * var(--psc-gutter-width) - var(--para-indent, 0px)) ); } diff --git a/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts b/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts index 87d5fe92..cc560d0b 100644 --- a/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts +++ b/packages/platform/src/usj-nodes.css.gutter-coverage.test.ts @@ -2,6 +2,7 @@ // (jsdom rewrites `import.meta.url` to an http URL, which breaks the stylesheet read; this test // only reads a file, so the node environment is the correct one anyway.) import { readFileSync } from "node:fs"; +import postcss, { type Rule } from "postcss"; import { describe, expect, it } from "vitest"; /** @@ -15,18 +16,26 @@ import { describe, expect, it } from "vitest"; * This stylesheet is mirrored into consumers, so the gutter block is contract: a gap here becomes a * gap there on the next sync. paranext-core runs the same derivation over its copies in * `extensions/src/platform-scripture-editor/src/usj-nodes-scss-coverage.test.ts`. + * + * Known limits, each unreachable in today's stylesheet: a `calc()` value is reported as unreadable + * rather than evaluated, and a selector's direction is read from `[dir=…]` / `:dir(…)` only. */ describe("usj-nodes.css .psc-gutter-markers.text-spacing coverage", () => { - it("derives a non-empty expectation from the base text-spacing rules", () => { - // If the base parser ever reads nothing, both coverage checks below would pass vacuously. - expect(EXPECTED_PARA_INDENT.size).toBeGreaterThan(40); - expect(EXPECTED_VERSE_TEXT_START.size).toBeGreaterThan(20); - // The parser must be able to see every margin it is asked to compensate. - expect(unreadableMarginSpellings()).toEqual([]); - expect(nestingProblems("margin-left")).toEqual([]); - expect(nestingProblems("margin-right")).toEqual([]); - expect(nestingProblems("text-indent")).toEqual([]); + it("can read every rule and value it derives from", () => { + // Rules nested in an at-rule (a media query, keyframes) would be scoped away by the browser; + // none of the tracked properties may be set there. + expect(nestedTrackedRules()).toEqual([]); + // A margin spelled as shorthand, a logical property, a keyword or a `calc()` cannot be derived + // from, and a `!important` would win over the compensation's own rule. + expect(unreadableDeclarations()).toEqual([]); + // If the parser ever read nothing, the comparisons below would pass vacuously. + expect(EXPECTED_PARA_INDENT.size).toBeGreaterThan(0); + expect(EXPECTED_VERSE_TEXT_START.size).toBeGreaterThan(0); + // The gutter values are identical for LTR and RTL by design, so no gutter rule may be + // direction-qualified: it would leave one direction uncompensated while counting as covered. + expect(directionQualifiedGutterRules("--para-indent")).toEqual([]); + expect(directionQualifiedGutterRules("--verse-text-start")).toEqual([]); }); it("spot-checks base margins and hanging indents against the USFM stylesheet", () => { @@ -36,44 +45,43 @@ describe("usj-nodes.css .psc-gutter-markers.text-spacing coverage", () => { ); // `p` has a positive first-line indent (2.5vw); only negative ones are hanging indents. expect(EXPECTED_VERSE_TEXT_START.has("p")).toBe(false); - // The table-row exclusion is only meaningful while those markers still have a base margin. - const ltrMargins = resolveBaseValues("margin-left", "ltr"); - NOT_COMPENSATED.forEach((marker) => expect(ltrMargins.has(marker)).toBe(true)); + // The table-row exclusion is only meaningful while that marker still has a real margin. + NOT_COMPENSATED.forEach((marker) => + expect(isNonZeroLength(BASE_INLINE_START_MARGIN.get(marker) ?? "0")).toBe(true), + ); }); it("every indented marker sets --para-indent equal to its text-spacing margin", () => { - expect(nestingProblems("--para-indent")).toEqual([]); - expect(directionQualifiedGutterRules("--para-indent")).toEqual([]); - // One value serves both directions only if the base margins agree. - expect(directionAsymmetries(EXPECTED_PARA_INDENT)).toEqual([]); - expect(valueMismatches(ACTUAL_PARA_INDENT, "--para-indent", EXPECTED_PARA_INDENT)).toEqual([]); - expect(unexpectedMarkers(ACTUAL_PARA_INDENT, "--para-indent", EXPECTED_PARA_INDENT)).toEqual( - [], - ); + // One value serves both directions only if the base margins agree, including a margin set + // in only one direction. + expect(directionAsymmetries("margin-left", "margin-right")).toEqual([]); + expect(ACTUAL_PARA_INDENT).toEqual(EXPECTED_PARA_INDENT); }); it("every hanging-indent marker sets --verse-text-start equal to its text-indent", () => { - expect(nestingProblems("--verse-text-start")).toEqual([]); - expect(directionQualifiedGutterRules("--verse-text-start")).toEqual([]); - expect( - valueMismatches(ACTUAL_VERSE_TEXT_START, "--verse-text-start", EXPECTED_VERSE_TEXT_START), - ).toEqual([]); - expect( - unexpectedMarkers(ACTUAL_VERSE_TEXT_START, "--verse-text-start", EXPECTED_VERSE_TEXT_START), - ).toEqual([]); + expect(directionAsymmetries("text-indent", "text-indent")).toEqual([]); + expect(ACTUAL_VERSE_TEXT_START).toEqual(EXPECTED_VERSE_TEXT_START); + // The focus box takes min(--verse-text-start, -(gutter) - --para-indent). A hanging indent on a + // marker with no margin would win that min() and start the box outside the editor. + const hangingWithoutMargin = [...EXPECTED_VERSE_TEXT_START.keys()].filter( + (marker) => !EXPECTED_PARA_INDENT.has(marker), + ); + expect(hangingWithoutMargin).toEqual([]); }); }); /** * Markers whose base rules indent them but which must NOT have gutter compensation. A real table - * row is a `` (`ImmutableTableRowNode`), not a `.para`, so the glyph rule never matches it. + * row (`\tr`) is a `` (`ImmutableTableRowNode`), not a `.para`, so the glyph rule never matches + * it. The obsolete `\tr1` and `\tr2` are not excluded: the converter matches `tr` exactly, so they + * become plain paragraphs and need an entry like any other indented marker. */ -const NOT_COMPENSATED = new Set(["tr", "tr1", "tr2"]); +const NOT_COMPENSATED = new Set(["tr"]); // Spot check against the USFM stylesheet (LeftMargin / FirstLineIndent in inches x 20 = vw): // https://github.com/ubsicap/usfm/blob/master/sty/usfm.sty. One marker per distinct value. The // derivation below guarantees base-to-gutter consistency, not base-to-spec; this catches a drift in -// one of these six, and a re-sync that changes any other marker's base value is not caught here. +// one of these markers; a re-sync that changes any other marker's base value is not caught here. const USFM_LEFT_MARGIN: { [key: string]: string } = { pi: "5vw", // 0.25" li1: "10vw", // 0.5" @@ -90,161 +98,158 @@ const USFM_FIRST_LINE_INDENT: { [key: string]: string } = { q4: "-2.5vw", // -0.125" }; +/** The properties this test derives from or checks; a nested or unreadable one of these fails. */ +const TRACKED_PROPERTIES = new Set([ + "margin-left", + "margin-right", + "text-indent", + "--para-indent", + "--verse-text-start", +]); + +/** Margin spellings the derivation cannot read; a marker rule using one must fail loudly. */ +const UNREADABLE_MARGIN_PROPERTIES = new Set(["margin", "margin-inline", "margin-inline-start"]); + type Direction = "ltr" | "rtl" | "agnostic"; -/** One `.usfm_` selector from a rule, with the writing direction it is scoped to. */ +/** One selector of a rule, with its `.usfm_` and the writing direction it is scoped to. */ interface MarkerSelector { + selector: string; marker: string; direction: Direction; } +/** A top-level rule that names at least one `.usfm_`. */ interface Block { - /** The rule's selector list, whitespace-collapsed and attribute quotes normalised to `'`. */ - selectors: string; - /** Each `.usfm_` in the selector list, classified per selector, not per list. */ + rule: Rule; markers: MarkerSelector[]; - declarations: string; } -const MARKER_CLASS = /\.usfm_([a-z0-9]+)/; +const root = postcss.parse(readFileSync(new URL("./usj-nodes.css", import.meta.url), "utf-8")); /** - * Classifies one selector (not a comma list) by the writing direction it is scoped to. Attribute - * quotes are normalised so `[dir="ltr"]` and `[dir='ltr']` compare equal: this file uses double - * quotes, the paranext-core SCSS copy single quotes. + * The `.usfm_` class in a selector. Marker classes may carry hyphens and capitals + * (`usfm_qt-s`, `usfm_xtSeeAlso`), so the match runs to the end of the class name; a `.usfm_` that + * does not match whole is a parser gap and throws rather than attributing the rule to a shorter + * marker. */ +function markerOf(selector: string): string | undefined { + if (!selector.includes(".usfm_")) return undefined; + const match = /\.usfm_([A-Za-z0-9-]+)(?![A-Za-z0-9_-])/.exec(selector); + if (!match) throw new Error(`Cannot read the marker class in "${selector}"`); + return match[1]; +} + +/** Attribute quotes normalised so `[dir="ltr"]` and `[dir='ltr']` compare equal. */ function directionOf(selector: string): Direction { - if (selector.includes("[dir='rtl']")) return "rtl"; - if (selector.includes("[dir='ltr']")) return "ltr"; + const normalised = selector.replace(/"/g, "'"); + if (normalised.includes("[dir='rtl']") || normalised.includes(":dir(rtl)")) return "rtl"; + if (normalised.includes("[dir='ltr']") || normalised.includes(":dir(ltr)")) return "ltr"; return "agnostic"; } -const css = readFileSync(new URL("./usj-nodes.css", import.meta.url), "utf-8").replace( - /\/\*[\s\S]*?\*\//g, - "", -); - -// The stylesheet is parsed as a flat list of `selector { declarations }` blocks. This regex cannot -// reliably read a rule nested inside another block (a media query or a keyframes at-rule), so a -// rule wrapped in one could be mis-parsed and its markers silently uncovered. `nestingProblems()` -// catches that by brace depth before the coverage checks run. Selector lists are split on commas -// and each selector classified on its own, so a group that mixes directions cannot misfile the -// whole group. -const blocks: Block[] = [...css.matchAll(/([^{}]+)\{([^}]+)\}/g)].map( - ([, selectors, declarations]) => { - const normalised = selectors.replace(/\s+/g, " ").replace(/"/g, "'").trim(); - const markers = normalised - .split(",") - .map((selector) => selector.trim()) - .flatMap((selector) => { - const marker = MARKER_CLASS.exec(selector)?.[1]; - return marker ? [{ marker, direction: directionOf(selector) }] : []; - }); - return { selectors: normalised, markers, declarations }; - }, -); - -/** Brace nesting depth at a character offset: 1 inside a top-level rule, >1 inside a nested one. */ -function braceDepthAt(index: number): number { - const before = css.slice(0, index); - return (before.match(/\{/g) ?? []).length - (before.match(/\}/g) ?? []).length; +/** A gutter compensation selector carries both scoping classes. */ +function isGutterSelector(selector: string): boolean { + return selector.includes("psc-gutter-markers") && selector.includes("text-spacing"); } -/** - * Reports `property` setters that sit deeper than one block, where a browser would scope them - * away but the flat block parser cannot reliably see them. - */ -function nestingProblems(property: string): string[] { - const setter = new RegExp(`${property}\\s*:`, "g"); - const nested = [...css.matchAll(setter)].filter((match) => braceDepthAt(match.index ?? 0) > 1); - return nested.length === 0 - ? [] - : [ - `${property}: ${nested.length} setter(s) are nested inside an @media/@keyframes block; ` + - `the flat parser below cannot reliably see them. Update the parser.`, - ]; +/** A base text-spacing selector: scoped by `.text-spacing` but not a gutter selector. */ +function isBaseSelector(selector: string): boolean { + return selector.includes("text-spacing") && !isGutterSelector(selector); } -/** - * Reads the value of one `property: value;` declaration out of a rule's declaration block. - * - * Only a setter of exactly that property counts. The name must start the block or follow a `;` or - * whitespace, so `margin-left` is not found inside `scroll-margin-left`; and it must be followed by - * `:`, so `margin-left-foo` and a `var(--x)` read of a custom property do not count as setting it. - * - * @param declarations The text between a rule's `{` and `}`. - * @param property The property name to read, e.g. `margin-left` or `--para-indent`. - * @returns The trimmed value, or `undefined` when the block does not set that property. - */ -function declarationValue(declarations: string, property: string): string | undefined { - const match = new RegExp(`(?:^|;|\\s)${property}\\s*:\\s*([^;]+)`).exec(declarations); - return match ? match[1].trim() : undefined; +/** The value a rule sets for `property`, whitespace-collapsed; the last declaration wins. */ +function declarationValue(rule: Rule, property: string): string | undefined { + let value: string | undefined; + rule.each((node) => { + if (node.type === "decl" && node.prop === property) value = node.value.replace(/\s+/g, " "); + }); + return value; } -/** A block is a gutter compensation rule when its selector carries both scoping classes. */ -function isGutterBlock(selectors: string): boolean { - return selectors.includes("psc-gutter-markers") && selectors.includes("text-spacing"); +const BLOCKS: Block[] = []; +root.walkRules((rule) => { + if (rule.parent?.type !== "root") return; + const markers = rule.selectors.flatMap((selector) => { + const marker = markerOf(selector); + return marker ? [{ selector, marker, direction: directionOf(selector) }] : []; + }); + if (markers.length > 0) BLOCKS.push({ rule, markers }); +}); + +/** Tracked properties set inside an at-rule, where the cascade would scope them away. */ +function nestedTrackedRules(): string[] { + const found: string[] = []; + root.walkDecls((decl) => { + if (!TRACKED_PROPERTIES.has(decl.prop) && !UNREADABLE_MARGIN_PROPERTIES.has(decl.prop)) return; + if (decl.parent?.type === "rule" && decl.parent.parent?.type !== "root") + found.push(`${decl.prop} inside a nested rule: ${decl.parent.selector}`); + }); + return found; } -/** A block is a base text-spacing rule: scoped by `.text-spacing` but not a gutter rule. */ -function isBaseBlock(selectors: string): boolean { - return selectors.includes("text-spacing") && !isGutterBlock(selectors); +/** + * Marker declarations the derivation cannot read: a `margin` shorthand or logical margin, a + * keyword or `calc()` in place of a plain length, or `!important` on a tracked property. + */ +function unreadableDeclarations(): string[] { + const found: string[] = []; + BLOCKS.forEach(({ rule, markers }) => { + if (!markers.some(({ selector }) => isBaseSelector(selector) || isGutterSelector(selector))) + return; + rule.each((node) => { + if (node.type !== "decl") return; + if (UNREADABLE_MARGIN_PROPERTIES.has(node.prop)) + found.push(`${rule.selector}: ${node.prop} is not derivable; use margin-left/right`); + if (!TRACKED_PROPERTIES.has(node.prop)) return; + if (node.important) found.push(`${rule.selector}: ${node.prop} !important`); + if (!/^-?\d*\.?\d+[a-z%]+$|^0$/.test(node.value.trim())) + found.push(`${rule.selector}: ${node.prop}: ${node.value} is not a plain length`); + }); + }); + return found; } -/** Maps each `.usfm_` to the value the gutter rules give it for `property`. */ -function getGutterMarkerValues(property: string): Map { +/** Maps each marker to the value the gutter rules give it for `property`. */ +function gutterValues(property: string): Map { const values = new Map(); - blocks - .filter((block) => isGutterBlock(block.selectors)) - .forEach((block) => { - const value = declarationValue(block.declarations, property); - if (value === undefined) return; - block.markers.forEach(({ marker }) => values.set(marker, value)); + BLOCKS.forEach(({ rule, markers }) => { + const value = declarationValue(rule, property); + if (value === undefined) return; + markers.forEach(({ selector, marker }) => { + if (isGutterSelector(selector)) values.set(marker, value); }); + }); return values; } /** - * Maps each `.usfm_` to the value the BASE text-spacing rules resolve for `property` in the - * given direction: a `[dir=…]` rule for that direction wins over a direction-agnostic one, matching - * the cascade (the qualified selector is more specific). Every value is kept here, zero included — - * a `margin-left: 0` LTR override must be allowed to beat an agnostic `5vw` before any filtering, - * or the filter would discard the winner and enforce the loser. + * Maps each marker to the value the BASE text-spacing rules resolve for `property` in the given + * direction: a `[dir=…]` rule for that direction wins over a direction-agnostic one, matching the + * cascade (the qualified selector is more specific). Zeros are kept: a `margin-left: 0` LTR + * override must be allowed to beat an agnostic `5vw` before any filtering. */ -function resolveBaseValues(property: string, direction: "ltr" | "rtl"): Map { +function baseValues(property: string, direction: "ltr" | "rtl"): Map { const agnostic = new Map(); const directed = new Map(); - blocks - .filter((block) => isBaseBlock(block.selectors)) - .forEach((block) => { - const value = declarationValue(block.declarations, property); - if (value === undefined) return; - block.markers.forEach((entry) => { - if (entry.direction === "agnostic") agnostic.set(entry.marker, value); - else if (entry.direction === direction) directed.set(entry.marker, value); - }); + BLOCKS.forEach(({ rule, markers }) => { + const value = declarationValue(rule, property); + if (value === undefined) return; + markers.forEach((entry) => { + if (!isBaseSelector(entry.selector)) return; + if (entry.direction === "agnostic") agnostic.set(entry.marker, value); + else if (entry.direction === direction) directed.set(entry.marker, value); }); + }); return new Map([...agnostic, ...directed]); } -/** - * Maps each marker with an inline-start margin in EITHER direction to the margin `--para-indent` - * must equal: the LTR `margin-left` where there is one, else the RTL `margin-right`. Deriving from - * the union means a marker indented only in RTL is still required to have an entry; - * `directionAsymmetries` then reports that its two margins disagree. - */ -function resolveInlineStartMargins(): Map { - const ltr = resolveBaseValues("margin-left", "ltr"); - const rtl = resolveBaseValues("margin-right", "rtl"); - return new Map([...rtl, ...ltr]); -} - /** A length that moves the box: anything other than a zero (`0`, `0px`, `0vw`, `0in`, ...). */ function isNonZeroLength(value: string): boolean { - return !/^-?0(\.0+)?[a-z%]*$/.test(value); + return !/^-?0(\.0+)?[a-z%]*$/i.test(value); } -/** A negative length — the hanging indent that pulls the first line before the border edge. */ +/** A negative length: the hanging indent that pulls the first line before the border edge. */ function isNegativeLength(value: string): boolean { return value.startsWith("-") && isNonZeroLength(value); } @@ -267,100 +272,48 @@ function needingCompensation( ); } -/** Reports each expected marker whose actual `property` value is missing or wrong. */ -function valueMismatches( - actual: Map, - property: string, - expected: Map, -): string[] { - return [...expected] - .filter(([marker, value]) => actual.get(marker) !== value) - .map( - ([marker, value]) => - `.usfm_${marker}: expected ${property}: ${value}, got ${actual.get(marker) ?? "none"}`, - ); -} - /** - * Reports gutter markers that set `property` but have no base rule calling for it — the reverse of - * `valueMismatches`, so a table-row entry or a stale entry left behind after a marker loses its - * margin is flagged too. + * Reports markers whose RTL value for `rtlProperty` differs from their LTR value for + * `ltrProperty`, including a value set in only one direction. The same custom property feeds both + * directions, so one value can only be right if the base rules agree. */ -function unexpectedMarkers( - actual: Map, - property: string, - expected: Map, -): string[] { - return [...actual] - .filter(([marker]) => !expected.has(marker)) +function directionAsymmetries(ltrProperty: string, rtlProperty: string): string[] { + const ltr = baseValues(ltrProperty, "ltr"); + const rtl = baseValues(rtlProperty, "rtl"); + return [...new Set([...ltr.keys(), ...rtl.keys()])] + .filter((marker) => (ltr.get(marker) ?? "0") !== (rtl.get(marker) ?? "0")) .map( - ([marker, value]) => - `.usfm_${marker}: sets ${property}: ${value} but no base text-spacing rule calls for it`, + (marker) => + `.usfm_${marker}: LTR ${ltrProperty} ${ltr.get(marker) ?? "none"} but RTL ` + + `${rtlProperty} ${rtl.get(marker) ?? "none"}; one value cannot serve both directions`, ); } -/** - * Reports gutter `property` rules qualified by writing direction (`[dir=…]`). The gutter values are - * identical for LTR and RTL by design, so a qualifier would leave one direction uncompensated while - * still counting as covered. - */ +/** Gutter rules for `property` qualified by writing direction, which the design forbids. */ function directionQualifiedGutterRules(property: string): string[] { - const setter = new RegExp(`${property}\\s*:`); - return blocks - .filter( - (block) => - isGutterBlock(block.selectors) && - setter.test(block.declarations) && - block.selectors.includes("[dir="), - ) - .map((block) => `${property}: direction-qualified selector "${block.selectors}"`); -} - -/** - * Reports base marker rules that set a margin through a spelling `declarationValue` does not read - * (the `margin` shorthand or a logical `margin-inline-*`). Such a rule would give a marker a - * margin that neither side of the coverage check can see, so it must fail loudly rather than pass - * silently. - */ -function unreadableMarginSpellings(): string[] { - return blocks - .filter((block) => isBaseBlock(block.selectors) && block.markers.length > 0) - .filter((block) => - /(?:^|;|\s)(?:margin|margin-inline(?:-start|-end)?)\s*:/.test(block.declarations), - ) - .map( - (block) => - `"${block.selectors}" sets a margin with a spelling this test does not read; use ` + - `margin-left / margin-right so the gutter compensation can be derived`, - ); + return BLOCKS.filter( + ({ rule, markers }) => + declarationValue(rule, property) !== undefined && + markers.some( + ({ selector, direction }) => isGutterSelector(selector) && direction !== "agnostic", + ), + ).map(({ rule }) => `${property}: direction-qualified selector "${rule.selector}"`); } -/** - * Reports markers whose RTL inline-start margin differs from their LTR one, including a margin set - * in only one direction. The same `--para-indent` feeds both the LTR `left` and the RTL `right` - * glyph calculation, so a marker whose two margins disagree cannot be compensated correctly in both - * directions by one value. - */ -function directionAsymmetries(expected: Map): string[] { - const ltr = resolveBaseValues("margin-left", "ltr"); - const rtl = resolveBaseValues("margin-right", "rtl"); - return [...expected.keys()] - .filter((marker) => ltr.get(marker) !== rtl.get(marker)) - .map( - (marker) => - `.usfm_${marker}: LTR margin-left ${ltr.get(marker) ?? "none"} but RTL margin-right ` + - `${rtl.get(marker) ?? "none"}; one --para-indent cannot serve both directions`, - ); -} +/** Each marker's inline-start margin: the LTR `margin-left`, else the RTL `margin-right`. */ +const BASE_INLINE_START_MARGIN = new Map([ + ...baseValues("margin-right", "rtl"), + ...baseValues("margin-left", "ltr"), +]); // Every base inline-start margin is a paragraph indent the glyph must be pulled back by. -const EXPECTED_PARA_INDENT = needingCompensation(resolveInlineStartMargins(), isNonZeroLength); +const EXPECTED_PARA_INDENT = needingCompensation(BASE_INLINE_START_MARGIN, isNonZeroLength); // Every base negative text-indent is a hanging indent the focus box can fall back to. const EXPECTED_VERSE_TEXT_START = needingCompensation( - resolveBaseValues("text-indent", "ltr"), + baseValues("text-indent", "ltr"), isNegativeLength, ); -const ACTUAL_PARA_INDENT = getGutterMarkerValues("--para-indent"); -const ACTUAL_VERSE_TEXT_START = getGutterMarkerValues("--verse-text-start"); +const ACTUAL_PARA_INDENT = gutterValues("--para-indent"); +const ACTUAL_VERSE_TEXT_START = gutterValues("--verse-text-start"); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cf250d62..06d371cb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -448,6 +448,9 @@ importers: specifier: ^5.1.0 version: 5.1.0 devDependencies: + postcss: + specifier: 8.5.26 + version: 8.5.26 react: specifier: ^18.3.1 version: 18.3.1