lua-ml icon indicating copy to clipboard operation
lua-ml copied to clipboard

Use OCamlformat?

Open lindig opened this issue 4 years ago • 3 comments

Ho do you feel about re-indenting all code using OCamlformat and introducing a make format targe for this? I'm using it now on all my current projects and it has changed my opinion about indentation. Yes, you can emphasize meaning by using alignment that an automatic tool can't create but I think the advantages of consistent indentation outweigh this. In particular tricky cases (if vs. match) where indentation can become misleading after changes.

lindig avatar Oct 05 '21 09:10 lindig

My main concern about formatting tools in established projects is the loss of revision history. It becomes impossible to trace a line to a specific commit using git blame if the last non-cosmetic change was before the introduction of the formatter.

So I wonder if history loss would be an acceptable price to pay for consistent indentation. There are also problems other than indentation now, such as chunks of commented-out code. Ideally "all good people of the world" should give that codebase a good review and clean-up.

If we can get anyone else involved in that, it may be a good idea to run the code through the formatter before the clean-up project begins because so far most changes are attributed to either you, me, or two more people (@Drup and @Leonidas-from-XIV).

dmbaturin avatar Oct 21 '21 05:10 dmbaturin

Dune supports reformatting out of the box with dune build @fmt there is not much that needs to be done to set it up. I myself have become a big fan of automatic formatting to the point where I usually don't even try to write sensible code, I just write whatever and let the formatter handle it looking consistent.

That said, I understand @dmbaturin's concern. There is no good solution for this, but as a stopgap it is possible to ignore certail revs for git blame. The downside is, that it doesn't automatically work on remote repos.

Leonidas-from-XIV avatar Oct 21 '21 08:10 Leonidas-from-XIV

Thanks for the comments. I understand @dmbaturin's concern as well. Given that the code doesn't see a lot of changes, I would argue that it is a good moment to do this, hoping to benefit from it in the future by making changes easier. Is git blame right now a frequent use case? I would doubt it and would still be in favour as another step in modernising the code base.

lindig avatar Oct 22 '21 18:10 lindig