Re: automated formatting

[David Kastrup]

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.

-- 
David Kastrup