Skip to content

Conversation

jonludlam
Copy link
Member

No description provided.

@dbuenzli
Copy link
Contributor

Btw. @jonludlam would it please be possible to sync the .ocp-indent file in the repo with the style mandated by ocamlformat ? They don't seem to agree which is quite annoying.

@jonludlam
Copy link
Member Author

Sounds perfectly reasonable to me. I don't currently use ocp-indent, but I see we've got a customised .ocp-indent file. Given we're using ocamlformat's default configuration, I wonder whether using ocp-indent's defaults work better. If you remove the .ocp-indent file altogether does it work better?

@jonludlam jonludlam merged commit b0c03d1 into ocaml:master Mar 11, 2021
@dbuenzli
Copy link
Contributor

Well I don't know exactly how ocamlformat works (and I'm not really interested in a tool that doesn't respect logical groupings it cannot know about). But it seems there is some kind of ocp-indent compatbility flag maybe you should invoke ocamlformat with that.

@jonludlam
Copy link
Member Author

I gave that a quick try, and the results weren't spectacular - in the sense that while it did reformat a load of files, a subsequent run of ocp-indent was a long way from a no-op.

@jonludlam
Copy link
Member Author

the ocamlformat parameter change caused:

 57 files changed, 1285 insertions(+), 1016 deletions(-)

whereas the subsequent ocp-indent caused:

81 files changed, 8946 insertions(+), 8946 deletions(-)

@dbuenzli
Copy link
Contributor

Well forget about that then.

@dbuenzli
Copy link
Contributor

But then if ocamlformat is supposed to be the truth and the .ocp-indent file is wrong then the latter should be removed.

I wouldn't mind tweaking my .emacs to use ocamlformat but I'm sure it's going to take me hours to get what I want – i.e. use ocp-indent on sources dominated by a .ocp-indent and ocamlformat on sources dominated by .ocamlformat. If someone already has the runes please post them somewhere :-)

@dbuenzli
Copy link
Contributor

Btw. the docs of this flag seems to document the map between ocamlformat options and .ocp-indent's ones.

@Julow
Copy link
Collaborator

Julow commented Mar 12, 2021

We already reformatted the whole project with OCamlformat without respecting ocp-indent. The simpler way to make them agree now is to change the ocp-indent config.

@dbuenzli
Copy link
Contributor

Indeed that's what I suggested :-)

@dbuenzli
Copy link
Contributor

But I'm still curious about ocamlformat, can it actually stand as an indentation engine in emacs ? Their docs seem to rather indicate that you should invoke it on save (much like you do with merlin). That's fine but not really helpful when you are coding.

@Julow
Copy link
Collaborator

Julow commented Mar 12, 2021

OCamlformat formats the whole file. It can't work as an indentation engine because it doesn't try to patch its input but just replaces it.
Personally, I use ocp-indent but I'm fine with it contraticting OCamlformat because I no longer pay attention to formatting while I type (I let the computer do it later).

@Julow
Copy link
Collaborator

Julow commented Mar 12, 2021

I fixed the match clause indentation but for other cases it'll still conflict: #637 (review)
Other cases are less frequent because they happen when writing code from scratch (eg. less frequent when inserting a line), I'd expect OCamlformat to rewrite it entirely.

@dbuenzli
Copy link
Contributor

It can't work as an indentation engine because it doesn't try to patch its input but just replaces it.

Ah thanks, I didn't get that.

Personally, I use ocp-indent but I'm fine with it contraticting OCamlformat because I no longer pay attention to formatting while I type (I let the computer do it later).

There are two issues with this:

  1. I know computer people like to defer everything to computers but I personally do think that code presentation is very important in practice and that automated formatting does have its limits. So I'd like a reasonable preview of what is going to happen and at least I'd like ocp-indent not too lie to much (e.g. about something trivial as identation levels).
  2. That's one more step you have to do before doing a PR and it's quite annoying to make your PR and then have the CI complain about the formatting.

Altogether from a UX coding point of view I find the current status not to be that great. I don't mind having more rigid structured code editing but I have the feeling that it's not the ocp-indent/ocamlformat that is going to bring us that. Anyways.

I fixed the match clause indentation

Thanks for that !

@dbuenzli
Copy link
Contributor

Another issue I have right now is that ocamlformat depends on odoc and that odoc master blows it which makes it a pain for pin testing. I guess this will be solved once the comment parser is factored out (though that won't lessen the pains believe me). Could we stop having recursive dependencies in the dev tools ?

@Julow
Copy link
Collaborator

Julow commented Mar 12, 2021

The plan is to extract the parser to its own opam package (still in Odoc's repository). That should be enough.

@dbuenzli
Copy link
Contributor

The plan is to extract the parser to its own opam package (still in Odoc's repository).

Yeah I know, it's not the first time this project runs in circles, we already had that structure a few years ago which was not a great idea either.

That should be enough.

Well the problem will remain the same. If you break the parser for odoc then you won't be able to PR odoc until you have fixed ocamlformat.

@dbuenzli
Copy link
Contributor

Well the problem will remain the same. If you break the parser for odoc then you won't be able to PR odoc until you have fixed ocamlformat.

Btw. one way out would be to make odoc not depend on the parser package, i.e. use it as a vendored dep.

For me it's quite important to be able to change odoc, pin it in opam (because I use to test odig which documents opam switches) and be able to PR to the odoc repo – which means I need a functioning ocamlformat regardless of the changes that happened to odoc.

@Julow
Copy link
Collaborator

Julow commented Mar 17, 2021

This is a known problem but it has been decided (not by me) that constraining which dependencies OCamlformat can have is not the right way of solving this. This sounds a bit like saying "it's not our problem" without proposing a workaround but I believe the outcome will be better if this is solved at the distribution level (eg. OS package managers, a specific mode of Opam).

Personally I use Nix to manage ocamlformat and other tools. I'd suggest making an opam switch dedicated to tools that doesn't depend on ocaml's version (ocamlformat, ocp-indent but not merlin) and calling them like this: opam exec --switch=tools -- ocamlformat .... I know this is not a good solution for the long term but it would solve your problem right now.

@dbuenzli
Copy link
Contributor

This is a known problem but it has been decided (not by me) that constraining which dependencies OCamlformat can have is not the right way of solving this.

I don't understand, that's not what I suggested.

I suggested ocamlformat uses the comment parser library but odoc does not, odoc uses it as a vendored lib (that's easy to do since it's going to stay in the repo).

That solve the problem without having to become utterly bureaucratic about all this and having to babysit multiple opam switches/nix setups or what not just to be able to contribute to odoc.

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.

3 participants