← Back to team overview

yade-dev team mailing list archive

clang-format (Was: Re: Removing trailing white space, yay or nay?)

 

Dear all,

there are several pre-defined styles in the clang-formatter.
I have recursively run all of them in the current trunk and I controlled,
how many lines were changed (+ lines added, - lines removed)


Style     | +      | -       | Line length
--------------------------------
LLVM      | 95796  | 71398   | 80
Google    | 95748  | 72468   | 80
Chromium  | 104646 | 72689   | ?
Mozilla   | 109952 | 70635   | 80
WebKit    | 76487  | 69974 * | ?
Microsoft | 97828  | 73115   | ?

As you can see the minimal diff was produced by the
WebKit-style. It can be caused by the fact, that some
styles are changing the line length of the code. So,
this parameter is also in the table.

After that I have took the WebKit style and played with
the type of indents: spaces, tabs (Always and ForIndentation).

UseTab          | IndentWidth  | +      | -
------------------------------------------------
Never           | 1            | 77654  | 71141
Never           | 2            | 73701  | 67188
Never           | 4            | 76487  | 69974
Never           | 6            | 78023  | 71510
Never           | 8            | 77785  | 71272
ForIndentation  | 1            | 77624  | 71111
ForIndentation  | 2            | 73336  | 66823
ForIndentation  | 4            | 74727  | 68214
ForIndentation  | 6            | 77538  | 71025
ForIndentation  | 8            | 63022  | 56509 *
Always          | 1            | 77623  | 71110
Always          | 2            | 73340  | 66827
Always          | 4            | 74722  | 68209
Always          | 8            | 63013  | 56500

I think that "tabs" won. Also "always" looks like too
restrictive, so I have chosen this parameter with the
indentwidth 8 as the minimal invasive.

The last one is the maximal line length:

ColumnLimit   | +      | -
--------------------------------
80            | 99719  | 62788
100           | 83550  | 61566
120           | 75988  | 61015
140           | 71907  | 60655  *
160           | 69721  | 60506
180           | 68393  | 60430
200           | 67445  | 60374
220           | 66670  | 60316
240           | 66146  | 60304

Results are interesting. Clear is that we need to
cut over long lines. My proposal would be 140,
though most of guidelines use 80.

Opinions?

Regards

Anton

Am Fr., 18. Okt. 2019 um 15:30 Uhr schrieb Janek Kozicki (yade)
<jkozicki-yade@xxxxxxxxx>:
>
> It’s a great idea. And I’m surprised how personal it is for me :) Code readability is top priority for me. I’m sure we will find the clang-format settings which suits us.
>
> Automatic checks if the recently touched files (e.g files from the current MR) were reformatted will definitely help us. At first this will add some unreadable diffs. But later it will be a refreshing breeze :)
>
> -- Janek Kozicki
> On 17 Oct 2019, 18:59 +0200, Anton Gladky <gladky.anton@xxxxxxxxx>, wrote:
>
> Hi,
>
> I propose to use the clang-format [1], which automatically formats
> the code according to the pre-defined rules. It has a nice support
> by most of IDEs and can also be used through command line.
>
> There are some pre-defined styles (LLVM, Google, Chromium, Mozilla, WebKit),
> and i think we just need to choose. Fine-tuning is also possible, but
> it is not necessary.
>
> Running this tool recursively all over the code is possible, but can produce
> a large, not reviewable diff. But formatting the files, touched during
> development process can step-by-step fix most parts of the code and be
> consistent in the future.
>
> I have prepared a WIP-merge request [2], where the .clang-format file is added
> and the Scene.cpp was reformatted using this tool. So you can check,
> whether it will work for us. Style format can be changed, but it looks like
> LLVM-style will produce the minimal changes.
>
> Pipelines can also check, whether the committer formatted the code
> with clang-format. At the end we will forget the formatting problem in Yade.
>
> Generally, I have a good experience with this tool and I hope it can
> work for this project as well. Comments are welcome.
>
> [1] https://clang.llvm.org/docs/ClangFormat.html
> [2] https://gitlab.com/yade-dev/trunk/merge_requests/298
>
> Regards
>
>
> Anton
>
> _______________________________________________
> Mailing list: https://launchpad.net/~yade-dev
> Post to : yade-dev@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~yade-dev
> More help : https://help.launchpad.net/ListHelp
>
> _______________________________________________
> Mailing list: https://launchpad.net/~yade-dev
> Post to     : yade-dev@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~yade-dev
> More help   : https://help.launchpad.net/ListHelp


Follow ups

References