Skip to content

Improve: extend option sequence-blank-line to let-bindings#1056

Closed
gpetiot wants to merge 4 commits intoocaml-ppx:masterfrom
gpetiot:preserve-blank-line
Closed

Improve: extend option sequence-blank-line to let-bindings#1056
gpetiot wants to merge 4 commits intoocaml-ppx:masterfrom
gpetiot:preserve-blank-line

Conversation

@gpetiot
Copy link
Copy Markdown
Collaborator

@gpetiot gpetiot commented Oct 8, 2019

This is WIP

Fix #1047

Few issues:

  • I suspect the location body.pexp_loc of Pexp_letop {let_; ands; body} is incorrect, I only checked by printing the location, the definition of letop_binding_body in parser.mly looks suspicious (should reuse more bits of let_binding_body) but I'm not really familiar enough with this code (any ideas @Julow ?)
  • The location of the bindings don't include the in keyword, not a bug, but as a consequence this feature does not work if there is a in then an empty line separating 2 definitions (hence the TO-FIX-tests)

Copy link
Copy Markdown
Collaborator

@Julow Julow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in keywords are not counted in any location, we will need to read a token from the source file.
I couldn't find anything wrong with the locations in letops. I'll look more into it.

in
fmt_let c ctx ~ext ~rec_flag ~bindings ~parens ~loc:pexp_loc
~attributes:pexp_attributes ~fmt_atrs ~fmt_expr ~indent_after_in
~blank_line_after
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fmt_let has enough context to compute that itself, it should be moved there.

@gpetiot gpetiot changed the title Improve: rename option sequence-blank-line to function-bank-line and extends it to let-bindings Improve: extend option sequence-blank-line to let-bindings Oct 9, 2019
@gpetiot gpetiot mentioned this pull request Oct 9, 2019
12 tasks
@gpetiot
Copy link
Copy Markdown
Collaborator Author

gpetiot commented Oct 15, 2019

Replaced by #1075 and #1077

@gpetiot gpetiot closed this Oct 15, 2019
@gpetiot gpetiot deleted the preserve-blank-line branch October 15, 2019 05:04
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.

Feature request: preserve more blank lines in function body

3 participants