Question: What does wrap-comments do?

[mbarbin]

This is a question about the formatting option wrap-comments=true.

I think I am misunderstanding its intent. I thought this would be an option to force inserting newline characters in regular non-docstring comments, so they fit into the margin. However, experimenting with this, this doesn't seem to be the case. I am not sure whether I should file a bug report, or I misunderstood how the option can be used.

Here is my config:

version = 0.27.0
profile = janestreet
wrap-comments = true

Here was an attempt at writing a long comment, with the hope that it would be automatically wrapped on fmt:

(* Hello, this file has [wrap-comments = true] enabled via its [.ocamlformat] config, but I am not sure what changes does it make. This comment for example is not wrapped when this file is formatted. Sounds like the setting is ignored. Or how long should a comment for it to be wrapped? I am keeping this comment here for the time being to monitor whether there'd be changes on this front. *)

[Julow]

This changed in #2371. Regular comments are formatted as doc-comments with the janestreet profile, independently of the wrap-comments option. But then, because parse-docstrings is not enabled, the comments are not formatted. There seems to be a logical problem here. An other thing is that doc-comments don't wrap with the janestreet profile, and that cannot be changed with an option. I may have fixed that in https://github.com/ocaml-ppx/ocamlformat/pull/2645

[mbarbin]

Thank you for looking into the issue!

Asterisk

I built a local version of ocamlformat including this fix:

$ ocamlformat --version
0.27.0-1-g4c94d48

and then tried it out with this config:

version=0.27.0-1-g4c94d48
ocaml-version=5.2
profile=janestreet
parse-docstrings=true
wrap-comments=true

I confirm now that a file like the following is stable through fmt. Thank you!

(* Hello.
 * This is a comment with asterisk chars
 * at the beginning of every lines.
 *
 * Yay!
 *)
let () = ()

Incidentally as I was editing the file manually, I came across this case:

(* Hello.
   * This is a comment with asterisk chars
 * at the beginning of every lines.
 *
 * Yay!
 *)
 let () = ()

$ ocamlformat mini_fmt.ml
ocamlformat: Cannot process "mini_fmt.ml".
  Please report this bug at https://github.com/ocaml-ppx/ocamlformat/issues.
  BUG: comment changed.
File "mini_fmt.ml", lines 1-6, characters 0-3:
Error: comment (*  Hello.
   * This is a comment with asterisk chars
 * at the beginning of every lines.
 *
 * Yay!
  *) dropped.
  BUG: comment changed.
File "mini_fmt.ml", lines 1-6, characters 0-2:
Error: comment (*  Hello.
   * This is a comment with asterisk chars
   * at the beginning of every lines.
   *
   * Yay!
 *) added.

As in, if the asterisk are not originally aligns, something unexpected happens. Reporting for your consideration. cours Asterixme, cours!!

Wrap-comments

I then tried to copy my long one line comment in another file, both as regular or doc-strings comment. I does not wrap.

An other thing is that doc-comments don't wrap with the janestreet profile, and that cannot be changed with an option.

I am not sure if the current fix intended to fix this as well. If it did, something is not working as expected.

[mbarbin]

Hello @Julow just wanted to make sure you see my last comment after the issue is closed. It sounds like it's missing another round of fixes for this and #2643

By the way I have tried the wrap-comments option with the conventional profile to get some experience with it, and I found it very nice. I look forward to using it with my current config. Thank you.

[Julow]

Thanks for your help :) I'll re-open the issue as a bug with asterisks is still here.

An other thing is that doc-comments don't wrap with the janestreet profile, and that cannot be changed with an option.

I am not sure if the current fix intended to fix this as well. If it did, something is not working as expected.

The fix was not intended to change this as that's the style used at Janestreet. One possibility would be to expose this as an option.

@tdelvecchio-jsc OCamlformat avoids wrapping paragraphs in comments when using the Janestreet profile. Is this intended or is this a workaround for something that could be improved ?

[mdelvecchio-jsc]

This is something @dvulakh is currently working on.

[mbarbin]

In the release notes for 0.28.0 it says the wrap-comments=true was fixed for janestreet profile, but this is not what I see when trying the binary from the release. Could someone please try to experiment with this, to confirm this?

I confirm the issue with the asterisk is fixed.

[dvulakh]

I expect to post a PR against oxcaml/ocamlformat that re-enables comment formatting in the janestreet profile soon (order of weeks?). I will then be happy to give some pointers if @Julow wants to upstream these changes.

[mbarbin]

@dvulakh your input will give me some material to think about. On one hand I am excited and thankful about your work on this, but on the other hand I don't know if I should be worried about some implications when you say:

I will then be happy to give some pointers if @Julow wants to upstream these changes.

it sounds like the janestreet profile on the main ocamlformat runs the risk of becoming a second class citizen, and I'm not sure how this plays in the long term.. Alrighty, we'll see how goes with this particular feature. Thanks!

[dvulakh]

I will let @Julow opine on how he thinks about the status of the janestreet profile in main ocamlformat.

It is true that Jane Street development has largely centered around our ocamlformat fork in recent years, but that's mostly because we need to format additional OxCaml syntax. This feature, on the other hand, is plausibly upstreamable.

[Julow]

@dvulakh I think it's better if your changes are upstreamed, when that's easy. Otherwise, your fork will diverge more and more which might be annoying to people that use OCaml and OxCaml at the same time. It'd also make incorporating upstream bug fixes harder.

@mbarbin The janestreet profile has been the formatting used inside Janestreet until now. It changed several time in the past, which might not be ideal for open source projects. Perhaps we could make a new profile that looks similar but is in control of the community ? As it seems that a lof of people like this profile.

[mbarbin]

@dvulakh I didn't realize Jane Street development for non OxCaml features were no longer done here, but in retrospect that makes sense as it should be a more convenient workflow! Thanks for letting me know.

@Julow Re new community profile inspired by the upstream janestreet for OCaml. I find this interesting! That would make sense to me, if there is demand. I tried a few times to see if I could switch to a profile for open source projects, but the convential profile is so different, that it felt like too big a departure and I didn't pursue so far.

Thank you for your answers!

[Julow]

I tried a few times to see if I could switch to a profile for open source projects, but the convential profile is so different, that it felt like too big a departure and I didn't pursue so far.

Have you tried the ocamlformat profile ? I'm also considering adding the ocsigen profile with these options: https://github.com/ocsigen/eliom/blob/master/.ocamlformat

[mbarbin]

Have you tried the ocamlformat profile ?

I had a while ago, but you motivated me to do some recent experiment again & Re: ocsigen example as well: I selected a repo I have as candidate for previews and created draft PRs with these 2 styles. https://github.com/mbarbin/cubzzle/pull/17 & https://github.com/mbarbin/cubzzle/pull/18

I have some reservations about both of the profiles ocsigen & ocamlformat. It seems unlikely that I'd want to adopt either ones without significant tweaking.

Generally speaking, I think I'd be more motivated to opt-in into a shared profile rather than having to maintain my own configuration. But it may not be realistic and I should be OK with having a few tweaks on top of an existing "close-enough" profile. That's what I was trying to accomplish with profile=janestreet + enabling better comments & doc-comments support. As it stands & based on what I see in these draft PRs my personal take is that if the wrap-comments was support for my current profile, that'd still be my preferred option as of today.

A question btw: I did another experiment and thought that when one uses --print-config to "expand" all current configuration from a profile, the resulting formatting would be the same. However I have tried in https://github.com/mbarbin/cubzzle/pull/19 to expand my current config that way, but this formats very differently (is that a bug? or have I not generated the config fields correctly?). Thank you!