Re: [Development] clang-format

[Frederik Gladhorn]

Hi Mitch,

thanks for trying!

On mandag 14. oktober 2019 14:51:41 CEST Mitch Curtis wrote:
> As a side note, I installed it on Ubuntu 18.04.3 with:
> 
> sudo apt-get install clang-format
> 
> but there was no git-clang-format binary, even though
> https://packages.ubuntu.com/search?searchon=contents=git-clang-for
> mat=exactfilename=disco=any said there should be, so I just
> made a symbolic link to it:
 
> sudo ln -s /usr/bin/git-clang-format-6.0 /usr/bin/git-clang-format
> 
> Would be great to know if there's a proper way to do this that I'm missing.

git-clang-format should be shipped with clang-format, but of course each linux 
distro does it's own thing, potentially. On (K)ubuntu 19.04 I have:
dpkg -S /usr/bin/git-clang-format
clang-format: /usr/bin/git-clang-format

So there it simply ships with clang-format and things work out of the box.
On macOS it also seems to work after getting clang from homebrew.
And on Windows it may just be working... fingers crossed.

I hope it turns into something useful :)

Cheers,
Frederik


> 
> 
> > 
> > 
> > ___
> > Development mailing list
> > Development@qt-project.org
> > https://lists.qt-project.org/listinfo/development




___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development

Re: [Development] clang-format

[Mitch Curtis]

> -Original Message-
> From: Development  On Behalf Of
> Frederik Gladhorn
> Sent: Monday, 14 October 2019 1:46 PM
> To: development@qt-project.org
> Subject: [Development] clang-format
> 
> Hi,
> 
> after just a bit over a year, we have a hint of progress on the state of using
> clang-format.
> 
> If you use qt5.git, you'll get the git hook for it set up, to automatically 
> run
> clang-format on the lines you changed.
> To benefit today, get the dev branch of qt5 and run init-repository -f 
> --force-
> hooks.
> The commit hook runs and shows a diff of what should be changed, but
> doesn't add anything, so your changes should be safe from unwanted
> reformatting no matter what.
> If you think the changes make sense, just run "git clang-format" and all the
> things you have added to the staging area will be reformatted.
> 
> git add myfile # stage a file
> git commit # try to commit, clang-format thinks you need to reformat git
> clang-format # apply the reformatting to the index git diff # see what clang-
> format did
> 
> The hook script which is probably far from optimal (written in sh) is found in
> qt/qtrepotools. You can also just link https://code.qt.io/cgit/qt/
> qtrepotools.git/tree/git-hooks/clang-format-pre-commit as pre-commit into
> your .git/hooks directory and you get to enjoy it.
> 
> I bet we'll have some issues and it's not quite perfect, but hey, small
> progress.
> 
> Enjoy re-formatting and feel relaxed about it too ;)
> 
> Cheers,
> Frederik

For anyone curious about how it looks, here's a dummy test I did:

mitch@mitch-ubuntu-18:~/dev/qt5.13/qtquickcontrols2$ git commit
clang-format output:
diff --git a/tests/auto/qquickmenubar/tst_qquickmenubar.cpp 
b/tests/auto/qquickmenubar/tst_qquickmenubar.cpp
index c0d9f8ed..9a22d26f 100644
--- a/tests/auto/qquickmenubar/tst_qquickmenubar.cpp
+++ b/tests/auto/qquickmenubar/tst_qquickmenubar.cpp
@@ -543,8 +543,7 @@ void tst_qquickmenubar::addRemove()
 QCOMPARE(menuBar->count(), 1);
 QVERIFY(!menuBar->menuAt(1));
 QVERIFY(!menuBar->itemAt(1));
-QCoreApplication::sendPostedEvents(
-menu1.data(), QEvent::DeferredDelete);
+QCoreApplication::sendPostedEvents(menu1.data(), QEvent::DeferredDelete);
 QVERIFY(!menu1.isNull());
 QCoreApplication::sendPostedEvents(menuBarItem1, QEvent::DeferredDelete);
 QVERIFY(menuBarItem1.isNull());
clang-format detected change in format, please run:
  git clang-format
and amend the change.

As a side note, I installed it on Ubuntu 18.04.3 with:

sudo apt-get install clang-format

but there was no git-clang-format binary, even though 
https://packages.ubuntu.com/search?searchon=contents=git-clang-format=exactfilename=disco=any
 said there should be, so I just made a symbolic link to it:

sudo ln -s /usr/bin/git-clang-format-6.0 /usr/bin/git-clang-format

Would be great to know if there's a proper way to do this that I'm missing.

> 
> 
> ___
> Development mailing list
> Development@qt-project.org
> https://lists.qt-project.org/listinfo/development
___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development

[Development] clang-format

[Frederik Gladhorn]

Hi,

after just a bit over a year, we have a hint of progress on the state of using 
clang-format.

If you use qt5.git, you'll get the git hook for it set up, to automatically 
run clang-format on the lines you changed.
To benefit today, get the dev branch of qt5 and run init-repository -f --force-
hooks.
The commit hook runs and shows a diff of what should be changed, but doesn't 
add anything, so your changes should be safe from unwanted reformatting no 
matter what.
If you think the changes make sense, just run "git clang-format" and all the 
things you have added to the staging area will be reformatted.

git add myfile # stage a file
git commit # try to commit, clang-format thinks you need to reformat
git clang-format # apply the reformatting to the index
git diff # see what clang-format did

The hook script which is probably far from optimal (written in sh) is found in 
qt/qtrepotools. You can also just link https://code.qt.io/cgit/qt/
qtrepotools.git/tree/git-hooks/clang-format-pre-commit as pre-commit into your 
.git/hooks directory and you get to enjoy it.

I bet we'll have some issues and it's not quite perfect, but hey, small 
progress.

Enjoy re-formatting and feel relaxed about it too ;)

Cheers,
Frederik



___
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development

Re: [Development] clang-format

[resurrection]

Interesting discussion about using clang-format Qt wide. As someone who 
implemented something like that in a project (of smaller size than Qt, but not 
all that small) some points:
 
- clang-format virtually annihilates A LOT of manual work. It might not seem 
like much but I have measured it and even had to work on two projects 
simultaneously where only one had clang-format. It was staggering how much time 
is spent on formatting when done manually. And how much it annoys you since you 
KNOW it can be done for you, automatically and always correctly. It also speed 
things up because if you make a formatting mistake it will be corrected as soon 
as you save the file in Qt Creator. So you don't bother correcting it, you just 
go on typing or hit ctrl+s if it annoys you to see it. You will get seriously 
addicted to that. It is really quite liberating.
 
- I have read many concerns here about loss of immediate history, that 
clang-format does not get it always right etc. Well we had those too but one 
thing beats them all - free consistency. I could hardly believe it but even 
though I did not like all of the rules we put in it the fact that they were 
consistent everywhere made reading the code far easier than before. You could 
not be surprised, you knew where will everything be. Whether it was includes 
and their ordering (the epic battles about that in pre-clang-format era...), 
alignment, wrapping, you name it. Once you could rely on the fact that it will 
be the same everywhere you suddenly did not have to focus on formatting which 
(again surprisingly) took a lot of code review time for all parties.
 
- Continuous integration. I cannot stress this one enough. Make it a standard 
in the repo, make everyone enable it in Qt Creator, make a git hook and make it 
part of the build. Our CI failed when someone committed incorrectly formatted 
file telling him as much even before compilation or anything. It happened 
rarely as most people used the git hook but still.
 
- Most people will be sceptical until they start working with it. I guess this 
is a given for any new technology or approach. I know I was. But I am yet to 
come by a colleague that would complain about it or who would want to remove 
it. It might not be perfect but the benefits far outweigh any issues.
 
And on more practical note:
 
- We converted all sources at once and it has not caused any problems - it's 
just formatting anyhow.
- Some files (very few) turned out to be somewhat broken but the reason for that was that 
they were curiously formatted in the first place. We fixed those as they were 
encountered. It generated negligible amount of extra "work" (as mentioned 
hitting Enter for new line and saving file to reformat typically was the trick).
 
I became a huge fan of clang-format thanks to making my work a lot more 
pleasant and less tedious experience. It might not seem like much but it really 
makes a big difference in daily work. I really hope it will be adopted Qt-wide.
 
My 2 cents and I would gladly answer any questions about practicalities as 
someone who did pretty much all of it from converting all sources to 
implementing the git hook and CI stuff.
 
Best wishes,


Michael Vlach
Software Engineer
NCR

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Frederik Gladhorn]

On onsdag 20. juni 2018 13.01.10 CEST Tor Arne Vestbø wrote:
> On 20 Jun 2018, at 12:13, Lars Knoll  wrote:
> 
> > 
> > I can’t see how clang-format will make you jump through any sort of hoops.
> > Creator already has a hook for doing it on file saving time afaik,
> > git-clang-format will clean up your change from the command line.
> 
> Good point, I was imagining it used only to verify style, not to
> auto-format. Still, starting out with a few non-controversial rules would
> be a good thing.

For now we're trying to get the format definition file into qt5 at all.
Once we have a file we agree on (and I'd be happy to see it evolve, it's in 
git, so please just contribute everyone), we can decide how to use it.

I see several things that would make sense.
1) Make it easy to use locally. That is mostly the case, just run:
   $ git clang-format
   And the lines you changed will be reformatted.

2) It's relatively easy to have a commit hook that complains or runs clang-
format for you.

3) Make it part of the sanity bot with a nice message explaining that it's 
possible to use clang-format and ask contributors to use it (with neutral or 
-1 effect).

Cheers,
Frederik
 
> Tor Arne 
> 
> 
> 
> > 
> > Lars
> > 
> > 
> >> On 20 Jun 2018, at 11:52, Tor Arne Vestbø  wrote:
> >> 
> >> How about we also start with only one or two  obvious rules that everyone
> >> agrees on? I don’t want Qt development to turn into Python PEP8 type
> >> rigid rules that makes you jump through a million hoops. If the latter
> >> is the goal here then I’m against enforcing clang-format, and it should
> >> be implemented as a bot that just warns, similar to the current style
> >> bot.
 
> >> - Tor Arne
> >> 
> >> 
> >>> On 20 Jun 2018, at 11:21, André Pönitz  wrote:
> >>> 
> >>> 
>  On Wed, Jun 20, 2018 at 06:30:26AM +, Lars Knoll wrote:
>  
>  
>  
> > On 19 Jun 2018, at 18:19, Ville Voutilainen
> >  wrote:
> > 
> > On 19 June 2018 at 19:13, Philippe  wrote:
> > 
> >>> For the above reasons I'd lean towards not running it globally and
> >>> just using it on new changes.
> >> 
> >> 
> >> +1, based on my clang-format experience on a big application.
> >> 
> >> BTW, keep in mind that you can disable clang-format on code
> >> sections with:
> >> 
> >> // clang-format off // clang-format on
> > 
> > 
> > When I last experienced a large-scale clang-format reformat, it
> > really hurt development during the churn. We should somehow manage
> > to do it during a time when there aren't many pending patches in the
> > pipeline. I'm not concerned about git-blame; that has never been a
> > problem after reformats. However, I do not care about indentation
> > nor do I want to spend time on it either way, it has no actual
> > effect on readability and maintainability of code, and consistency
> > outside the file you're in has never mattered to me one bit.
> > 
> > IOW, I'm not opposed to reformats and auto-checking of clang-format
> > (or even hooking it), but I do not see it as a thing with all that
> > great return-of-investment.
>  
>  
>  It helps in that you do not need to point those things out in code
>  reviews, and that I (and others) won’t even create changes with wrong
>  formatting that I’ll need to fix up later on. It’s part of a larger
>  story, where I would like to get as much automatic checking of changes
>  done before humans start reviewing.
> >>> 
> >>> 
> >>> It's also a cultural thing.
> >>> 
> >>> Quite a few people seem to take less offense from a "Your formatting is
> >>> bad" when the comment comes from a bot than when it comes from a human.
> >>> 
> >>> 
> >>> 
>  One idea could be to introduce this incrementally. Let’s first start
>  off with enforcing it for new changes. Then we run it globally over
>  the code base shortly before Qt 6.0 is being released. At that time
>  merges shouldn’t be as much of a problem (as we’ll probably
>  cherry-pick into Qt 5.15) and by then all new changes in Gerrit will
>  be properly formatted (due to the earlier hook).
> >>> 
> >>> 
> >>> Incrementally sounds good to me.
> >>> 
> >>> Still I am a bit of a fence here. So far I've seen a couple of auto-
> >>> formatting attempts biting back, so I thinl it would help to convince
> >>> me
> >>> to see the kind of changes that would happen first before deciding
> >>> on the global change.
> >>> 
> >>> Andre'
> >>> ___
> >>> Development mailing list
> >>> Development@qt-project.org
> >>> http://lists.qt-project.org/mailman/listinfo/development
> > 
> > 
> 
> 
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development




