Upgrade to ocamlformat 0.26.0

[Julow]

The changed formatting of @@ let ... (any operator) cause huge diffs in the command descriptions. The changed indentation of if and let in parentheses also has a big impact.

The aim of this commit is to gather feedback.

Changelog can be found here: https://github.com/ocaml-ppx/ocamlformat/blob/main/CHANGES.md

[emillon]

Once 3.9.0 is out I'll upgrade to 0.25.0 first so we can see what's going on more precisely (I'll also enable .git-blame-ignore-revs).

[rgrinberg]

IMO the old code looked better here.

[Julow]

What do you think of the similar change in cmd code, where the let-expression is much bigger and the operators are not chained ?

[rgrinberg]

The old version is better there too. The additional line only pays off when the let expression has some ugly wrapping due to the lack of horizontal space. There was no such issue there.

[Alizter]

Can ocamlformat pick the one that causes the least wrapping? (at least under the hood)

[Julow]

Not really, OCamlformat cannot compare two possible formatting. Only the input can be considered (though, multiple iterations on the whole code can be made).
But it can format differently whether there would be no wrapping at all and it can detect chained operators.

Such formatting could be possible:

<|> let+ p = Profile.decode in Profile p
<|> ...

falling back to this as soon as it doesn't fit on the line:

<|>
let+ p = Profile.decode in
(* longer code *)
Profile p
<|> ...

I'm mentioning chained operators because to me, the cmd code looks better with less indentation.
To me, that indentation can cause ugly wrapping means that it does.

[Julow]

By looking at the diff again, I find that the reduced indentation do save lines by removing some wrapping. I also do not find a lot of chained operators so my previous suggestion is not valid.

Though, it should be possible have this formatting, as long as it fits on one line:

...
<|> let+ p = Profile.decode in Profile p

Should I add this before the next release ?

[emillon]

not a huge fan of saving lines at all cost, I think that the old version was better.

[Julow]

I went back and found the reason for this change: ocaml-ppx/ocamlformat#2315
It's a consistency change, in the previous version, the line was broken on regular lets but not letops:

let _ =
  Cmd.v info
  @@ let+ trimmed_size = () in
     ()

let _ =
  ()
  <|> let+ p = Profile.decode in
      Profile p

let _ =
  Cmd.v info
  @@
  let trimmed_size = () in
  ()

let _ =
  ()
  <|>
  let p = Profile.decode in
  Profile p

The other form exists in the code: bin/describe/describe_workspace.ml line 378.

This diff can go away by keeping the line break from the source code, whether it's a let or a letop. (ocaml-ppx/ocamlformat#2396)
We try to add as few of these as possible but the regression is large enough to justify it.

I'll merge this patch and update this preview PR after the weekend.

[Julow]

Once 3.9.0 is out I'll upgrade to 0.25.0 first so we can see what's going on more precisely (I'll also enable .git-blame-ignore-revs).

Let me know and I'll rebase this.

[emillon]

Let me know and I'll rebase this.

you can go ahead!

[Julow]

I rebased and re-ran the preview with the @@ let+ fix.

[emillon]

No need to do that in this PR since this will change

[Julow]

Right!

I'll make sure to remove that and fix the version line before I mark the PR as ready.

[Julow]

Though, do you think this PR could be merged unsquashed ?

[emillon]

for now I prefer doing this in 2 steps

[Julow]

Ok!

[emillon]

In my opinion, these are all improvements. Let's wait for @rgrinberg's opinion as well.

[Julow]

Thanks :) Unless a problem is found, I intent to release today.

[Julow]

OCamlformat 0.26.0 has been released! This PR can be merged or closed if it's a bit too early to upgrade.

[emillon]

I rebased this; this can land once #8311 is merged.

[Alizter]

Indentation here is kind of funny. Is this expected @Julow?

[Julow]

It is expected: The -> was moved to the same line as the arguments to avoid the previous indentation. The arguments no longer fit on the line and have to break.

The arguments are indented more than the body for disambiguation.

[Alizter]

LGTM, just a non-blocking comment.