Discussion:
[Development] Resolving coding style contentions
Sze Howe Koh
2018-11-18 13:30:26 UTC
Permalink
Hi all,

I can't find a final decision on this topic:
http://lists.qt-project.org/pipermail/development/2016-June/026058.html

The arguments from both sides were numerous (browse through
http://lists.qt-project.org/pipermail/development/2016-June/thread.html ).
I would like to avoid a re-hash of the same arguments.

I'd just like to know: Did the Qt Project ever reach a decision? If not,
how can we reach one?


Thanks and regards,
Sze-Howe
Thiago Macieira
2018-11-18 19:49:09 UTC
Permalink
On Sunday, 18 November 2018 05:30:26 PST Sze Howe Koh wrote:
> I'd just like to know: Did the Qt Project ever reach a decision? If not,
> how can we reach one?

I don't think we did. Right now, it's up to the maintainer's discretion.

QtCore opts into the rule: newlines are whitespace.

Corollary: you can only insert a newline where a space is allowed by the
coding convention. And since our coding conventions are that commas and semi-
colons have no space to the left, you can't insert a newline there either.

Exception: complex #if, as in the declaration of qCompilerCpuFeatures in
qsimd_p.h and qsimd_x86_p.h:

static const quint64 qCompilerCpuFeatures = 0
#if defined __ARM_NEON__
| CpuFeatureNEON
#endif
#if defined __ARM_FEATURE_CRC32
| CpuFeatureCRC32
#endif
#if defined __mips_dsp
| CpuFeatureDSP
#endif
#if defined __mips_dspr2
| CpuFeatureDSPR2
#endif
;

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Edward Welbourne
2018-11-19 10:34:09 UTC
Permalink
On Sunday, 18 November 2018 05:30:26 PST Sze Howe Koh wrote:
>> I'd just like to know: Did the Qt Project ever reach a decision? If not,
>> how can we reach one?

(Linked to the discussion of constructor initializers like

MyClass(type param)
: MyBase(param)
, myMember(param)

using comma at start of line, for anyone who hasn't followed the link.)

Thiago Macieira (18 November 2018 20:49) replied
> I don't think we did. Right now, it's up to the maintainer's discretion.

My (highly unreliable) memory concurs.

> QtCore opts into the rule: newlines are whitespace.

(I wonder if we'll ever get round to renaming that character class
"spacing characters" - I've been using blackspace in my reverse-video
emacs windows for a quarter century now; and, before that, I used quite
a lot of greenspace. But web-browsers insist on it being white.)

> Corollary: you can only insert a newline where a space is allowed by the
> coding convention.

I note a glaring exception to that: after the opening parenthesis of
a parameter list, if the line would otherwise be too long:

auto variable = QCharacteristicallyVerboseClassName::characteristicallyLongFunctionName(
firstParameter, secondParameter, thirdParameter, fourthParameter);

is tolerated (well, maybe not the naming ...), but a space *without*
the line-break between open-paren and firstParameter is forbidden.

It remains that we got no consensus on the newline-comma pattern
for classes; it mixes virtues and vices. Some maintainers will let you
do that. Some won't. This reviewer has mixed feelings about it.

> And since our coding conventions are that commas and semi-
> colons have no space to the left, you can't insert a newline there either.
>
> Exception: complex #if, as in the declaration of qCompilerCpuFeatures in
> qsimd_p.h and qsimd_x86_p.h:
>
> static const quint64 qCompilerCpuFeatures = 0
> #if defined __ARM_NEON__
> | CpuFeatureNEON
> #endif
> #if defined __ARM_FEATURE_CRC32
> | CpuFeatureCRC32
> #endif
> #if defined __mips_dsp
> | CpuFeatureDSP
> #endif
> #if defined __mips_dspr2
> | CpuFeatureDSPR2
> #endif
> ;

(The "no space allowed" point is just at the end, before the semicolon.)
I see this as more an example of the one rule to rule them all:
* When strictly following a rule makes your code look bad, feel free to break it
https://wiki.qt.io/Qt_Coding_Style#General_exception

It remains that this can apply equally justly to the newline-comma
case (at least) when the last member of the class is subject to #if-ery,

MyClass(type param)
: MyBase(param), myMember(param)
#if QT_CONFIG(myfeature)
, myFeatureSpecificMember (param)
#endif

which, all things considered, is the case that I believe motivated the
newline-comma pattern among various peers I've known who liked it.

Eddy.
Thiago Macieira
2018-11-19 16:37:07 UTC
Permalink
On Monday, 19 November 2018 02:34:09 PST Edward Welbourne wrote:
> I note a glaring exception to that: after the opening parenthesis of
> a parameter list, if the line would otherwise be too long:
>
> auto variable =
> QCharacteristicallyVerboseClassName::characteristicallyLongFunctionName(
> firstParameter, secondParameter, thirdParameter, fourthParameter);
>
> is tolerated (well, maybe not the naming ...), but a space *without*
> the line-break between open-paren and firstParameter is forbidden.

Right, when your line would be too long, you need to break *somewhere*. Take
this example of a long sequence with no spacing:


QObject::tr("%1%2%3%4").arg(someFunction()).arg(otherFunction()).arg(thirdFunction()).arg(lookMaNoSpaces());

This has no space at all, so much that not even the email composer can break
it anywhere.

In this case, it's often that we break before the dot, so the next line starts
with punctuation.

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Ville Voutilainen
2018-11-19 20:03:06 UTC
Permalink
On Mon, 19 Nov 2018 at 18:37, Thiago Macieira <***@intel.com> wrote:
>
> On Monday, 19 November 2018 02:34:09 PST Edward Welbourne wrote:
> > I note a glaring exception to that: after the opening parenthesis of
> > a parameter list, if the line would otherwise be too long:
> >
> > auto variable =
> > QCharacteristicallyVerboseClassName::characteristicallyLongFunctionName(
> > firstParameter, secondParameter, thirdParameter, fourthParameter);
> >
> > is tolerated (well, maybe not the naming ...), but a space *without*
> > the line-break between open-paren and firstParameter is forbidden.
>
> Right, when your line would be too long, you need to break *somewhere*. Take
> this example of a long sequence with no spacing:
>
>
> QObject::tr("%1%2%3%4").arg(someFunction()).arg(otherFunction()).arg(thirdFunction()).arg(lookMaNoSpaces());
>
> This has no space at all, so much that not even the email composer can break
> it anywhere.
>
> In this case, it's often that we break before the dot, so the next line starts
> with punctuation.

I personally tend to split such things after an opening parenthesis.
Getting back to allowing ctor-initializers to be written
with a comma starting a line, I think we should just allow it; the
benefit of not having noise in a diff seems to outweigh
the minor aesthetics of it.
Thiago Macieira
2018-11-19 20:41:46 UTC
Permalink
On Monday, 19 November 2018 12:03:06 PST Ville Voutilainen wrote:
> I personally tend to split such things after an opening parenthesis.
> Getting back to allowing ctor-initializers to be written
> with a comma starting a line, I think we should just allow it; the
> benefit of not having noise in a diff seems to outweigh
> the minor aesthetics of it.

It is allowed, unless the maintainer objects to it.

I object to it in QtCore.

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Thiago Macieira
2018-11-19 20:55:41 UTC
Permalink
On Monday, 19 November 2018 12:41:46 PST Thiago Macieira wrote:
> It is allowed, unless the maintainer objects to it.
>
> I object to it in QtCore.

I also insist that the opening brace in non-nested classes, enums and in any
functions be placed in a new line (unless the entire function is in one line)

bool X::foo() { return m_value; } // ok

class Foo { // not ok
public:
bool foo() const { // not ok
return m_value;
}
};

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Ville Voutilainen
2018-11-19 20:59:24 UTC
Permalink
On Mon, 19 Nov 2018 at 22:41, Thiago Macieira <***@intel.com> wrote:
>
> On Monday, 19 November 2018 12:03:06 PST Ville Voutilainen wrote:
> > I personally tend to split such things after an opening parenthesis.
> > Getting back to allowing ctor-initializers to be written
> > with a comma starting a line, I think we should just allow it; the
> > benefit of not having noise in a diff seems to outweigh
> > the minor aesthetics of it.
>
> It is allowed, unless the maintainer objects to it.
>
> I object to it in QtCore.

I'm suggesting that you stop objecting to it.
Lars Knoll
2018-11-20 08:28:49 UTC
Permalink
> On 19 Nov 2018, at 21:59, Ville Voutilainen <***@gmail.com> wrote:
>
> On Mon, 19 Nov 2018 at 22:41, Thiago Macieira <***@intel.com> wrote:
>>
>> On Monday, 19 November 2018 12:03:06 PST Ville Voutilainen wrote:
>>> I personally tend to split such things after an opening parenthesis.
>>> Getting back to allowing ctor-initializers to be written
>>> with a comma starting a line, I think we should just allow it; the
>>> benefit of not having noise in a diff seems to outweigh
>>> the minor aesthetics of it.
>>
>> It is allowed, unless the maintainer objects to it.
>>
>> I object to it in QtCore.
>
> I'm suggesting that you stop objecting to it.

I suggest that we stop arguing about coding styles by defining one through a tool (aka clang-format and one format file for all of Qt). I know that one will never be perfect for all cases (and that you can always find corner cases where manual formatting might be slightly better), but consistency and finally stopping to argue about coding style esp. in code reviews (by automating things) is more important.

Cheers,
Lars
Matthew Woehlke
2018-11-21 17:50:59 UTC
Permalink
On 20/11/2018 03.28, Lars Knoll wrote:
> I suggest that we stop arguing about coding styles by defining one
> through a tool (aka clang-format and one format file for all of Qt).

Ugh... clang-format is only okay if you want LLVM's code style, and it
does obnoxious things like forcibly reflowing your arguments to
"maximize space usage" (even when it results in aesthetically worse
code) if you enable any line length options.

Personally, I would encourage looking at uncrustify instead; it can
apply as much or as little formatting as you want, for virtually
whatever format you want. (Astyle was nice, once upon a time, but is
basically dead AFAIK.)

(Disclaimer: I contribute to uncrustify, so I'm biased :-).)

--
Matthew
Elvis Stansvik
2018-11-21 18:40:41 UTC
Permalink
Den ons 21 nov. 2018 kl 18:52 skrev Matthew Woehlke <***@gmail.com>:
>
> On 20/11/2018 03.28, Lars Knoll wrote:
> > I suggest that we stop arguing about coding styles by defining one
> > through a tool (aka clang-format and one format file for all of Qt).
>
> Ugh... clang-format is only okay if you want LLVM's code style, and it
> does obnoxious things like forcibly reflowing your arguments to
> "maximize space usage" (even when it results in aesthetically worse
> code) if you enable any line length options.
>
> Personally, I would encourage looking at uncrustify instead; it can
> apply as much or as little formatting as you want, for virtually
> whatever format you want. (Astyle was nice, once upon a time, but is
> basically dead AFAIK.)

Perfect. So instead of bikeshedding code style, we've progressed to
bikeshedding code stylers :) (100% kidding).

Elvis

>
> (Disclaimer: I contribute to uncrustify, so I'm biased :-).)
>
> --
> Matthew
> _______________________________________________
> Development mailing list
> ***@lists.qt-project.org
> https://lists.qt-project.org/listinfo/development
Ville Voutilainen
2018-11-21 19:13:08 UTC
Permalink
On Wed, 21 Nov 2018 at 20:42, Elvis Stansvik <***@gmail.com> wrote:
> Perfect. So instead of bikeshedding code style, we've progressed to
> bikeshedding code stylers :) (100% kidding).

We're hopefully not going to bikeshed code style after we've had the
Final Battle on it, aka what the rules for whatever formatting tool
are. :D
(and sure, it's possible that we're going to bikeshed the tool)
But at that point I'd expect that we _are_ going to argue about code
styles for hopefully One Last Time. :)
Ville Voutilainen
2018-11-21 19:11:04 UTC
Permalink
On Wed, 21 Nov 2018 at 19:51, Matthew Woehlke <***@gmail.com> wrote:
>
> On 20/11/2018 03.28, Lars Knoll wrote:
> > I suggest that we stop arguing about coding styles by defining one
> > through a tool (aka clang-format and one format file for all of Qt).
>
> Ugh... clang-format is only okay if you want LLVM's code style, and it

It has been used successfully in projects that don't use LLVM's style.
Marco Bubke
2018-11-21 19:19:01 UTC
Permalink
Some people use clang format already for Qt Creator. My experience so far is very positive. Actually the best part is that there is no time any more spend on formatting, especially for chats about very personal aesthetics.

________________________________
From: Development <development-***@lists.qt-project.org> on behalf of Ville Voutilainen <***@gmail.com>
Sent: Wednesday, November 21, 2018 8:11:04 PM
To: Matthew Woehlke
Cc: Thiago Macieira; Qt development mailing list
Subject: Re: [Development] Resolving coding style contentions

On Wed, 21 Nov 2018 at 19:51, Matthew Woehlke <***@gmail.com> wrote:
>
> On 20/11/2018 03.28, Lars Knoll wrote:
> > I suggest that we stop arguing about coding styles by defining one
> > through a tool (aka clang-format and one format file for all of Qt).
>
> Ugh... clang-format is only okay if you want LLVM's code style, and it

It has been used successfully in projects that don't use LLVM's style.
Matthew Woehlke
2018-11-21 19:27:14 UTC
Permalink
On 21/11/2018 14.11, Ville Voutilainen wrote:
> On Wed, 21 Nov 2018 at 19:51, Matthew Woehlke wrote:
>> On 20/11/2018 03.28, Lars Knoll wrote:
>>> I suggest that we stop arguing about coding styles by defining one
>>> through a tool (aka clang-format and one format file for all of Qt).
>>
>> Ugh... clang-format is only okay if you want LLVM's code style, and it
>
> It has been used successfully in projects that don't use LLVM's style.

Really? When I looked into using it for one of my work projects, it was
hopelessly unconfigurable and was going to necessitate switching to a
significantly different code style than what most of the code was using.

Okay, pedantically, LLVM *or Google*, or I think there are maybe 1-2
other styles it supports... However, if what you want deviates from
those presents, your choices are either a) too bad, change the code
style of your entire source base, or b) don't use clang-format.
Clang-format simply doesn't have the configurability to support anything
beyond its "blessed styles" with a very small number of possible tweaks.

