Skip to content

Fix override parsing and preserve comments in JSX applications - #18

Merged
davesnx merged 3 commits into
mainfrom
fix/formatter-review-regressions
Oct 1, 2026
Merged

davesnx merged 3 commits into
mainfrom
fix/formatter-review-regressions

Conversation

@davesnx

@davesnx davesnx commented Sep 16, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #16 and #17. Companion parser fix: ocaml-mlx/mlx#47.

Restore object-override expressions

The two-token GREATER RBRACE closer made ordinary override expressions ambiguous, including examples without a comparison:

{< x = true && false >}
{< x = if condition then 1 else 2 >}

Adding parentheses let these parse, but the formatter removed them and then failed its own syntax check. The previous workaround only handled top-level > comparisons.

  • Emit a distinct GREATER_BEFORE_RBRACE token for the closing angle while leaving } to be lexed separately. The name makes clear that the token consumes only >, not the brace.
  • Update both vendored parsers, including JSX and object-type closers; preserve whitespace before the brace.
  • Remove the narrow parenthesis workaround now that normal override expressions parse again.

Preserve comments on hand-written JSX applications

Previously, this failed with BUG: comment changed because conversion to JSX omitted the commented unit argument:

(App.createElement ~children:xs (* keep *) ()) [@JSX]
  • Retain the unit argument's location during JSX classification.
  • Keep ordinary application syntax when the omitted unit has comments.
  • Use the same classification for nested-child/prop parentheses, including spreads.
  • Continue canonicalizing uncommented applications to <App>...xs</App>.

Tests

Added regressions for override comparisons, boolean operators, conditionals, local bindings, functions, match/try expressions, empty/local overrides, nested braces, object types, and JSX closing tags. Comment regressions cover unit-argument placement and applications nested in children, spreads, and props, including idempotency checks.

Passed the full local suite with OCaml 5.5.0 and Menhir 20230608:

DEVELOPER_DIR=/Library/Developer/CommandLineTools \
SDKROOT=/Library/Developer/CommandLineTools/SDKs/MacOSX15.4.sdk \
opam exec --switch=/Users/davesnx/Code/github/ocaml-mlx/mlx -- dune runtest

Publication note

The initial commit was accidentally pushed to main and reverted in 252fe2f. This PR reapplies the fixes through review and includes the explicit token name; main's content was restored before this branch was published.

@davesnx
davesnx merged commit 2ba843f into main Oct 1, 2026
10 checks passed
davesnx added a commit that referenced this pull request Oct 1, 2026
* origin/main:
  Fix override parsing and preserve comments in JSX applications (#18)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant