Skip to content

Conversation

@Julow
Copy link
Collaborator

@Julow Julow commented Sep 7, 2023

These are changes from #2314 that I extracted and polished a bit.

This is a work-in-progress, the goal is to cause no regressions on the default profile.

@Julow Julow marked this pull request as ready for review September 8, 2023 09:55
@Julow
Copy link
Collaborator Author

Julow commented Sep 8, 2023

Ready for review. There's a regression that I would like to keep:

   | (* Cancellation requested. Build jobs are immediately rejected in this
        state *)
-    Restarting_build of
-      Memo.Invalidation.t
+    Restarting_build of Memo.Invalidation.t

The constructor decl used to break after the of due to the comment.

@Julow
Copy link
Collaborator Author

Julow commented Sep 8, 2023

The constructor declaration change is moved to a different PR for easier blaming and better changelog: #2429

Copy link
Collaborator

@gpetiot gpetiot left a comment

Choose a reason for hiding this comment

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

LGTM!

Fun arguments are not aligned after the `fun` keyword and are not
aligned inside parentheses.

This requires tweaking the box structure.
`Params.Exp.box_fun_decl_args` has the same intent as the other `get_*`
functions but works by composing bigger pieces rather than returning
pieces to compose.
@Julow
Copy link
Collaborator Author

Julow commented Sep 8, 2023

While looking at the diffs, I found 3 similar changes that I added here. I'll merge if test_branch reports no regression.

@Julow
Copy link
Collaborator Author

Julow commented Sep 11, 2023

No diff in test_branch, merging!

@Julow Julow merged commit c7b89a0 into ocaml-ppx:main Sep 11, 2023
Julow added a commit to Julow/opam-repository that referenced this pull request Sep 15, 2023
….26.1)

CHANGES:

### Changed

- Compatible with OCaml 5.1.0 (ocaml-ppx/ocamlformat#2412, @Julow)
  The syntax of let-bindings changed sligthly in this version.
- Improved ocp-indent compatibility (ocaml-ppx/ocamlformat#2428, @Julow)
- \* Removed extra break in constructor declaration with comment (ocaml-ppx/ocamlformat#2429, @Julow)
- \* De-indent the `object` keyword in class types (ocaml-ppx/ocamlformat#2425, @Julow)
- \* Consistent formatting of arrows in class types (ocaml-ppx/ocamlformat#2422, @Julow)

### Fixed

- Fix dropped attributes on a begin-end in a match case (ocaml-ppx/ocamlformat#2421, @Julow)
- Fix dropped attributes on begin-end in an if-then-else branch (ocaml-ppx/ocamlformat#2436, @gpetiot)
- Fix non-stabilizing comments before a functor type argument (ocaml-ppx/ocamlformat#2420, @Julow)
- Fix crash caused by module types with nested `with module` (ocaml-ppx/ocamlformat#2419, @Julow)
- Fix ';;' formatting between doc-comments and toplevel directives (ocaml-ppx/ocamlformat#2432, @gpetiot)
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
….26.1)

CHANGES:

### Changed

- Compatible with OCaml 5.1.0 (ocaml-ppx/ocamlformat#2412, @Julow)
  The syntax of let-bindings changed sligthly in this version.
- Improved ocp-indent compatibility (ocaml-ppx/ocamlformat#2428, @Julow)
- \* Removed extra break in constructor declaration with comment (ocaml-ppx/ocamlformat#2429, @Julow)
- \* De-indent the `object` keyword in class types (ocaml-ppx/ocamlformat#2425, @Julow)
- \* Consistent formatting of arrows in class types (ocaml-ppx/ocamlformat#2422, @Julow)

### Fixed

- Fix dropped attributes on a begin-end in a match case (ocaml-ppx/ocamlformat#2421, @Julow)
- Fix dropped attributes on begin-end in an if-then-else branch (ocaml-ppx/ocamlformat#2436, @gpetiot)
- Fix non-stabilizing comments before a functor type argument (ocaml-ppx/ocamlformat#2420, @Julow)
- Fix crash caused by module types with nested `with module` (ocaml-ppx/ocamlformat#2419, @Julow)
- Fix ';;' formatting between doc-comments and toplevel directives (ocaml-ppx/ocamlformat#2432, @gpetiot)
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.

2 participants