kicad-developers team mailing list archive

[John Beard <john.j.beard@xxxxxxxxx>]

Hi,

Indentation is not defined well in the coding style documentation
(actually, lots of things are not mentioned in the document). Thus, by
the rules of thermodynamics, KiCad has a healthy mix of different
styles.

The clang-fomat file specifies the following:

* IndentWidth: 4 # This is the "block indent"
* ContinuationIndentWidth: 8 # This is the indent when continuing a
line, as you found
* AlignAfterOpenBracket: DontAlign # indent following lines of a
function call using the ContinuationIndentWidth

So you get:

someLongFunction( argument1,
        argument2 );

If we change AlignAfterOpenBracket to Align, this will produce:

someLongFunction( argument1,
                  argument2 );

If this is "correct" KiCad style, we should change clang-format.

WRT to template spacing inside < >, again the policy says nothing.
Clang format uses SpacesInAngles: false, which gives <FOO> rather than
< Foo >. Notably, no-spaces appears to outnumber with-spaces roughly
3.5-1 in extant code.

So both these issues are a mismatch of clang-format and "the right
style". Or vice versa.

One thing that you absolutely should be careful of with clang-format
is auto-generated files (like wxFormBuilder and CPP files). These
files should not be auto-formatted. I plan to submit a change to
disable checks on these files (it's been on my stack for some time!).

Cheers,

John

On Wed, Mar 20, 2019 at 4:56 PM Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>
> Brian,
>
> On 3/20/2019 12:46 PM, Brian wrote:
> > I'm trying to get better at not breaking the KiCad style guidelines, but
> > then I see the clang-format tool make suggestions like this:
> >
> > - static LIB_TEXT* loadText( std::unique_ptr< LIB_PART >& aPart,
> > LINE_READER& aReader,
> > - int aMajorVersion, int aMinorVersion );
> > - static LIB_RECTANGLE* loadRectangle( std::unique_ptr< LIB_PART >& aPart,
> > - LINE_READER& aReader );
> > + static LIB_TEXT* loadText( std::unique_ptr<LIB_PART>& aPart,
> > LINE_READER& aReader,
> > + int aMajorVersion, int aMinorVersion );
> > + static LIB_RECTANGLE* loadRectangle( std::unique_ptr<LIB_PART>& aPart,
> > LINE_READER& aReader );
> >
> > This doesn't seem right to me, in particular pulling the second line of
> > the loadText definition leftward and removing the spaces around template
> > parameters.  It makes me reluctant to blindly do a `git clang-format`
> > before a commit.
> >
> > Comments / advice?
>
> This is why I didn't allow this to be always hooked when making commits.
>  I don't trust them to do exactly what I want.  Technically we do not
> define this indentation.  It can be either indented one level (4 spaces)
> or aligned with the first parameter.  I prefer the latter.  In either
> case, this indentation doesn't appear to be correct.  I don't pretend to
> be a clang-format expert but I'm guessing this can be fixed with the
> appropriate setting.  Your best bet is to configure your editor to
> perform the code formatting to match the kicad coding policy.
>
> Cheers,
>
> Wayne
>
> >
> > Thanks,
> > -Brian
> >
> >
> >
> > _______________________________________________
> > 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