Julian Foad <julianfoad@btopenworld.com> writes:
> Can that last change hunk really be true: a whole load of array
> creation and function calls just achieved the setting of two local
> variables to the empty string? I'm pretty sure about it. Hint: an
> earlier call to svn_opt_args_to_target_array2 has used up the
> command-line targets in 'os', so the only targets parsed by this call
> are from the temporary array passed as 'known_targets' in the third
> argument. How bizarre.
(I'm responding to this mail in order to quote the above paragraph,
but don't worry, the patch I'm actually looking at is the revised
version you sent fixing the native-vs-utf8 problem.)
Maybe the point of the old code was to convert the artificial targets
to UTF-8 or something, although that seems like it would have been a
clumsy way to do it, and they didn't need converting anyway, since
both "." and "" are UTF-8 already. <shrug> I dunno, but anyway your
change looks like a distinct improvement to me :-).
-Karl
> - Julian
>
> Avoid using a deprecated function. No functional change.
>
> * subversion/clients/cmdline/diff-cmd.c
> (parse_path): New helper function.
> (svn_cl__diff): Simplify the code for argument parsing, using a helper,
> and avoid using the recently deprecated svn_opt_args_to_target_array.
>
> Index: subversion/clients/cmdline/diff-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/diff-cmd.c (revision 11829)
> +++ subversion/clients/cmdline/diff-cmd.c (working copy)
> @@ -38,6 +38,24 @@
>
> /*** Code. ***/
>
> +/* Parse PATH of the form "path[@rev]" into TRUEPATH and *REV,
> + overwriting *REV only if "@rev" was present. */
> +static svn_error_t *
> +parse_path (svn_opt_revision_t *rev,
> + const char **truepath,
> + const char *path,
> + apr_pool_t *pool)
> +{
> + svn_opt_revision_t new_rev;
> + const char *new_path;
> +
> + SVN_ERR (svn_opt_parse_path (&new_rev, &new_path, path, pool));
> + *truepath = svn_path_canonicalize (new_path, pool);
> + if (new_rev.kind != svn_opt_revision_unspecified)
> + *rev = new_rev;
> + return SVN_NO_ERROR;
> +}
> +
> /* An svn_opt_subcommand_t to handle the 'diff' command.
> This implements the `svn_opt_subcommand_t' interface. */
> svn_error_t *
> @@ -76,15 +94,14 @@
> && opt_state->end_revision.kind == svn_opt_revision_unspecified)
> {
> /* The 'svn diff OLD_URL[@OLDREV] NEW_URL[@NEWREV]' case matches. */
> - SVN_ERR (svn_opt_args_to_target_array (&targets, os,
> - opt_state->targets,
> - &(opt_state->start_revision),
> - &(opt_state->end_revision),
> - TRUE, /* extract @revs */ pool));
> -
> - old_target = APR_ARRAY_IDX (targets, 0, const char *);
> - new_target = APR_ARRAY_IDX (targets, 1, const char *);
> - targets->nelts = 0;
> +
> + /* The array of regular targets will be empty */
> + targets = apr_array_make (pool, 1, sizeof (const char *));
> +
> + SVN_ERR (parse_path (&opt_state->start_revision, &old_target,
> + os->argv[os->ind], pool));
> + SVN_ERR (parse_path (&opt_state->end_revision, &new_target,
> + os->argv[os->ind + 1], pool));
>
> if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
> opt_state->start_revision.kind = svn_opt_revision_head;
> @@ -93,28 +110,20 @@
> }
> else if (opt_state->old_target)
> {
> - apr_array_header_t *tmp, *tmp2;
> -
> /* The 'svn diff --old=OLD[@OLDREV] [--new=NEW[@NEWREV]]
> [PATH...]' case matches. */
>
> SVN_ERR (svn_opt_args_to_target_array2 (&targets, os,
> opt_state->targets, pool));
>
> - tmp = apr_array_make (pool, 2, sizeof (const char *));
> - APR_ARRAY_PUSH (tmp, const char *) = (opt_state->old_target);
> - APR_ARRAY_PUSH (tmp, const char *) = (opt_state->new_target
> - ? opt_state->new_target
> - : APR_ARRAY_IDX (tmp, 0,
> - const char *));
> -
> - SVN_ERR (svn_opt_args_to_target_array (&tmp2, os, tmp,
> - &(opt_state->start_revision),
> - &(opt_state->end_revision),
> - TRUE, /* extract @revs */ pool));
> + SVN_ERR (parse_path (&opt_state->start_revision, &old_target,
> + opt_state->old_target, pool));
>
> - old_target = APR_ARRAY_IDX (tmp2, 0, const char *);
> - new_target = APR_ARRAY_IDX (tmp2, 1, const char *);
> + SVN_ERR (parse_path (&opt_state->end_revision, &new_target,
> + opt_state->new_target
> + ? opt_state->new_target
> + : opt_state->old_target,
> + pool));
>
> if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
> opt_state->start_revision.kind = svn_path_is_url (old_target)
> @@ -126,7 +135,6 @@
> }
> else
> {
> - apr_array_header_t *tmp, *tmp2;
> svn_boolean_t working_copy_present = FALSE, url_present = FALSE;
>
> /* The 'svn diff [-r M[:N]] [TARGET[@REV]...]' case matches. */
> @@ -138,17 +146,8 @@
>
> svn_opt_push_implicit_dot_target (targets, pool);
>
> - tmp = apr_array_make (pool, 2, sizeof (const char *));
> - APR_ARRAY_PUSH (tmp, const char *) = ".";
> - APR_ARRAY_PUSH (tmp, const char *) = ".";
> -
> - SVN_ERR (svn_opt_args_to_target_array (&tmp2, os, tmp,
> - &(opt_state->start_revision),
> - &(opt_state->end_revision),
> - TRUE, /* extract @revs */ pool));
> -
> - old_target = APR_ARRAY_IDX (tmp2, 0, const char *);
> - new_target = APR_ARRAY_IDX (tmp2, 1, const char *);
> + old_target = "";
> + new_target = "";
>
> /* Check to see if at least one of our paths is a working copy
> path. */
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Nov 12 16:48:29 2004