___
Development mailing list

Re: [Development] clang-format

[Shawn Rutledge]

> On 3 Jul 2018, at 14:12, Dominik Holland  
> wrote:
> 
> 
>>> I thought we were going to run it as a fancy style bot, complaining if
>>> the code isn't per the format-file, but allowing us to ignore it if we
>>> feel the tailored code-formatting is better? Doesn't this mean the bot
>>> will complain a lot? 
>> If there is the need for some human intervention after a bot work, for
>> code formatting purposes, this would defeat any productivity goal.
> 
> Not necessarily. In one of our previous project we patched the Sanity
> Bot to run clang-format over the patch (only the patch, not the complete
> files) and provide the complains as bot comment.
> You could ignore it, or just copy and paste one command from the comment
> and it would fix all the issues in your patch (again only the patch, not
> the complete file).
> 
> Just pasting a single command doesn't sound like it would defeat any
> productivity goal to me.

I think the way to start is with a commit hook to run clang-format on each 
patch, in a way which will reformat only changed lines and ignore other lines 
in the file.  After we try that for a while we’ll see whether we like the 
results.

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Dominik Holland]

>> I thought we were going to run it as a fancy style bot, complaining if
>> the code isn't per the format-file, but allowing us to ignore it if we
>> feel the tailored code-formatting is better? Doesn't this mean the bot
>> will complain a lot? 
> If there is the need for some human intervention after a bot work, for
> code formatting purposes, this would defeat any productivity goal.

Not necessarily. In one of our previous project we patched the Sanity
Bot to run clang-format over the patch (only the patch, not the complete
files) and provide the complains as bot comment.
You could ignore it, or just copy and paste one command from the comment
and it would fix all the issues in your patch (again only the patch, not
the complete file).

Just pasting a single command doesn't sound like it would defeat any
productivity goal to me.

Dominik

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Tor Arne Vestbø]

On 3 Jul 2018, at 13:42, Lars Knoll  wrote:
> 
>  To some extent it might introduce new ones, but the idea was to aim for a 
> format file that is very close to our existing rules.

It seems the extent is wider than first outlined. And even if it’s very close 
to the existing rules, those rules will now be strictly enforced, instead of 
leaving it up to developers to use common sense. The absolute worst thing about 
PEP8 e.g. is its strict line length rule, which forces you to awkwardly wrap 
code in weird places or split lines up into multiple statements, just for the 
sake of being able to read the code on a 80s terminal.

Tor Arne 

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Lars Knoll]

On 3 Jul 2018, at 12:13, Tor Arne Vestbø 
mailto:tor.arne.ves...@qt.io>> wrote:



On 3 Jul 2018, at 10:26, Lars Knoll mailto:lars.kn...@qt.io>> 
wrote:

On 2 Jul 2018, at 16:52, Tor Arne Vestbø 
mailto:tor.arne.ves...@qt.io>> wrote:



On 2 Jul 2018, at 16:49, Lars Knoll mailto:lars.kn...@qt.io>> 
wrote:


On 2 Jul 2018, at 13:35, Tor Arne Vestbø 
mailto:tor.arne.ves...@qt.io>> wrote:


On 2 Jul 2018, at 12:56, Svenn-Arne Dragly 
mailto:svenn-arne.dra...@qt.io>> wrote:

There are also many nice options set in the clang-format config found in Qt 
Creator's sources[2] which I think are interesting. For instance, 
"BinPackParameters: false" and "BinPackArguments: false" makes sure you to 
either put all arguments on one line or give if arguments will have one line 
each. This might be in the controversial category, but it is nice to enable 
while developing. It makes clang-format reflow the code consistently just by 
moving a single argument to a new line and running clang-format afterwards.

I oppose mandating this style, through clang-format or otherwise.

Having a common style that we start following is worth something. And yes, 
everybody will always find some details he won't like. So we won't get anywhere 
if everybody wants it exactly his way.

Why not ease into this with the non-controversial style-rules first?

clang-format will produce one way how the output is formatted. It will reformat 
your sources a certain way with less definitions in the file as well. So it's 
most likely better to have more rules defined as it'll give something closer to 
our implicitly used coding style.

Really?? That sounds like a key feature, I’m surprised clang-format doesn’t 
allow it to leave code untouched.

If that’s the case then I’m not convinced this exercise is worth the churn. I 
thought we were going to run it as a fancy style bot, complaining if the code 
isn’t per the format-file, but allowing us to ignore it if we feel the tailored 
code-formatting is better? Doesn’t this mean the bot will complain a lot?

The idea is to only run it on lines that the change touches anyway. That is 
apparently possible, so you will not get a full reformat of the file(s) you 
touched.

We already have a style guide. Introducing clang-format to the mix does two 
things:

 1. Formalizes the style guide (to a certain degree)
 2. Introduces new style rules

The second point was new to me, I was under the impression we were only going 
to use it as a convenient tool to improve #1.

It does formalise the rules, something I think is a good thing. To some extent 
it might introduce new ones, but the idea was to aim for a format file that is 
very close to our existing rules.

Cheers,
Lars

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Philippe]

> Really?? That sounds like a key feature, I’m surprised clang-format doesn’t 
> allow it to leave code untouched. 

As already mentionned, there is always the option to tag code sections:
// clang-format off
...
// clang-format on

> I thought we were going to run it as a fancy style bot, complaining if
> the code isn’t per the format-file, but allowing us to ignore it if we
> feel the tailored code-formatting is better? Doesn’t this mean the bot
> will complain a lot? 

If there is the need for some human intervention after a bot work, for
code formatting purposes, this would defeat any productivity goal.

Philippe


On Tue, 3 Jul 2018 10:13:06 +
Tor Arne Vestbø  wrote:

> 
> 
> > On 3 Jul 2018, at 10:26, Lars Knoll  wrote:
> > 
> >> On 2 Jul 2018, at 16:52, Tor Arne Vestbø  wrote:
> >> 
> >> 
> >> 
> >>> On 2 Jul 2018, at 16:49, Lars Knoll  wrote:
> >>> 
> >>> 
>  On 2 Jul 2018, at 13:35, Tor Arne Vestbø  wrote:
>  
>  
> > On 2 Jul 2018, at 12:56, Svenn-Arne Dragly  
> > wrote:
> > 
> > There are also many nice options set in the clang-format config found 
> > in Qt Creator's sources[2] which I think are interesting. For instance, 
> > "BinPackParameters: false" and "BinPackArguments: false" makes sure you 
> > to either put all arguments on one line or give if arguments will have 
> > one line each. This might be in the controversial category, but it is 
> > nice to enable while developing. It makes clang-format reflow the code 
> > consistently just by moving a single argument to a new line and running 
> > clang-format afterwards.
>  
>  I oppose mandating this style, through clang-format or otherwise.
> >>> 
> >>> Having a common style that we start following is worth something. And 
> >>> yes, everybody will always find some details he won't like. So we won't 
> >>> get anywhere if everybody wants it exactly his way. 
> >> 
> >> Why not ease into this with the non-controversial style-rules first? 
> > 
> > clang-format will produce one way how the output is formatted. It will 
> > reformat your sources a certain way with less definitions in the file as 
> > well. So it's most likely better to have more rules defined as it'll give 
> > something closer to our implicitly used coding style.
> 
> Really?? That sounds like a key feature, I’m surprised clang-format doesn’t 
> allow it to leave code untouched. 
> 
> If that’s the case then I’m not convinced this exercise is worth the churn. I 
> thought we were going to run it as a fancy style bot, complaining if the code 
> isn’t per the format-file, but allowing us to ignore it if we feel the 
> tailored code-formatting is better? Doesn’t this mean the bot will complain a 
> lot? 
> 
> We already have a style guide. Introducing clang-format to the mix does two 
> things: 
> 
>   1. Formalizes the style guide (to a certain degree)
>   2. Introduces new style rules
> 
> The second point was new to me, I was under the impression we were only going 
> to use it as a convenient tool to improve #1.
> 
> Tor Arne 
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Svenn-Arne Dragly]

On 07/03/2018 10:26 AM, Lars Knoll wrote:

On 2 Jul 2018, at 16:52, Tor Arne Vestbø  > wrote:
>
>
>
>> On 2 Jul 2018, at 16:49, Lars Knoll >> > wrote:

>>
>>
>>> On 2 Jul 2018, at 13:35, Tor Arne Vestbø >>> > wrote:

>>>
>>>
 On 2 Jul 2018, at 12:56, Svenn-Arne Dragly  > wrote:


 There are also many nice options set in the clang-format config found in Qt 
 Creator's sources[2] which I think are interesting. For instance, 
 "BinPackParameters: false" and "BinPackArguments: false" makes sure you to 
 either put all arguments on one line or give if arguments will have one 
 line each. This might be in the controversial category, but it is nice to 
 enable while developing. It makes clang-format reflow the code consistently 
 just by moving a single argument to a new line and running clang-format 
 afterwards.

>>>
>>> I oppose mandating this style, through clang-format or otherwise.
>>
>> Having a common style that we start following is worth something. And yes, 
>> everybody will always find some details he won't like. So we won't get 
>> anywhere if everybody wants it exactly his way.

>
> Why not ease into this with the non-controversial style-rules first?

clang-format will produce one way how the output is formatted. It will reformat
your sources a certain way with less definitions in the file as well. So it's
most likely better to have more rules defined as it'll give something closer to
our implicitly used coding style.

Cheers,
Lars

Not necessarily. With fewer options set, clang-format does leave some 
things as is, so the output is not always the same. For instance, if you 
leave "BinPackArguments" unset and set "ColumnLimit: 0", clang-format 
will respect your choices regarding argument placement. The following 
code is for instance kept intact in that case:


    auto result = myFunction(arg1, arg2,
 argument3, arg4, argument5,
 0, nullptr);

And similarly, this is kept intact:

    auto result = myFunction(arg1, arg2, argument3, arg4,
 argument5, 0, nullptr);

Setting "BinPackArguments: false" or "ColumnLimit: 100" does on the 
other hand make clang-format reformat the above function call accordingly.


I agree with you, Tor Arne. I think it is better to start out with a 
config with only uncontroversial settings enabled. This already gets us 
pretty far and we'll have fewer arguments about style. And to be clear, 
I also don't want to mandate setting "BinPackArguments: false". I wanted 
to mention it because I found it nice to enable locally, even though it 
is far from perfect. If anything should be mandated with regards to 
argument placement, it should probably be more sophisticated or simply 
rely on a fixed column count.


However, a comprehensive clang-format config would be the best in the 
long run. So even though I think we should start with a minimal config, 
I also think we should start discussing the controversial options in 
separate threads/code reviews. Going through the different options in 
Creator's config is a good start.


Svenn-Arne

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Tor Arne Vestbø]

> On 3 Jul 2018, at 10:26, Lars Knoll  wrote:
> 
>> On 2 Jul 2018, at 16:52, Tor Arne Vestbø  wrote:
>> 
>> 
>> 
>>> On 2 Jul 2018, at 16:49, Lars Knoll  wrote:
>>> 
>>> 
 On 2 Jul 2018, at 13:35, Tor Arne Vestbø  wrote:
 
 
> On 2 Jul 2018, at 12:56, Svenn-Arne Dragly  
> wrote:
> 
> There are also many nice options set in the clang-format config found in 
> Qt Creator's sources[2] which I think are interesting. For instance, 
> "BinPackParameters: false" and "BinPackArguments: false" makes sure you 
> to either put all arguments on one line or give if arguments will have 
> one line each. This might be in the controversial category, but it is 
> nice to enable while developing. It makes clang-format reflow the code 
> consistently just by moving a single argument to a new line and running 
> clang-format afterwards.
 
 I oppose mandating this style, through clang-format or otherwise.
