Repository navigation
Fix JSX elements closed directly before |] and } - #46
Merged
Merged
Conversation
`[|<div>aa</div>|]` failed to parse: the `>`-operator rule lexed `>|` as an infix operator, so the end tag never produced GREATER. Add a dedicated `>|]` rule to both lexers that gives the `>` back and lets `|]` lex separately as BARRBRACKET. An operator like `>|` can never be legally followed by `]` without parentheses, so no legal program changes meaning; `1>|2`, `[|1>|2|]` and `>|=` are covered by tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`{x = <div>a</div>}` failed to parse: `>}` lexed as the single
GREATERRBRACE token (the object-override closer), so the JSX end tag
never produced GREATER. Steal `>}` in both lexers (backtrack, return
GREATER, let `}` lex as RBRACE) and compensate in both grammars by
closing object overrides with the two-token sequence GREATER RBRACE.
Object override `{< x = 2 >}` keeps parsing in both spacings.
Known narrow regression: an override field ending in an
unparenthesized comparison directly before the closer
(`{< x = 1 > 2 >}`) now needs parentheses (`{< x = (1 > 2) >}`);
pinned in tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Formatter port: ocaml-mlx/ocamlformat-mlx#17. Note from testing over there: the override-comparison limitation is broader than the description above — an unparenthesized |
`(x : <m : int>)` failed to parse because `<m` lexes as the JSX element-open token even in type position, where JSX can never occur. Both grammars now accept the fused token as `<` plus the first method label: object_type gains a meth_list_jsx alternative whose first field comes from JSX_LIDENT and which continues into the ordinary meth_list. Lexers untouched; no new menhir conflicts; spaced object types, `< .. >`, and expression JSX covered by regression tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
Added a third fix to this PR (also ported to ocaml-mlx/ocamlformat-mlx#17): object types written without a space after |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
davesnx
approved these changes
Sep 16, 2026
andreypopp
added a commit
to ocaml-mlx/ocamlformat-mlx
that referenced
this pull request
Sep 16, 2026
* Fix JSX elements closed directly before |] and } Port of ocaml-mlx/mlx#46 to both vendored parsers: a dedicated `>|]` lexer rule backtracks to give the `>` back so `|]` lexes as BARRBRACKET, and `>}` is stolen the same way with the object-override grammar compensating by closing with GREATER RBRACE. Also fix Pexp_override printing to keep parentheses around a bare `>` comparison used as a field value (any field, not just the last one): dropping them produced output that fails the reparse self-check, since an unparenthesized `a > b` in an override field list is ambiguous with the closing `>` under the new two-token close. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Accept object types written without a space after < `(x : <m : int>)` failed to parse because `<m` lexes as the JSX element-open token even in type position, where JSX can never occur. Both vendored grammars now accept the fused token as `<` plus the first method label: field/field_semi are parameterized over the label symbol, and object_type gains a meth_list_jsx alternative fed by an inline jsx_first_label rule. Lexers untouched; spaced object types, `< .. >`, and expression JSX are covered by regression tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Condense comments to single lines Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
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.
Two lexing bugs where a JSX end tag directly touching a closing delimiter failed to parse:
Fixes
>|]lexer rule backtracks to give the>back (closing the tag) and lets|]lex asBARRBRACKET. An operator like>|can never legally be followed by]without parentheses, so no valid program changes meaning;1>|2,[|1>|2|]and>|=are covered by tests. Same technique as the earlier removal of the>]/GREATERRBRACKETrule that made list literals work.>}is stolen the same way, and both grammars now close object overrides with the two-token sequenceGREATER RBRACEinstead ofGREATERRBRACE.{< x = 2 >}keeps parsing in both spacings (and{< x = 2 > }becomes newly accepted).Both fixes are applied to the two parsers (mlx preprocessor and merlin reader), with cram tests including the merlin
-convbridge.Known narrow regression
An override field ending in an unparenthesized comparison directly before the closer —
{< x = 1 > 2 >}— now requires parentheses:{< x = (1 > 2) >}(pinned in tests). The parser shifts the final>as a comparison; stock OCaml's single-token>}avoided the ambiguity. Menhir picked up exactly one new silently-solved shift/reduce conflict from this; no reduce/reduce.🤖 Generated with Claude Code