I haven't looked into whether Qt's is one of those "blessed styles". If
it is... fine, whatever. If it isn't, uncrustify can almost certainly
produce whatever style Qt wants, which is a claim clang-format most
certainly can't make.

(Besides, for me, the whole reflow thing was sort of a deal-killer.)

BTW, does clang-format know how to correctly style old-style SIGNALS/SLOTS?

--
Matthew
Elvis Stansvik
2018-11-21 19:35:09 UTC
Permalink
Den ons 21 nov. 2018 kl 20:28 skrev Matthew Woehlke <***@gmail.com>:
>
> On 21/11/2018 14.11, Ville Voutilainen wrote:
> > On Wed, 21 Nov 2018 at 19:51, Matthew Woehlke wrote:
> >> On 20/11/2018 03.28, Lars Knoll wrote:
> >>> I suggest that we stop arguing about coding styles by defining one
> >>> through a tool (aka clang-format and one format file for all of Qt).
> >>
> >> Ugh... clang-format is only okay if you want LLVM's code style, and it
> >
> > It has been used successfully in projects that don't use LLVM's style.
>
> Really? When I looked into using it for one of my work projects, it was
> hopelessly unconfigurable and was going to necessitate switching to a
> significantly different code style than what most of the code was using.

We use the following .clang-format at work:

Standard: Cpp11
ColumnLimit: 0
SortIncludes: true
IncludeIsMainRegex: "(Test)?$"
BasedOnStyle: WebKit
PointerAlignment: Right
AlignAfterOpenBracket: Align
BreakBeforeBraces: Custom
BraceWrapping:
AfterClass: true
AfterControlStatement: false
AfterEnum: false
AfterFunction: true
AfterNamespace: false
AfterStruct: true
BeforeCatch: false
BeforeElse: false
IndentBraces: false
ForEachMacros: [ foreach, Q_FOREACH, BOOST_FOREACH ]

I'd say it's Qt/KDE-ish. Certainly not perfect by any means, and yea
clang-format can do some quite annoying stuff (the re-flowing you
mention).

But for the most part, we just get on with it and live with some less
then perfect formatting for some things. All in all it's been a real
relief for all involved since we started mandating all code to be
auto-formatted, and saves a lot of time at review.

I'll definitely look at uncrustify though, thanks for the tip (and for
building it!).

Elvis

>
> Okay, pedantically, LLVM *or Google*, or I think there are maybe 1-2
> other styles it supports... However, if what you want deviates from
> those presents, your choices are either a) too bad, change the code
> style of your entire source base, or b) don't use clang-format.
> Clang-format simply doesn't have the configurability to support anything
> beyond its "blessed styles" with a very small number of possible tweaks.
>
> I haven't looked into whether Qt's is one of those "blessed styles". If
> it is... fine, whatever. If it isn't, uncrustify can almost certainly
> produce whatever style Qt wants, which is a claim clang-format most
> certainly can't make.
>
> (Besides, for me, the whole reflow thing was sort of a deal-killer.)
>
> BTW, does clang-format know how to correctly style old-style SIGNALS/SLOTS?
>
> --
> Matthew
> _______________________________________________
> Development mailing list
> ***@lists.qt-project.org
> https://lists.qt-project.org/listinfo/development
Matthew Woehlke
2018-11-21 19:57:30 UTC
Permalink
On 21/11/2018 14.35, Elvis Stansvik wrote:
> Den ons 21 nov. 2018 kl 20:28 skrev Matthew Woehlke:
> We use the following .clang-format at work:
> [snip]
>
> I'd say it's Qt/KDE-ish. Certainly not perfect by any means, and yea
> clang-format can do some quite annoying stuff (the re-flowing you
> mention).
>
> But for the most part, we just get on with it and live with some less
> then perfect formatting for some things. All in all it's been a real
> relief for all involved since we started mandating all code to be
> auto-formatted, and saves a lot of time at review.

