← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Add clang-format configuration

 

Hi Chris,

This is definitely loads better than uncrustify. I think it would be
great if we can get a really solid config here that is officially
accepted as "the" reference format, especially as it integrates well
with editors (unlike uncrustify). I find it very hard to spot all
formatting mistakes I make, especially as KiCad is already unique in
format but not always consistent: when everything kind of stands out,
nothing does, if that makes sense.

The official KiCad style guide is very short on actual details about
formatting. Specifically, indentation of continued lines is something
I never know how to do:

sometype somevar = somefunction( foobarbarfooquuxbar,
        foobarbarfooquuxbar().GetBar() );

or (as hinted in the Strings section of the style guide):

sometype somevar = somefunction( foobarbarfooquuxbar,

foobarbarfooquuxbar().GetBar() );

Also, constructor parameter/init lists:

FOOBAR_CLASS::FOOBAR_CLASS( foobarbarfooquuxbar,
        foobarbarfooquuxbar ) : // what indent here, space before colon?
                BASE_CLASS( 0 ), // and what indent here?
                m_member( 0 )
{}

As for items in the _format-clang file that Chris submitted:

* I personally quite like the BinPackArguments: false style (each
argument on new line) but that doesn't seem to be the normal style,
and it does take quite a bit of space.
* ContinuationIndentWidth: 4 seems a bit shallow (assuming this is how
we want to indent, see above).

It also doesn't seem to format things like this in "table" style for
the second parameter:

    { KIGFX::OPENGL_ANTIALIASING_MODE::NONE, 0 }, // Default
    { KIGFX::OPENGL_ANTIALIASING_MODE::SUBSAMPLE_HIGH, 1 },

Can we live without this for the sake of having a proper way to check
formatting? You can still add "// clang-format off" if it's critical.

Cheers,

John


On Mon, Mar 13, 2017 at 9:48 PM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
> I'm OK with this.  If clang handles indenting the _() macro properly,
> that is definitely an improvement over uncrustify.
>
> On 3/13/2017 9:42 AM, Chris Pavlina wrote:
>> Hi,
>>
>> Currently, our preferred code formatting tool is Uncrustify. Uncrustify
>> is bad. It does things like this:
>>
>> wxString m_choiceAntialiasingChoices[] =
>>         {
>>             _( "No Antialiasing" ),
>>             _( "Subpixel Antialiasing (High Quality)" ),      _(
>>                     "Subpixel Antialiasing (Ultra Quality)" ),_( "Supersampling (2x)" ), _(
>>                     "Supersampling (4x)" )
>>         };
>>
>> Yes, that's an actual Uncrustify output with our configuration, posted
>> on IRC this morning.
>>
>> Per metacollin's suggestion I looked into clang-format, which does a
>> MUCH nicer job because it uses clang's parser to actually understand the
>> code it's formatting. I wrote up a clang-format config following the
>> KiCad coding style. Here is the output of clang-format on the same code:
>>
>> wxString m_choiceAntialiasingChoices[] = { _( "No Antialiasing" ),
>>     _( "Subpixel Antialiasing (High Quality)" ),
>>     _( "Subpixel Antialiasing (Ultra Quality)" ),
>>     _( "Supersampling (2x)" ),
>>     _( "Supersampling (4x)" ) };
>>
>> It's not perfect, but it's way better than Uncrustify.
>>
>> The attached patch adds a _clang-format configuration file to the root
>> of the repository.
>>
>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp


Follow ups

References