fix typst color regex bug and add typst-css unit tests#14469
Merged
Conversation
CSS `border` and `border-color` declarations lost any tokens that
preceded an `rgb()` / `rgba()` color because `s:find('%w+%b()', start)`
in `translate_border` and `consume_color` is unanchored — it would skip
past width/style (or earlier color) tokens to match the parens-bearing
form. With `border: 0px solid rgb(255, 0, 0)` the width and style were
never seen, leaving the default `medium` (2.25pt) thickness.
Adding `^` anchors the search at `start` so the leading token is
consumed first, matching how named/hex colors already worked.
Also adds a luaunit-based unit-test convention:
- tests/unit-lua/*.test.lua (luaunit suites; ends in os.exit(lu...))
- tests/smoke/lua-unit/lua-unit.test.ts runs each via `quarto run` with
LUA_PATH set so `require('luaunit')` and filter-module requires work.
Fixes #14460
While writing tests for #14460, found and fixed five latent bugs in src/resources/filters/modules/typst_css.lua: - parse_color: brand-color reference path (var(--brand-NAME)) crashed when NAME contained digits. The capture class [%a--]* excluded %d, so e.g. var(--brand-red-50) failed to match and fell into a broken error branch that concatenated an undefined `v` and `return null`. Widened the class to [%w-] and fixed the error branch to use the actual color string and return nil. - output_color: implicit global `zero` (missing `local`) leaked from the no-color-with-opacity path. Made it local. - translate_font_weight: passed `null` (an undefined global -> nil) to output_warning, silently bypassing any user-supplied warnings collector. Changed to forward `warnings`. - consume_width / consume_style: `fbeg, fend = ...` without `local` leaked them as globals on every call. Added `local`. Each fix is covered by a regression test in tests/unit-lua/typst-css.test.lua.
- parse_multiple was declared without `local`, leaking as a global on
every module load. Made it `local function` (forward use in
translate_border still works since the local is declared above).
- Removed `set_brand_mode = set_brand_mode,` from the export table:
set_brand_mode is never defined in this module and has no callers
anywhere, so the export silently resolved to nil — a phantom API
surface.
- Removed dead `local mult = 1` initial assignment (immediately
overwritten in every branch); now just `local mult` then assigned.
- Removed unused `local dash` in translate_border_style.
- Removed a duplicate `local function quote(s)` definition (two
identical copies of the same function body, the first one dead).
- Removed a trailing-whitespace nit.
Two regression tests added in tests/unit-lua/typst-css.test.lua:
- TestModuleNoGlobalLeak.testParseMultipleNotGlobal — `_G.parse_multiple`
must be nil after the module loads.
- TestModuleExports.testNoPhantomExports — every key listed on the LHS
of the module's `return { ... }` block must resolve to a non-nil
value on the loaded module. (pairs() can't see this because Lua
tables drop nil-valued keys, so the test reads the source file.)
While locking in coverage for every public function in
src/resources/filters/modules/typst_css.lua, found and fixed five more
latent bugs:
- parse_opacity: tonumber() can return nil (e.g. for `rgba(0 0 0 / abc)`),
but the value was passed directly to math.min(1.0, ...) which raises
"number expected, got nil" and crashes the filter. Now returns nil
when the operand isn't a number, on both the percent and fraction
branches.
- parse_color: hex colors of length 1, 2, 5, or 7 (and any with non-hex
characters) were silently accepted as 1-, 2-, 2-, or 3-component rgb
values because gmatch '..' just dropped trailing odd digits and there
was no character validation. Now requires exactly 3, 4, 6, or 8 hex
digits and emits an "invalid hex color" warning otherwise.
- length_units: 'dvmin ' (with a stray trailing space) caused
parse_length_unit('10dvmin') to fall through to the shorter 'vmin',
silently rejecting any `dvmin` length. Removed the space.
- translate_font_weight: CSS keywords are case-insensitive, but the
module's lookup tables only had lowercase entries, so values like
'BOLD' or 'Normal' fell into the "invalid font weight" branch. Now
lowercases the input before lookup.
- translate_font_family_list: a whitespace-only input like ' ' was
emitted as `("",)` because the gmatch matched the whitespace run as
one token, leading-space trim made it empty, but it was still inserted
as a quoted empty family. Now also strips trailing whitespace and
skips empty results.
Twenty-five new unit tests (5 failing-regression + 20 lock-in) added to
tests/unit-lua/typst-css.test.lua, covering parse_opacity, parse_color
(hex paths), parse_color-via-rgb, output_color, parse_length_unit (the
dvmin case), translate_border (default propagation + ordering),
translate_font_weight, translate_font_family_list, and
expand_side_shorthand. Total Lua unit tests: 47, all passing.
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rgb()#14460: anchor the%w+%b()color matcher intranslate_borderandconsume_colorso width/style tokens that precede anrgb()/rgba()color aren't skipped (e.g.border: 0px solid rgb(255,0,0)now correctly suppresses the border instead of rendering at the default 2.25pt).src/resources/filters/modules/typst_css.luaand fixed ten more latent bugs found there. Highlights:var(--brand-NAME)references crashed the Typst filter whenNAMEcontained digits (e.g.--brand-red-50).parse_opacity('not-a-number')crashed withmath.min(1.0, nil).dvminlength unit silently rejected (stray trailing space in the unit table).BOLD/Normalnot recognized asbold/normal(CSS keywords are case-insensitive, but the module's lookups were case-sensitive).#fffffsilently parsed as 2-component colors.locals leaking globals (zero,fbeg,fend,parse_multiple) on every module load.set_brand_modewas a phantom export —set_brand_mode = set_brand_modein the export table but never defined anywhere; never had a caller.tests/unit-lua/<feature>.test.lua— luaunit suites that end inos.exit(lu.LuaUnit.run()).tests/smoke/lua-unit/lua-unit.test.ts— Deno smoke test that runs each listed Lua suite viaquarto runwithLUA_PATHset, surfacing luaunit failures through the Deno test runner.typst_css.lua; the same shape works for any other Lua module that wants direct unit coverage.typst_css.lua, including regression tests for all bugs fixed here.luacheckon the module is now clean (was 9 warnings).Test plan
cd tests && ./run-fast-tests.sh smoke/lua-unit/lua-unit.test.ts→ 1 Deno test passes (47 luaunit subtests, all green)rgb()#14460 (named/hex/rgb/rgba zero-px borders) to Typst → all four cells render bare, matching the issue's expected output