Sure. I'm not against automatic styling tools, by any means! I just
wanted to point out that a) clang-format isn't the only plausible
option, and b) clang-format has shortcomings, especially if the style
you want isn't quite the style *it* wants.

For a large project, especially one with a large group of contributors,
forcibly changing the existing code style can be... problematic.

> I'll definitely look at uncrustify though, thanks for the tip (and for
> building it!).

I'm not the author; just one of the current active contributors.

However, I do recommend it. It has some faults (but it's also under
active development and patches are encouraged), however I think it is a
better tool than clang-format in some ways.

Clang-format is a tool for dictating *to* you how to format your code.
If you disagree with it, you are wrong, because clang-format is always
right.

Uncrustify is a tool for helping to enforce a style that *you* choose.
It will enforce as much or as little as you like with whatever
subtleties you want.

(Uncrustify is also stupidly easy and quick to build compared to clang,
which is something to keep in mind if you're expecting developers to run
it locally.)

--
Matthew
Philippe
2018-11-22 06:54:59 UTC
Permalink
I have been using various c++ formatters since 1995 (first one was C-Vision from Gimpel, pretty
good at that time).
But since I tried clang-format, I don't look elsewhere anymore.
Why? Because its parser is unmatched, and with today's possible
complicated C++ constructs, this is a must-have.

