← Back to team overview

yade-dev team mailing list archive

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

 

Hi Anton,
It's not yet all clear to me how it will work.
What would be the workflow in general for, let's say, a kdevelop/kate user?
Edit, then "git-clang-format" before commit?

I'm particularly curious about your statement in the giltab thread [1]:
"Reformatting everything is not the best idea. It will change everything
and can hurt the history. I would propose to do it step by step, only by
changing the files."

Do you suggest that each time a file will be modified it should be also
re-formatted, but no systematic reformatting would be done beyond that?
Isn't it maximizing the risk of mixing real changes with formatting? I
would think reformatting everything in one single commit would make the
history much more clear.

Last thing I'm thinking about: how will we avoid that different authors
with (even slightly) different editing tools and auto-formating settings
end up in committing conflicting auto-formats?

Bruno

[1] https://gitlab.com/yade-dev/trunk/merge_requests/298#note_233001456



On Sun, 20 Oct 2019 at 21:47, Anton Gladky <gladky.anton@xxxxxxxxx> wrote:

> 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
>
> _______________________________________________
> 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
>


-- 
-- 
_______________
Bruno Chareyre
Associate Professor
ENSE³ - Grenoble INP
Lab. 3SR
BP 53
38041 Grenoble cedex 9
Tél : +33 4 56 52 86 21
________________

Email too brief?
Here's why: email charter
<https://marcuselliott.co.uk/wp-content/uploads/2017/04/emailCharter.jpg>

Follow ups

References