fix: highlight the case and prql keywords and the brackets in the lezer grammar - #6307
fix: highlight the case and prql keywords and the brackets in the lezer grammar#6307prql-bot wants to merge 2 commits into
Conversation
…er grammar
The style tags for `case`, `( )`, `[ ]`, `{ }` and `| ,` named terms the
grammar never declared, so they matched nothing and those tokens rendered
unstyled. Declare the bracket and separator literals in `@tokens`, and
switch `case` and `prql` to `kw<>` so they are named terms, then add the
missing `prql` style tag.
prql-bot
left a comment
There was a problem hiding this comment.
Self-review, so a COMMENT rather than an approval.
The change does what it claims: I rebuilt the parser and ran the suite (106 passing), confirmed both new cases fail against 37f81bb, and checked that every selector in highlight.js now resolves to a term in the built parser — no dead tag is left behind. Two points.
The tree gained prql and case nodes, and nothing pins them. The description says the existing tests are unaffected because fileTests skips punctuation. That is right for the brackets — defaultIgnore in @lezer/generator's test module is /\W/.test(type.name) — but it does not cover the two keywords. kw<> gives them word names, so they now appear in the tree the way let already does in misc.txt: QueryDefinition(prql,NamedArg(...)) and CaseExpression(case,TupleExpression(...)). The 104 existing cases stay green only because no .txt case exercises either rule — there is no prql header and no case expression anywhere in test/*.txt. So this PR changes the shape of the tree downstream consumers walk and leaves the new shape untested; a couple of cases in misc.txt would pin it in the suite's own idiom. The description is also worth correcting, since it currently reads as though the tree is unchanged.
The test pins nine tokens rather than the property that failed. The bug here is that styleTags drops an unmatched selector silently — a tenth selector added later, for a token none of these snippets happen to contain, fails exactly the same way and still passes this test. Splitting the spec out of the styleTags call makes the general check about ten lines:
export const highlightSpec = { ... };
export const prqlHighlight = styleTags(highlightSpec);and the test then asserts every selector in highlightSpec resolves to a name in parser.nodeSet.types. I ran that assertion against this branch and it reports nothing missing, so it passes as-is and would catch the next dead tag without needing a snippet per token. Whether that is worth the extra export is a judgement call; the snippet tests are still useful either way, since they pin the tag each token gets rather than only that it gets one.
Unrelated to the code: #6308 (tend/update-workflows) carries 553cd4f in its history, so this change is currently sitting in two open PRs.
…resolves Naming the two keywords through kw<> adds them to the tree, which no .txt case exercised; two misc.txt cases pin the new shape. Splitting the selector map out of the styleTags call lets a test assert every selector resolves to a grammar term, so a future dead tag fails here rather than degrading silently.
|
Both points actioned in b08b03f, plus a correction to the third. The tree shape is now pinned. The review is right that The general check is in. The selector map is exported as #6308 does not carry this commit. Its head |
Problem
Five of the style tags in the lezer grammar's
highlight.jsmatch no term in the grammar, so nine tokens render unstyled in any editor built on this parser: thecasekeyword,(,),[,],{,},,and|. Theprqlquery header has no style tag at all, so it is unstyled too.Lezer only creates a named term for a literal it is told to name. The bracket and separator literals appear inline in rules and are never declared, so they are anonymous — a selector naming one matches nothing.
caseandprqlare written as bare@specialize<identPart, "…">rather than through the grammar'skw<>template, which is what adds[@name={term}]; that is whymodule,let,this,that,nullandindo highlight and these two do not. Nothing reports the mismatch:styleTagsdrops an unmatched selector silently, so the parser builds and loads clean.Against a parser built from
main, the tags that do land areletandtrue; every token listed above comes back with no tag:Solution
Declare the bracket and separator literals in the
@tokensblock, alongside the"="[@name=Equals]that is already there — an undecorated literal declaration names each term after itself. SwitchQueryDefinitionandCaseExpressiontokw<>so their keywords become named terms, and add the missingprqlentry tohighlight.jsont.keyword, matching the keyword face the emacs, nano, GtkSourceView and KSyntaxHighlighting grammars give it after #6210.All nine tokens plus
prqlthen carry their intended tag.Tree changes for downstream consumers
Naming the two keywords adds them to the parse tree, the way
letalready appears: a query header now parses asQueryDefinition(prql,NamedArg(…))and a case expression asCaseExpression(case,TupleExpression(…)). Anything walking the tree by child position sees one extra leading child on those two nodes.The brackets and separators are the wider change, not the smaller one: naming a literal puts it in the tree, so every rule built from one gains it as a child.
TupleExpression,ArrayExpression,NestedPipeline,ParenthesizedExpressionandModulegain both delimiters, andAnnotationgains its closing}(its@{opener is a separate literal and still undeclared) — whereselect {a, b}gaveTupleExpression(Identifier,Identifier), its tuple now holds the two braces and the comma as well. The grammar suite doesn't see any of that, becausedefaultIgnorein@lezer/generator's test module skips any node whose name is non-word, which is why no.txtcase needed updating; consumers do see it.The one consumer in this repo changes behaviour, for the better.
web/prql-codemirror-demo/src/lang-prql/prql.tsregistersfoldNodeProp.add({"ArrayExpression TupleExpression": foldInside}), andfoldInsidefolds from the end of the first child to the start of the last. Against37f81bbthose were the first and last tuple items, so foldingcollapsed only the
,\nbetweenaandb; with the delimiters named it collapses the whole body inside the braces, which is whatfoldInsideis for. Nothing else in the repo reads this tree — the demo'scomplete.tsmatches node names only, andweb/playgroundhighlights through its own Monaco Monarch tokenizer rather than this grammar.The existing 104 grammar tests stayed green through the keyword naming only because none of them contained a
prqlheader or acaseexpression, so two cases intest/misc.txtnow pin the new shape; both fail against37f81bb.Testing
test/test-highlight.jsis new. Its snippet cases map each tag to its own class and assert the exact[token, tag]pairs a snippet produces, pinning the tag each token gets rather than only that it gets one; both fail onmain.Snippets only cover the tokens they contain, though, and the underlying bug is that an unmatched selector is dropped in silence — a tenth selector added later for a token no snippet uses would fail the same way and still pass. So the selector map is now exported from
highlight.jsasprqlHighlightSpecand passed tostyleTagsseparately, and a third case asserts every selector in it resolves to a term inparser.nodeSet.types. Adding a selector for a non-existent term makes that case fail with the term's name.This does not add keywords the grammar cannot parse.
into,type,internal,func,importandenumare still absent from the lezer grammar, which needs rules rather than a keyword list; #6210 excluded it for that reason, and #2052 tracks the missing grammar coverage.Found during the nightly rolling survey of
grammars/prql-lezer/src/highlight.js.