Yes, it is not as much customizable as some other tools, and I can regret
that sometimes, but reliability is prime for me.

For the small story: I use different clang-format settings for class
declarations and the rest of the code.
To achieve this, I have a simple custom C++ parser that identifies code
sections with class declarations: these source code sections are
formatted with different settings (clang-format can be executed to
format custom code ranges).

Philippe

On Wed, 21 Nov 2018 14:57:30 -0500
Matthew Woehlke <***@gmail.com> wrote:

> On 21/11/2018 14.35, Elvis Stansvik wrote:
> > Den ons 21 nov. 2018 kl 20:28 skrev Matthew Woehlke:
> > We use the following .clang-format at work:
> > [snip]
> >
> > I'd say it's Qt/KDE-ish. Certainly not perfect by any means, and yea
> > clang-format can do some quite annoying stuff (the re-flowing you
> > mention).
> >
> > But for the most part, we just get on with it and live with some less
> > then perfect formatting for some things. All in all it's been a real
> > relief for all involved since we started mandating all code to be
> > auto-formatted, and saves a lot of time at review.
>
> Sure. I'm not against automatic styling tools, by any means! I just
> wanted to point out that a) clang-format isn't the only plausible
> option, and b) clang-format has shortcomings, especially if the style
> you want isn't quite the style *it* wants.
>
> For a large project, especially one with a large group of contributors,
> forcibly changing the existing code style can be... problematic.
>
> > I'll definitely look at uncrustify though, thanks for the tip (and for
> > building it!).
>
> I'm not the author; just one of the current active contributors.
>
> However, I do recommend it. It has some faults (but it's also under
> active development and patches are encouraged), however I think it is a
> better tool than clang-format in some ways.
>
> Clang-format is a tool for dictating *to* you how to format your code.
> If you disagree with it, you are wrong, because clang-format is always
> right.
>
> Uncrustify is a tool for helping to enforce a style that *you* choose.
> It will enforce as much or as little as you like with whatever
> subtleties you want.
>
> (Uncrustify is also stupidly easy and quick to build compared to clang,
> which is something to keep in mind if you're expecting developers to run
> it locally.)
>
> --
> Matthew
> _______________________________________________
> Development mailing list
> ***@lists.qt-project.org
> https://lists.qt-project.org/listinfo/development
Frederik Gladhorn
2018-11-22 09:30:01 UTC
Permalink
Please test the clang-format file we have:
https://code.qt.io/cgit/qt/qt5.git/tree/_clang-format