>>> 
>>> Having a common style that we start following is worth something. And yes, 
>>> everybody will always find some details he won't like. So we won't get 
>>> anywhere if everybody wants it exactly his way. 
>> 
>> Why not ease into this with the non-controversial style-rules first? 
> 
> clang-format will produce one way how the output is formatted. It will 
> reformat your sources a certain way with less definitions in the file as 
> well. So it's most likely better to have more rules defined as it'll give 
> something closer to our implicitly used coding style.

Really?? That sounds like a key feature, I’m surprised clang-format doesn’t 
allow it to leave code untouched. 

If that’s the case then I’m not convinced this exercise is worth the churn. I 
thought we were going to run it as a fancy style bot, complaining if the code 
isn’t per the format-file, but allowing us to ignore it if we feel the tailored 
code-formatting is better? Doesn’t this mean the bot will complain a lot? 

We already have a style guide. Introducing clang-format to the mix does two 
things: 

  1. Formalizes the style guide (to a certain degree)
  2. Introduces new style rules

The second point was new to me, I was under the impression we were only going 
to use it as a convenient tool to improve #1.

Tor Arne 
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Lars Knoll]

On 2 Jul 2018, at 16:52, Tor Arne Vestbø 
mailto:tor.arne.ves...@qt.io>> wrote:



On 2 Jul 2018, at 16:49, Lars Knoll mailto:lars.kn...@qt.io>> 
wrote:


On 2 Jul 2018, at 13:35, Tor Arne Vestbø 
mailto:tor.arne.ves...@qt.io>> wrote:


On 2 Jul 2018, at 12:56, Svenn-Arne Dragly 
mailto:svenn-arne.dra...@qt.io>> wrote:

There are also many nice options set in the clang-format config found in Qt 
Creator's sources[2] which I think are interesting. For instance, 
"BinPackParameters: false" and "BinPackArguments: false" makes sure you to 
either put all arguments on one line or give if arguments will have one line 
each. This might be in the controversial category, but it is nice to enable 
while developing. It makes clang-format reflow the code consistently just by 
moving a single argument to a new line and running clang-format afterwards.

I oppose mandating this style, through clang-format or otherwise.

Having a common style that we start following is worth something. And yes, 
everybody will always find some details he won't like. So we won't get anywhere 
if everybody wants it exactly his way.

Why not ease into this with the non-controversial style-rules first?

clang-format will produce one way how the output is formatted. It will reformat 
your sources a certain way with less definitions in the file as well. So it's 
most likely better to have more rules defined as it'll give something closer to 
our implicitly used coding style.

Cheers,
Lars

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Tor Arne Vestbø]

> On 2 Jul 2018, at 16:49, Lars Knoll  wrote:
> 
> 
>> On 2 Jul 2018, at 13:35, Tor Arne Vestbø  wrote:
>> 
>> 
>>> On 2 Jul 2018, at 12:56, Svenn-Arne Dragly  wrote:
>>> 
>>> There are also many nice options set in the clang-format config found in Qt 
>>> Creator's sources[2] which I think are interesting. For instance, 
>>> "BinPackParameters: false" and "BinPackArguments: false" makes sure you to 
>>> either put all arguments on one line or give if arguments will have one 
>>> line each. This might be in the controversial category, but it is nice to 
>>> enable while developing. It makes clang-format reflow the code consistently 
>>> just by moving a single argument to a new line and running clang-format 
>>> afterwards.
>> 
>> I oppose mandating this style, through clang-format or otherwise.
> 
> Having a common style that we start following is worth something. And yes, 
> everybody will always find some details he won't like. So we won't get 
> anywhere if everybody wants it exactly his way. 

Why not ease into this with the non-controversial style-rules first? 

Tor Arne 

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Lars Knoll]

> On 2 Jul 2018, at 13:35, Tor Arne Vestbø  wrote:
> 
> 
>> On 2 Jul 2018, at 12:56, Svenn-Arne Dragly  wrote:
>> 
>> There are also many nice options set in the clang-format config found in Qt 
>> Creator's sources[2] which I think are interesting. For instance, 
>> "BinPackParameters: false" and "BinPackArguments: false" makes sure you to 
>> either put all arguments on one line or give if arguments will have one line 
>> each. This might be in the controversial category, but it is nice to enable 
>> while developing. It makes clang-format reflow the code consistently just by 
>> moving a single argument to a new line and running clang-format afterwards.
> 
> I oppose mandating this style, through clang-format or otherwise.

Having a common style that we start following is worth something. And yes, 
everybody will always find some details he won't like. So we won't get anywhere 
if everybody wants it exactly his way. 

I didn't know that creator had a clang-format file. I had a quick chat with 
some of the team here in Berlin, and the file is a result of discussions they 
had some time ago. They are apparently mostly happy with the style.

I would really like to go with one format file for all of Qt, so why not use 
the one from Creator?

And just to repeat: The reasons I want some automated formatting and checking 
is to remove manual work in the reviewing process. It'll help both the people 
reviewing as well as making it easier for new contributors to create patches 
that are consistent with our coding style. Longer term, I want to have as much 
checking as possible (e.g. compile checking and maybe even auto testing) 
happening before the manual code review is being done.

Cheers,
Lars

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Tor Arne Vestbø]

> On 2 Jul 2018, at 12:56, Svenn-Arne Dragly  wrote:
> 
> There are also many nice options set in the clang-format config found in Qt 
> Creator's sources[2] which I think are interesting. For instance, 
> "BinPackParameters: false" and "BinPackArguments: false" makes sure you to 
> either put all arguments on one line or give if arguments will have one line 
> each. This might be in the controversial category, but it is nice to enable 
> while developing. It makes clang-format reflow the code consistently just by 
> moving a single argument to a new line and running clang-format afterwards.

I oppose mandating this style, through clang-format or otherwise.

Tor Arne 

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Svenn-Arne Dragly]

On 06/20/2018 01:01 PM, Tor Arne Vestbø wrote:

Good point, I was imagining it used only to verify style, not to auto-format. 
Still, starting out with a few non-controversial rules would be a good thing.


I agree. For instance, it is clear from the patch discussion[1] that 
line length/column count is controversial. I think it would be best to 
set this to 0 for now to be able to move forward and get the benefits of 
clang-format for all the other things that we do agree on. Setting it to 
0 means that clang-format will respect your decisions about line length 
unless you contradict other rules.


There are also many nice options set in the clang-format config found in 
Qt Creator's sources[2] which I think are interesting. For instance, 
"BinPackParameters: false" and "BinPackArguments: false" makes sure you 
to either put all arguments on one line or give if arguments will have 
one line each. This might be in the controversial category, but it is 
nice to enable while developing. It makes clang-format reflow the code 
consistently just by moving a single argument to a new line and running 
clang-format afterwards.


Svenn-Arne

[1] https://codereview.qt-project.org/#/c/233051/
[2] 
https://github.com/qt-creator/qt-creator/blob/master/dist/clangformat/.clang-format


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Philippe]

> It doesn't help me all that much to have
> familiar formatting and naming in a translation
> unit

This is a good skill. But the idea is to help developers that don't have
this skill.

Philippe

On Fri, 22 Jun 2018 19:47:14 +0300
Ville Voutilainen  wrote:

> On 22 June 2018 at 19:39, Scott Bloom  wrote:
> > I cant even get my developers to agree the emacs takes to many fingers, and 
> > VI(m) is the only editor they need
> >
> > Let alone, where the brackets and spaces belong
> >
> > Let alone, if statements on the same line as the conditional
> >
> > The problem ive seen, is while you may LOVE the curly brackts on the same 
> > line, you will never convince me... ??
> 
> The problem I have with this oft-recurring formatting discussion is
> that it's advertised as a big benefit, sometimes
> downplaying its cost. It doesn't help me all that much to have
> familiar formatting and naming in a translation
> unit; that consistency is dwarfed by the effort needed to understand
> the logical design of the code to reasonable
> modify/refactor/maintain it.
> 
> I think it's reasonable and harmless to clang-format new changes; I
> continue to be unconvinced of the cost/benefit ratio
> of reformatting all of our existing code.


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Scott Bloom]

> I cant even get my developers to agree the emacs takes to many fingers, and 
> VI(m) is the only editor they need
>
> Let alone, where the brackets and spaces belong
>
> Let alone, if statements on the same line as the conditional
>
> The problem ive seen, is while you may LOVE the curly brackts on the 
> same line, you will never convince me... 

The problem I have with this oft-recurring formatting discussion is that it's 
advertised as a big benefit, sometimes downplaying its cost. It doesn't help me 
all that much to have familiar formatting and naming in a translation unit; 
that consistency is dwarfed by the effort needed to understand the logical 
design of the code to reasonable modify/refactor/maintain it.

I think it's reasonable and harmless to clang-format new changes; I continue to 
be unconvinced of the cost/benefit ratio of reformatting all of our existing 
code.

Agreed.

I have moved to the "over all policy" of don’t check in crap code...   And run 
your styler as you need to, on code you are working on, if it helps you 
understand it.  

And "try" to avoid checking in "format only changes"

Scott
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Ville Voutilainen]

On 22 June 2018 at 19:39, Scott Bloom  wrote:
> I cant even get my developers to agree the emacs takes to many fingers, and 
> VI(m) is the only editor they need
>
> Let alone, where the brackets and spaces belong
>
> Let alone, if statements on the same line as the conditional
>
> The problem ive seen, is while you may LOVE the curly brackts on the same 
> line, you will never convince me... 

The problem I have with this oft-recurring formatting discussion is
that it's advertised as a big benefit, sometimes
downplaying its cost. It doesn't help me all that much to have
familiar formatting and naming in a translation
unit; that consistency is dwarfed by the effort needed to understand
the logical design of the code to reasonable
modify/refactor/maintain it.

I think it's reasonable and harmless to clang-format new changes; I
continue to be unconvinced of the cost/benefit ratio
of reformatting all of our existing code.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Scott Bloom]

I cant even get my developers to agree the emacs takes to many fingers, and 
VI(m) is the only editor they need

Let alone, where the brackets and spaces belong

Let alone, if statements on the same line as the conditional

The problem ive seen, is while you may LOVE the curly brackts on the same line, 
you will never convince me... 




-Original Message-
From: Development  On 
Behalf Of Philippe
Sent: Friday, June 22, 2018 09:12
To: development@qt-project.org
Subject: Re: [Development] clang-format

>  I usually come to the conclusion, code in the style you want, unless 
> it's a bad format.

"bad format" is subjective and the use of clang-format precisely bypasses 
subjectivity.

> But IMO, wholesale "format changes" have zero value to the customer, 
> so any pain associated with them, should be weighed against the time 
> and effort necessary to implement a format change that is being taken 
> away from customer oriented development.

I have a different POV: everything that eases the developer's work, means 
spared development time, hence more productivity, hence benefits to customers 
at the end.

Not having to take care of code-formatting, thanks to clang-format, eases 
programming.

And looking at someone else code with a familiar format, gives the feeling "I 
am home" (provided some naming guideline is respected).

Philippe

On Fri, 22 Jun 2018 15:34:09 +
Scott Bloom  wrote:

> Fair enough.. It just seems that this thread has fundamentally become a 
> religious issue over format, and when it should be forced on people...
> 
> I fight this all the time in my organization... I usually come to the 
> conclusion, code in the style you want, unless it's a bad format.  And like 
> pornography, I cant define it but I know it when I see it...
> 
> But IMO, wholesale "format changes" have zero value to the customer, so any 
> pain associated with them, should be weighed against the time and effort 
> necessary to implement a format change that is being taken away from customer 
> oriented development.
> 
> Scott
> 
> -Original Message-
> From: Development 
>  On Behalf Of 
> Thiago Macieira
> Sent: Friday, June 22, 2018 08:26
> To: development@qt-project.org
> Subject: Re: [Development] clang-format
> 
> On Friday, 22 June 2018 07:40:58 PDT Scott Bloom wrote:
> > In a series of wrapper scripts, essentially, everything checked in 
> > was converted to the check in format.  However each developer had 
> > their own format .  And on checkout, the code would be compare to it.
> > 
> > On "Diff" analysis of two reversions, you had to see the diff based 
> > on the checked in format.
> > 
> > It really made things, that much simpler... and we never heard 
> > people bitch and moan about where the curley brackets belong
> 
> Git has such a functionality, it's called the clean/smudge filters. See man 
> gitattributes for more information.
> 
> But that still means the code you see is subject to the smudge filter's 
> whims. 
> We've concluded that it doesn't always do the right thing.
> 
> And besides, there's something new that didn't exist in the old days: code 
> reviews. Gerrit has no such functionality and in any case, reviewers need to 
> agree between themselves on what they ar reviewing.
> 
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
> 
> 
> 
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Philippe]

