You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2004/05/14 13:13:39 UTC
Re: svn commit: r9709 - trunk/subversion/libsvn_client
jpieper@tigris.org writes:
> Modified: trunk/subversion/libsvn_client/ra.c
> ==============================================================================
> --- trunk/subversion/libsvn_client/ra.c (original)
> +++ trunk/subversion/libsvn_client/ra.c Fri May 14 05:41:37 2004
> @@ -359,14 +359,22 @@
> we'll either use the path, or, if was copied, use its
> copyfrom_path. */
> change = apr_hash_get (changed_paths, path, APR_HASH_KEY_STRING);
> - if (change)
> +
> + /* If PATH was not newly added in this revision, then it may or may
> + not have also been part of a moved subtree. In this case, set a
> + default previous path, but still look through the parents of this
> + path for a possible copy event. */
> + if (change && change->action != 'A' && change->action != 'R')
> + prev_path = path;
> +
> + if (change && (change->action == 'A' || change->action == 'R'))
> {
> + /* PATH is new in this revision. This means it cannot have been
> + part of a copied subtree. */
> if (change->copyfrom_path)
> prev_path = apr_pstrdup (pool, change->copyfrom_path);
> - else if (change->action == 'A')
> - prev_path = NULL;
> else
> - prev_path = path;
> + prev_path = NULL;
>
> *prev_path_p = prev_path;
> if (action_p)
Josh, could you a) fix the comment which is above the `change =
apr_hash_get' line to reflect the new truth, and maybe simplify the
logic above with an extra level of indentation? Something like:
/* See if PATH was explicitly changed in this revision. */
change = apr_hash_get (changed_paths, path, APR_HASH_KEY_STRING);
if (change)
{
/* If PATH was not newly added in this revision, then it may or may
not have also been part of a moved subtree. In this case, set a
default previous path, but still look through the parents of this
path for a possible copy event. */
if (change->action != 'A' && change->action != 'R')
{
prev_path = path;
}
else
{
/* PATH is new in this revision. This means it cannot have been
part of a copied subtree. */
if (change->copyfrom_path)
prev_path = apr_pstrdup (pool, change->copyfrom_path);
*prev_path_p = prev_path;
if (action_p)
*action_p = change->action;
if (copyfrom_rev_p)
*copyfrom_rev_p = change->copyfrom_rev;
return SVN_NO_ERROR;
}
}
[...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9709 - trunk/subversion/libsvn_client
Posted by Josh Pieper <jj...@pobox.com>.
C. Michael Pilato wrote:
> Josh Pieper <jj...@pobox.com> writes:
>
> > Thanks for the feedback. It should be all cleaned up in r9711.
>
> Cool. I'll trust you tested it, 'cause I didn't. :-)
Well, it at least passed all the diff tests, which I think exercises
all the code paths...
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9709 - trunk/subversion/libsvn_client
Posted by "C. Michael Pilato" <cm...@collab.net>.
Josh Pieper <jj...@pobox.com> writes:
> Thanks for the feedback. It should be all cleaned up in r9711.
Cool. I'll trust you tested it, 'cause I didn't. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r9709 - trunk/subversion/libsvn_client
Posted by Josh Pieper <jj...@pobox.com>.
C. Michael Pilato wrote:
> Josh, could you a) fix the comment which is above the `change =
> apr_hash_get' line to reflect the new truth, and maybe simplify the
> logic above with an extra level of indentation? Something like:
>
> /* See if PATH was explicitly changed in this revision. */
> change = apr_hash_get (changed_paths, path, APR_HASH_KEY_STRING);
> if (change)
> {
> /* If PATH was not newly added in this revision, then it may or may
> not have also been part of a moved subtree. In this case, set a
> default previous path, but still look through the parents of this
> path for a possible copy event. */
> if (change->action != 'A' && change->action != 'R')
> {
> prev_path = path;
> }
> else
> {
> /* PATH is new in this revision. This means it cannot have been
> part of a copied subtree. */
> if (change->copyfrom_path)
> prev_path = apr_pstrdup (pool, change->copyfrom_path);
>
> *prev_path_p = prev_path;
> if (action_p)
> *action_p = change->action;
> if (copyfrom_rev_p)
> *copyfrom_rev_p = change->copyfrom_rev;
> return SVN_NO_ERROR;
> }
> }
> [...]
Thanks for the feedback. It should be all cleaned up in r9711.
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org