Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Represent 'let f _ = function' in the CST #2596

Merged
merged 22 commits into from
Oct 24, 2024

Conversation

Julow
Copy link
Collaborator

@Julow Julow commented Oct 23, 2024

Currently, the CST represents the arguments of 'let f _ =' in a way that
is incompatible with the changed function representation from OCaml 5.2.
These two were represented the same way in the CST:

let f _ = function ...
let f _ = (function ...)

To fix this problem, the representation for let bindings bodies is
changed to 'function_body', matching the 5.2 representation.

The ctx used while formatting let bindings is changed and this caused a large number of changes in the code and removed a few unnecessary parentheses in the output.

This fixes various inconsistencies and will help write parens rules for
functions.
Currently, the CST represents the arguments of 'let f _ =' in a way that
is incompatible with the changed function representation from OCaml 5.2.
These two are represented the same way in the CST:

    let f _ = function ...
    let f _ = (function ...)

To fix this problem, the representation for let bindings bodies is
changed to 'function_body', matching the 5.2 representation.

This is still work in progress, Ast rules need to be tweaked, a comment
is dropped and some formattings are broken.
And reuse code between 'Lb' and 'Pexp_let' Ast rules.
@Julow
Copy link
Collaborator Author

Julow commented Oct 24, 2024

This fixes a critical bug in #2544 and fixes bad indentation and unwanted parentheses in let bindings.

@Julow Julow merged commit 500977d into ocaml-ppx:main Oct 24, 2024
9 of 10 checks passed
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