On Fri, 2010-12-10 at 18:09 +0530, Noorul Islam K M wrote:
> Julian Foad <julian.f...@wandisco.com> writes:
>
> > Noorul Islam K M wrote:
> >
> >> Attached is the patch for svn/diff-cmd.c. All tests pass.
> >
> > Hi Noorul. Thanks for mentioning that all tests pass - that's good to
> > know.
> >
> >> + svn_cl__assert_homogeneous_target_type(targets);
> >> +
> >> /* Check to see if at least one of our paths is a working copy
> >> path. */
> >> for (i = 0; i < targets->nelts; ++i)
> >
> > After you have asserted that all the targets are of the same type, there
> > is no need for a loop that checks all of them in turn, just to find out
> > whether they are paths or URLs, is there?
> >
>
> If you see the code below, it is using the variable
> working_copy_present.
Yes, it is, but you don't need a loop. All the targets are the same
type, so after examining the first one there is no point in examining
the rest.
- Julian
> Also I attached the wrong patch. I am not sure how that
> happened. Somehow my first version of the patch got attached. Here is
> the correct one.
>
> Thanks and Regards
> Noorul
>
> plain text document attachment (diff-cmd-new-mixed-func.txt)
> Index: subversion/svn/diff-cmd.c
> ===================================================================
> --- subversion/svn/diff-cmd.c (revision 1044208)
> +++ subversion/svn/diff-cmd.c (working copy)
> @@ -260,7 +260,7 @@
> }
> else
> {
> - svn_boolean_t working_copy_present = FALSE, url_present = FALSE;
> + svn_boolean_t working_copy_present = FALSE;
>
> /* The 'svn diff [-r N[:M]] [targ...@rev]...]' case matches. */
>
> @@ -272,22 +272,20 @@
> old_target = "";
> new_target = "";
>
> + SVN_ERR(svn_cl__assert_homogeneous_target_type(targets));
> +
> /* Check to see if at least one of our paths is a working copy
> path. */
> for (i = 0; i < targets->nelts; ++i)
> {
> const char *path = APR_ARRAY_IDX(targets, i, const char *);
> if (! svn_path_is_url(path))
> - working_copy_present = TRUE;
> - else
> - url_present = TRUE;
> + {
> + working_copy_present = TRUE;
> + break;
> + }
> }
>
> - if (url_present && working_copy_present)
> - return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> - _("Cannot mix repository and working copy "
> - "targets"));
> -
> if (opt_state->start_revision.kind == svn_opt_revision_unspecified
> && working_copy_present)
> opt_state->start_revision.kind = svn_opt_revision_base;