>  I usually come to the conclusion, code in the style you want,
> unless it's a bad format. 

"bad format" is subjective and the use of clang-format precisely
bypasses subjectivity.

> But IMO, wholesale "format changes" have zero value to the customer, so any
> pain associated with them, should be weighed against the time and
> effort necessary to implement a format change that is being taken away from
> customer oriented development.

I have a different POV: everything that eases the developer's work,
means spared development time, hence more productivity, hence benefits to
customers at the end.

Not having to take care of code-formatting, thanks to clang-format, eases
programming.

And looking at someone else code with a familiar format, gives the
feeling "I am home" (provided some naming guideline is respected).

Philippe

On Fri, 22 Jun 2018 15:34:09 +
Scott Bloom  wrote:

> Fair enough.. It just seems that this thread has fundamentally become a 
> religious issue over format, and when it should be forced on people...
> 
> I fight this all the time in my organization... I usually come to the 
> conclusion, code in the style you want, unless it's a bad format.  And like 
> pornography, I cant define it but I know it when I see it...
> 
> But IMO, wholesale "format changes" have zero value to the customer, so any 
> pain associated with them, should be weighed against the time and effort 
> necessary to implement a format change that is being taken away from customer 
> oriented development.
> 
> Scott
> 
> -Original Message-
> From: Development  On 
> Behalf Of Thiago Macieira
> Sent: Friday, June 22, 2018 08:26
> To: development@qt-project.org
> Subject: Re: [Development] clang-format
> 
> On Friday, 22 June 2018 07:40:58 PDT Scott Bloom wrote:
> > In a series of wrapper scripts, essentially, everything checked in was 
> > converted to the check in format.  However each developer had their 
> > own format .  And on checkout, the code would be compare to it.
> > 
> > On "Diff" analysis of two reversions, you had to see the diff based on 
> > the checked in format.
> > 
> > It really made things, that much simpler... and we never heard people 
> > bitch and moan about where the curley brackets belong
> 
> Git has such a functionality, it's called the clean/smudge filters. See man 
> gitattributes for more information.
> 
> But that still means the code you see is subject to the smudge filter's 
> whims. 
> We've concluded that it doesn't always do the right thing.
> 
> And besides, there's something new that didn't exist in the old days: code 
> reviews. Gerrit has no such functionality and in any case, reviewers need to 
> agree between themselves on what they ar reviewing.
> 
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
>   Software Architect - Intel Open Source Technology Center
> 
> 
> 
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Thiago Macieira]

On Friday, 22 June 2018 08:34:09 PDT Scott Bloom wrote:
> But IMO, wholesale "format changes" have zero value to the customer, so any
> pain associated with them, should be weighed against the time and effort
> necessary to implement a format change that is being taken away from
> customer oriented development.

Right. I'm ok with the bot announcing formatting errors and getting people to 
agree on a single format.

So long as we agree that there's no whitespace before a comma.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Thiago Macieira]

On Friday, 22 June 2018 07:40:58 PDT Scott Bloom wrote:
> In a series of wrapper scripts, essentially, everything checked in was
> converted to the check in format.  However each developer had their own
> format .  And on checkout, the code would be compare to it.
> 
> On "Diff" analysis of two reversions, you had to see the diff based on the
> checked in format.
> 
> It really made things, that much simpler... and we never heard people bitch
> and moan about where the curley brackets belong

Git has such a functionality, it's called the clean/smudge filters. See man 
gitattributes for more information.

But that still means the code you see is subject to the smudge filter's whims. 
We've concluded that it doesn't always do the right thing.

And besides, there's something new that didn't exist in the old days: code 
reviews. Gerrit has no such functionality and in any case, reviewers need to 
agree between themselves on what they ar reviewing.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Frederik Gladhorn]

OK, so for now, my take-away from this thread is that we should simply create 
nice commit-hooks that help everyone along. As a next step we can also enhance 
the sanity bot with friendly messages.
I ran clang-format a few times over different parts of the code-base and I 
agree that it sometimes ends up with stuff looking worse. Either use the opt 
out for those parts or live with it.
Not having to debate the format again and again is a big win in my opinion.

Let's move the _clang-format file from qtrepotools to qt5.git:
https://codereview.qt-project.org/#/c/233051/
https://codereview.qt-project.org/#/c/233050/

Cheers,
Frederik

On mandag 18. juni 2018 11.04.33 CEST Frederik Gladhorn wrote:
> Hi all,
> 
> as part of the closing ceremony of this year's Qt Contributors' Summit we
> agreed to start using clang-format, to have fewer discussions around coding
> style and rather focus on the actual code.
> 
> I have not yet thought about all angles, how to best implement this, here
> are some notes:
> 
> We have a clang-format file in qtrepotools. You can use it today, simply
> make sure it's in any parent directory of the files you are editing. I'd
> actually propose simply moving this into the root directory of qt5.git.
> https://code.qt.io/cgit/qt/qtrepotools.git/tree/config/_clang-format
> 
> If you want to clean one file:
> clang-format -i myfile.cpp/h
> 
> To clean a commit (only modifies the working tree):
> git clang-format
> 
> 
> Getting clang-format seems easy enough:
> On macOS: brew install clang-format
> Linux: install clang-format
> Windows: it comes with normal clang
> 
> Then there is the tooling/workflow perspective. Creator and other IDEs have
> support (you may need to enable the beautifier plugin in about plugins).
> I imagine we add this to the sanity bot ("git clang-format --diff -q" should
> return empty, otherwise post a message).
> 
> Local hooks are basically the same, any ideas how to best set up the git
> hooks appreciated :)
> 
> And then there is the big question when we run it once over the entire
> codebase.
> 
> Cheers,
> Frederik
> 
> 
> 
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development




___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Thiago Macieira]

On Thursday, 21 June 2018 16:56:00 PDT Ville Voutilainen wrote:
> While I appreciate your git-fu, I must bat my eyelids if we consider that
> a normal procedure for people wishing to submit patches.

Not many people have over 100 pending patches for Qt. In fact, you can 
probably count them in the fingers of one hand.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Ville Voutilainen]

On 22 June 2018 at 02:36, Olivier Goffart  wrote:
> On 2018-06-21 16:02, Thiago Macieira wrote:
>>
>> On Thursday, 21 June 2018 04:51:38 PDT Frederik Gladhorn wrote:
>>>
>>> checkout the change before we run clang-format
>>> run clang-format on the changed files
>>> push on top of the formatting change
>>
>>
>> I'd appreciate such a script. As I said, I will dedicate no more than 15
>> minutes total for rebasing after such a massive change that deals with
>> whitespace only and that includes writing such a script.
>
>
>
> I suggest you first rebase on top of the change right before the whitespace
> changes and fix all conflicts as you usually do.
>
>
> Then:
>
>   git rebase -x "git-clang-format HEAD^" -i  
>
> On conflicts, use git mergetool and always pick your changes, or even
> git checkout --ours
>
> This should probably go quite fast.

While I appreciate your git-fu, I must bat my eyelids if we consider that
a normal procedure for people wishing to submit patches.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Olivier Goffart]

On 2018-06-21 16:02, Thiago Macieira wrote:

On Thursday, 21 June 2018 04:51:38 PDT Frederik Gladhorn wrote:

checkout the change before we run clang-format
run clang-format on the changed files
push on top of the formatting change


I'd appreciate such a script. As I said, I will dedicate no more than 15
minutes total for rebasing after such a massive change that deals with
whitespace only and that includes writing such a script.



I suggest you first rebase on top of the change right before the whitespace 
changes and fix all conflicts as you usually do.



Then:

  git rebase -x "git-clang-format HEAD^" -i  

On conflicts, use git mergetool and always pick your changes, or even
git checkout --ours

This should probably go quite fast.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Thiago Macieira]

On Thursday, 21 June 2018 04:51:38 PDT Frederik Gladhorn wrote:
> checkout the change before we run clang-format
> run clang-format on the changed files
> push on top of the formatting change

I'd appreciate such a script. As I said, I will dedicate no more than 15 
minutes total for rebasing after such a massive change that deals with 
whitespace only and that includes writing such a script.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Frederik Gladhorn]

On tirsdag 19. juni 2018 17.51.28 CEST Thiago Macieira wrote:
> On Tuesday, 19 June 2018 02:34:09 PDT Lars Knoll wrote:
> > Currently, we only merge from 5.11 to dev, so I hope this won’t be too
> > bad.
> > But we could of course also run the tool over both branches.
> 
> I have right now 149 changes on top of qtbase's dev branch. For something
> that is ostensibly a whitespace change, I will dedicate no more than 1
> minute per change to resolve a conflict or 15 minutes total to rebase my
> branch once it goes in.
> 
> Any change that times out will be dropped. That includes all the Qt 6
> container work, the un-accepted QtDBus updates (which include a
> QDBusConnection::connect taking PMF), a large refactoring of
> QFileSystemEngine that no one has reviewed yet despite being on Gerrit for
> over a year, some work for CBOR and a few other miscellaneous changes.

That should be scriptable afaict, correct me if I'm wrong.

checkout the change before we run clang-format
run clang-format on the changed files
push on top of the formatting change

Cheers,
Frederik






___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Jean-Michaël Celerier]

> I never seen anything QtCreator had trouble with.

really ?
paste this in a .cpp and indent it :

#define WHATEVER

struct foo
{
WHATEVER

public:
explicit foo();
};

struct bar
{
public:
explicit bar();
};




---
Jean-Michaël Celerier
http://www.jcelerier.name

On Wed, Jun 20, 2018 at 3:08 PM, Allan Sandfeld Jensen 
wrote:

> On Mittwoch, 20. Juni 2018 14:09:20 CEST Ivan Donchevskii wrote:
> > One more thing about clang-format.
> >
> > It might be really nice if we use it as a default formatting tool in Qt
> > Creator. And I really want to experiment with it and see how clang-format
> > can replace the indenter that we currently use (which has a lot of bug
> > reports about broken formatting for example with modern C++).
> >
>
> I never seen anything QtCreator had trouble with. But on the subject,
> there is
> one standard indentation features QtCreator has that clang-format can't
> reproduce. When you line-break an expression, clang-format will indent it
> a
> fixed amount from the beginning of the next line, where QtCreator will try
> to
> find places to line up with from the line above, including indenting from
> the
> last paranthesis For instance:
>
> int a = foobar(x) + foo(looongfunctionname(
>  looongvariablename));
>
> vs
>
> int a = foobar(x) + foo(looongfunctionname(
> looongvariablename));
>
>
> This semantic indentation is not something clang-format can do at the
> moment.
> At least if someone knows how, I would love to know, I have search
> everywhere,
> including the clang-format code.
>
> It is not so bad in this case, but once you have multiple line-break like
> that
> in a long function call, clang-formated code is essentionally just
> unformatted.
>
> In any case if we want to enforce clang-format everywhere, perhaps we
> shouldn't have QtCreator do smarter and more readable indentation that we
> do
> not allow in our repository?
>
> 'Allan
>
>
>
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Philippe]

> When you line-break an expression, clang-format will indent it a 
>fixed amount from the beginning of the next line,
> where QtCreator will try to find places to line up with from the line above, 
> including indenting from the 
> last paranthesis For instance:

I don't use QtCreator, but clang-format does it as you describe for QtCreator.
And this works very nicely (...most of the time).
Now, I don't remember which one of the many settings is responsible for this.

Philippe


On Wed, 20 Jun 2018 15:08:29 +0200
Allan Sandfeld Jensen  wrote:

