[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r33558 - trunk/subversion/libsvn_subr

From: Greg Stein <gstein_at_gmail.com>
Date: Thu, 9 Oct 2008 01:06:08 -0700

Certainly. As I said, "I've always let it slide", favoring personal
expression over rigid style rules.

On Wed, Oct 8, 2008 at 9:02 PM, Karl Fogel <kfogel_at_red-bean.com> wrote:
> "Greg Stein" <gstein_at_gmail.com> writes:
>> Why all the parens around those tests? Parens are not needed due to
>> basic precedence. Also note that the other == tests don't add
>> unnecessary parens, so the ones that you added don't even "follow
>> prevalent convention for the file".
>>
>> And (unrelated) note that I've always hated unnecessary parens. There
>> are a lot of them in the svn codebase where devs had said "it helps to
>> clarify" or "just in case precedence isn't obvious, the parens help".
>> Stuff like that, so I've always let it slide. But I still don't like
>> it :-)
>
> Being consistent within the same conditional expression would be good,
> no argument there.
>
> But beyond basic consistency, let's just let devs do whatever they're
> comfortable with when it comes to parenthesizing for precedence. The
> differences in readability are pretty small -- for some of us,
> non-existent.
>
> -Karl
>
>> On Wed, Oct 8, 2008 at 1:01 PM, <rhuijben_at_tigris.org> wrote:
>>> Author: rhuijben
>>> Date: Wed Oct 8 13:01:12 2008
>>> New Revision: 33558
>>>
>>> Log:
>>> Following up to r33553, use the new enum values within the #if defined(WIN32)
>>> blocks. This should fix the Windows builds.
>>>
>>> * subversion/libsvn_subr/dirent_uri.c
>>> (canonicalize), (get_longest_ancestor_length),
>>> (is_child), (is_ancestor): Check for type_dirent instead of not uri for the
>>> Windows specific cases.
>>>
>>> Modified:
>>> trunk/subversion/libsvn_subr/dirent_uri.c
>>>
>>> Modified: trunk/subversion/libsvn_subr/dirent_uri.c
>>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/dirent_uri.c?pathrev=33558&r1=33557&r2=33558
>>> ==============================================================================
>>> --- trunk/subversion/libsvn_subr/dirent_uri.c Wed Oct 8 13:01:12 2008 (r33557)
>>> +++ trunk/subversion/libsvn_subr/dirent_uri.c Wed Oct 8 13:01:12 2008 (r33558)
>>> @@ -204,7 +204,7 @@ canonicalize(path_type_t type, const cha
>>> #if defined(WIN32) || defined(__CYGWIN__)
>>> /* On Windows permit two leading separator characters which means an
>>> * UNC path. */
>>> - if (! uri && *src == '/')
>>> + if ((type == type_dirent) && *src == '/')
>>> *(dst++) = *(src++);
>>> #endif /* WIN32 or Cygwin */
>>> }
>>> @@ -271,7 +271,7 @@ canonicalize(path_type_t type, const cha
>>> #if defined(WIN32) || defined(__CYGWIN__)
>>> /* Skip leading double slashes when there are less than 2
>>> * canon segments. UNC paths *MUST* have two segments. */
>>> - if (! uri && canon[0] == '/' && canon[1] == '/')
>>> + if ((type == type_dirent) && canon[0] == '/' && canon[1] == '/')
>>> {
>>> if (canon_segments < 2)
>>> return canon + 1;
>>> @@ -380,7 +380,7 @@ get_longest_ancestor_length(path_type_t
>>> return 1;
>>> #if defined(WIN32) || defined(__CYGWIN__)
>>> /* X:/foo and X:/bar returns X:/ */
>>> - if (!uris &&
>>> + if ((types == type_dirent) &&
>>> last_dirsep == 2 && path1[1] == ':' && path1[2] == '/'
>>> && path2[1] == ':' && path2[2] == '/')
>>> return 3;
>>> @@ -436,7 +436,7 @@ is_child(path_type_t type, const char *p
>>> {
>>> if (path1[i - 1] == '/'
>>> #if defined(WIN32) || defined(__CYGWIN__)
>>> - || (! uri && path1[i - 1] == ':')
>>> + || ((type == type_dirent) && path1[i - 1] == ':')
>>> #endif /* WIN32 or Cygwin */
>>> )
>>> if (path2[i] == '/')
>>> @@ -473,7 +473,7 @@ is_ancestor(path_type_t type, const char
>>> if (strncmp(path1, path2, path1_len) == 0)
>>> return path1[path1_len - 1] == '/'
>>> #if defined(WIN32) || defined(__CYGWIN__)
>>> - || (! uri && path1[path1_len - 1] == ':')
>>> + || ((type == type_dirent) && path1[path1_len - 1] == ':')
>>> #endif /* WIN32 or Cygwin */
>>> || (path2[path1_len] == '/' || path2[path1_len] == '\0');
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
>>> For additional commands, e-mail: svn-help_at_subversion.tigris.org
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
>> For additional commands, e-mail: dev-help_at_subversion.tigris.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-09 10:06:24 CEST

This is an archived mail posted to the Subversion Dev mailing list.