lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: automated formatting


From: Carl Sorensen
Subject: Re: automated formatting
Date: Tue, 28 Jan 2020 17:11:27 +0000
User-agent: Microsoft-MacOutlook/10.10.10.191111


On 1/28/20, 1:50 AM, "lilypond-devel on behalf of Han-Wen Nienhuys" 
<lilypond-devel-bounces+c_sorensen=address@hidden on behalf of address@hidden> 
wrote:

    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.
    > >
<snip>
    > >
    > >> * 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++.
    >
<snip>
    >
    > 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.

I admit that when I saw a request for clang as a requirement for development I 
was somewhat dismayed.  Like David, I felt like we had a workable tool, and we 
could stick with it.

HOWEVER, if moving to clang-format is a price we have to pay to have Han-Wen 
contributing his expertise to the project, I would willingly pay it. 

UNLESS moving to clang-format would drive David K away from contributing his 
expertise to the project.

I really hope we can find a solution that will satisfy both David and Han-Wen, 
because I think that both of them are instrumental to the health of LilyPond.  
And I love having LilyPond available!
    
    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.
    
    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 agree that completely automatic formatting is a worthy goal.  I think we 
should have it.   If we can only have it with clang-format, then we should 
probably use clang-format.  If we can have it with both clang-format and fixcc, 
then the decision between the two tools needs to be based on criteria other 
than completely automatic formatting.

    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`

The problem you have with fixcc.py is similar to the problem I would have with 
clang-format.  It's not set up on my system yet in a way that I can run.  And 
I'd need to make some changes.  As I said above, I'd be willing to do that.
    
    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.

If there is agreement on basic goals, then I'm fully confident that an 
agreement on the means to achieve the goals can be reached.

Thanks,

Carl
 


reply via email to

[Prev in Thread] Current Thread [Next in Thread]