-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA224 Yes, we can wait a few days for your input. :-) The vast majority of this file was written in the style I prefer: https://git.trinitydesktop.org/cgit/tdelibs/tree/tdecore/tdehw/tdehardwared… I noticed on quick glance that at least one alternate styles snuck in over time, so the revision ID specified in that link is an older version without many of the changes that introduced those alternate styles. I find it very easy to read on mutiple screens, on the Web and in Kate, from small laptops up to my main workstation. Comments are of course welcome! Tim

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA224 I prefer hard tabs, this way each developer can set the space the way they want to see it. Tim

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA224 AStyle. In fact I was working on this in late 2012: https://git.trinitydesktop.org/cgit/scripts/tree/astyle Tim

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA224 Just one quick comment; I still need to carefully review your other suggestions and comment on them later: Detached braces are a big problem for me, severely reducing code legibility. Is this something you are flexible on or is code like this: void foo() { // Do stuff } hard for you to read? Thanks! Tim

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 <snip> In my programming carrier I have worked with different styles with different people, so if needed I can adapt to a different style and in general I am flexible. Whatever consistent style is better than "no style". The "attached brackets" vs "detached brackets" styles are two "schools of thoughts", so no wonder that if someone is used to one way he will find the other one harder to read, at least until he gets used to it. Having said all that, Slavek's comments described exactly my view on the bracket point: brackets at the end of the line are harder to find when scrolling along long blocks. I guess there is no easy solution, either we choose one way or the other. A possible way to mitigate the problem for the developers of the other school of thought could be the following (a little awkward but I haven't found anything better yet): 1) we create two (or more) style files and respective beautify files, for example one for attached brackets and one for detached brackets. The more options are in common between the two styles the better 2) we choose as "official style" the one most developers are happy with. 3) the developers of the other school can still use their style to edit code locally before committing using these steps: a) git pull b) beautify_to_the_style_I_like.sh c) edit, compile, test locally until happy d) beautify_to_TDE_official_style.sh e) commit It's a little bit uncomfortable, but at least its a possible way of working with one's own favorite code style. Note that point d) should become mandatory for all developers before any commit, to make sure the code style is maintain over time. Other ideas? Cheers Michele

On Thursday 01 of January 2015 07:22:08 Michele Calgaro wrote: Only just for show I attach a style that I used in my projects. As you can see, I'm used to small indentation => code did not run to the right side so quickly. I do not have infinitely wide screen :) Unusually looking definition of function parameters is based from the days when it was not used automatic generation of documentation => description of the parameters was not in a separate documenting comment, but the definition is self-commenting. Simple symbols used in comments <=> mark parameters simultaneously input and output, <= mark output parameters and => mark input parameters. At the same time, I have kept the habit of the parameters order: at first 'i/o' parameters, then 'o' parameters and the last 'i' parameters. One thing that might be useful for our common rules for TDE is - strictly habit always use braces - even in cases where the block is a single line. As I said in the beginning - I attach it only as a demonstration of my habits. I am also flexible and ready to use the style that we will confirm as default TDE style. -- Slávek