> On Mittwoch, 20. Juni 2018 14:09:20 CEST Ivan Donchevskii wrote:
> > One more thing about clang-format.
> > 
> > It might be really nice if we use it as a default formatting tool in Qt
> > Creator. And I really want to experiment with it and see how clang-format
> > can replace the indenter that we currently use (which has a lot of bug
> > reports about broken formatting for example with modern C++).
> > 
> 
> I never seen anything QtCreator had trouble with. But on the subject, there 
> is 
> one standard indentation features QtCreator has that clang-format can't 
> reproduce. When you line-break an expression, clang-format will indent it a 
> fixed amount from the beginning of the next line, where QtCreator will try to 
> find places to line up with from the line above, including indenting from the 
> last paranthesis For instance:
> 
> int a = foobar(x) + foo(looongfunctionname(
>looongvariablename));
> 
> vs
> 
> int a = foobar(x) + foo(looongfunctionname(
> looongvariablename));
> 
> 
> This semantic indentation is not something clang-format can do at the moment. 
> At least if someone knows how, I would love to know, I have search 
> everywhere, 
> including the clang-format code. 
> 
> It is not so bad in this case, but once you have multiple line-break like 
> that 
> in a long function call, clang-formated code is essentionally just 
> unformatted.
> 
> In any case if we want to enforce clang-format everywhere, perhaps we 
> shouldn't have QtCreator do smarter and more readable indentation that we do 
> not allow in our repository?
> 
> 'Allan
>   
> 
> 
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Eirik Aavitsland]

On 20. juni 2018 13:23, Morten Sørvig wrote:




On 19 Jun 2018, at 17:33, Allan Sandfeld Jensen  wrote:

Btw. Just for your information.

I have attached a few random examples of what we can look forward too after
running an auto-"beautifying" tool over our hand-formated Qt code. And these
changes are NOT something we can configure out ouf of in clang-format, it is
just cases where it can't do any 
better.___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development


Thanks, this should really be the first thing we do: run a pre-project
so that we can have an informed discussion.

The point of machine formatting is perhaps not to make code beautiful, rather
it is to have automated formatting in order to offload work from the humans.


Looking at qConvertRgb16To32:

Manual formatting:
 return 0xff00
 | c) << 3) & 0xf8) | (((c) >> 2) & 0x7))
 | c) << 5) & 0xfc00) | (((c) >> 1) & 0x300))
 | c) << 8) & 0xf8) | (((c) << 3) & 0x7));

Clang-format:
 return 0xff00 | c) << 3) & 0xf8) | (((c) >> 2) & 0x7)) | c) << 5) & 
0xfc00) | (((c) >> 1) & 0x300))
| c) << 8) & 0xf8) | (((c) << 3) & 0x7));

The manually formatted version clearly has something going for it.



Yup, I also did some manual testing on existing Qt code, and I generally 
found that


- clang-format's intra-line changes were fine (correcting white space in 
expressions etc.)


- but whenever clang-format would add or remove linebreaks, which it 
would quite often do, the changes were typically detrimental to 
readability, and often significantly so (as in the example above).


In my view, the readability improvements of the former in no way 
outweighs the readability damage of the latter. So to me this tool (or 
its config) is not ready to be trusted with the power of doing automatic 
(not developer reviewed & approved) changes.


- Eirik Aa.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Allan Sandfeld Jensen]

On Mittwoch, 20. Juni 2018 14:09:20 CEST Ivan Donchevskii wrote:
> One more thing about clang-format.
> 
> It might be really nice if we use it as a default formatting tool in Qt
> Creator. And I really want to experiment with it and see how clang-format
> can replace the indenter that we currently use (which has a lot of bug
> reports about broken formatting for example with modern C++).
> 

I never seen anything QtCreator had trouble with. But on the subject, there is 
one standard indentation features QtCreator has that clang-format can't 
reproduce. When you line-break an expression, clang-format will indent it a 
fixed amount from the beginning of the next line, where QtCreator will try to 
find places to line up with from the line above, including indenting from the 
last paranthesis For instance:

int a = foobar(x) + foo(looongfunctionname(
 looongvariablename));

vs

int a = foobar(x) + foo(looongfunctionname(
looongvariablename));


This semantic indentation is not something clang-format can do at the moment. 
At least if someone knows how, I would love to know, I have search everywhere, 
including the clang-format code. 

It is not so bad in this case, but once you have multiple line-break like that 
in a long function call, clang-formated code is essentionally just 
unformatted.

In any case if we want to enforce clang-format everywhere, perhaps we 
shouldn't have QtCreator do smarter and more readable indentation that we do 
not allow in our repository?

'Allan



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Ivan Donchevskii]

One more thing about clang-format.

It might be really nice if we use it as a default formatting tool in Qt 
Creator. And I really want to experiment with it and see how clang-format can 
replace the indenter that we currently use (which has a lot of bug reports 
about broken formatting for example with modern C++).


BR,
Ivan



From: Development  
on behalf of Sérgio Martins via Development 
Sent: Wednesday, June 20, 2018 1:56 PM
To: Frederik Gladhorn
Cc: Development; development@qt-project.org
Subject: Re: [Development] clang-format

On 2018-06-18 10:04, Frederik Gladhorn wrote:
> Hi all,
>
> as part of the closing ceremony of this year's Qt Contributors' Summit
> we
> agreed to start using clang-format, to have fewer discussions around
> coding
> style and rather focus on the actual code.
>
> I have not yet thought about all angles, how to best implement this,
> here are
> some notes:
>
> We have a clang-format file in qtrepotools. You can use it today,
> simply make
> sure it's in any parent directory of the files you are editing. I'd
> actually
> propose simply moving this into the root directory of qt5.git.
> https://code.qt.io/cgit/qt/qtrepotools.git/tree/config/_clang-format
>
> If you want to clean one file:
> clang-format -i myfile.cpp/h
>
> To clean a commit (only modifies the working tree):
> git clang-format
>
>
> Getting clang-format seems easy enough:
> On macOS: brew install clang-format
> Linux: install clang-format
> Windows: it comes with normal clang
>
> Then there is the tooling/workflow perspective. Creator and other IDEs
> have
> support (you may need to enable the beautifier plugin in about
> plugins).
> I imagine we add this to the sanity bot ("git clang-format --diff -q"
> should
> return empty, otherwise post a message).
>
> Local hooks are basically the same, any ideas how to best set up the
> git hooks
> appreciated :)
>
> And then there is the big question when we run it once over the entire
> codebase.

I find clang-format a bit limited and always need to manually format
some things that clang-format doesn't allow to tune.

It's quite useful when integrated with gerrit so it can automatically -1
the most common mistakes, but I wouldn't run it on the entire codebase.

So YES to all you said, except the massive cleanup.




Regards,
--
Sérgio Martins | sergio.mart...@kdab.com | Senior Software Engineer
Klarälvdalens Datakonsult AB, a KDAB Group company
Tel: Sweden (HQ) +46-563-540090, USA +1-866-777-KDAB(5322)
KDAB - The Qt, C++ and OpenGL Experts
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Sérgio Martins via Development]

On 2018-06-18 10:04, Frederik Gladhorn wrote:

Hi all,

as part of the closing ceremony of this year's Qt Contributors' Summit 
we
agreed to start using clang-format, to have fewer discussions around 
coding

style and rather focus on the actual code.

I have not yet thought about all angles, how to best implement this, 
here are

some notes:

We have a clang-format file in qtrepotools. You can use it today, 
simply make
sure it's in any parent directory of the files you are editing. I'd 
actually

propose simply moving this into the root directory of qt5.git.
https://code.qt.io/cgit/qt/qtrepotools.git/tree/config/_clang-format

If you want to clean one file:
clang-format -i myfile.cpp/h

To clean a commit (only modifies the working tree):
git clang-format


Getting clang-format seems easy enough:
On macOS: brew install clang-format
Linux: install clang-format
Windows: it comes with normal clang

Then there is the tooling/workflow perspective. Creator and other IDEs 
have
support (you may need to enable the beautifier plugin in about 
plugins).
I imagine we add this to the sanity bot ("git clang-format --diff -q" 
should

return empty, otherwise post a message).

Local hooks are basically the same, any ideas how to best set up the 
git hooks

appreciated :)

And then there is the big question when we run it once over the entire
codebase.


I find clang-format a bit limited and always need to manually format 
some things that clang-format doesn't allow to tune.


It's quite useful when integrated with gerrit so it can automatically -1 
the most common mistakes, but I wouldn't run it on the entire codebase.


So YES to all you said, except the massive cleanup.




Regards,
--
Sérgio Martins | sergio.mart...@kdab.com | Senior Software Engineer
Klarälvdalens Datakonsult AB, a KDAB Group company
Tel: Sweden (HQ) +46-563-540090, USA +1-866-777-KDAB(5322)
KDAB - The Qt, C++ and OpenGL Experts
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Tor Arne Vestbø]

On 20 Jun 2018, at 12:13, Lars Knoll  wrote:
> 
> I can’t see how clang-format will make you jump through any sort of hoops. 
> Creator already has a hook for doing it on file saving time afaik, 
> git-clang-format will clean up your change from the command line.

Good point, I was imagining it used only to verify style, not to auto-format. 
Still, starting out with a few non-controversial rules would be a good thing.

Tor Arne 


> 
> Lars
> 
>> On 20 Jun 2018, at 11:52, Tor Arne Vestbø  wrote:
>> 
>> How about we also start with only one or two  obvious rules that everyone 
>> agrees on? I don’t want Qt development to turn into Python PEP8 type rigid 
>> rules that makes you jump through a million hoops. If the latter is the goal 
>> here then I’m against enforcing clang-format, and it should be implemented 
>> as a bot that just warns, similar to the current style bot.
>> 
>> - Tor Arne
>> 
>>> On 20 Jun 2018, at 11:21, André Pönitz  wrote:
>>> 
 On Wed, Jun 20, 2018 at 06:30:26AM +, Lars Knoll wrote:
 
 
> On 19 Jun 2018, at 18:19, Ville Voutilainen
>  wrote:
> 
> On 19 June 2018 at 19:13, Philippe  wrote:
>>> For the above reasons I'd lean towards not running it globally and
>>> just using it on new changes.
>> 
>> +1, based on my clang-format experience on a big application.
>> 
>> BTW, keep in mind that you can disable clang-format on code
>> sections with:
>> 
>> // clang-format off // clang-format on
> 
> When I last experienced a large-scale clang-format reformat, it
> really hurt development during the churn. We should somehow manage
> to do it during a time when there aren't many pending patches in the
> pipeline. I'm not concerned about git-blame; that has never been a
> problem after reformats. However, I do not care about indentation
> nor do I want to spend time on it either way, it has no actual
> effect on readability and maintainability of code, and consistency
> outside the file you're in has never mattered to me one bit.
> 
> IOW, I'm not opposed to reformats and auto-checking of clang-format
> (or even hooking it), but I do not see it as a thing with all that
> great return-of-investment.
 
 It helps in that you do not need to point those things out in code
 reviews, and that I (and others) won’t even create changes with wrong
 formatting that I’ll need to fix up later on. It’s part of a larger
 story, where I would like to get as much automatic checking of changes
 done before humans start reviewing.
>>> 
>>> It's also a cultural thing.
>>> 
>>> Quite a few people seem to take less offense from a "Your formatting is
>>> bad" when the comment comes from a bot than when it comes from a human. 
>>> 
 One idea could be to introduce this incrementally. Let’s first start
 off with enforcing it for new changes. Then we run it globally over
 the code base shortly before Qt 6.0 is being released. At that time
 merges shouldn’t be as much of a problem (as we’ll probably
 cherry-pick into Qt 5.15) and by then all new changes in Gerrit will
 be properly formatted (due to the earlier hook).
>>> 
>>> Incrementally sounds good to me.
>>> 
>>> Still I am a bit of a fence here. So far I've seen a couple of auto-
>>> formatting attempts biting back, so I thinl it would help to convince me
>>> to see the kind of changes that would happen first before deciding
>>> on the global change.
>>> 
>>> Andre'
>>> ___
>>> Development mailing list
>>> Development@qt-project.org
>>> http://lists.qt-project.org/mailman/listinfo/development
> 

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Lars Knoll]

I can’t see how clang-format will make you jump through any sort of hoops. 
Creator already has a hook for doing it on file saving time afaik, 
git-clang-format will clean up your change from the command line.

