You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2008/06/04 15:19:35 UTC
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
blair@tigris.org writes:
> Author: blair
> Date: Wed Jun 4 08:01:31 2008
> New Revision: 31583
>
> Log:
> * subversion/libsvn_wc/entries.c:
> Style change. Replace all
> if (! strcmp(a, b))
> with
> if (strcmp(a, b) == 0)
I see this has got into hacking when I wasn't paying attention :-(
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Blair Zajac <bl...@orcaware.com>.
Philip Martin wrote:
> Karl Fogel <kf...@red-bean.com> writes:
>
>> Philip Martin <ph...@codematters.co.uk> writes:
>>> blair@tigris.org writes:
>>>> Author: blair
>>>> Date: Wed Jun 4 08:01:31 2008
>>>> New Revision: 31583
>>>>
>>>> Log:
>>>> * subversion/libsvn_wc/entries.c:
>>>> Style change. Replace all
>>>> if (! strcmp(a, b))
>>>> with
>>>> if (strcmp(a, b) == 0)
>>> I see this has got into hacking when I wasn't paying attention :-(
>> Heh. Didn't know you minded; can you survive? :-)
>
> If there is a consensus, yes.
>
> To my eye strcmp() == 0 is as ugly as the construct 5 == x; I find
> !strcmp() is as natural as !ptr. entries.c used both strcmp styles
> and ! form was dominant so at least one other developer must agree
> with me. I realise styles change; years ago when I started writing C
> the people I worked with would only ever have written strcmp() == 0 if
> it was mixed with strcmp() > 0 or strcmp() < 0. Writing strcmp() == 0
> on it's own it would probably have attracted comments referring to
> Pascal, or Modula2, and the subsequent debate would inevitably include
> "of course a real programmer would use FORTRAN".
I like using 5 == x even if its ugly since it prevents coding mistakes.
The other one strcmp() == 0 I don't think is ugly and I don't feel strongly
about either style, but since it went into hacking.html, I don't have a problem
using that style.
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Branko Čibej <br...@xbc.nu>.
Stefan Sperling wrote:
>>>>
>>>>
>>> Heh. When I started programming, many people had already forgotten
>>> that these languages ever existed :)
>>>
>>>
>> Yes, I assumed as much.
>>
>> -- Brane
>>
>
> You hardly ever say things I cannot agree with, but at the same time
> you're grumpier than gcc -Wall -pedantic.
>
*rofl*
Thanks, you degrumpified me for a bit here. :)
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jun 04, 2008 at 11:00:41PM +0200, Branko Čibej wrote:
> Stefan Sperling wrote:
> > On Wed, Jun 04, 2008 at 08:30:48PM +0100, Philip Martin wrote:
> >
> >> To my eye strcmp() == 0 is as ugly as the construct 5 == x; I find
> >> !strcmp() is as natural as !ptr. entries.c used both strcmp styles
> >> and ! form was dominant so at least one other developer must agree
> >> with me. I realise styles change; years ago when I started writing C
> >> the people I worked with would only ever have written strcmp() == 0 if
> >> it was mixed with strcmp() > 0 or strcmp() < 0.
> >>
> >
> > In my mind, ! is a boolean operator, and strcmp() does not return
> > a boolean value:
> >
> > RETURN VALUES
> > The strcmp() and strncmp() return an integer greater than, equal to, or
> > less than 0, according as the string s1 is greater than, equal to, or
> > less than the string s2.
> >
> > For pointers, ! makes sense -- the pointer is either valid or it isn't.
> >
>
> I can think of several billion pointer values that are invalid and yet
> not NULL.
Yes, but most code just wants to know whether a pointer is NULL
or not before dereferencing or doing arithmetic on it.
> And it strikes my that far too much effort is being spent on stylistic
> nits lately. Yes, consistent style is important, but working code is
> more important.
Could it be that you perceive recent questions and remarks about coding
style I posted to the list as the stylistic nits you mention?
By doing this, I'm simply trying to figure out what the conventions are,
and try to adhere to them. Not every little thing is documented in HACKING.
Overall, I find Subversion's code remarkably consistent and readable.
The only inconsistencies I know of so far are uses of strcmp, putting
a space after ! or not, and putting a newline after function return
types or not.
FWIW, I don't mind dealing with such inconsistencies at all, as long as
I know that people have agreed on being inconsistent. Which is the case
for all the above, except that strcmp() seems to be reopened for debate.
Before this thread I wasn't consciously aware that there was a (recently
introduced) preferred convention for strcmp().
> To misquote some US president, if you can't read the code, get out of the
> project.
I was trying to provide a mnemonic for people who find strcmp() == 0
unnatural. Where was I saying "eww I can't read this"?
> >> Writing strcmp() == 0
> >> on it's own it would probably have attracted comments referring to
> >> Pascal, or Modula2, and the subsequent debate would inevitably include
> >> "of course a real programmer would use FORTRAN".
> >>
> >
> > Heh. When I started programming, many people had already forgotten
> > that these languages ever existed :)
> >
>
> Yes, I assumed as much.
>
>-- Brane
You hardly ever say things I cannot agree with, but at the same time
you're grumpier than gcc -Wall -pedantic.
Stefan
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Branko Čibej <br...@xbc.nu>.
Karl Fogel wrote:
> Branko Čibej <br...@xbc.nu> writes:
>
>> I'd suggest to leave well enough alone, and let people write
>> !strcmp() or '! strcmp()' or '0 == strcmp()' or 'strcmp() == 0' or
>> !strcmp() != 0' or whatever suits them as long as the meaning is
>> obvious and the code is correct.
>>
>
> Anyone object if I take it out of hacking.html? I don't care that
> strongly about it, and some people care strongly that we not care
> strongly...
>
FWIW, I've used at least three of the variants I listed in various
situations, depending on surrounding code, mood, sunspot activity and
the like. I don't think I ever got the urge to go back and change my
code, or other peoples', to fit my preference-of-the-day.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by David Glasser <gl...@davidglasser.net>.
On Thu, Jun 5, 2008 at 9:19 AM, Karl Fogel <kf...@red-bean.com> wrote:
> Anyone object if I take it out of hacking.html? I don't care that
> strongly about it, and some people care strongly that we not care
> strongly...
Yeah, I don't care either way; I think if people *actually* want to
make strcmp-as-equal more readable, then we should just define
svn_cstring_equals.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Karl Fogel wrote on Thu, 5 Jun 2008 at 12:19 -0400:
> Branko Čibej <br...@xbc.nu> writes:
> > I'd suggest to leave well enough alone, and let people write
> > !strcmp() or '! strcmp()' or '0 == strcmp()' or 'strcmp() == 0' or
> > !strcmp() != 0' or whatever suits them as long as the meaning is
> > obvious and the code is correct.
>
> Anyone object if I take it out of hacking.html? I don't care that
> strongly about it, and some people care strongly that we not care
> strongly...
>
+1
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Karl Fogel <kf...@red-bean.com>.
Branko Čibej <br...@xbc.nu> writes:
> Yes. Changing the coding style in a trivial way that doesn't actually
> improve readability for most people isn't reasonable.
Okay. So this is a different point than I thought you were making.
You don't object to enforcing stylistic consistency in general; you
object to *this particular convention*, because you think it doesn't
improve readability for most people -- and that all the strcmp choices
are about equally okay.
For all I know, you might be right; it's hard to take good surveys about
this stuff. The way currently documented in hacking.html is more
readable for me, but I could live with any of the other ways too, it's
not a big deal.
> We did this once with the space-before-paren thing, though that had
> somewhat stronger arguments on its side because most people (who
> weren't used to RMS's creative destructivism) were a bit put off by
> the code.
Yeah, 90% of people don't like GNU-style, beats me why :-). Anyway,
there's a technical advantage to the new way: it's easier to search for
function calls now, because they look less like conditional guards. So
I think that was a good change, even though I voted against it.
> Changing the spots on the bikeshed from mauve to lavender won't make a
> noticeable difference except that the other half of developers are
> going to be unhappy. :(
>
> I'd suggest to leave well enough alone, and let people write
> !strcmp() or '! strcmp()' or '0 == strcmp()' or 'strcmp() == 0' or
> !strcmp() != 0' or whatever suits them as long as the meaning is
> obvious and the code is correct.
Anyone object if I take it out of hacking.html? I don't care that
strongly about it, and some people care strongly that we not care
strongly...
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Branko Čibej <br...@xbc.nu>.
Karl Fogel wrote:
> Gently enforcing stylistic consistency is a perfectly reasonable thing
> for a project to do.
Yes. Changing the coding style in a trivial way that doesn't actually
improve readability for most people isn't reasonable.
We did this once with the space-before-paren thing, though that had
somewhat stronger arguments on its side because most people (who weren't
used to RMS's creative destructivism) were a bit put off by the code.
Changing the spots on the bikeshed from mauve to lavender won't make a
noticeable difference except that the other half of developers are going
to be unhappy. :(
I'd suggest to leave well enough alone, and let people write '!strcmp()
or '! strcmp()' or '0 == strcmp()' or 'strcmp() == 0' or '!strcmp() !=
0' or whatever suits them as long as the meaning is obvious and the code
is correct.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Mark Phippard <ma...@gmail.com>.
On Wed, Jun 4, 2008 at 5:09 PM, Karl Fogel <kf...@red-bean.com> wrote:
> Branko Čibej <br...@xbc.nu> writes:
>> And it strikes my that far too much effort is being spent on stylistic
>> nits lately. Yes, consistent style is important, but working code is
>> more important.
>
> Far too much effort is being spent lately pointing out how much effort
> is being spent on stylistic nits...
>
> Seriously: I don't see reviews where the patch is rejected solely for
> stylistic nits. Do you? And I try to always make substantive code
> comments along with stylistic ones, or when there are only stylistic
> comments, I go out of my way to point out that the style questions are
> not all that important. I see others doing similarly.
>
> Gently enforcing stylistic consistency is a perfectly reasonable thing
> for a project to do. If we were to do it in overeager and hard-nosed
> ways -- ways that could waste reviewers' time or drive away potential
> contributors -- then I'd think you were on to something. But, I don't
> see any of that going on. Do you?
FWIW, other than this I do not remember a whole lot of style
conversation going on lately. Also, we have several new committers
and contributors, which is great. So it would not surprise me if some
of the old conversations were being rehashed.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Karl Fogel <kf...@red-bean.com>.
Branko Čibej <br...@xbc.nu> writes:
> And it strikes my that far too much effort is being spent on stylistic
> nits lately. Yes, consistent style is important, but working code is
> more important.
Far too much effort is being spent lately pointing out how much effort
is being spent on stylistic nits...
Seriously: I don't see reviews where the patch is rejected solely for
stylistic nits. Do you? And I try to always make substantive code
comments along with stylistic ones, or when there are only stylistic
comments, I go out of my way to point out that the style questions are
not all that important. I see others doing similarly.
Gently enforcing stylistic consistency is a perfectly reasonable thing
for a project to do. If we were to do it in overeager and hard-nosed
ways -- ways that could waste reviewers' time or drive away potential
contributors -- then I'd think you were on to something. But, I don't
see any of that going on. Do you?
If there's an actual conflict between working code and stylistic nits,
then absolutely, we should favor working code. But the mere existence
of style comments does not mean there is such a conflict.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Branko Čibej <br...@xbc.nu>.
Stefan Sperling wrote:
> On Wed, Jun 04, 2008 at 08:30:48PM +0100, Philip Martin wrote:
>
>> To my eye strcmp() == 0 is as ugly as the construct 5 == x; I find
>> !strcmp() is as natural as !ptr. entries.c used both strcmp styles
>> and ! form was dominant so at least one other developer must agree
>> with me. I realise styles change; years ago when I started writing C
>> the people I worked with would only ever have written strcmp() == 0 if
>> it was mixed with strcmp() > 0 or strcmp() < 0.
>>
>
> In my mind, ! is a boolean operator, and strcmp() does not return
> a boolean value:
>
> RETURN VALUES
> The strcmp() and strncmp() return an integer greater than, equal to, or
> less than 0, according as the string s1 is greater than, equal to, or
> less than the string s2.
>
> For pointers, ! makes sense -- the pointer is either valid or it isn't.
>
I can think of several billion pointer values that are invalid and yet
not NULL.
And it strikes my that far too much effort is being spent on stylistic
nits lately. Yes, consistent style is important, but working code is
more important.
To misquote some US president, if you can't read the code, get out of
the project.
>> Writing strcmp() == 0
>> on it's own it would probably have attracted comments referring to
>> Pascal, or Modula2, and the subsequent debate would inevitably include
>> "of course a real programmer would use FORTRAN".
>>
>
> Heh. When I started programming, many people had already forgotten
> that these languages ever existed :)
>
Yes, I assumed as much.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Stefan Sperling wrote on Wed, 4 Jun 2008 at 22:06 +0200:
> On Wed, Jun 04, 2008 at 08:30:48PM +0100, Philip Martin wrote:
> > To my eye strcmp() == 0 is as ugly as the construct 5 == x; I find
> > !strcmp() is as natural as !ptr. entries.c used both strcmp styles
> > and ! form was dominant so at least one other developer must agree
> > with me. I realise styles change; years ago when I started writing C
> > the people I worked with would only ever have written strcmp() == 0 if
> > it was mixed with strcmp() > 0 or strcmp() < 0.
>
> In my mind, ! is a boolean operator, and strcmp() does not return
> a boolean value:
>
Yeah, but the difference between
strcmp("foo", "bar") == 0
and
strcmp("foo", "bar") != 0
is exactly one character. With
! strcmp("foo", "bar")
you can tell immediately that it tests for equality.
</bikeshed>
> > Writing strcmp() == 0
> > on it's own it would probably have attracted comments referring to
> > Pascal, or Modula2, and the subsequent debate would inevitably include
> > "of course a real programmer would use FORTRAN".
>
> Heh. When I started programming, many people had already forgotten
> that these languages ever existed :)
>
I wonder what languages they teach in high schools these days...
> Stefan
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jun 04, 2008 at 08:30:48PM +0100, Philip Martin wrote:
> To my eye strcmp() == 0 is as ugly as the construct 5 == x; I find
> !strcmp() is as natural as !ptr. entries.c used both strcmp styles
> and ! form was dominant so at least one other developer must agree
> with me. I realise styles change; years ago when I started writing C
> the people I worked with would only ever have written strcmp() == 0 if
> it was mixed with strcmp() > 0 or strcmp() < 0.
In my mind, ! is a boolean operator, and strcmp() does not return
a boolean value:
RETURN VALUES
The strcmp() and strncmp() return an integer greater than, equal to, or
less than 0, according as the string s1 is greater than, equal to, or
less than the string s2.
For pointers, ! makes sense -- the pointer is either valid or it isn't.
> Writing strcmp() == 0
> on it's own it would probably have attracted comments referring to
> Pascal, or Modula2, and the subsequent debate would inevitably include
> "of course a real programmer would use FORTRAN".
Heh. When I started programming, many people had already forgotten
that these languages ever existed :)
Stefan
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Philip Martin <ph...@codematters.co.uk>.
Karl Fogel <kf...@red-bean.com> writes:
> Philip Martin <ph...@codematters.co.uk> writes:
>> blair@tigris.org writes:
>>> Author: blair
>>> Date: Wed Jun 4 08:01:31 2008
>>> New Revision: 31583
>>>
>>> Log:
>>> * subversion/libsvn_wc/entries.c:
>>> Style change. Replace all
>>> if (! strcmp(a, b))
>>> with
>>> if (strcmp(a, b) == 0)
>>
>> I see this has got into hacking when I wasn't paying attention :-(
>
> Heh. Didn't know you minded; can you survive? :-)
If there is a consensus, yes.
To my eye strcmp() == 0 is as ugly as the construct 5 == x; I find
!strcmp() is as natural as !ptr. entries.c used both strcmp styles
and ! form was dominant so at least one other developer must agree
with me. I realise styles change; years ago when I started writing C
the people I worked with would only ever have written strcmp() == 0 if
it was mixed with strcmp() > 0 or strcmp() < 0. Writing strcmp() == 0
on it's own it would probably have attracted comments referring to
Pascal, or Modula2, and the subsequent debate would inevitably include
"of course a real programmer would use FORTRAN".
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r31583 - trunk/subversion/libsvn_wc
Posted by Karl Fogel <kf...@red-bean.com>.
Philip Martin <ph...@codematters.co.uk> writes:
> blair@tigris.org writes:
>> Author: blair
>> Date: Wed Jun 4 08:01:31 2008
>> New Revision: 31583
>>
>> Log:
>> * subversion/libsvn_wc/entries.c:
>> Style change. Replace all
>> if (! strcmp(a, b))
>> with
>> if (strcmp(a, b) == 0)
>
> I see this has got into hacking when I wasn't paying attention :-(
Heh. Didn't know you minded; can you survive? :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org