yade-dev team mailing list archive
-
yade-dev team
-
Mailing list archive
-
Message #14878
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
-
Removing trailing white space, yay or nay?
From: Robert Caulk, 2019-10-17
-
Re: Removing trailing white space, yay or nay?
From: Janek Kozicki (yade), 2019-10-17
-
Re: Removing trailing white space, yay or nay?
From: Jerome Duriez, 2019-10-17
-
Re: Removing trailing white space, yay or nay?
From: Anton Gladky, 2019-10-17
-
Re: Removing trailing white space, yay or nay?
From: Janek Kozicki (yade), 2019-10-18
-
clang-format (Was: Re: Removing trailing white space, yay or nay?)
From: Anton Gladky, 2019-10-20