On Thursday 01 of January 2015 21:43:08 Timothy Pearson wrote: Wow, indentation in twin is yet another - I would say, very confusing - because the brackets are at the same level of indentation as the block, so is lost completely. Awful. I am aware that the more popular now are brackets at the end of lines. And I, as well as Michele, earlier mentioned that we are flexibile and we have no problem adapt to it. In fact, it was enough for the pleasure to me knowing that along with Michele we have the same preference :) But what would prefer a smaller indentation in the switch cases - as featured Michele. Unfortunately I do not know whether astyle can be set to such a manner. See attachment. -- Slávek

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA224 <snip> My personal study of these adaptation phenomena has led me to the conclusion that, in most cases, the "adaption" is more an automated hiding of an inefficient and/or error-prone style from the level of the programmer's conciousness. Basically it shows up to an outside observer as a slowing down of the programmer's work without his or her knowledge, similar to clunky computer interfaces slowing down common tasks without the user's knowledge or inefficient factory stations draining employee productivity. It can even manifest as a subtle almost subconcious dislike of working on the code without the programmer ever really becoming aware of why. This is something we want to avoid if at all possible, hence this discussion. :-) Now of course there are cases where true adaption does take place, but there are limits on how far out the style can be from the programmer's natural expectations before this no longer holds. ;-) So, to be clear, this is what is desired? do_something(); switch(foo) { case bar: a=1; case baz: a=2; case asd: a=3; default: a=0; } do_something_else(); My question is this: Say I have a long case statement (dozens of cases with subsequent processing blocks, enough to run off both vertical edges of my screen) and it's "conveniently" located in the middle of a large source file with other case blocks and "normal code" above and below it. With the style above I will have problems finding the end of the case block, which is why I started indenting them like this: do_something(); switch(foo) { case bar: a=1; case baz: a=2; case asd: a=3; default: a=0; } do_something_else(); There are actually several instances of the pathological case described above in the TDE source tree right now, in the udev and network-manager interfaces among other locations, and I expect the number of these to grow as more DBUS-based system abstraction layers are forced on us (e.g. systemd, logind, and who knows what else down the line). Being able to easily say "yeah, I know what that case block does, get me to the next line of code after the case handler" by simply following the indentation proved quite valuable for me when writing the TDE hardware library. Rebuttals welcome. :-) I will definitely enforce the chosen style through an automated method; I'm not sure if I can practically hook into GIT for pre-commit as we have so many modules but I will be adding something to the server to make sure the style doesn't inadvertently "drift" over time. This is the current consensus as far as I can tell: 1.) Every conditional uses braces 2.) Hard tabs 3.) Conditional indentation like so: if (foo) { a=0; } else if (bar) { a=1; } else { a=2; } We still need to discuss how to handle large chains of conditionals, e.g. these pathological cases: https://git.trinitydesktop.org/cgit/tdelibs/tree/tdecore/tdehw/tdehardwared… Is the style shown there acceptable? Also on the docket are how to handle simple variable assignments; there are several ways I've seen: a=2; a = 2; a= 2; a =2; I'm fairly undecided on this. For consistency with larger statements "a = 1;" should probably be used but it seems such a waste of space compared with "a=1;". Thoughts? One other issue is the whitespace within conditional expressions. As you can see I'm fairly undecided on this as well, having used multiple styles already: https://git.trinitydesktop.org/cgit/tdelibs/tree/tdecore/tdehw/tdehardwared… https://git.trinitydesktop.org/cgit/tdelibs/tree/tdecore/tdehw/tdehardwared… Given a choice I slightly prefer the style shown on line 2518; my only strong preference here is that I don't want to see any whitespace between two matched parenthesis: Correct: execute_something() Wrong: execute_something( ) Thoughts are welcome here as well. Thanks! Tim

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA224 Ah, OK, that is a valid complaint. The case style shown above is fine and we'll go with that. My only remaining question then is should we also be forcing whitespace between each case block for legibility, something like this? do_something(); switch(foo) { case bar: a=1; break; case baz: { a=2; ...long case block... c=4; break; } case asd: a=3; break; default: a=0; } do_something_else(); I am curious as to why you find this more intuitive than having the logical operator before the comparison. Roughly translating to English the above style reads something like: if the box is black AND <pause> the box is not square AND <pause> the sphere is purple OR the sphere is NOT PRESENT THEN versus if the box is black <pause> AND the box is not square <pause> AND the sphere is purple <pause> OR the sphere is NOT PRESENT <pause> THEN You can see how in the latter style the relevant logical operator is stated first, then the expression, with all required information to understand the conditional contained on one line instead of spanning two lines. The latter is easier for me to grasp all of the conditions required in a large expression; the former requires a bit more effort. It isn't a big deal either way for me, and I'd like Slavek's input before deciding, but I am curious as to the thought processes behind the suggested style. :-) I'm going to lean towards forcing spaces for consistency. What is your opinion Slavek? OK, we'll go with that then. Good catch! Agreed. As the number of finalized rules is steadily growing I have started an Etherpad here to codify them and keep track of where we are at: http://trinity.etherpad.trinitydesktop.org/87 Tim

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA224 OK, agreed and added to official spec. Wouldn't the lack of a curly brace state the same thing, given our rules on curly braces? Point taken. However, I think we need to add a rule as well that states all conditionals must be enclosed in parenthesis; I have cleaned up so many compiler warnings and so much buggy code becase of lazy programmers in this respect that I don't want to see another unenclosed conditional. ;-) If this rule is enforced, your second example becomes: if ((a == 3) && ((b == 5) || (c == 4) || ((d == 10) && (e == 20)))) which is a bit easier to read, but overall this style is still harder to read than your first example when more than one condition is present on one line. Perhaps we should allow both styles and simply state that the style providing "best legibility" should be used? The number of long/complex ifs in the codebase are why I am insisting this be hammered out properly. :-) We haven't head from Slavek in a while so I guess we'll keep drawing up the specification and he can review it and comment when he has time. Yes! This is Hungarian notation for member variable scope, and should be strongly enforced. Added to spec for new code, though I don't think we can go back and fix this automatically. I prefer this: MyClass::MyClass(int i, char c, int *p) : m_i(i), m_c(c), m_p(p), m_d(d) { do_something(); } and where a derived class is being created: MyClass::MyClass(int i, char c, int *p) : TQObject(), m_i(i), m_c(c), m_p(p), m_d(d) { do_something(); } Is this acceptable? I have also added a question to the Etherpad regarding loop control statement formatting; if you could respond to that I'd appreciate it. Thanks! Tim

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA224 The problem with allowing these is that if I need to refactor the code to add in a single conditional I need to add parenthesis, which a.) is an additional source of error and b.) messes up the difference tracking making it hard for other developers to see what the functional change was. I think I'm going to override you on this one and force the parenthesis. ;-) But the base class(es) are fundamentally different than the other members; I like the visual distinction on first glance. Slavek, what is your opinion here? While that is more compact, it introduces a problem with maintainability. Say I need to add a new member variable, strongly associated with m_c. That changes the definition to: MyClass::MyClass(int i, char c, int *p) : m_i(i), m_c(c), m_cRel(c_r), m_p(p), m_d(d) { do_something(); } which a.) required quite a bit of editing (enough to make me think twice about adding the new member variable!) and b.) now shows two lines as completely changed in the difference output. The less-compact style makes it easy to add the variable, and shows at a glance what changed in the difference output. Tim

