lilypond-devel
[Top][All Lists]
Advanced

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

GOP-PROP 3: C++ formatting


From: Graham Percival
Subject: GOP-PROP 3: C++ formatting
Date: Wed, 22 Jun 2011 00:30:53 +0100
User-agent: Mutt/1.5.20 (2009-06-14)

This one will be contentious.


** Proposal summary

Speaking academically, C++ code style is a "solved problem". Let’s
pick one of the existing solutions, and let a computer deal with
this. Humans should not waste their time, energy, and creativity
manually adding tabs or spaces to source code.

I see three main contenders:

    * emacs x.y.z with extra post-processing: our current
      “official” style with scripts/auxiliar/fixcc.py
    * astyle 2.0.2, possibly with extra post-processing
    * uncrustify 0.58 

I’m torn between emacs and astyle, but the choice of exact program
pales in comparison to simply having (and using!) an automatic C++
code formatter.


** Rationale

New contributors sometimes struggle to follow our indentation and
code style – this is especially difficult when parts of our
existing source code doesn’t have a consistent style. This is
problematic... we want new contributors to be struggling with the
lilypond architecture, not playing games in their text editors!

we’ve lost time, energy, and contributors because of this. Two bad
examples: (the second one hopefully hasn’t lost us a contributor –
but her patch was delayed by three weeks because of indentation
issues, and that can’t be very encouraging)
        
http://codereview.appspot.com/1724041/
http://codereview.appspot.com/4490045/

This is also the worst “administrative” time-wasters in recent
LilyPond history; we’ve had numerous discussions without actually
resolving anything. Previous discussions on tabs v. spaces include
the following:
        
http://thread.gmane.org/gmane.comp.gnu.lilypond.devel/22691
http://lists.gnu.org/archive/html/lilypond-devel/2009-04/msg00076.html

The vagueness and confusion in past discussions were not helped by
our existing code being inconsistent; for example indenting
namespaces.

flower/include/yaffut.h
flower/include/std-vector.h

Another problem is that the scripts/auxiliar/fixcc.py depends on
emacs, but emacs’ formatting changes between versions.


** Eliminate tabs

I’m going to make the bold step of assuming that we will eliminate
tabs in all C++ files. I personally like the idea of tabs, but
from an examination of source code styles (both official and
unofficial) in various projects, I must admit that this ship has
sailed. Too many programs/editors don’t support tabs. Too many
people find them confusing. Too many cut&paste jobs from html
results in spaces instead of tabs.
        

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Spaces_vs._Tabs
http://www.rosegardenmusic.com/wiki/dev:coding_style
http://techbase.kde.org/Policies/Kdelibs_Coding_Style

** Branches and patch sizes

http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=heads

I’ve prepared a few different versions of our source code for easy
examination and comparison. The base for all this work is
dev/gperciva, which has these two patches applied:

271K   0001-Whitespace-remove-trailing-spaces-in-.h-.cc.patch
1.7M   0002-Whitespace-tabs-8-spaces-in-.h-.cc.patch

This eliminates a bunch of "false positives", enabling us to
compare the real changes between the different source code
formatters.
fixcc

Our “official” but unsupported tool ‘scripts/auxiliar/fixcc.py’
(with emacs 23.1.1.) produces this: (after re-converting tabs back
to spaces)

789K   0001-fixcc.py-.h-.cc-followed-by-fixing-tabs.patch

astyle 2.02

astyle seems fairly stable and produces somewhat decent results.
http://astyle.sourceforge.net/

360K   0001-astyle-2.02-.h-.cc.patch

uncrustify 0.58

uncrustify is a relatively tool for indentation. It has tons of
configuration options (400 at last count).
http://uncrustify.sourceforge.net/

I’m not eager to start relying on a "pre-stable" tool, but I
figured it was worth spending an hour experimenting with options.
NB: uncrustify was only ran against lily/*.cc, not flower/ or
lily/include/

294K   0001-uncrustify-process-with-0.58.patch


** Detailed analysis of emacs+post vs. astyle

You can compare the results directly with this:
 
git diff origin/dev/gperciva-fixcc origin/dev/gperciva-astyle

At first glance, I prefer the "look" of the emacs +
post-processing instead of astyle, but most of the differences
look relatively easy to change with a similar "post-processing"
script. Maybe we could even send fixes updatream, with a view to
using plain astyle 2.0.3 when it’s out?

A few specific problems with astyle:

    * it doesn’t indent enum in gnu style
    * loses some nice comment formatting in
      ‘lily/axis-group-interface.cc’ ; we could replace some
      spaces with _ to indicate a space, but that’s a bit icky.
      Annoyingly, astyle doesn’t seem to have an "ignore
      multi-line comments" options. 


** Practical considerations

Not everybody has emacs installed, and the C++ formatting varies
between versions. That said, we could probably avoid that problem
by giving a custom .el file with the C++ formatting from whichever
version of emacs we prefer (23.1.1, 23.2, ...?)

uncrustify is unstable.

astyle seems fairly well-supported. For emacs users concerned
about integration, there’s a astyle-hooks.el:
        
http://astyle.sourceforge.net/scripts.html
http://stackoverflow.com/questions/801983/how-do-i-use-astyle-within-emacs

There’s also the consideration of how much energy we want to spend
arguing about this. My preference is just to say "screw it, let’s
just use fixcc.py strictly". I don’t like tying the code
formatting to a specific text editor (much less one whose
formatting changes with different versions!), but astyle and
uncrustify just don’t seem "there" yet.


** Implementation notes

No plan for how to do this yet; it will be a mess.

lilydev needs to have whatever tool we end up choosing. Patches
will break, unless somebody applies the patch to the un-indented
source code, then run the indentation tool, then make a new patch,
etc. Massive confusion.

Cheers,
- Graham



reply via email to

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