From kde-core-devel Sat Jul 05 13:41:23 2008 From: Lubos Lunak Date: Sat, 05 Jul 2008 13:41:23 +0000 To: kde-core-devel Subject: Consensus on the kdelibs coding style Message-Id: <200807051541.24021.l.lunak () suse ! cz> X-MARC-Message: https://marc.info/?l=kde-core-devel&m=121526534025820 On Thursday 03 of July 2008, Aaron J. Seigo wrote: > i suggest to not restart this conversation every time the topic veers > towards coding style You know, you are actually right about this. It is pointless to have such discussions repeatedly, especially given that they every single time, including the first one, ended with no conclusion and just ignoring it, getting tired of it or dismissing it as has-been-already-discussed (which is kind of funny, given that those previous discussions never had a conclusion). It is also silly to have some people pushing this way and some the other way the way it's been happening. And, given that a coding style is eventually bound to spread more and more, I think it makes sense to finally sort this out. It's awfully late but awfully late is still better than too late. Therefore I suggest we restart this exactly once more. And this time, preferably with something looking at least a bit like conclusion and getting reasonable choices. I think the current policy sucks, but what sucks a lot more is the way we got it - by changing agreement ([1]) about having a coding style ('a' style, I was there and the vote I remember didn't have 'Qt' anywhere in it) into having to use the Qt style without having a choice ([2]) and ignoring any feedback (am I the only one who sees the contradictions e.g. between [3] and [4]?). I was away by that time, missed the party and reading the whole thread later was rather an experience (and BTW, I read it again now, all of it, just to make sure I remember it correctly). Now the situation is that the Qt style is at techbase, marked as 'recommended but not enforced at all', which basically means anything, and that's how people currently handle it. Also, to give some numbers, I quickly checked how current code complies with it, by using a simple metric of the {} placement ({ trailing previous line vs { column-aligned with } ), where trailing { means compliancy (an obvious overstatement, but whatever). Using [5] gives about 50% vs 50% in kdecore and about 60% vs 40% in kdeui. After almost two years of the policy being "effective", using a metric based on the most basic rule of the policy that very likely seriously overestimates it? [Note: This is not '60% of kdeui codes follows the policy', far from it. I myself read it as 'the policy has been largely ignored anyway', in contrary to e.g. [6]] So I think it would make sense to get to a clearer status of the policy, even if that would be people just agreeing with going with the Qt style, regardless of the awful way we got it. And, to actually have some options, unlike the last time (many of the support votes there were actually along the lines of "I don't like the Qt style, but let's go with it anyway"), I would like to propose an alternative. In contrary to the Qt style, which defines even completely useless things like requiring {}'s around a single-line if() body statement (where's the point in that?), I'd prefer something simple. [Now, if you're happy with the Qt style, the way we got it and getting occassionally pointed out that you put a space incorrectly, or you just don't care and couldn't be bothered less, just say so and that's it, stop here. In the worst case that I'm the only one who doesn't like this, then I've wasted my time, and I find it worth the effort. Given that I've worked on many different pieces of code in KDE with different styles and could handle it quite well, I guess I dislike this issue a lot. Onto my proposal:] ===== Coding style proposal ===== As the numbers above show, we can still switch to anything we want, and there's not much sense in being overly detailed, since that will probably never be followed (it's not quite in the KDE spirit to be pedantic, is it?). Therefore I'd like to sum the policy as "4 spaces, no tabs, {}'s column-aligned, indent everything". Plus the somewhat automatic "don't write messy code". Plus possibly some additional recommendations I find obvious for people who can't live without them, as non-mandatory. Plus "only major contributors to a file can reformat it". Rationale: - This alone should take care of more than 90% of problems that people have with inconsistent code, at a relatively low cost compared to the Qt style. KDE has never enforced many things and if we actually want to get somewhere with the coding style thing, IMHO we should try slowly and possibly extend it later if things go well rather than the all-or-nothing approach of the Qt style proposal. As already said, we have better things to do, so who here wants to spend their time on checking whether they have one space more or less? - "4 spaces, no tabs" - this appears to be the most used style in kdelibs, and other options don't seem to have big advantages - "{}'s column-aligned" - matching { and } are in the same column, as e.g. in [7]. If the purpose of having a common coding style is to improve readibility, we should actually use one that itself does so, right? The Wikipedia section [7] is quite clear on the advantages (I've never really learnt to visually identify blocks with the trailing { style, after all those years), and the only disadvantage there, about taking more space, is only minor (especially e.g. in the Qt style, where other rules encourage this anyway). As the numbers above show, there is no clear preference for one style, so we should pick the one that makes code more readable. - "indent everything" - i.e. indent everything that looks like a logical sub-section (if/while/for/... bodies, public/protected/private sections in a class, list of functions in such sections, case labels, code withing those case blocks, etc.). A simple rule, without exceptions. With one, actually, namespaces, for the purely practical reason that otherwise files using them would be almost completely indented one level because of this. - "only major contributors to a file can reformat it" - this can hopefully reduce opposition as people working actually on their code can keep it the way they like it and are used to it, while still not blocking adjustments when new people pick up unmaintained code or there are several contributors to one file. BTW, on a personal note, I don't think coding style of files matters much when it comes to getting contributors. I ended up maintaining files in various places that have at least 3 quite different styles and it didn't really matter with other people (not) contributing there. In fact, come to think of it, the biggest contributions have been to the code which is formatted in my personal style, not that I think that was the reason though. I don't think insisting on every single space is helping contributors. That's it. These are all that I think would actually make a difference, and if somebody thinks it would make more sense differently, just say it. And there should be presumably additional reasonable recommendations on more clean code like "one variable declaration per line, spaces around operators" etc., but not mandatory. ===== Example ===== // one of possible ways, as the point is not being too pedantic class Foo { public: Foo( int a ); void bar( int a = 0 ); }; Foo::Foo() { if( a == 2 ) { bar( 1 ); bar( 2 ); } else bar(); } ===== Links ===== [1] http://lists.kde.org/?l=kde-core-devel&m=115348018523989&w=2 [2] http://lists.kde.org/?l=kde-core-devel&m=115340952719876&w=2 [3] http://lists.kde.org/?l=kde-core-devel&m=115342346704536&w=2 [4] http://lists.kde.org/?l=kde-core-devel&m=115342327025907&w=2 [5] $ find [subdir] -name *.cpp -o -name *.h | xargs cat a.cpp $ cat a.cpp | sed 's/\t/ /g' >b.cpp $ cat b.cpp | sed 's#/\*.*\*/ *$##' >c.cpp $ cat c.cpp | sed 's#//.*##' >d.cpp $ cat d.cpp | grep '^ .*[^ ] *{ *$' >atrail.txt $ cat d.cpp | grep '^ *{' | [manual removing of arrays] > aalign.txt $ wc -l atrail.txt aalign.txt [6] http://lists.kde.org/?l=kde-core-devel&m=115360828511229&w=2 [7] http://en.wikipedia.org/wiki/Indent_style#Allman_style_.28bsd_in_Emacs.29 -- Lubos Lunak KDE developer -------------------------------------------------------------- SUSE LINUX, s.r.o. e-mail: l.lunak@suse.cz , l.lunak@kde.org Lihovarska 1060/12 tel: +420 284 028 972 190 00 Prague 9 fax: +420 284 028 951 Czech Republic http://www.suse.cz