Re: automated formatting

[Han-Wen Nienhuys]

On Tue, Jan 28, 2020 at 8:08 PM David Kastrup <address@hidden> wrote:
> >>    Can you provide me with a presubmit hook that applies fixcc? Can you
> >>    guarantee that, if fixcc has run on the code, there will be no further
> >>    remarks on code formatting during review?
> >>
> >> I think that it's a really good idea to have presubmit hooks.  I
> >> believe, but can't guarantee, that if all code were automatically
> >> reformatted on submission (by either fixcc or clang-format) there
> >> would be virtually no comments on formatting.  And you could
> >> completely ignore any that were made.
> >
> > I'm all for making it possible for a contributor to set up something
> > like this if he pleases, but I think it's a bad idea to rely on this
> > level of automation and integration with a specific source-control
> > tool.

Sorry, I was unclear. I mean: a local precommit hook (which wouldn't
be mandatory to install)

> To clarify: before you joined the project, there were recurrent
> reformatting passes preferably at "quiescent" times to bring consistent
> style into the code base.  There were large discussions about it but it
> was sort-of agreed that having a uniform style presented to people
> reading the code was a reasonably good idea and that the tool chosen and
> maintained did not cause noticeable damage (formatting of scm was a more
> tricky proposition and the respective script available by now just
> defers to Emacs for doing it).


I ran some of the discussion points by the authors of clang-format
(they work in the Google Munich office.)

Their suggestions:

* formatting can and will change subtly across versions. Difference
will be relatively small, because large users (eg. google) have an
auto-formatted code base, and don't want needless churn on their code
base.

* If there are doubts, it's best to recommend the "diff" option
instead 
(https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting),
which reformats diffs, instead of whole files.

* we could introduce an automated check that verifies the diff against
formatting problems. Either as part of the build, in git-cl, or as
suggested pre-commit hook in the docs.

* statically linked binaries of clang-format are available here:
https://github.com/angular/clang-format/tree/master/bin ; we could
standardize on whatever the Angular project recommends (which is v11,
at the moment.)

> Basically, no real great solution offered itself then (and I don't see
> that we can do this significantly better now, even while the particular
> tool to use may be subject to debate) and the adapted solution was to
> encourage using the fixcc scripts on one's own contribution and
> regularly apply it to the repository at reasonable points of time.
>
> I am not sure how far this policy dates back, but while Graham was
> project leader, it was done regularly.  There was no intent by me not
> following this practice: I simply dropped the ball.
>
> I'd be willing to pick up on it since the adopted compromise was sort of
> agreed upon by the active developers.
>
> The set of active developers has changed by now, and of course we also
> try angling for the mystic developer just missing one particular
> tool/workflow in order to become productive or productive again.  There
> is little enough sense in turning this into a showdown rather than an
> attempt of finding a consensus everyone can find themselves able to live
> with.  I'll do (respectively already have done in a few issues) what I
> feel brings the comparatively simple fixcc tool back up to scratch
> (fittingly for my non-use of it, its --sloppy option was broken) and
> then hopefully we'll be in a better position of deciding how to
> continue.

So here is my list of desires, priority from most to least important.
I'd be happy with 1), but I think going to 2) or even 3) would be good
for the LilyPond project as a whole.

1. Recognize clang-format as a "good enough", so I can use it locally
on my diffs and not get reviews on style.

2. Recognize clang-format as recommended, and document how to set it
up for contributors

3. Recognize clang-format as standard, and setup a formatting test on
diffs in the Makefile.

4. Recognize a certain version of clang-format as standard, and setup
a formatting test on the source tree in the Makefile. Users can setup
integration with emacs, vim, CLion, Eclipse etc.

-- 
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen