Format compiler sources with ocamlformat

[aspeddro]

This PR is a proposal to add ocamlformat on test group with-test and ocaml-lsp-server to the with-dev-setup group. A feature introduced in opam v2.2.0

Currently ocamlformat is a standard dependency and there are several .ocamlformat with disable which disables formatting. I actually don't know why.

One problem is synchronizing ocamlformat and ocaml-lsp-server since ocamlformat is a dependency on ocaml-lsp-server see rescript-lang/rescript-vscode#832. This generates some conflicts because in CONTRIBUTING.md it is instructed to install ocaml-lsp-server separately.

By marking ocamlformat(with-test) and ocaml-lsp-server with with-dev-setup we can synchronize boths.

Other changes

• I deleted all .ocamlformat with disable
• Added some files in .ocamlformat-ignore. This files has top level directive #if
• Formated files
• Added format check on CI

Now we can format on save.

[aspeddro]

js_parser should be ignored, right?

[cknitt]

Thanks a lot, @aspeddro!

Some remarks:

• Yes, js_parser is vendored and should therefore be ignored.
• The contents of the ml folder is mostly vendored from OCaml (although already heavily modified) and should probably be ignored, too, as far as I remember from previous discussions. @cristianoc right?
• Format check is actually already run here https://github.com/rescript-lang/rescript-compiler/blob/a8647fd229b8ddf9acda77a5aad4fb95168a1462/scripts/ciTest.js#L45

[cristianoc]

Thanks a lot, @aspeddro!

Some remarks:

• Yes, js_parser is vendored and should therefore be ignored.
• The contents of the ml folder is mostly vendored from OCaml (although already heavily modified) and should probably be ignored, too, as far as I remember from previous discussions. @cristianoc right?
• Format check is actually already run here https://github.com/rescript-lang/rescript-compiler/blob/a8647fd229b8ddf9acda77a5aad4fb95168a1462/scripts/ciTest.js#L45

Reformatting ml is good. It was postponed because some cppo preprocessing was not compatible.
The only thing is, this should be merged when there are hardly any outstanding PRs, or one will get horrible conflicts.

[cknitt]

Reformatting ml is good. It was postponed because some cppo preprocessing was not compatible.
The only thing is, this should be merged when there are hardly any outstanding PRs, or one will get horrible conflicts.

I would then suggest that we wait until 12.0-alpha.1 is out (including @cristianoc's latest uncurried cleanup work) and I have done the backports to the v11 branch. Then would be a good time to do the reformatting before starting the work on alpha 2.

Another thing: After this PR, ocaml-lsp-server will be installed in CI (unnecessarily). Not sure if that's much an issue though now that @cometkim has improved opam caching.

[aspeddro]

Ok, added jscom/js_parser/** in .ocamlformat-ignore and removed .{ml,mli} changes (easier to sync with master)

[aspeddro]

Another thing: After this PR, ocaml-lsp-server will be installed in CI (unnecessarily)

I think it might be useful in the process of updating ocamlformat and ocaml-lsp-server. To ensure that there is no conflict between the two.

[cometkim]

After this PR, ocaml-lsp-server will be installed in CI (unnecessarily).

A common strategy is to separate the dev-only and build-only workflows and run them simultaneously. As I mentioned in here

But it will duplicates cache, I think intalling deps including dev-setup is good enough for now

[cknitt]

• I deleted all .ocamlformat with disable

I don't see that in the current changes.

[aspeddro]

After this PR, ocaml-lsp-server will be installed in CI (unnecessarily)

I added ocamlformat to the test group (with-test). ocaml-lsp-server is no longer installed no CI

[aspeddro]

I don't see that in the current changes.

Updated

[aspeddro]

@cknitt, If everything is ok I can format the files to pass the test.

[cknitt]

Thanks a lot! Looks good to me.

[cknitt]

@cristianoc @zth Fine with you, too?

[tsnobip]

Great PR! I can't count how many times I had to struggle to sync ocaml lsp and ocamlformat when working on the compiler.