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

RE: [PATCH] [merge-tracking] Code clarity fix

From: Jonathan Gilbert <o2w9gs702_at_sneakemail.com>
Date: 2006-09-06 21:41:35 CEST

At 7:32 PM 06/09/2006, Daniel Berlin wrote:
> I'm not sure what you think this patch does, but it's not right to
> remove these cases.
>
> The cases below that you are removing combine ranges in case you have
> the following:
>
> in1 = [0, 10]
> in2 = [4, 7]
> and
> in1= [1, 3]
> in2 = [0, 10]
>
> In both cases, one range completely subsumes the other, so we just use
> one of the ranges.
>
> It has nothing to do with revert ranges.

At 10:03 PM 06/09/2006 +0530, Kamesh Jayachandran wrote:
> //Takes care of in1 = [0, 10] in2 = [4, 7]
> else if (in1->start <= in2->start
> && in1->end >= in2->start)
> {
> (*output)->start = in1->start;
> (*output)->end = MAX(in1->end, in2->end);
> return TRUE;
> }
> //Takes care of in1= [1, 3] in2 = [0, 10]
>
> else if (in2->start <= in1->start
> && in2->end >= in1->start)
> {
> (*output)->start = in2->start;
> (*output)->end = MAX(in1->end, in2->end);
> return TRUE;
> }
>
> The code that is removed by the patch try to handle revert ranges which
> should be a no-op.

I have been watching this thread for some time now, and it seems that a
much simpler algorithm could be used for what's needed. Basically, we want
to merge two ranges if they are touching in any way, right? And it's safe
to assume that the "end" field will always be >= the "start" field, right?

If so, then consider the following code:

static svn_boolean_t
range_contains_revision(svn_merge_range_t *range, svn_revnum_t revision)
{
  return (range->start <= revision) && (range->end >= revision);
}

static svn_boolean_t
ranges_are_adjacent(svn_merge_range_t *in1, svn_merge_range_t *in2)
{
  return (in1->end + 1 == in2->start) || (in2->end + 1 == in1->start);
}

static svn_boolean_t
combine_ranges(svn_merge_range_t **output, svn_merge_range_t *in1,
               svn_merge_range_t *in2)
{
  if (range_contains_revision(in1, in2->start)
   || range_contains_revision(in2, in1->start)
   || ranges_are_adjacent(in1, in2))
    {
      (*output)->start = MIN(in1->start, in2->start);
      (*output)->end = MAX(in1->end, in2->end);
      return TRUE;
    }

  return FALSE;
}

Basically, if the ranges are "touching", then either one of them will start
in the middle of the other one, or one of them will start immediately after
the other one. In either case, the start & end can be computed in the same
way.

Are there any holes in this? If not, I think it's a lot more readable than
what's currently in mergeinfo.c on the merge-tracking branch. :-)

Jonathan Gilbert

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Sep 6 21:40:32 2006

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.