It's in qt5.git and the CI is smart enough not to run any tests when changing
only the format file, so please contribute to it, if you think it's not good
enough. I had to configure Creator to base things on a file, from there on it
worked nicely. Although I'd prefer a mode where it only formats changed lines
instead of everything.

Then there is git clang-format which should re-format only the changes you are
making while contributing. I'd like to see that integrated as at least a hint
in either the sanity bot or a new bot.
In addition we should have a commit hook, anyone volunteering to write one
will have my full support.

I have to agree though that some of the re-flowing and other details of clang-
format are far from perfect in my opinion. If anyone did the work for
Uncrustify instead and showed that it creates more happiness than clang-
format, I would support that too. Does Uncrustify support ranges so it will
only re-format the parts of a file that were touched?

So please give either tool a chance and let's see if we can reach consensus.

Cheers,
Frederik

In the meantime I'll enjoy the meta bike-shed and wonder if we should create
an abstraction layer on top :P
Elvis Stansvik
2018-11-22 09:45:50 UTC
Permalink
Den tors 22 nov. 2018 kl 10:31 skrev Frederik Gladhorn
<***@qt.io>:
>
> Please test the clang-format file we have:
> https://code.qt.io/cgit/qt/qt5.git/tree/_clang-format
>
> It's in qt5.git and the CI is smart enough not to run any tests when changing
> only the format file, so please contribute to it, if you think it's not good
> enough. I had to configure Creator to base things on a file, from there on it
> worked nicely. Although I'd prefer a mode where it only formats changed lines
> instead of everything.
>
> Then there is git clang-format which should re-format only the changes you are
> making while contributing. I'd like to see that integrated as at least a hint
> in either the sanity bot or a new bot.
> In addition we should have a commit hook, anyone volunteering to write one
> will have my full support.
>
> I have to agree though that some of the re-flowing and other details of clang-
> format are far from perfect in my opinion. If anyone did the work for
> Uncrustify instead and showed that it creates more happiness than clang-
> format, I would support that too. Does Uncrustify support ranges so it will
> only re-format the parts of a file that were touched?
>
> So please give either tool a chance and let's see if we can reach consensus.
>
> Cheers,
> Frederik
>
> In the meantime I'll enjoy the meta bike-shed and wonder if we should create
> an abstraction layer on top :P

