Re: [PATCH v4 01/12] Add .clang-format with defaults matching what's specified in the style guide

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

 




> On 16 Feb 2018, at 10:07, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> 
> On Thu, Feb 15, 2018 at 05:06:14PM +0100, Christophe de Dinechin wrote:
>> From: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>> 
>> There are two use cases for this .clang-format:
>> 
>> 1. Local reformatting of source code, e.g. using Emacs clang-format.el package.
>>   This is basically a wawy to accelerate routine reformatting actions during
> 
> 'way'
> 
>>   heavy editing or refactoring.
>> 
>> 2. Global reformatting of the source code, e.g. using the 'make reformat'
>>   rule in the 'build' makefiles.
>> 
>> Presently, our source code is not entirely ready for this kind of
>> refactoring yet, mostly because several of our headers are not
>> self-contained, so you end up with errors like:
>> 
>>    In file included from agent-msg-filter.c:25:
>>    In file included from ./red-common.h:37:
>>    In file included from ./spice.h:27:
>>    ./spice-migration.h:47:31: error: unknown type name 'SpiceServer'
> 
> My understanding was also that this .clang-format file got the
> formatting wrong in some cases when applied over the codebase?

I did improve the settings, see below “some setting changes”.

Frediano’s comment was based on a push where I had not yet noticed that

1. LLVM defaults to aligning decls, so I added AlignConsecutiveAssignments: false and AlignConsecutiveDeclarations: false.
2. The ‘Linux’ brace alignment was not correct for structs, which prompted me to change it that setting to ‘Mozilla'

The result after these changes is here: https://gitlab.com/c3d/spice-server/commit/19e7c5bb97b0ce9d489c16229ab9caf39080145d

There are still issues, but they seem to be more related to:

- Misinterpretation of some macro code, e.g. https://gitlab.com/c3d/spice-server/commit/19e7c5bb97b0ce9d489c16229ab9caf39080145d#c75ac8e2c001e819bae810b126f19ba02f1bac2a_35_35

- Special manual formatting, e.g. https://gitlab.com/c3d/spice-server/commit/19e7c5bb97b0ce9d489c16229ab9caf39080145d#1f4f5fab2bf5fd2a2296b15eba820fc32802b59d_239_235. Those can be protected with a "// clang-format off” comment.

- The return type on a separate line is systematically removed. I don’t know if we want that or not. We could set AlwaysBreakAfterDefinitionReturnType: TopLevel.

For the rest, I think it’s doing a rather good job, and there are many places where it markedly improves things.


> 
> 
> 
>> Changes since v3:
>> - Documented rationale and use cases in commit log
>> - Some setting changes to more closely match existing server practice
> 
> NB: This part belongs below the -- line as this usually should not be pushed
> to the upstream repository

Ah, yes. Thanks.

> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]