On Tuesday 13 of January 2015 00:25:15 Timothy Pearson wrote: Ok, I understand and accept. -- Slávek

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 01/17/2015 11:52 PM, Slávek Banko wrote: Good idea, makes the code easier to read. Cheers Michele

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 2015/01/17 11:30 PM, Slávek Banko wrote: This is good for me too, I actually almost wanted to propose it myself :-) Cheers Michele

On Saturday 17 of January 2015 15:59:19 Slávek Banko wrote: Wow, now I noticed a great debate about the spaces / no-spaces on Ehterpad. I prefer to continue the discussion here. I would say that there already was a consensus regarding spaces in the assignments - ie "a = b + c" and already there was a consensus regarding spaces around operators in conditions such as if, while - ie "if (a == b)". All was fine until we had a case of 'for'. At its core, it does not contain anything new - uses the assignment and uses conditions => for both there already was a consensus. So one would expect that there is no problem. Michele proposed: for (i = 0; i < 1; i = i + 2) { } advantage - is fully in line with already agreed practices disadvantage - are harder to distinguish the individual 'for' expressions Tim proposed: for (i=0; i<1; i=i+2) { } advantage - are well distinguishable individual 'for' expressions disadvantages - difficult to read individual 'for' expressions as such - is inconsistent with already agreed practices I understand Tim's argument. But that we would either to 'for' give exemption from other rules - which does not sound too good, or we would have to change already agreed rules - which is even worse option. I really would not want to because of the 'for' reduce the readability of all other conditions and assignments. But there is another thing - more complex 'for' where individual parts can contain more assignments and more conditions - in such cases Tim's argument will be less important - on the contrary will be greater problem reduced readability each expressions as such. Therefore I have two proposals: 1) As with the 'if' use 'excessive' brackets: for ((i = 0); (i < 1); (i = i + 2)) { } 2) More complex 'for' break into lines: for ((i = iteratorInicialization(arg)); (i != iteratorEnd(arg)); (i = iteratorNext(arg))) { } What do you think about that? -- Slávek

On Sunday 18 of January 2015 03:58:34 Michele Calgaro wrote: Yeah, it looks good! Simple and clearly visible. Two spaces seem to me to be sufficient. I have no objection to any of these methods. Opening brace is on the front of the line, but its good visibility deteriorates text followed brace on the same line. Although the style is very compact, it seems to me less readable. -- Slávek

Dne út 20. ledna 2015 Timothy Pearson napsal(a): Two spaces seem to me sufficient to good readability, while there was no violated clarity. Writing assignments and conditions in the 'for' without spaces results in inconsistence style with herself. Or, if you propose to remove spaces in assignments and conditions in all code, will make hard to read all code. For example - without spaces it simply disgust: for (x=1,y=5; x<10&&y>0; x++,y--) { } with spaces It's not beautiful, but at least readable: for (x = 1, y = 5; x < 10 && y > 0; x++, y--) { } Therefore, the use of two spaces seems to me the least painful. Just my two cents. -- Slávek

On Friday 09 of January 2015 03:40:45 Timothy Pearson wrote: Incidentally, the 'else' is block like everyone other, so it "should have" their own brackets and indentation: if (foo) { a=0; } else { if (bar) { a=1; } else { a=2; } } I understand that in cases of use strictly as 'elseif' is acceptable to omit the brackets. However, only I'm not sure if complicated cases are a source of compiler warnings 'ambiguous else'. -- Slávek