Re: [Development] kdelibs coding style
> From: Thiago Macieira >To: development@qt-project.org >Sent: Thursday, May 2, 2013 11:09 AM >Subject: Re: [Development] kdelibs coding style >On quinta-feira, 2 de maio de 2013 16.12.00, Daniel Teske wrote: >> > I tried to compromise, but it seems like we don't want to compromise. >> Apparently you think there is some benefit, but so far you haven't actually >> explained what that benefit is. You should explain what those are before >> trying to change a rule. >Making it easier to bring KDE developers in to Qt coding, and making it easier >to bring KDE code in too. >It's not that big of a benefit. But it's also not big of a change either. I'd suggest that the change would be one of consistency between practice and documentation as well if current practice is to optionally allow the braces that way as seems to be indicated by this thread. $0.02 Ben ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On quinta-feira, 2 de maio de 2013 16.12.00, Daniel Teske wrote: > > I tried to compromise, but it seems like we don't want to compromise. > > Apparently you think there is some benefit, but so far you haven't actually > explained what that benefit is. You should explain what those are before > trying to change a rule. Making it easier to bring KDE developers in to Qt coding, and making it easier to bring KDE code in too. It's not that big of a benefit. But it's also not big of a change either. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On Tuesday 30 Apr 2013 20:45:40 Thiago Macieira wrote: > On terça-feira, 30 de abril de 2013 19.51.40, Daniel Teske wrote: > > > So you suggest that KDE adopt the Qt style and Qt doesn't change > > > anything? > > > > Why do they need to be the same? > > That was the request: can they be the same? They are identical except for > the braces-on-if. > > I tried to compromise, but it seems like we don't want to compromise. Apparently you think there is some benefit, but so far you haven't actually explained what that benefit is. You should explain what those are before trying to change a rule. daniel ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On Wed, May 01, 2013 at 05:38:44AM +1000, Lorn Potter wrote: > > On 01/05/2013, at 2:47 AM, Oswald Buddenhagen > wrote: > > > On Tue, Apr 30, 2013 at 09:01:51AM -0700, Thiago Macieira wrote: > >> On segunda-feira, 29 de abril de 2013 23.30.59, André Pönitz wrote: > >>> The rules are ok as they are (pretty much as _any_ set of consistently > >>> applied and not-completely-weird rules). Opening them up only leads to > >>> more review bikeshedding. > >>> > >>> It's not that people actively change coding style in old Qt code, and > >>> there's always the "don't stick to the rules if it makes you look bad" > >>> excuse. > >> > >> Let's not try to change any of the rules, since this leads to > >> bikeshedding, > >> like André said. It clearly has already become that. > >> > > so let's meta-bikeshed instead? > > > >> So let's just change the interpretation: the "don't stick to the rules" > >> allows > >> us to accept a slightly different style if that makes it nicer. That > >> should be > >> enough to apply to the case of braces in single-line ifs. > >> > > now, that is kinda ridiculous. the cop-out rule exists to justify > > exceptions in corner cases, not to give everyone a free pass for their > > coding style preferences. > > If I recall, it was also to give developers a little freedom in their > expression. I took it more like a "we don't want to force rules on you that make you look stupid". There are projects with weird rules without such fallback, and it's at times ridiculous to watch those. I am quite happy Qt takes a somewhat more pragmatic approach here by providing the fallback. "No rules" is bad, "too strict rules" is also bad. Somewhere in between there's a sane middle ground. The current set of rules seems to be somewhere there. "Giving developers a little freedom in their expression" sounds wrong, and dangerous. I understand there are areas in the sources that must have been written with something like that in mind. But at the end of the day, we are not talking about an exhibition of Modern Art here. People come and go. The code is meant to be read and understood(!) by complete strangers. Any weirdness added, ranging from using unusual concepts to unnecessary style deviation from what is documented, or "common", makes that harder. As dull as it sounds: Just stick to the crowd when choosing coding style. Andre' ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On Tue, Apr 30, 2013 at 12:38 PM, Lorn Potter wrote: > > On 01/05/2013, at 2:47 AM, Oswald Buddenhagen > wrote: > >> On Tue, Apr 30, 2013 at 09:01:51AM -0700, Thiago Macieira wrote: >>> On segunda-feira, 29 de abril de 2013 23.30.59, André Pönitz wrote: The rules are ok as they are (pretty much as _any_ set of consistently applied and not-completely-weird rules). Opening them up only leads to more review bikeshedding. It's not that people actively change coding style in old Qt code, and there's always the "don't stick to the rules if it makes you look bad" excuse. >>> >>> Let's not try to change any of the rules, since this leads to bikeshedding, >>> like André said. It clearly has already become that. >>> >> so let's meta-bikeshed instead? >> >>> So let's just change the interpretation: the "don't stick to the rules" >>> allows >>> us to accept a slightly different style if that makes it nicer. That should >>> be >>> enough to apply to the case of braces in single-line ifs. >>> >> now, that is kinda ridiculous. the cop-out rule exists to justify >> exceptions in corner cases, not to give everyone a free pass for their >> coding style preferences. > > If I recall, it was also to give developers a little freedom in their > expression. I had assumed it was also to prevent people wasting time on big arguments over whitespace. So long as the code isn't a hideous mess then you are permitted to focus on more important edits. Drawing it back to Thiago's question, the style guides sound close enough already that I don't think we'll get real gains by worrying about it. I feel that the cost of a few extra braces here or a few fewer braces there is less than a twenty e-mail debate, which seems to be the alternative ;) . -- Alan Alpert ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On 01/05/2013, at 2:47 AM, Oswald Buddenhagen wrote: > On Tue, Apr 30, 2013 at 09:01:51AM -0700, Thiago Macieira wrote: >> On segunda-feira, 29 de abril de 2013 23.30.59, André Pönitz wrote: >>> The rules are ok as they are (pretty much as _any_ set of consistently >>> applied and not-completely-weird rules). Opening them up only leads to >>> more review bikeshedding. >>> >>> It's not that people actively change coding style in old Qt code, and >>> there's always the "don't stick to the rules if it makes you look bad" >>> excuse. >> >> Let's not try to change any of the rules, since this leads to bikeshedding, >> like André said. It clearly has already become that. >> > so let's meta-bikeshed instead? > >> So let's just change the interpretation: the "don't stick to the rules" >> allows >> us to accept a slightly different style if that makes it nicer. That should >> be >> enough to apply to the case of braces in single-line ifs. >> > now, that is kinda ridiculous. the cop-out rule exists to justify > exceptions in corner cases, not to give everyone a free pass for their > coding style preferences. If I recall, it was also to give developers a little freedom in their expression. > on a personal note, while i strongly sympathize with the diff > minimization philosophy, i also feel quite a dislike for excess braces. > adding to that the in this case pretty persuasive consideration of the > status quo, i'd like this discussion to die. NOW. > ___ > Development mailing list > Development@qt-project.org > http://lists.qt-project.org/mailman/listinfo/development Lorn Potter Senior Software Engineer, QtSensors/QtSensorGestures ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On 01/05/2013, at 2:01 AM, Thiago Macieira wrote: > On segunda-feira, 29 de abril de 2013 23.30.59, André Pönitz wrote: >> The rules are ok as they are (pretty much as _any_ set of consistently >> applied and not-completely-weird rules). Opening them up only leads to >> more review bikeshedding. >> >> It's not that people actively change coding style in old Qt code, and >> there's always the "don't stick to the rules if it makes you look bad" >> excuse. > > Let's not try to change any of the rules, since this leads to bikeshedding, > like André said. It clearly has already become that. > > So let's just change the interpretation: the "don't stick to the rules" > allows > us to accept a slightly different style if that makes it nicer. That should > be > enough to apply to the case of braces in single-line ifs. I have preferred and written always using braces. These were always meant as guidelines not hard rules. Lorn Potter Senior Software Engineer, QtSensors/QtSensorGestures ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On terça-feira, 30 de abril de 2013 19.51.40, Daniel Teske wrote: > > So you suggest that KDE adopt the Qt style and Qt doesn't change anything? > > Why do they need to be the same? That was the request: can they be the same? They are identical except for the braces-on-if. I tried to compromise, but it seems like we don't want to compromise. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On Tuesday 30 Apr 2013 19:44:45 Thiago Macieira wrote: > On terça-feira, 30 de abril de 2013 18.47.33, Oswald Buddenhagen wrote: > > on a personal note, while i strongly sympathize with the diff > > minimization philosophy, i also feel quite a dislike for excess braces. > > adding to that the in this case pretty persuasive consideration of the > > status quo, i'd like this discussion to die. NOW. > > So you suggest that KDE adopt the Qt style and Qt doesn't change anything? Why do they need to be the same? daniel ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On terça-feira, 30 de abril de 2013 18.47.33, Oswald Buddenhagen wrote: > on a personal note, while i strongly sympathize with the diff > minimization philosophy, i also feel quite a dislike for excess braces. > adding to that the in this case pretty persuasive consideration of the > status quo, i'd like this discussion to die. NOW. So you suggest that KDE adopt the Qt style and Qt doesn't change anything? -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On Tue, Apr 30, 2013 at 09:01:51AM -0700, Thiago Macieira wrote: > On segunda-feira, 29 de abril de 2013 23.30.59, André Pönitz wrote: > > The rules are ok as they are (pretty much as _any_ set of consistently > > applied and not-completely-weird rules). Opening them up only leads to > > more review bikeshedding. > > > > It's not that people actively change coding style in old Qt code, and > > there's always the "don't stick to the rules if it makes you look bad" > > excuse. > > Let's not try to change any of the rules, since this leads to bikeshedding, > like André said. It clearly has already become that. > so let's meta-bikeshed instead? > So let's just change the interpretation: the "don't stick to the rules" > allows > us to accept a slightly different style if that makes it nicer. That should > be > enough to apply to the case of braces in single-line ifs. > now, that is kinda ridiculous. the cop-out rule exists to justify exceptions in corner cases, not to give everyone a free pass for their coding style preferences. on a personal note, while i strongly sympathize with the diff minimization philosophy, i also feel quite a dislike for excess braces. adding to that the in this case pretty persuasive consideration of the status quo, i'd like this discussion to die. NOW. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On segunda-feira, 29 de abril de 2013 23.30.59, André Pönitz wrote: > The rules are ok as they are (pretty much as _any_ set of consistently > applied and not-completely-weird rules). Opening them up only leads to > more review bikeshedding. > > It's not that people actively change coding style in old Qt code, and > there's always the "don't stick to the rules if it makes you look bad" > excuse. Let's not try to change any of the rules, since this leads to bikeshedding, like André said. It clearly has already become that. So let's just change the interpretation: the "don't stick to the rules" allows us to accept a slightly different style if that makes it nicer. That should be enough to apply to the case of braces in single-line ifs. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On Tue, Apr 30, 2013 at 07:37:36AM -0600, Charley Bay wrote:
> Samuel sayeth:
>
> > A good reason for using the
> >
> > Foo()
> > : x(0)
> > , y(0)
> > {
> > }
> >
> > syntax for initializer lists after all is to make diffs easier to read
> > when adding or removing a member variable. The same argument could be
> > made for permitting braces for one-line if-statements.
> >
>
> +1, that's a really helpful style for the base/member initializer list.
+1, if we're really starting a vote. It makes diffs reading, line add/remove
and copy-pasting more practical (at least for me).
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On Tue, Apr 30, 2013 at 5:13 PM, Thiago Macieira wrote: > On terça-feira, 30 de abril de 2013 08.54.31, Samuel Rødal wrote: >> A good reason for using the >> >> Foo() >> : x(0) >> , y(0) > > This is extremely ugly and should be discouraged. If it is about the sum of all opinions, I agree with Thiago: -1. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On terça-feira, 30 de abril de 2013 08.54.31, Samuel Rødal wrote: > A good reason for using the > > Foo() > : x(0) > , y(0) This is extremely ugly and should be discouraged. -- Thiago Macieira - thiago.macieira (AT) intel.com Software Architect - Intel Open Source Technology Center signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On 04/29/2013 10:31 PM, Olivier Goffart wrote: > > I support the change to optionally permit the braces. > +1. Have always disliked this particular style. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
Samuel sayeth:
> A good reason for using the
>
> Foo()
> : x(0)
> , y(0)
> {
> }
>
> syntax for initializer lists after all is to make diffs easier to read
> when adding or removing a member variable. The same argument could be
> made for permitting braces for one-line if-statements.
>
+1, that's a really helpful style for the base/member initializer list.
--charley
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On 04/29/2013 10:31 PM, Olivier Goffart wrote:
> On Monday 29 April 2013 09:43:55 Thiago Macieira wrote:
>> On segunda-feira, 29 de abril de 2013 17.16.49, Sergio Martins wrote:
>>> The thing is, I just discovered that the kdelibs style[1] has a little
>>> difference from Qt, it uses braces for one-line if statements.
>>>
>>> This seems to be a documentation/policy only rule, as the kdelibs code is
>>> full of if's without braces.
>>
>> Remember that kdelibs isn't completely ported over to the kdelibs coding
>> style.
>>
>> So you may be seeing code that simply needs to be fixed when it's next
>> touched.
>>
>> The brace-on-one-line-if was adopted for KDE for a couple of good reasons:
>>
>> 1) when you add a second line, the if line does not need to be changed
>> 2) conversely, if the if had a second line and you remove it, you don't need
>> to change the if
>> 3) you're less likely to make this mistake:
>> if (condition);
>> doThat();
>>
>> The reason Qt requests that the braces not be added is a simple one:
>> 1) it's uglier
>>
>> With that in mind, I would suggest we change *both* coding styles to
>> optionally permit the braces, and strongly suggest it for more-complex
>> conditionals. In particular, it should suggest you leave the braces if
>> you've reduced the statements to one line.
>
> I support the change to optionally permit the braces.
+1
A good reason for using the
Foo()
: x(0)
, y(0)
{
}
syntax for initializer lists after all is to make diffs easier to read
when adding or removing a member variable. The same argument could be
made for permitting braces for one-line if-statements.
--
Samuel
___
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On Mon, Apr 29, 2013 at 10:37:10PM +0200, Giuseppe D'Angelo wrote: > On 29 April 2013 21:20, Thiago Macieira wrote: > > On segunda-feira, 29 de abril de 2013 18.52.16, Giuseppe D'Angelo wrote: > >> On 29 April 2013 18:43, Thiago Macieira wrote: > >> > With that in mind, I would suggest we change *both* coding styles to > >> > optionally permit the braces, and strongly suggest it for more-complex > >> > conditionals. > >> > >> In Qt the braces are already mandatory if the condition spans over > >> more than one line (which is typical for complex conditions, as other > >> rules demand for lines to be properly wrapped). Are you suggesting to > >> loosen this? > > > > No, I am suggesting adding that rule, because I didn't know it was there (it > > wasn't there when I last looked at the document, in my early days in > > Trolltech). > > It's there [1] > > * Exception 1: Use braces also if the parent statement covers several lines > > / wraps: > > and > > Keep lines shorter than 100 characters; wrap if necessary > > [1] http://qt-project.org/wiki/Qt_Coding_Style > > I also support the idea of avoiding to remove braces if the body > collapses to one statement only. The rules are ok as they are (pretty much as _any_ set of consistently applied and not-completely-weird rules). Opening them up only leads to more review bikeshedding. It's not that people actively change coding style in old Qt code, and there's always the "don't stick to the rules if it makes you look bad" excuse. I personally would be happy enough if at least new code pretended to roughly stick to the existing rules, before inventing new ones... Andre' ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On 29 April 2013 21:20, Thiago Macieira wrote: > On segunda-feira, 29 de abril de 2013 18.52.16, Giuseppe D'Angelo wrote: >> On 29 April 2013 18:43, Thiago Macieira wrote: >> > With that in mind, I would suggest we change *both* coding styles to >> > optionally permit the braces, and strongly suggest it for more-complex >> > conditionals. >> >> In Qt the braces are already mandatory if the condition spans over >> more than one line (which is typical for complex conditions, as other >> rules demand for lines to be properly wrapped). Are you suggesting to >> loosen this? > > No, I am suggesting adding that rule, because I didn't know it was there (it > wasn't there when I last looked at the document, in my early days in > Trolltech). It's there [1] > * Exception 1: Use braces also if the parent statement covers several lines / > wraps: and > Keep lines shorter than 100 characters; wrap if necessary [1] http://qt-project.org/wiki/Qt_Coding_Style I also support the idea of avoiding to remove braces if the body collapses to one statement only. -- Giuseppe D'Angelo ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On Monday 29 April 2013 09:43:55 Thiago Macieira wrote: > On segunda-feira, 29 de abril de 2013 17.16.49, Sergio Martins wrote: > > The thing is, I just discovered that the kdelibs style[1] has a little > > difference from Qt, it uses braces for one-line if statements. > > > > This seems to be a documentation/policy only rule, as the kdelibs code is > > full of if's without braces. > > Remember that kdelibs isn't completely ported over to the kdelibs coding > style. > > So you may be seeing code that simply needs to be fixed when it's next > touched. > > The brace-on-one-line-if was adopted for KDE for a couple of good reasons: > > 1) when you add a second line, the if line does not need to be changed > 2) conversely, if the if had a second line and you remove it, you don't need > to change the if > 3) you're less likely to make this mistake: > if (condition); > doThat(); > > The reason Qt requests that the braces not be added is a simple one: > 1) it's uglier > > With that in mind, I would suggest we change *both* coding styles to > optionally permit the braces, and strongly suggest it for more-complex > conditionals. In particular, it should suggest you leave the braces if > you've reduced the statements to one line. I support the change to optionally permit the braces. -- Olivier Woboq - Qt services and support - http://woboq.com - http://code.woboq.org ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On segunda-feira, 29 de abril de 2013 18.52.16, Giuseppe D'Angelo wrote: > On 29 April 2013 18:43, Thiago Macieira wrote: > > With that in mind, I would suggest we change *both* coding styles to > > optionally permit the braces, and strongly suggest it for more-complex > > conditionals. > > In Qt the braces are already mandatory if the condition spans over > more than one line (which is typical for complex conditions, as other > rules demand for lines to be properly wrapped). Are you suggesting to > loosen this? No, I am suggesting adding that rule, because I didn't know it was there (it wasn't there when I last looked at the document, in my early days in Trolltech). Also, Creator adds an extra indent to complex conditionals: if (a && b) c; -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On 29 April 2013 18:43, Thiago Macieira wrote: > > With that in mind, I would suggest we change *both* coding styles to > optionally permit the braces, and strongly suggest it for more-complex > conditionals. In Qt the braces are already mandatory if the condition spans over more than one line (which is typical for complex conditions, as other rules demand for lines to be properly wrapped). Are you suggesting to loosen this? -- Giuseppe D'Angelo ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
Re: [Development] kdelibs coding style
On segunda-feira, 29 de abril de 2013 17.16.49, Sergio Martins wrote: > The thing is, I just discovered that the kdelibs style[1] has a little > difference from Qt, it uses braces for one-line if statements. > > This seems to be a documentation/policy only rule, as the kdelibs code is > full of if's without braces. Remember that kdelibs isn't completely ported over to the kdelibs coding style. So you may be seeing code that simply needs to be fixed when it's next touched. The brace-on-one-line-if was adopted for KDE for a couple of good reasons: 1) when you add a second line, the if line does not need to be changed 2) conversely, if the if had a second line and you remove it, you don't need to change the if 3) you're less likely to make this mistake: if (condition); doThat(); The reason Qt requests that the braces not be added is a simple one: 1) it's uglier With that in mind, I would suggest we change *both* coding styles to optionally permit the braces, and strongly suggest it for more-complex conditionals. In particular, it should suggest you leave the braces if you've reduced the statements to one line. -- Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org Software Architect - Intel Open Source Technology Center PGP/GPG: 0x6EF45358; fingerprint: E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358 signature.asc Description: This is a digitally signed message part. ___ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
