[PATCH] [merge-tracking] clarity fix for subversion/libsvn_subr/mergeinfo.c

[Kamesh Jayachandran <ka...@collab.net>]

Hi All,
Find the attached patch.

With regards
Kamesh Jayachandran

[[[
Patch by: Kamesh Jayachandran <ka...@collab.net>

Had a tough time initially in understanding what 'elt2 & elt1' is
120 lines of while loop. Though my mind now can easily map what
'elt2 & elt1' is, it would have saved few minutes if names had been
bit more hinting.

Doc correction to function header 'parse_revlist'.

* subversion/libsvn_subr/mergeinfo.c
  (parse_revlist):
   Doc header fix as follows,
   Typo fix 'revisioneelement' -> 'revisionelement',
   'revisionrange | REVISION' -> 'revisionelement' in defining 
'revisionlist'.

  (rangelist_intersect_or_remove):
   Variable name change from 'elt1' to 'cur_wboard_range'
   'elt2' to 'cur_erase_range'.

]]]

Re: [PATCH] [merge-tracking] clarity fix for subversion/libsvn_subr/mergeinfo.c

[Daniel Rall <dl...@collab.net>]

On Sun, 09 Jul 2006, Kamesh Jayachandran wrote:
...
> Had a tough time initially in understanding what 'elt2 & elt1' is
> 120 lines of while loop. Though my mind now can easily map what
> 'elt2 & elt1' is, it would have saved few minutes if names had been
> bit more hinting.

I agree that more descriptive and consistently named variable names
would improve the comprehensibility of this routine.

> Doc correction to function header 'parse_revlist'.
> 
> * subversion/libsvn_subr/mergeinfo.c
>  (parse_revlist):
>   Doc header fix as follows,
>   Typo fix 'revisioneelement' -> 'revisionelement',
>   'revisionrange | REVISION' -> 'revisionelement' in defining 
> 'revisionlist'.

Committed as r20485.

>  (rangelist_intersect_or_remove):
>   Variable name change from 'elt1' to 'cur_wboard_range'
>   'elt2' to 'cur_erase_range'.

While I'm in favor of this type of change, I wonder if we shouldn't
remove the eraser/whiteboard metaphor at the same time, since it
simply isn't applicable to the "intersect" mode of this function.
Thoughts?

- Dan