Hi Alan,
I have a few questions on your patch (inline).
On 04/23/2009 11:11 AM, Alan Wood wrote:
> Hi,
>
> Here is an updated version of the patch in which I have reverted two lines that I
> changed as I have been unable to convince myself that the change was necessary.
>
> Can't actually come up with a test case that uses the lines so I will leave them alone.
>
> Index: subversion/libsvn_client/blame.c
> ===================================================================
> --- subversion/libsvn_client/blame.c (revision 37376)
> +++ subversion/libsvn_client/blame.c (working copy)
> @@ -526,18 +526,18 @@
> assert(walk->start == walk_merged->start);
>
> if (walk->next->start < walk_merged->next->start)
> - {
> - struct blame *tmp = blame_create(chain_merged, walk_merged->next->rev,
> + { /* split walk_merged in two so start points the same */
> + struct blame *tmp = blame_create(chain_merged, walk_merged->rev,
> walk->next->start);
> - tmp->next = walk_merged->next->next;
> + tmp->next = walk_merged->next;
> walk_merged->next = tmp;
> }
>
> if (walk->next->start > walk_merged->next->start)
> - {
> - struct blame *tmp = blame_create(chain, walk->next->rev,
> + { /* split walk in two so start points the same */
> + struct blame *tmp = blame_create(chain, walk->rev,
> walk_merged->next->start);
> - tmp->next = walk->next->next;
> + tmp->next = walk->next;
> walk->next = tmp;
> }
> }
> @@ -781,7 +781,7 @@
> sb->data, FALSE, iterpool));
> else
> SVN_ERR(receiver(receiver_baton, line_no, SVN_INVALID_REVNUM,
> - NULL, SVN_INVALID_REVNUM, NULL, NULL,
> + NULL, merged_rev, merged_rev_props, merged_path,
Why is this chunk needed?
> sb->data, TRUE, iterpool));
> }
> if (eof) break;
> Index: subversion/svn/blame-cmd.c
> ===================================================================
> --- subversion/svn/blame-cmd.c (revision 37376)
> +++ subversion/svn/blame-cmd.c (working copy)
> @@ -178,8 +178,9 @@
> revision which put the line in its current state, so we use the
> earliest revision. If we ever switch to a backward blame algorithm,
> we may need to adjust this. */
> - if (merged_revision < revision)
> - {
> + if (SVN_IS_VALID_REVNUM(merged_revision) &&
> + (!SVN_IS_VALID_REVNUM(revision) || merged_revision < revision))
> + {
Same question here.
The test case you provided - committed in r37719 - doesn't need those
two changes to pass.
Lieven
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2240565
Received on 2009-05-13 19:35:10 CEST