Don't forget that always when a project starts discussing
auto-formatting, there also needs to be a big discussion about whether
to do a mass formatting :) With one camp usually vehemently against it
due to the destruction of git blame and the destruction of carefully
aligned `//` comments et.c., and the other camp thinking it worth it
because of the consistency it brings. So you'll have plenty to enjoy
:)

Elvis

>
>
>
> _______________________________________________
> Development mailing list
> ***@lists.qt-project.org
> https://lists.qt-project.org/listinfo/development
Uwe Rathmann
2018-11-22 10:02:51 UTC
Permalink
On Thu, 22 Nov 2018 09:30:01 +0000, Frederik Gladhorn wrote:

> If anyone did the work for
> Uncrustify instead and showed that it creates more happiness than clang-
> format,

I was using uncrustify for quite some time in qskinny, because of it
being way more configurable. Unfortunately it has several bugs ( f.e with
classes inside anynymous namespaces ) and I always had to manually post
process my files after running it.

My 2 cents,
Uwe
Matthew Woehlke
2018-11-22 16:41:04 UTC
Permalink
On 22/11/2018 04.30, Frederik Gladhorn wrote:
> Does Uncrustify support ranges so it will
> only re-format the parts of a file that were touched?

I don't think so, though if someone cared enough, it might be possible
to add that. (Uncrustify tracks the original location of its tokens, so
plausibly you could make it output only the selected range after
reformatting.)

However... I think ranges are a crutch. If a file is correctly formatted
to begin with, I don't see any value in only formatting a range. I'm
also concerned there could be instances when trying to only format a
given range could lead to errors. (Maybe the range is a "hint" which can
be expanded until the formatted and original output have a line break at
the same token? But even this might result in screwy formatting...)

BTW, while there are hopefully few if any left, what does clang-format
do to normalized SIGNAL/SLOT?

--
Matthew
Eike Ziller
2018-11-23 08:39:20 UTC
Permalink
> On 22. Nov 2018, at 17:41, Matthew Woehlke <***@gmail.com> wrote:
>
> On 22/11/2018 04.30, Frederik Gladhorn wrote:
>> Does Uncrustify support ranges so it will
>> only re-format the parts of a file that were touched?
>
> I don't think so, though if someone cared enough, it might be possible
> to add that. (Uncrustify tracks the original location of its tokens, so
> plausibly you could make it output only the selected range after
> reformatting.)
>
> However... I think ranges are a crutch.

> If a file is correctly formatted
> to begin with,

That’s exactly the point. Forcing a codebase to be completely reformatted when adopting a style severely limits the options for introducing it.

> I don't see any value in only formatting a range.

It makes it possible to adopt a formatter without reformatting the whole code base.
It makes it possible for individuals to use a formatter while a generic consent is not reached (yet?),
since then the formatter can be used for only the part that was touched in a patch anyhow.
That also helps in creating style settings for the formatter in the first place, and testing what works well, what not so well, what might even be fixed in the tool upstream.
E.g. currently some Qt Creator developers use clang-format. Since the format is tweaked so it mostly represents what is already used the new code fits into the code base, but the rest of the code is not affected by the line breaking or unbreaking etc that clang-format would enforce on it.

> I'm
> also concerned there could be instances when trying to only format a
> given range could lead to errors. (Maybe the range is a "hint" which can
> be expanded until the formatted and original output have a line break at
> the same token?

AFAIK something like this is what clang-format does, it can affect a few lines above or below. (At least that is what happens in the Qt Creator integration.)

> But even this might result in screwy formatting...)
>
> BTW, while there are hopefully few if any left, what does clang-format
> do to normalized SIGNAL/SLOT?

Is that a relevant question for the Qt 5 code base?

Br, Eike

--
Eike Ziller
Principal Software Engineer

The Qt Company GmbH
Rudower Chaussee 13
D-12489 Berlin
***@qt.io
http://qt.io
Geschäftsführer: Mika Pälsi,
Juha Varelius, Mika Harjuaho
Sitz der Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B
Matthew Woehlke
2018-11-27 16:42:49 UTC
Permalink
On 23/11/2018 03.39, Eike Ziller wrote:
> On 22. Nov 2018, at 17:41, Matthew Woehlke wrote:
>> If a file is correctly formatted to begin with,
>
> That’s exactly the point. Forcing a codebase to be completely
> reformatted when adopting a style severely limits the options for
> introducing it.

Eh... I find inconsistent style within a file to be the absolute worst
thing you can do, style-wise. So, while I understand your argument, I'm
still not sure I agree. Better, if you have an existing style, to use a
tool that can enforce what you already have.

OTOH, something you could do with uncrustify that you can't do with
clang-format is gradually build up a "complete" set of style rules, a
few rules at a time.

>> BTW, while there are hopefully few if any left, what does clang-format
>> do to normalized SIGNAL/SLOT?
>
> Is that a relevant question for the Qt 5 code base?

I don't know; is it? I guess I would hope not, but offhand I don't know.

(BTW, what about uic? AFAIK that still generates SIGNAL/SLOT
connections. OT: Any plans to change that?)

--
Matthew
Sze Howe Koh
2018-11-23 14:10:35 UTC
Permalink
On Tue, 20 Nov 2018 at 16:29, Lars Knoll <***@qt.io> wrote:
>
> > On 19 Nov 2018, at 21:59, Ville Voutilainen <***@gmail.com> wrote:
> >
> > On Mon, 19 Nov 2018 at 22:41, Thiago Macieira <***@intel.com> wrote:
> >>
> >> On Monday, 19 November 2018 12:03:06 PST Ville Voutilainen wrote:
> >>> I personally tend to split such things after an opening parenthesis.
> >>> Getting back to allowing ctor-initializers to be written
> >>> with a comma starting a line, I think we should just allow it; the
> >>> benefit of not having noise in a diff seems to outweigh
> >>> the minor aesthetics of it.
> >>
> >> It is allowed, unless the maintainer objects to it.
> >>
> >> I object to it in QtCore.
> >
> > I'm suggesting that you stop objecting to it.
>
> I suggest that we stop arguing about coding styles by defining one through a tool (aka clang-format and one format file for all of Qt). I know that one will never be perfect for all cases (and that you can always find corner cases where manual formatting might be slightly better), but consistency and finally stopping to argue about coding style esp. in code reviews (by automating things) is more important.
>
> Cheers,
> Lars

Here's a small record of intent:
https://wiki.qt.io/index.php?title=Qt_Coding_Style&type=revision&diff=34756&oldid=34328

I wanted to link Lars' email, but that seems to have disappeared from
the archive.


Regards,
Sze-Howe
Edward Welbourne
2018-11-26 10:22:20 UTC
Permalink
Sze Howe Koh (23 November 2018 15:10) let us know
> Here's a small record of intent:
> https://wiki.qt.io/index.php?title=Qt_Coding_Style&type=revision&diff=34756&oldid=34328

Thanks.

> I wanted to link Lars' email, but that seems to have disappeared from
> the archive.

Yup, it's in a known gap in that mail archive, that TQtC's sysadmins are
looking into; suspected fall-out of the transition to https.
I have pinged them about it (again) ...

Eddy.
Loading...