DevHeads.net

Kdelibs Coding Style vs. preparations for Qt5

Hi,

what about adapting the Kdelibs Coding Style to the upcoming preparations for
the porting to Qt5? A lot of (KDE) projects follow that kdelibs one, but there
is (at least?) one rule which seems to conflict with the recommendations for
the preparations:

--- 8< ---
Qt Includes

If you add #includes for Qt classes, use both the module and class name. This
allows library code to be used by applications without excessive compiler
include paths.

Example:
// wrong
#include <QString>

// correct
#include <QtCore/QString>
--- 8< ---
From <a href="http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes" title="http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes">http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes</a>

Of course the current Qt Coding Style, which is the base of the kdelibs one,
does not mention anything about that, given that its about their own headers.

Now the Porting from Qt 4 to Qt 5 guide from KDAB recommends this:
--- 8< ---
Fixing up includes

[...]

Or more portably (Which works in Qt 4 and Qt 5):
#include <QWidget>
--- 8< ---
From <a href="http://www.kdab.com/porting-from-qt-4-to-qt-5/" title="http://www.kdab.com/porting-from-qt-4-to-qt-5/">http://www.kdab.com/porting-from-qt-4-to-qt-5/</a>

So what to do about this?

Will kde-frameworks be Qt5-only, so not need to support both Qt4 and Qt5 and
thus to use module-less Qt includes? Or will the includes be if-def'ed?
So will projects which refer to the Kdelibs Coding Style need to add an
exception to their rules for the includes, if they want to prepare for Qt5?

Or does the rule need adaption?

Cheers
Friedrich

Comments

Re: Kdelibs Coding Style vs. preparations for Qt5

By David Faure at 12/29/2012 - 06:06

On Saturday 29 December 2012 02:47:13 Friedrich W. H. Kossebau wrote:
At some point it will be, yes.
Right now it supports both, so a bunch of QtGui/ was removed in #include
statements.

Well, for frameworks that intend to be "as close to Qt as possible" they
should do the same (for the convenience of developers who don't use
qmake/cmake but set up their project configuration in their IDE by hand, for
instance Visual Studio).
This means re-adding the missing QtWidgets/ in public headers once Qt5 is
required in KF5.

But for sure this doesn't apply to "projects which refer to the kdelibs coding
style". There's no good reason for apps to do this, only porting trouble comes
from that.

Re: Kdelibs Coding Style vs. preparations for Qt5

By Kevin Ottens at 12/29/2012 - 16:58

On Saturday 29 December 2012 11:06:30 David Faure wrote:
Out of curiosity, is it something which got documented in the Qt project? Or
that's more a custom? (doesn't make it less valid, I'm being curious here and
couldn't find the info)

Regards.

Re: Kdelibs Coding Style vs. preparations for Qt5

By Thiago Macieira at 12/29/2012 - 17:17

On sábado, 29 de dezembro de 2012 22.58.49, Kevin Ottens wrote:
syncqt complains if public headers have Qt includes that aren't in the
<QtModule/ClassName> or <QtModule/classname.h> form.

Re: Kdelibs Coding Style vs. preparations for Qt5

By Stephen Kelly at 12/29/2012 - 18:48

syncqt is a Qt-internal tool. It is relevant to the headers of Qt itself,
but not relevant outside it.

Thanks,

Steve.

Re: Kdelibs Coding Style vs. preparations for Qt5

By Thiago Macieira at 12/29/2012 - 18:56

On domingo, 30 de dezembro de 2012 00.48.01, Stephen Kelly wrote:
True, but it's relevant to the discussion.

The discussion is about making KF5 coding style as close as possible to Qt's,
potentially to permit the codebase to be imported into Qt as an addon later.
If the codebase is imported into Qt, it will be built with Qt's buildsystem
(whatever that is at that time). Right now, that includes a syncqt step.

Even if the code isn't imported into Qt and is never subject to syncqt, the
question still was whether that was documented. It isn't documented in a wiki,
but it is effectively documented by having syncqt spew warnings.

Re: Kdelibs Coding Style vs. preparations for Qt5

By Kevin Ottens at 12/30/2012 - 02:27

On Saturday 29 December 2012 21:56:41 Thiago Macieira wrote:
Note that wasn't even on my radar.

Thanks, indeed that's what I was after.

Regards.

Re: Kdelibs Coding Style vs. preparations for Qt5

By Kevin Ottens at 12/29/2012 - 04:48

Hello,

On Saturday 29 December 2012 02:47:13 Friedrich W. H. Kossebau wrote:
Since I've been skimming quite a bit through kdelibs codebase recently,
needless to say this rule is in practice void as it's clearly not followed.

Yep.

I *personally* prefer the fully qualified style with the module name. That
said, since we don't have per module namespaces to go with it and the like
it's rather cosmetic more than anything. Now add to that the fact that the
fully qualified style will generate more work when you port (for classes
moving from one module to another) it sounds like it's the right move to drop
them completely and use only the class name for includes[*].

For now it supports both, but it's very likely that by the end of january it
will be Qt5 only.

With what I wrote above I think the natural conclusion is to adapt the rule.
We should push to use the class name only includes I think.

Also, contributing to Qt itself more, I noticed some differences in style
between both (namely: we ignore space around operators, and we for braces
around single line statements). Your question makes me wonder if we should
update our own KDE Frameworks style to be closer of the Qt one. The aim here
would be to be consistent accross the whole stack.

Actually looking at both documents now, I see we should already follow the
spacing around operators today... I think the intent of our coding style was
to be "the Qt style with extra exceptions" but somewhere in the wording this
gets lost. Maybe we should do a better job at it... If we go toward being
closer to the Qt style maybe the proper way out would be to shorten the
document quite a bit saying: use the Qt style + includes section + astyle
section + emacs & vim section. Right now by duplicating some but not all of
the Qt style we're diluting the messaging it seems. Opinions?

Thanks for bringing it up though, it's definitely the right time for that.

Regards.

PS: Please, before hitting reply think twice! This type of thread can easily
degenerate in bikeshedding. So put your personal preferences at the door as I
tried to, it's really not the point.

PPS: My PS might sound obvious and unneeded to some of you, but I got burnt
enough times with coding styles discussions to be extra cautious now. It's
really easy to loose perspective and I might end up doing the mistake myself,
the reminder is for me too. :-)

[*] Note that, History might prove me wrong there at some point if Qt6
introduces classes with the same name in different modules... if that's the
case I hope it'll come with consistent namespacing though. :-)

Re: Kdelibs Coding Style vs. preparations for Qt5

By Albert Astals Cid at 12/29/2012 - 10:25

El Dissabte, 29 de desembre de 2012, a les 10:48:48, Kevin Ottens va escriure:
Would there be any chance to have the style check done by a pre-commit hook?
Or at least have a command-line tool that checks it for me?

Personally I find it is quite hard to remember all the different coding styles
of the bazillion projects I controbiute to and since i can't remember they in
my head i have to find the page that describes the coding style and check it
manually each time i want to contribute to the respective projects, and to be
honest that is quite boring and lowers my morale.

Cheers,
Albert

Re: Kdelibs Coding Style vs. preparations for Qt5

By Kevin Ottens at 12/29/2012 - 16:48

On Saturday 29 December 2012 16:25:54 Albert Astals Cid wrote:
If anyone is willing to put the time and effort I would definitely welcome
that!

But it clearly need some more investigation (use krazy or astyle? how much
setup for dev? how reliable? etc.) and experimentation.

Yep, that's one more reason for trying to be closer to the Qt style as I
mentioned in my previous email. But I'm waiting to see if someone has
practical problems with such a move to have a clearer opinion.

Regards.

Re: Re: Kdelibs Coding Style vs. preparations for Qt5

By Martin =?ISO-88... at 12/29/2012 - 13:11

On Saturday 29 December 2012 16:25:54 Albert Astals Cid wrote:
Assuming we can perfectly check the coding style with astyle it would be quite
nice to have that as a pre-commit hook. Would be very nice to have it in the
repos as it also moves away the nitpicks from code reviews.

And no, Krazy check is for that too late as it's after commit.

Cheers
Martin

Re: Kdelibs Coding Style vs. preparations for Qt5

By Milian Wolff at 12/29/2012 - 15:31

On Saturday 29 December 2012 19:11:17 Martin Gräßlin wrote:
Note that something like that would be very welcome for me in KDevelop related
projects as well. I've investigated it a few times already and it is sadly not
yet perfectly feasable in my opinion. My approach was to format the new about-
to-be-committed file and compare it to the unformatted file - if they both are
equal everything is fine and we can continue. Otherwise some style guideline
was violated and the commit was denied.

While it was fairly trivial to write this up, the tool fails due to the
imperfection of astyle and uncrustify. Both have still problems when
formatting a few files, especially when it comes to macro usages or similar.
Furthermore I was not yet able to write a "perfect" rule set for either astyle
or uncrustify.

Generally though, I would very much welcome any effort in that direction. If
we could at least catch a few common issues it should help a lot.

Cheers

Re: Kdelibs Coding Style vs. preparations for Qt5

By Allen Winter at 12/29/2012 - 14:50

On Saturday 29 December 2012 07:11:17 PM Martin Gräßlin wrote:
If you have the Krazy command line tool installed (<a href="https://gitorious.org/krazy" title="https://gitorious.org/krazy">https://gitorious.org/krazy</a>)
then you could write a pre-commit hook that runs 'krazy2 --style' on the files
being committed.

Keeping in mind that I haven't tested the Krazy "kdelibs" style checker in a long time.
But I am accepting bug reports and patches.

-Allen

Re: Re: Kdelibs Coding Style vs. preparations for Qt5

By Martin =?ISO-88... at 12/30/2012 - 02:55

On Saturday 29 December 2012 14:50:46 Allen Winter wrote:
Cheers
Martin

Re: Kdelibs Coding Style vs. preparations for Qt5

By Laszlo Papp at 12/29/2012 - 15:32

./krazy2 --style
Unknown option: style

Besides, it would be nice to get something automatically fixed when
possible (i.e. not just a simple run as krazy usually does).

Laszlo

Re: Kdelibs Coding Style vs. preparations for Qt5

By Adriaan de Groot at 12/29/2012 - 11:34

On Saturday, December 29, 2012 04:25:54 PM Albert Astals Cid wrote:
Wouldn't that typically be a Krazy thing to check (and you can run Krazy
checks from the command-line)? It'd be post-hoc, but at least the style issues
can then be dealt with by those folks who run around fixing up Krazy things.

[ade]

Re: Kdelibs Coding Style vs. preparations for Qt5

By Stephen Kelly at 12/29/2012 - 05:02

I agree.

We have a buildsystem that is good enough that we can specify the
directories to look for the 'class name' headers in, and it is in the
buildsystem that that stuff belongs.

See the kinds of practical problems 'module includes' create:

<a href="http://thread.gmane.org/gmane.comp.programming.tools.cmake.user/44780/focus=44893" title="http://thread.gmane.org/gmane.comp.programming.tools.cmake.user/44780/focus=44893">http://thread.gmane.org/gmane.comp.programming.tools.cmake.user/44780/fo...</a>

Thanks,

Steve.