Re: automated formatting

[Han-Wen Nienhuys]

On Tue, Jan 28, 2020 at 12:18 AM David Kastrup <address@hidden> wrote:
>
> David Kastrup <address@hidden> writes:
>
> > Han-Wen Nienhuys <address@hidden> writes:
> >
> >> On Mon, Jan 27, 2020 at 11:49 AM David Kastrup <address@hidden> wrote:
> >>> > I want to propose to move to automated formatting for our C++ code.
> >>> >
> >>> > I put up a .clang-format code that mostly mimicks our style at
> >>> >
> >>> >   https://codereview.appspot.com/561340043
> >>> >
> >>> > I have a lot of good experience with automating code formatting.. It
> >>> > removes drudgery for code authors, obviates discussions over style in
> >>> > code review, and generally elevates the level of discourse in our
> >>> > reviews.
> >>> >
> >>> > What do you all think?
> >>>
> >>> scripts/auxiliar/fixcc.py and astyle 2.04 is what we standardized on.
> >>
> >> Yes, I am proposing that we change and standardize on clang-format.
> >>
> >>> > The current config modifies about 11k lines, mostly because of
> >>> > different line breaks (necessary to keep the 80 column limit.)
> >>>
> >>> Any particular reason to change the automated style to a different one?
> >>
> >> Several reasons
> >>
> >> * clang-format is a *complete* formatter. It reformats files to a
> >> canonical formatting regardless of how badly you mangle the input.
> >
> > Do we want to mangle the input badly?
> >
> >> For example, if you introduce a comment line of 200 chars. In
> >> clang-format, this will be reformatted to the specified column limit.
> >> Astyle/fixcc doesn't know what to do with it.
> >
> > Comments often contain ASCII-art and formatted tables.  Formatting those
> > is not helpful.
>
> Here are examples:
>
>   /*
>    * this case (distant half collide),
>    *
>    *    |
>    *  x |
>    * | x
>    * |
>    *
>    * the noteheads may be closer than this case (close half collide)
>    *
>    *    |
>    *    |
>    *   x
>    *  x
>    * |
>    * |
>    *
>    */
>
>
>               /* TODO:
>
>               For some cases we should kern some more: when the
>               distance between the next or prev note is too large, we'd
>               get large white gaps, eg.
>
>               |
>               X|
>               |X  <- kern this.
>               |
>               X
>
>               */
>
>
> >
> >> * clang-format is based on clang itself, and has an understanding of
> >> the source code being formatted. This makes it better than fixcc which
> >> uses regular expressions (sic) to make sense of C++.
>
> For better or worse, we still do a lot with preprocessor macros that are
> applied in the manner of declarations and statements.  They are
> formatted in a superficial manner, reflecting the kind of construct they
> are mimicking (declarations, function calls) for the sake of human
> readers.  Reflecting the underlying C++ realities instead would be a
> distraction.  I don't want to diss Clang here: it is unlikely that it
> would actually dig down to the C++ level for analysing these constructs
> instead of staying at the preprocessor level.  But what I am saying is
> that we usually want to make code readable to _humans_ and that means
> patterning it in a manner where deep semantic analysis should not be a
> requirement.
>
> I understand that it was chiefly my neglect of our existing reformatting
> regime that caused you to look for a solution for a problem
> unnecessarily and invest time into it.  I apologize for that, and am
> willing to bear the cost in future cherry-picks into the 2.20 stable
> branch for reformatting both 2.20.0 and 2.21.0 before release.
>
> But the additional cost for changing to a completely different
> reformatting system just does not seem warranted as long as there are no
> obvious deficiencies to be seen with what we have now.

I think you are missing the larger point I have been trying to make.

In Salzburg, people asked "what scares contributors away", and "what
can we do to suck Han-Wen/Mike back into the project"?

The project cares about code formatting, as evidenced by fixcc.py.
However, having to deal with code formatting, both while writing code,
and during review, is an absolute turn-off for me. If you want to suck
me (and others) in, you have to make the code formatting process as
transparent and automatic as possible.

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?

The point with completely automatic formatting is that you can simply
add a rule to "make check" (or some sort of "check" if we were to use
github or gitlab) that prevents the code ever to stray from the
defined standard.

I know that Dan also has issues with the current code formatting process.

If you reject clang-format (on grounds that I don't agree with, BTW),
then can you come up with another solution that lets me (and other
contributors) stop worrying about formatting in some other way? Note
that I can't even run fixcc (see below).

BTW, If you want to peruse clang-format output, you can go to
https://review.gerrithub.io/c/hanwen/lilypond/+/483012. As you can see
in

https://review.gerrithub.io/c/hanwen/lilypond/+/483012/1/lily/note-collision.cc

the ASCII art is unscathed. However, the file shows several problems
that fixcc didn't bother to fix (note-collision.cc is not in your
fixcc demo change.)

My machine is a 2012 T420 (i5 2540M), which is considerably slower than yours

$ time python2 scripts/auxiliar/fixcc.py --sloppy $(git ls-files '*.cc' '*.hh')
..
getopt.GetoptError: option --sloppy not recognized

$ time python2 scripts/auxiliar/fixcc.py $(git ls-files '*.cc' '*.hh')
Artistic Style Version 3.1
Warning: try to use Artistic Style Version 2.04.
Please limit use of this version to files with changed code.
Too many files with this version.  See `astyle --help`

Would it help if chatted with you over the telephone? I'm not sure if
we are in agreement of the basic goals we are trying to achieve.

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