Re: Kdelibs Coding Style vs. preparations for Qt5
Hello, On Saturday 29 December 2012 02:47:13 Friedrich W. H. Kossebau wrote: --- 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 http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes 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. 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. Yep. 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 http://www.kdab.com/porting-from-qt-4-to-qt-5/ So what to do about this? 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[*]. Will kde-frameworks be Qt5-only, so not need to support both Qt4 and Qt5 and thus to use module-less Qt includes? For now it supports both, but it's very likely that by the end of january it will be Qt5 only. 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? 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. :-) -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud patron of KDE, http://www.kdab.com signature.asc Description: This is a digitally signed message part.
Re: Kdelibs Coding Style vs. preparations for Qt5
Kevin Ottens wrote: We should push to use the class name only includes I think. 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: http://thread.gmane.org/gmane.comp.programming.tools.cmake.user/44780/focus=44893 Thanks, Steve.
Re: Kdelibs Coding Style vs. preparations for Qt5
On Saturday 29 December 2012 02:47:13 Friedrich W. H. Kossebau wrote: Will kde-frameworks be Qt5-only, so not need to support both Qt4 and Qt5 and thus to use module-less Qt includes? At some point it will be, yes. Right now it supports both, so a bunch of QtGui/ was removed in #include statements. 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? 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. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE, in particular KDE Frameworks 5
Re: Kdelibs Coding Style vs. preparations for Qt5
El Dissabte, 29 de desembre de 2012, a les 10:48:48, Kevin Ottens va escriure: Hello, On Saturday 29 December 2012 02:47:13 Friedrich W. H. Kossebau wrote: --- 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 http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes 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. 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. Yep. 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 http://www.kdab.com/porting-from-qt-4-to-qt-5/ So what to do about this? 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[*]. Will kde-frameworks be Qt5-only, so not need to support both Qt4 and Qt5 and thus to use module-less Qt includes? For now it supports both, but it's very likely that by the end of january it will be Qt5 only. 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? 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. 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 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
On Saturday, December 29, 2012 04:25:54 PM Albert Astals Cid wrote: 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? 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: Re: Kdelibs Coding Style vs. preparations for Qt5
On Saturday 29 December 2012 16:25:54 Albert Astals Cid wrote: 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? yeah that should be possible. At my old day-job I created a pre-commit hook to basically grep for some common coding style violations. Only problem: it checks for all files, so it would e.g. also check QML files which follow a different coding style. 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 signature.asc Description: This is a digitally signed message part.
Re: Kdelibs Coding Style vs. preparations for Qt5
On Saturday 29 December 2012 07:11:17 PM Martin Gräßlin wrote: On Saturday 29 December 2012 16:25:54 Albert Astals Cid wrote: 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? yeah that should be possible. At my old day-job I created a pre-commit hook to basically grep for some common coding style violations. Only problem: it checks for all files, so it would e.g. also check QML files which follow a different coding style. 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. If you have the Krazy command line tool installed (https://gitorious.org/krazy) 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: Kdelibs Coding Style vs. preparations for Qt5
On Saturday 29 December 2012 19:11:17 Martin Gräßlin wrote: On Saturday 29 December 2012 16:25:54 Albert Astals Cid wrote: 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? yeah that should be possible. At my old day-job I created a pre-commit hook to basically grep for some common coding style violations. Only problem: it checks for all files, so it would e.g. also check QML files which follow a different coding style. 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. 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 -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.
Re: Kdelibs Coding Style vs. preparations for Qt5
On Sat, Dec 29, 2012 at 7:50 PM, Allen Winter win...@kde.org wrote: If you have the Krazy command line tool installed ( https://gitorious.org/krazy) then you could write a pre-commit hook that runs 'krazy2 --style' on the files being committed. ./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
On Saturday 29 December 2012 16:25:54 Albert Astals Cid wrote: El Dissabte, 29 de desembre de 2012, a les 10:48:48, Kevin Ottens va escriure: Hello, On Saturday 29 December 2012 02:47:13 Friedrich W. H. Kossebau wrote: --- 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 http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes 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. 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. Yep. 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 http://www.kdab.com/porting-from-qt-4-to-qt-5/ So what to do about this? 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[*]. Will kde-frameworks be Qt5-only, so not need to support both Qt4 and Qt5 and thus to use module-less Qt includes? For now it supports both, but it's very likely that by the end of january it will be Qt5 only. 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? 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. 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? 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. 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. 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. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud patron of KDE, http://www.kdab.com signature.asc Description: This is a digitally signed message part.
Re: Kdelibs Coding Style vs. preparations for Qt5
On Saturday 29 December 2012 11:06:30 David Faure wrote: 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. 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. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud patron of KDE, http://www.kdab.com signature.asc Description: This is a digitally signed message part.
Re: Kdelibs Coding Style vs. preparations for Qt5
Thiago Macieira wrote: On sábado, 29 de dezembro de 2012 22.58.49, Kevin Ottens wrote: On Saturday 29 December 2012 11:06:30 David Faure wrote: 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. 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) syncqt complains if public headers have Qt includes that aren't in the QtModule/ClassName or QtModule/classname.h form. syncqt is a Qt-internal tool. It is relevant to the headers of Qt itself, but not relevant outside it. Thanks, Steve.
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 http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes 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 http://www.kdab.com/porting-from-qt-4-to-qt-5/ 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