[PATCH 1.12] A coding style for the server

Daniel Stone daniel at fooishbar.org
Wed Jan 18 20:49:06 PST 2012


Hi,

On 19 January 2012 01:11, Trevor Woerner <twoerner at gmail.com> wrote:
> On Tue, Jan 17, 2012 at 10:02 PM, Daniel Stone <daniel at fooishbar.org> wrote:
>> indent -linux -bad -bap -blf -bli0 -br -brs -cbi0 -cdw -nce -cs -i4 -hnl
>> -l80 -lc80 -lp -nbbo -nbc -nbfda -nprs -npcs -npsl -saf -sai -saw -nut
>
> About a dozen of the options you specify are already included in the
> -linux option. As such your list only needs to include:
>
> -linux -bad -blf -bli0 -cbi0 -cdw -nce -cs -i4 -lc80 -nbbo -nbc -nbfda -nut
>
> In other words the following are already included in -linux:
>
> -bap -br -brs -hnl -l80 -lp -nprs -npcs -npsl -saf -sai -saw

Good looking out, thanks.

> From reading the man page it appears the -nbbo (no break after
> boolean) option conflicts with the -hnl (honour newlines) option such
> that -hnl cancels -nbbo, therefore -nbbo is ignored.

Ah! I think just removing -hnl and retaining -nbbo is the right thing
to do here.

> Personally I don't like it when a developer submits a patch that
> intermixes code changes with formatting changes. As such I'm not fond
> of the -lp (continue at parenthesis) option because it has the
> side-effect of encouraging this behaviour. If, for example, a given
> function has 5 arguments and the code is refactored to change the name
> of the function such that the new name is a different length from the
> previous name, the patch will then contain the changed line plus 4
> lines of formatting changes to keep the arguments "lined up".

Nod, it is unfortunate.  I don't think it's too much of a problem
though, as if you're changing the names of several functions in one
go, you should perhaps take it as a hint to further split your patch.
:)

> You mentioned that cuddling the else slightly outnumbered not cuddling
> (was there a vote that I missed, or are you basing this on an
> examination of the existing code?). However you then provide the -nce
> (don't cuddle else) option. Personally I prefer -nce, but if you'd
> like to go with the majority then -ce should be used instead.

Actually, non-cuddling else was slightly in the majority -- this was
just based on a quick couple of greps, as was the decision to cuddle
if/for/while statements with their opening greps.  For full
disclosure, I prefer it this way, but was pleasantly surprised to see
the numbers agree with my preference.  If the numbers didn't agree, I
would've sucked it up and gone the other way.

Cheers,
Daniel


More information about the xorg-devel mailing list