Lars

> On 20 Jun 2018, at 11:52, Tor Arne Vestbø  wrote:
> 
> How about we also start with only one or two  obvious rules that everyone 
> agrees on? I don’t want Qt development to turn into Python PEP8 type rigid 
> rules that makes you jump through a million hoops. If the latter is the goal 
> here then I’m against enforcing clang-format, and it should be implemented as 
> a bot that just warns, similar to the current style bot.
> 
> - Tor Arne
> 
>> On 20 Jun 2018, at 11:21, André Pönitz  wrote:
>> 
>>> On Wed, Jun 20, 2018 at 06:30:26AM +, Lars Knoll wrote:
>>> 
>>> 
 On 19 Jun 2018, at 18:19, Ville Voutilainen
  wrote:
 
 On 19 June 2018 at 19:13, Philippe  wrote:
>> For the above reasons I'd lean towards not running it globally and
>> just using it on new changes.
> 
> +1, based on my clang-format experience on a big application.
> 
> BTW, keep in mind that you can disable clang-format on code
> sections with:
> 
> // clang-format off // clang-format on
 
 When I last experienced a large-scale clang-format reformat, it
 really hurt development during the churn. We should somehow manage
 to do it during a time when there aren't many pending patches in the
 pipeline. I'm not concerned about git-blame; that has never been a
 problem after reformats. However, I do not care about indentation
 nor do I want to spend time on it either way, it has no actual
 effect on readability and maintainability of code, and consistency
 outside the file you're in has never mattered to me one bit.
 
 IOW, I'm not opposed to reformats and auto-checking of clang-format
 (or even hooking it), but I do not see it as a thing with all that
 great return-of-investment.
>>> 
>>> It helps in that you do not need to point those things out in code
>>> reviews, and that I (and others) won’t even create changes with wrong
>>> formatting that I’ll need to fix up later on. It’s part of a larger
>>> story, where I would like to get as much automatic checking of changes
>>> done before humans start reviewing.
>> 
>> It's also a cultural thing.
>> 
>> Quite a few people seem to take less offense from a "Your formatting is
>> bad" when the comment comes from a bot than when it comes from a human. 
>> 
>>> One idea could be to introduce this incrementally. Let’s first start
>>> off with enforcing it for new changes. Then we run it globally over
>>> the code base shortly before Qt 6.0 is being released. At that time
>>> merges shouldn’t be as much of a problem (as we’ll probably
>>> cherry-pick into Qt 5.15) and by then all new changes in Gerrit will
>>> be properly formatted (due to the earlier hook).
>> 
>> Incrementally sounds good to me.
>> 
>> Still I am a bit of a fence here. So far I've seen a couple of auto-
>> formatting attempts biting back, so I thinl it would help to convince me
>> to see the kind of changes that would happen first before deciding
>> on the global change.
>> 
>> Andre'
>> ___
>> Development mailing list
>> Development@qt-project.org
>> http://lists.qt-project.org/mailman/listinfo/development

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Tor Arne Vestbø]

How about we also start with only one or two  obvious rules that everyone 
agrees on? I don’t want Qt development to turn into Python PEP8 type rigid 
rules that makes you jump through a million hoops. If the latter is the goal 
here then I’m against enforcing clang-format, and it should be implemented as a 
bot that just warns, similar to the current style bot.

- Tor Arne

> On 20 Jun 2018, at 11:21, André Pönitz  wrote:
> 
>> On Wed, Jun 20, 2018 at 06:30:26AM +, Lars Knoll wrote:
>> 
>> 
>>> On 19 Jun 2018, at 18:19, Ville Voutilainen
>>>  wrote:
>>> 
>>> On 19 June 2018 at 19:13, Philippe  wrote:
> For the above reasons I'd lean towards not running it globally and
> just using it on new changes.
 
 +1, based on my clang-format experience on a big application.
 
 BTW, keep in mind that you can disable clang-format on code
 sections with:
 
 // clang-format off // clang-format on
>>> 
>>> When I last experienced a large-scale clang-format reformat, it
>>> really hurt development during the churn. We should somehow manage
>>> to do it during a time when there aren't many pending patches in the
>>> pipeline. I'm not concerned about git-blame; that has never been a
>>> problem after reformats. However, I do not care about indentation
>>> nor do I want to spend time on it either way, it has no actual
>>> effect on readability and maintainability of code, and consistency
>>> outside the file you're in has never mattered to me one bit.
>>> 
>>> IOW, I'm not opposed to reformats and auto-checking of clang-format
>>> (or even hooking it), but I do not see it as a thing with all that
>>> great return-of-investment.
>> 
>> It helps in that you do not need to point those things out in code
>> reviews, and that I (and others) won’t even create changes with wrong
>> formatting that I’ll need to fix up later on. It’s part of a larger
>> story, where I would like to get as much automatic checking of changes
>> done before humans start reviewing.
> 
> It's also a cultural thing.
> 
> Quite a few people seem to take less offense from a "Your formatting is
> bad" when the comment comes from a bot than when it comes from a human. 
> 
>> One idea could be to introduce this incrementally. Let’s first start
>> off with enforcing it for new changes. Then we run it globally over
>> the code base shortly before Qt 6.0 is being released. At that time
>> merges shouldn’t be as much of a problem (as we’ll probably
>> cherry-pick into Qt 5.15) and by then all new changes in Gerrit will
>> be properly formatted (due to the earlier hook).
> 
> Incrementally sounds good to me.
> 
> Still I am a bit of a fence here. So far I've seen a couple of auto-
> formatting attempts biting back, so I thinl it would help to convince me
> to see the kind of changes that would happen first before deciding
> on the global change.
> 
> Andre'
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[André Pönitz]

On Wed, Jun 20, 2018 at 06:30:26AM +, Lars Knoll wrote:
> 
> 
> > On 19 Jun 2018, at 18:19, Ville Voutilainen
> >  wrote:
> > 
> > On 19 June 2018 at 19:13, Philippe  wrote:
> >>> For the above reasons I'd lean towards not running it globally and
> >>> just using it on new changes.
> >> 
> >> +1, based on my clang-format experience on a big application.
> >> 
> >> BTW, keep in mind that you can disable clang-format on code
> >> sections with:
> >> 
> >> // clang-format off // clang-format on
> > 
> > When I last experienced a large-scale clang-format reformat, it
> > really hurt development during the churn. We should somehow manage
> > to do it during a time when there aren't many pending patches in the
> > pipeline. I'm not concerned about git-blame; that has never been a
> > problem after reformats. However, I do not care about indentation
> > nor do I want to spend time on it either way, it has no actual
> > effect on readability and maintainability of code, and consistency
> > outside the file you're in has never mattered to me one bit.
> > 
> > IOW, I'm not opposed to reformats and auto-checking of clang-format
> > (or even hooking it), but I do not see it as a thing with all that
> > great return-of-investment.
> 
> It helps in that you do not need to point those things out in code
> reviews, and that I (and others) won’t even create changes with wrong
> formatting that I’ll need to fix up later on. It’s part of a larger
> story, where I would like to get as much automatic checking of changes
> done before humans start reviewing.

It's also a cultural thing.

Quite a few people seem to take less offense from a "Your formatting is
bad" when the comment comes from a bot than when it comes from a human. 

> One idea could be to introduce this incrementally. Let’s first start
> off with enforcing it for new changes. Then we run it globally over
> the code base shortly before Qt 6.0 is being released. At that time
> merges shouldn’t be as much of a problem (as we’ll probably
> cherry-pick into Qt 5.15) and by then all new changes in Gerrit will
> be properly formatted (due to the earlier hook).

Incrementally sounds good to me.

Still I am a bit of a fence here. So far I've seen a couple of auto-
formatting attempts biting back, so I thinl it would help to convince me
to see the kind of changes that would happen first before deciding
on the global change.

Andre'
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Kari Oikarinen]

On 20.06.2018 09:30, Lars Knoll wrote:
>
>
>> On 19 Jun 2018, at 18:19, Ville Voutilainen 
 wrote:

>>
>> On 19 June 2018 at 19:13, Philippe  wrote:
 For the above reasons I'd lean towards not running it globally and 
just using it

 on new changes.
>>>
>>> +1, based on my clang-format experience on a big application.
>>>
>>> BTW, keep in mind that you can disable clang-format on code 
sections with:

>>>
>>> // clang-format off
>>> // clang-format on
>>
>> When I last experienced a large-scale clang-format reformat, it really
>> hurt development
>> during the churn. We should somehow manage to do it during a time when
>> there aren't
>> many pending patches in the pipeline. I'm not concerned about
>> git-blame; that has never
>> been a problem after reformats. However, I do not care about
>> indentation nor do I want
>> to spend time on it either way, it has no actual effect on readability
>> and maintainability
>> of code, and consistency outside the file you're in has never mattered
>> to me one bit.
>>
>> IOW, I'm not opposed to reformats and auto-checking of clang-format
>> (or even hooking it),
>> but I do not see it as a thing with all that great return-of-investment.
>
> It helps in that you do not need to point those things out in code 
reviews, and that I (and others) won’t even create changes with wrong 
formatting that I’ll need to fix up later on. It’s part of a larger 
story, where I would like to get as much automatic checking of changes 
done before humans start reviewing.

>
> One idea could be to introduce this incrementally. Let’s first start 
off with enforcing it for new changes. Then we run it globally over the 
code base shortly before Qt 6.0 is being released. At that time merges 
shouldn’t be as much of a problem (as we’ll probably cherry-pick into Qt 
5.15) and by then all new changes in Gerrit will be properly formatted 
(due to the earlier hook).

>

I think this is a good approach. It includes a period of time between 
taking the
formatting options into use and applying them globally. There's a good 
chance
that the used clang-format settings may change due to discussion as more 
people

and code are exposed to it. It would be a shame to have to globally reformat
*again* after a short period of time.

(I'm still not sold on the benefit of running the reformatting on 
existing code
outweighs the cost, but the suggested time would probably give the best 
balance

there.)

--
Kari
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Lars Knoll]

> On 19 Jun 2018, at 18:19, Ville Voutilainen  
> wrote:
> 
> On 19 June 2018 at 19:13, Philippe  wrote:
>>> For the above reasons I'd lean towards not running it globally and just 
>>> using it
>>> on new changes.
>> 
>> +1, based on my clang-format experience on a big application.
>> 
>> BTW, keep in mind that you can disable clang-format on code sections with:
>> 
>> // clang-format off
>> // clang-format on
> 
> When I last experienced a large-scale clang-format reformat, it really
> hurt development
> during the churn. We should somehow manage to do it during a time when
> there aren't
> many pending patches in the pipeline. I'm not concerned about
> git-blame; that has never
> been a problem after reformats. However, I do not care about
> indentation nor do I want
> to spend time on it either way, it has no actual effect on readability
> and maintainability
> of code, and consistency outside the file you're in has never mattered
> to me one bit.
> 
> IOW, I'm not opposed to reformats and auto-checking of clang-format
> (or even hooking it),
> but I do not see it as a thing with all that great return-of-investment.

It helps in that you do not need to point those things out in code reviews, and 
that I (and others) won’t even create changes with wrong formatting that I’ll 
need to fix up later on. It’s part of a larger story, where I would like to get 
as much automatic checking of changes done before humans start reviewing.

One idea could be to introduce this incrementally. Let’s first start off with 
enforcing it for new changes. Then we run it globally over the code base 
shortly before Qt 6.0 is being released. At that time merges shouldn’t be as 
much of a problem (as we’ll probably cherry-pick into Qt 5.15) and by then all 
new changes in Gerrit will be properly formatted (due to the earlier hook).

Cheers,
Lars

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Christian Gagneraud]

On 20 June 2018 at 03:33, Allan Sandfeld Jensen  wrote:
> Btw. Just for your information.
>
> I have attached a few random examples of what we can look forward too after
> running an auto-"beautifying" tool over our hand-formated Qt code. And these
> changes are NOT something we can configure out ouf of in clang-format, it is
> just cases where it can't do any better.

You can surround code with "// clang-format off" and "// clang-format on".
We've switched to clang-format in git hooks when clang-6 got out, and
it works well.



> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Ville Voutilainen]

