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

Please reconsider multiline deprecations #1997

Closed
vphantom opened this issue Jan 28, 2022 · 6 comments
Closed

Please reconsider multiline deprecations #1997

vphantom opened this issue Jan 28, 2022 · 6 comments

Comments

@vphantom
Copy link

I am currently using four options which were deprecated in recent commits, which make me wary of upgrading beyond version 0.19:

I can live without if-then-else if I must (big adjustment, but I can see the appeal), and I'm not sure what function-indent-nested and match-indent-nested actually do anymore, but particularly painful to me would be losing closing-on-separate-line which makes things much easier to read and especially edit/diff. I get that LISPy languages save a line by closing on the last line, but I don't really see OCaml as a LISP. 😉

There was no explanation with the pull requests themselves so I'd like to take this opportunity to ask: are they deprecated because they are covered by other options or recent default behavior, or because too few people use them? Would a bounty help resuscitate indicate-multiline-delimiters in particular?

@gpetiot
Copy link
Collaborator

gpetiot commented Jan 29, 2022

Hi, there are multiple reasons as to why we are deprecating all these options:

  • first of all it's making the maintenance of ocamlformat unsustainable: some options have more than 3 values, that have not much in common, that results of big pattern-matchings to handle this in multiple parts of the code. It's a big drag to fixing some bugs or developing new features. We still regularly see some users asking for even more options, it was a mistake from me to say yes a bit too easily at first I admit and it's making more difficult for users to "lose" some of them;
  • a lot of options are not that used indeed, making the maintenance cost not worth it;
  • a lot of bugs arise in particular combinations of options, that do not play well together. This forced us to sometimes define all combinations of some option combinations in big nested pattern-matchings as you may imagine, once again the resulting code is not pretty. There are too many options/values of options making it impossible to have tests for every combination so of course a lot of bad interactions are not foreseen and tested;
  • the goal of ocamlformat is not to be customizable to the extent of reproducing every style we may encounter, we would like to provide common styles among the ocaml community, we know there is not a single style (I wish) and that's why we offer multiple profiles. This point makes it similar to auto code formatters available in other languages, like rustfmt, hfmt (and probably more).

To fix most of these problems we are heading towards a different direction (not official yet, hence why the current options are not yet removed) which is to only allow ocamlformat to be customized by profiles, that would all have their own specificities:

  • default/conventional: a very common style, mostly driven by project of the ocaml platform and the mirage ecosystem;
  • ocamlformat: having keywords/symbols/key elements of the language on the left margin to avoid having to look up until the end of the line to have an idea of the structure of the code;
  • janestreet: playing well with ocp-indent.

Personally I would like to add a profile that is diff-friendly, something always expands the code vertically to minimize the number of impacted lines when bits are added/removed, in the spirit of indicate-multiline-delimiters = closing-on-separate-line.
Instead of having to test and maintain a combination of every value of every option we would only have a small number of profiles to maintain/test/make consistent. Options (current ones) would just be an implementation detail.

Also see #1923

@vphantom
Copy link
Author

Thank you. I'll close this then. Please consider waiting until there's a viable alternative before removing indicate-multiline-delimiters outright.

@aantron
Copy link

aantron commented Feb 16, 2022

I second the request to wait on removing these options. Though I haven't looked at the profiles in detail recently, in the past the profiles were not diff-friendly. I mostly use a diff-friendly style, and I'd like to avoid these removals impacting that too greatly.

To give a concrete example, I prefer for the if and else branch expressions to be indented the same and written on separate lines, in case the if-then-else will need to be inverted or become a match, or the cases are expanded. if-then-else: fit-or-vertical satisfies this:

let () =
  if true then
    ()
  else
    ()

Of the remaining non-deprecated options, compact and keyword-first produce diff-unfriendly output:

let () = if true then () else ()

(I separately also almost never want this output for legibility reasons, regardless of diffs).

This output remains diff-unfriendly if one of the cases gets longer, but not the other, because they will try to do

if true then
  ...
else ()    (* diff entanglement between else keyword and () *)

I'd be happy to contribute to a profile, or contribute a profile, once ocamlformat makes the switch.

aantron added a commit to aantron/dream that referenced this issue Feb 16, 2022
Dream was using some of the removed options based on misconceptions
about what they did. Others seem to have good substitutes among the
remaining options.

For if-then-else = fit-or-vertical, see

  ocaml-ppx/ocamlformat#1997 (comment)
@gpetiot
Copy link
Collaborator

gpetiot commented Feb 23, 2022

I opened #2020, don't hesitate to give some feedback if you have time to give it a try!

@c-cube
Copy link

c-cube commented Mar 1, 2022

Please preserve at least the k-r option for if-then-else!

Also, earlier:

This point makes it similar to auto code formatters available in other languages, like rustfmt, hfmt (and probably more).

This ship sailed 20 years ago. It'd be nice to have one universal style but it will never happen (or would you consider removing the janestreet style, for example? I don't think so). This kind of thing needs to be there from day 1, not year 25, it's not a realistic objective in 2022.

@bluddy
Copy link

bluddy commented Mar 2, 2022

I think if you're going to be moving in the direction of profiles, it probably makes sense to add the profiles of prolific community members such as @c-cube and @antron as well. Look at the community members who wrote a lot of code on opam and give them a chance to come up with their own profile.

Ultimately you're probably going to have to factor out the common patterns between these profiles anyway, so that you'll leave only the options these profiles use (similar to yapf for python). You can then have configurations such as "@c-cube + if-then=X", which is exactly what yapf does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants