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

Re: svn commit: r921713 - /subversion/trunk/subversion/libsvn_subr/mergeinfo.c

From: Paul Burba <ptburba_at_gmail.com>
Date: Fri, 19 Mar 2010 14:01:32 -0400

On Tue, Mar 16, 2010 at 8:58 AM, Julian Foad <julian.foad_at_wandisco.com> wrote:
> On Mon, 2010-03-15, Paul Burba wrote:
>> In http://svn.apache.org/viewvc?view=revision&revision=923389 I fixed
>> the bug where the order of the rangelist arguments to
>> svn_rangelist_intersect() could produce different results.  Now the
>> intersection of two ranges with different inheritance always results
>> in an inheritable rangelist, regardless of the order in which they are
>> passed.
>>
>> I also changed the doc strings for svn_rangelist_intersect() and
>> svn_mergeinfo_intersect2() to reflect the existing behavior where the
>> resulting intersection of two non-inheritable ranges is also
>> non-inheritable, not inheritable as had been claimed.
>>
>> So now everything is consistent: The intersection of two ranges is
>> only non-inheritable if both of the ranges are non-inheritable.
>
> Thank you, Paul.  That's great!  Review...
>
>
>> +/* If DO_REMOVE is true, then remove any overlapping ranges described by
>> +   RANGELIST1 from RANGELIST2 and place the results in *OUTPUT.  When
>> +   DO_REMOVE is true, RANGELIST1 is effectively the "eraser" and RANGELIST2
>> +   the "whiteboard".
>> +
>> +   If DO_REMOVE is false, then capture the intersection between RANGELIST1
>> +   and RANGELIST2 and place the results in *OUTPUT.  The ordering of
>> +   RANGELIST1 and RANGELIST2 doesn't matter when DO_REMOVE is false.
>> +
>> +   If CONSIDER_INHERITANCE is true, then take the inheritance of the
>> +   ranges in RANGELIST1 and RANGELIST2 into account when comparing them
>> +   for intersection, see the doc string for svn_rangelist_intersection().
>> +
>> +   If CONSIDER_INHERITANCE is true, then ranges with differing inheritance
>> +   may intersect, but the resulting intersection is non-inheritable only
>> +   if both ranges were non-inheritable, e.g.:
>
> I think the last para means to say "If CONSIDER_INHERITANCE is
> *false* ...".

Indeed I did!

>> -                              const apr_array_header_t *eraser,
>> -                              const apr_array_header_t *whiteboard,
>> +                              const apr_array_header_t *rangelist1,
>> +                              const apr_array_header_t *rangelist2,
>
> I see where you were going with that rename, but actually the names
> "eraser" and "whiteboard" were rather helpful when thinking about the
> "remove" case.  "1" and "2" would be OK if we keep reminding ourselves
> that we're "removing 1 from 2" (rather than calculating "1 - 2"), but
> unfortunately the correspondence of names is now slightly more confusing
> than it already was:
>
>  rangelist1 - elt2 - j
>  rangelist2 - elt1 - i - lasti - wboardelt
>
> :-)  Could you clean up the names so they all correspond, like:
>
>  rangelist1 - elt1 - i1
>  rangelist2 - elt2 - i2 - lasti2 - working_elt2
>
> ?
>
> I'm suggesting "working_elt2" because it looks like "wboardelt" is a
> "working copy with local mods" of the last newly visited element from
> rangelist2.  I couldn't quite grasp this from the comment "/* Instead of
> making a copy of the entire array of rangelist2 elements, we just keep a
> copy of the current rangelist2 element that needs to be used, and modify
> our copy if necessary. */"
>
> - Julian

Made those changes http://svn.apache.org/viewvc?view=revision&revision=925356

Paul
Received on 2010-03-19 19:02:03 CET

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