On 19 June 2018 at 19:13, Philippe  wrote:
>> For the above reasons I'd lean towards not running it globally and just 
>> using it
>> on new changes.
>
> +1, based on my clang-format experience on a big application.
>
> BTW, keep in mind that you can disable clang-format on code sections with:
>
> // clang-format off
> // clang-format on

When I last experienced a large-scale clang-format reformat, it really
hurt development
during the churn. We should somehow manage to do it during a time when
there aren't
many pending patches in the pipeline. I'm not concerned about
git-blame; that has never
been a problem after reformats. However, I do not care about
indentation nor do I want
to spend time on it either way, it has no actual effect on readability
and maintainability
of code, and consistency outside the file you're in has never mattered
to me one bit.

IOW, I'm not opposed to reformats and auto-checking of clang-format
(or even hooking it),
but I do not see it as a thing with all that great return-of-investment.
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Philippe]

> For the above reasons I'd lean towards not running it globally and just using 
> it
> on new changes.

+1, based on my clang-format experience on a big application.

BTW, keep in mind that you can disable clang-format on code sections with:

// clang-format off
// clang-format on

Philippe

On Mon, 18 Jun 2018 12:23:53 +0300
Kari Oikarinen  wrote:

> 
> 
> On 18.06.2018 12:04, Frederik Gladhorn wrote:
> 
> 
> Other parts sound good, so I'll just touch on the big question.
> 
>  > And then there is the big question when we run it once over the entire
>  > codebase.
> 
> I'd hesitate to ever run it over the entire codebase.
> 
> * It will ruin plain git blame, since so much will point to that particular
>commit. Yes, you can use `git blame -w` to avoid whitespace changes, but 
> that
>does not catch rewrapped lines.
> 
> * Open changes would need to be rebased on top of it. When would be a good 
> point
>in time with few open changes?
> 
> * Which branch do you run it in? If an early one, there's many merges to do. 
> If
>a late one, all the subsequent merges are tricky.
> 
> It is quite a bit of pain while the benefit isn't that big. Actively worked on
> areas would shape up incrementally anyway and the other areas are not read 
> that
> much, so the damage of inconsistent formatting is limited.
> 
> For the above reasons I'd lean towards not running it globally and just using 
> it
> on new changes.
> 
> -- Kari
> ___
> Development mailing list
> Development@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Thiago Macieira]

On Tuesday, 19 June 2018 02:34:09 PDT Lars Knoll wrote:
> Currently, we only merge from 5.11 to dev, so I hope this won’t be too bad.
> But we could of course also run the tool over both branches.

I have right now 149 changes on top of qtbase's dev branch. For something that 
is ostensibly a whitespace change, I will dedicate no more than 1 minute per 
change to resolve a conflict or 15 minutes total to rebase my branch once it 
goes in.

Any change that times out will be dropped. That includes all the Qt 6 
container work, the un-accepted QtDBus updates (which include a 
QDBusConnection::connect taking PMF), a large refactoring of QFileSystemEngine 
that no one has reviewed yet despite being on Gerrit for over a year, some 
work for CBOR and a few other miscellaneous changes.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Thiago Macieira]

On Monday, 18 June 2018 02:04:33 PDT Frederik Gladhorn wrote:
> as part of the closing ceremony of this year's Qt Contributors' Summit we
> agreed to start using clang-format, to have fewer discussions around coding
> style and rather focus on the actual code.

Are we going to review the resulting format?

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Allan Sandfeld Jensen]

Btw. Just for your information.

I have attached a few random examples of what we can look forward too after 
running an auto-"beautifying" tool over our hand-formated Qt code. And these 
changes are NOT something we can configure out ouf of in clang-format, it is 
just cases where it can't do any better.Before:

// QStringRef <> QByteArray
inline QT_ASCII_CAST_WARN bool operator==(const QStringRef , const 
QByteArray ) { return lhs.compare(rhs) == 0; }
inline QT_ASCII_CAST_WARN bool operator!=(const QStringRef , const 
QByteArray ) { return lhs.compare(rhs) != 0; }
inline QT_ASCII_CAST_WARN bool operator< (const QStringRef , const 
QByteArray ) { return lhs.compare(rhs) <  0; }
inline QT_ASCII_CAST_WARN bool operator> (const QStringRef , const 
QByteArray ) { return lhs.compare(rhs) >  0; }
inline QT_ASCII_CAST_WARN bool operator<=(const QStringRef , const 
QByteArray ) { return lhs.compare(rhs) <= 0; }
inline QT_ASCII_CAST_WARN bool operator>=(const QStringRef , const 
QByteArray ) { return lhs.compare(rhs) >= 0; }

inline QT_ASCII_CAST_WARN bool operator==(const QByteArray , const 
QStringRef ) { return rhs.compare(lhs) == 0; }
inline QT_ASCII_CAST_WARN bool operator!=(const QByteArray , const 
QStringRef ) { return rhs.compare(lhs) != 0; }
inline QT_ASCII_CAST_WARN bool operator< (const QByteArray , const 
QStringRef ) { return rhs.compare(lhs) >  0; }
inline QT_ASCII_CAST_WARN bool operator> (const QByteArray , const 
QStringRef ) { return rhs.compare(lhs) <  0; }
inline QT_ASCII_CAST_WARN bool operator<=(const QByteArray , const 
QStringRef ) { return rhs.compare(lhs) >= 0; }
inline QT_ASCII_CAST_WARN bool operator>=(const QByteArray , const 
QStringRef ) { return rhs.compare(lhs) <= 0; }

After:

// QStringRef <> QByteArray
inline QT_ASCII_CAST_WARN bool operator==(const QStringRef , const 
QByteArray )
{
return lhs.compare(rhs) == 0;
}
inline QT_ASCII_CAST_WARN bool operator!=(const QStringRef , const 
QByteArray )
{
return lhs.compare(rhs) != 0;
}
inline QT_ASCII_CAST_WARN bool operator<(const QStringRef , const 
QByteArray ) { return lhs.compare(rhs) < 0; }
inline QT_ASCII_CAST_WARN bool operator>(const QStringRef , const 
QByteArray ) { return lhs.compare(rhs) > 0; }
inline QT_ASCII_CAST_WARN bool operator<=(const QStringRef , const 
QByteArray )
{
return lhs.compare(rhs) <= 0;
}
inline QT_ASCII_CAST_WARN bool operator>=(const QStringRef , const 
QByteArray )
{
return lhs.compare(rhs) >= 0;
}

inline QT_ASCII_CAST_WARN bool operator==(const QByteArray , const 
QStringRef )
{
return rhs.compare(lhs) == 0;
}
inline QT_ASCII_CAST_WARN bool operator!=(const QByteArray , const 
QStringRef )
{
return rhs.compare(lhs) != 0;
}
inline QT_ASCII_CAST_WARN bool operator<(const QByteArray , const 
QStringRef ) { return rhs.compare(lhs) > 0; }
inline QT_ASCII_CAST_WARN bool operator>(const QByteArray , const 
QStringRef ) { return rhs.compare(lhs) < 0; }
inline QT_ASCII_CAST_WARN bool operator<=(const QByteArray , const 
QStringRef )
{
return rhs.compare(lhs) >= 0;
}
inline QT_ASCII_CAST_WARN bool operator>=(const QByteArray , const 
QStringRef )
{
return rhs.compare(lhs) <= 0;
}


Before:

#define QT_MEMCPY_USHORT(dest, src, length) \
do {  \
/* Duff's device */   \
ushort *_d = (ushort*)(dest); \
const ushort *_s = (const ushort*)(src);\
int n = ((length) + 7) / 8;   \
switch ((length) & 0x07)  \
{ \
case 0: do { *_d++ = *_s++; Q_FALLTHROUGH(); \
case 7:  *_d++ = *_s++; Q_FALLTHROUGH(); \
case 6:  *_d++ = *_s++; Q_FALLTHROUGH(); \
case 5:  *_d++ = *_s++; Q_FALLTHROUGH(); \
case 4:  *_d++ = *_s++; Q_FALLTHROUGH(); \
case 3:  *_d++ = *_s++; Q_FALLTHROUGH(); \
case 2:  *_d++ = *_s++; Q_FALLTHROUGH(); \
case 1:  *_d++ = *_s++; \
} while (--n > 0);\
} \
} while (false)

inline ushort qConvertRgb32To16(uint c)
{
   return (((c) >> 3) & 0x001f)
| (((c) >> 5) & 0x07e0)
| (((c) >> 8) & 0xf800);
}

inline QRgb qConvertRgb16To32(uint c)
{
return 0xff00
| c) << 3) & 0xf8) | (((c) >> 2) & 0x7))
| c) << 5) & 0xfc00) | (((c) >> 1) & 0x300))
| c) << 8) & 0xf8) | (((c) << 3) & 0x7));
}


After:

#define QT_MEMCPY_USHORT(dest, src, length) 
   \
do {
   \
/* Duff's device */ 
   \
ushort *_d = (ushort *)(dest);  
   \
const ushort *_s = (const ushort 

Re: [Development] clang-format

[Liang Qi]

> On 19 Jun 2018, at 09:58, Joerg Bornemann  wrote:
> 
>> * Which branch do you run it in? If an early one, there's many merges to do. 
>> If
>>   a late one, all the subsequent merges are tricky.
> 
> Our commit policy suggests that the right branch would be dev.
> You're right that the merges will be harder. What's the opinion of our merge 
> master?
> 
> 

We will have 5.11 for a while, perhaps a 5.11.2 after summer at least. So 
better to do it in 5.11, and after it got merged in dev, then do it in dev 
again. That should help merge a lot.

— Liang
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Allan Sandfeld Jensen]

On Montag, 18. Juni 2018 11:23:53 CEST Kari Oikarinen wrote:
> On 18.06.2018 12:04, Frederik Gladhorn wrote:
> 
> 
> Other parts sound good, so I'll just touch on the big question.
> 
>  > And then there is the big question when we run it once over the entire
>  > codebase.
> 
> I'd hesitate to ever run it over the entire codebase.
> 
I would prefer the same. Especially since the first few iterations of the 
formating are not going to be good. If the goal is to minimize the amount of 
back-and-forth during code-review, then we only need to apply it to new 
patches.

And while there are ways around the noise generated, there is really no good 
reason to generate all that noise and auto-uglify the code.

'Allan


___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Lars Knoll]

> On 19 Jun 2018, at 09:58, Joerg Bornemann  wrote:
> 
> On 06/18/2018 11:23 AM, Kari Oikarinen wrote:
> 
>> I'd hesitate to ever run it over the entire codebase.
>> * It will ruin plain git blame, since so much will point to that particular
>>   commit. Yes, you can use `git blame -w` to avoid whitespace changes, but 
>> that
>>   does not catch rewrapped lines.
> 
> In my experience, plain "git blame" is not enough in most cases anyway. You 
> go back in history and skip commits you're not interested in. One cleanup 
> commit more to skip doesn't really hurt.

I agree, and Qt Creator makes that actually pretty easy in practice.
> 
>> * Open changes would need to be rebased on top of it. When would be a good 
>> point
>>   in time with few open changes?
> 
> Summer holidays? :)
> I guess there's no perfect point in time. Some open changes will have to be 
> rebased. This also happens if someone else is working in the area your commit 
> touched. Just a minor annoyance IMO.

Some time during summer is fine for me. But we’ll need to have the infra in 
place, so that formatting is enforced for new changes.
> 
>> * Which branch do you run it in? If an early one, there's many merges to do. 
>> If
>>   a late one, all the subsequent merges are tricky.
> 
> Our commit policy suggests that the right branch would be dev.
> You're right that the merges will be harder. What's the opinion of our merge 
> master?

Currently, we only merge from 5.11 to dev, so I hope this won’t be too bad. But 
we could of course also run the tool over both branches.

Cheers,
Lars

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Liang Qi]

On Mon, 18 Jun 2018 at 11:23, Kari Oikarinen  wrote:

> On 18.06.2018 12:04, Frederik Gladhorn wrote:
> 
>
> Other parts sound good, so I'll just touch on the big question.
>
>  > And then there is the big question when we run it once over the entire
>  > codebase.
>
> I'd hesitate to ever run it over the entire codebase.
>
> * It will ruin plain git blame, since so much will point to that particular
>commit. Yes, you can use `git blame -w` to avoid whitespace changes,
> but that
>does not catch rewrapped lines.
>
> git-hyper-blame ?
https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html

