[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] svn_opt_args_to_target_array2 continued [was: svn commit: r11809 ...]

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2004-11-11 02:42:38 CET

Julian Foad wrote:
> I expect to get rid of the remaining uses soon, so it will be a true
> deprecation.

How's this?

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.

- 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 Thu Nov 11 02:42:53 2004

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.