--Liang
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format

[Kari Oikarinen]

On 18.06.2018 12:04, Frederik Gladhorn wrote:


Other parts sound good, so I'll just touch on the big question.

> And then there is the big question when we run it once over the entire
> codebase.

I'd hesitate to ever run it over the entire codebase.

* It will ruin plain git blame, since so much will point to that particular
  commit. Yes, you can use `git blame -w` to avoid whitespace changes, 
but that

  does not catch rewrapped lines.

* Open changes would need to be rebased on top of it. When would be a 
good point

  in time with few open changes?

* Which branch do you run it in? If an early one, there's many merges to 
do. If

  a late one, all the subsequent merges are tricky.

It is quite a bit of pain while the benefit isn't that big. Actively 
worked on
areas would shape up incrementally anyway and the other areas are not 
read that

much, so the damage of inconsistent formatting is limited.

For the above reasons I'd lean towards not running it globally and just 
using it

on new changes.

--
Kari
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

[Development] clang-format

[Frederik Gladhorn]

Hi all,

as part of the closing ceremony of this year's Qt Contributors' Summit we 
agreed to start using clang-format, to have fewer discussions around coding 
style and rather focus on the actual code.

I have not yet thought about all angles, how to best implement this, here are 
some notes:

We have a clang-format file in qtrepotools. You can use it today, simply make 
sure it's in any parent directory of the files you are editing. I'd actually 
propose simply moving this into the root directory of qt5.git.
https://code.qt.io/cgit/qt/qtrepotools.git/tree/config/_clang-format

If you want to clean one file:
clang-format -i myfile.cpp/h

To clean a commit (only modifies the working tree):
git clang-format


Getting clang-format seems easy enough:
On macOS: brew install clang-format
Linux: install clang-format
Windows: it comes with normal clang

Then there is the tooling/workflow perspective. Creator and other IDEs have 
support (you may need to enable the beautifier plugin in about plugins).
I imagine we add this to the sanity bot ("git clang-format --diff -q" should 
return empty, otherwise post a message).

Local hooks are basically the same, any ideas how to best set up the git hooks 
appreciated :)

And then there is the big question when we run it once over the entire 
codebase.

Cheers,
Frederik



___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format config file.

[Matthew Woehlke]

On 2016-06-30 06:52, Olivier Goffart wrote:
> [clang-format] follows the style closely, but there are some cases
> that might not be following existing practices. The question is if we
> want to try to fix clang-format to be able to cope with them, or
> simply adapt the coding style to fit clang-format config.
> 
> Some of the things that are not possible with clang-format if to have all the 
> function in one line like we sometimes have in our headers:
> 
>   inline const QString operator+(const QString , const QString )
>   { QString t(s1); t += s2; return t; }

FWIW, I vote for fixing clang-format... because Qt is probably not the
only project out there that would prefer to use such format :-).

-- 
Matthew

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format config file.

[Olivier Goffart]

On Donnerstag, 30. Juni 2016 14:26:34 CEST Lorenz Haas wrote:
> Hi,
> 
> > The style disabled any re-wraping of the comments, because the qdoc rules
> > are not encoded in clang-format. So comments will not be touched.
> 
> just for the record: clang-format can exclude specific comment types
> from re-wrapping. Thus if you/we do not mind a "either re-wrap all
> comments or leave them all as they are" rule, CommentPragmas could be
> used, e.g.:
> 
> CommentPragmas: '^!|^:|^=|^~'

Thanks for your suggestion. I updated the file.
(I used only ! and :, I don't know what = or ~ was refering to)

-- 
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format config file.

[Lorenz Haas]

Hi,

> The style disabled any re-wraping of the comments, because the qdoc rules are
> not encoded in clang-format. So comments will not be touched.

just for the record: clang-format can exclude specific comment types
from re-wrapping. Thus if you/we do not mind a "either re-wrap all
comments or leave them all as they are" rule, CommentPragmas could be
used, e.g.:

CommentPragmas: '^!|^:|^=|^~'

Cheers
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format config file.

[Olivier Goffart]

On Donnerstag, 30. Juni 2016 13:01:30 CEST Dominik Holland wrote:
> Hi Olivier,
> 
> Am 06/30/2016 um 12:52 PM schrieb Olivier Goffart:
> > Hi,
> > 
> > I have made a clang-format config file. I believe it should go into the
> > qtrepotools repository, so I made this pull request:
> > https://codereview.qt-project.org/163941
> > 
> > You can download this file from there and put it in the parent directory
> > of your qt repositories.  (clang-format search all the parent folders for
> > a .clang-format file)
> > 
> > We don't want to reformat existing files, but this help to format only the
> > lines that are touched in a given commit.  The git-clang-format tool can
> > be used for that.
> > 
> > The git-clang-format tool is part of the clang. So it is probably packaged
> > in your distribution as part of clang.
> > 
> > To use it, simply commit as before, but before pushing, run:
> >   git-clang-format HEAD^
> >   git diff
> >   # look at the changes made by clang-format, possibly retify some of it
> >   git commit -a --amend
> > 
> > Then you can push.
> > 
> > If you want to run git-clang-format on several commit separately, the
> > --exec feature of git rebase comes handy:
> >   git rebase -i --exec "git-clang-format HEAD^"
> > 
> > The rebase will stop if clang-format does changes so you can apply the
> > changes to that last commit and continue with git rebase --continue
> > 
> > ⁂
> > 
> > The conf file is based on the WebKit style which is the closest builtin
> > style to Qt. Then it was adjusted to fit better to the existing Qt style.
> > 
> > It follows the style closely, but there are some cases that might not be
> > following existing practices. The question is if we want to try to fix
> > clang- format to be able to cope with them, or simply adapt the coding
> > style to fit clang-format config.
> 
> I think it would be cool to have a built-in Qt config, as it would be
> very convenient also for other developers which work with Qt.
> 
> > Some of the things that are not possible with clang-format if to have all
> > the> 
> > function in one line like we sometimes have in our headers:
> >   inline const QString operator+(const QString , const QString )
> >   { QString t(s1); t += s2; return t; }
> > 
> > The style disabled any re-wraping of the comments, because the qdoc rules
> > are not encoded in clang-format. So comments will not be touched.
> > 
> > But in general, I have been trying it on a few commits and it works quite
> > well. You should try it as well and report the problems so they can be
> > addressed.
> 
> thx for uploading this.
> 
> We used clang-format in one of our projects and what we've seen is that
> the clang-format version used for the formatting is important and can
> lead to different results.
> 
> Which version did you use ?

I was using clang 3.8.


> Debian(unstable) is currently offering the following versions for me:
> 
> clang-format - Tool to format C/C++/Obj-C code
> clang-format-3.5 - Tool to format C/C++/Obj-C code
> clang-format-3.6 - Tool to format C/C++/Obj-C code
> clang-format-3.7 - Tool to format C/C++/Obj-C code
> clang-format-3.8 - Tool to format C/C++/Obj-C code
> clang-format-3.9 - Tool to format C/C++/Obj-C code
> clang-format-3.4 - Tool to format C/C++/Obj-C code
> 
> If the version is not packaged for your distribution you might be able
> to download a binary version here: http://llvm.org/releases/download.html
> 
> Dominik

-- 
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Re: [Development] clang-format config file.

[Dominik Holland]

Hi Olivier,

Am 06/30/2016 um 12:52 PM schrieb Olivier Goffart:
> Hi,
> 
> I have made a clang-format config file. I believe it should go into the 
> qtrepotools repository, so I made this pull request:
> https://codereview.qt-project.org/163941
> 
> You can download this file from there and put it in the parent directory of 
> your qt repositories.  (clang-format search all the parent folders for a 
> .clang-format file)
> 
> We don't want to reformat existing files, but this help to format only the 
> lines that are touched in a given commit.  The git-clang-format tool can be 
> used for that.
> 
> The git-clang-format tool is part of the clang. So it is probably packaged in 
> your distribution as part of clang.
> 
> To use it, simply commit as before, but before pushing, run:
> 
>   git-clang-format HEAD^
>   git diff  
>   # look at the changes made by clang-format, possibly retify some of it
>   git commit -a --amend
> 
> Then you can push.
> 
> If you want to run git-clang-format on several commit separately, the --exec 
> feature of git rebase comes handy:
> 
>   git rebase -i --exec "git-clang-format HEAD^"
> 
> The rebase will stop if clang-format does changes so you can apply the 
> changes 
> to that last commit and continue with git rebase --continue
> 
> ⁂
> 
> The conf file is based on the WebKit style which is the closest builtin style 
> to Qt. Then it was adjusted to fit better to the existing Qt style.
> 
> It follows the style closely, but there are some cases that might not be 
> following existing practices. The question is if we want to try to fix clang-
> format to be able to cope with them, or simply adapt the coding style to fit 
> clang-format config.

I think it would be cool to have a built-in Qt config, as it would be
very convenient also for other developers which work with Qt.

> 
> Some of the things that are not possible with clang-format if to have all the 
> function in one line like we sometimes have in our headers:
> 
>   inline const QString operator+(const QString , const QString )
>   { QString t(s1); t += s2; return t; }
> 
> 
> The style disabled any re-wraping of the comments, because the qdoc rules are 
> not encoded in clang-format. So comments will not be touched.
> 
> But in general, I have been trying it on a few commits and it works quite 
> well. You should try it as well and report the problems so they can be 
> addressed.
> 


thx for uploading this.

We used clang-format in one of our projects and what we've seen is that
the clang-format version used for the formatting is important and can
lead to different results.

Which version did you use ?

Debian(unstable) is currently offering the following versions for me:

clang-format - Tool to format C/C++/Obj-C code
clang-format-3.5 - Tool to format C/C++/Obj-C code
clang-format-3.6 - Tool to format C/C++/Obj-C code
clang-format-3.7 - Tool to format C/C++/Obj-C code
clang-format-3.8 - Tool to format C/C++/Obj-C code
clang-format-3.9 - Tool to format C/C++/Obj-C code
clang-format-3.4 - Tool to format C/C++/Obj-C code

If the version is not packaged for your distribution you might be able
to download a binary version here: http://llvm.org/releases/download.html

Dominik

-- 
Dominik Holland
SENIOR SOFTWARE ENGINEER

Pelagicore AG
Balanstr. 55, 81541 Munich, Germany
+49 (0)171 760 25 96
dominik.holl...@pelagicore.com
www.pelagicore.com

___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

[Development] clang-format config file.

[Olivier Goffart]

Hi,

I have made a clang-format config file. I believe it should go into the 
qtrepotools repository, so I made this pull request:
https://codereview.qt-project.org/163941

You can download this file from there and put it in the parent directory of 
your qt repositories.  (clang-format search all the parent folders for a 
.clang-format file)

We don't want to reformat existing files, but this help to format only the 
lines that are touched in a given commit.  The git-clang-format tool can be 
used for that.

The git-clang-format tool is part of the clang. So it is probably packaged in 
your distribution as part of clang.

To use it, simply commit as before, but before pushing, run:

  git-clang-format HEAD^
  git diff  
  # look at the changes made by clang-format, possibly retify some of it
  git commit -a --amend

Then you can push.

If you want to run git-clang-format on several commit separately, the --exec 
feature of git rebase comes handy:

  git rebase -i --exec "git-clang-format HEAD^"

The rebase will stop if clang-format does changes so you can apply the changes 
to that last commit and continue with git rebase --continue

⁂

The conf file is based on the WebKit style which is the closest builtin style 
to Qt. Then it was adjusted to fit better to the existing Qt style.

It follows the style closely, but there are some cases that might not be 
following existing practices. The question is if we want to try to fix clang-
format to be able to cope with them, or simply adapt the coding style to fit 
clang-format config.

Some of the things that are not possible with clang-format if to have all the 
function in one line like we sometimes have in our headers:

  inline const QString operator+(const QString , const QString )
  { QString t(s1); t += s2; return t; }


The style disabled any re-wraping of the comments, because the qdoc rules are 
not encoded in clang-format. So comments will not be touched.

But in general, I have been trying it on a few commits and it works quite 
well. You should try it as well and report the problems so they can be 
addressed.

-- 
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development