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

Re: Diff --old --new: too many peg revision specifiers

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2006-03-16 00:54:13 CET

Greg Hudson wrote:
> On Tue, 2006-03-14 at 18:19 +0000, Julian Foad wrote:
>
>>If we support "-r N[:M] --old...", as with other commands it should mean
>>"operate on revisions N and M, or the revision range from N to M, of the
>>object", but this is neither what it currently means nor a good fit for the
>>syntax which specified TWO separate objects (--old and --new), so that's why I
>>say we should (eventually) drop the "-r" option from this syntax altogether.
>
> I was surprised to learn that svn diff --old= --new= allowed the -r
> option, and although it was probably my fault, I agree that we should
> get rid of it whenever it's practical to do so.

Excellent.

The essential first patch to avoid proliferating this problem is to remove "-c"
from the help message for this syntax, and preferably disallow it at the
option-parsing stage (which is a bit clumsy, since extra logic is required in
order to distinguish it from "-r"). The "-c" hasn't been released yet, so we
just need to make sure this gets back-ported into 1.4.0 and there'll be no
compatibility worries in that regard.

Patch attached. OK so far?

Next step: I think we shouldn't drop support for "-r" before v2, but I think it
is OK for us to deprecate an aspect of the User Interface in a minor release
just as we deprecate APIs. I propose to remove the mention of "-r" from the
main description, and add a note stating it is accepted for backward
compatibility, and describing its meaning.

- Julian

Remove the '-c/--change' option from the 'svn diff --old [--new]' syntax,
because '-r' has a non-standard meaning for which '-c' doesn't make sense,
and is provided only for backward compatibility anyway.

* subversion/svn/main.c
  (svn_cl__cmd_table): Remove '-c' from help for 'svn diff --old' syntax.
  (main): Error if '-c' and '--old' are both specified.

Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c (revision 18892)
+++ subversion/svn/main.c (working copy)
@@ -293,7 +293,7 @@ const svn_opt_subcommand_desc2_t svn_cl_
   { "diff", svn_cl__diff, {"di"}, N_
     ("Display the differences between two revisions or paths.\n"
      "usage: 1. diff [-c M | -r N[:M]] [TARGET[@REV]...]\n"
- " 2. diff [-c M | -r N[:M]] --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \\\n"
+ " 2. diff [-r N[:M]] --old=OLD-TGT[@OLDREV] [--new=NEW-TGT[@NEWREV]] \\\n"
      " [PATH...]\n"
      " 3. diff OLD-URL[@OLDREV] NEW-URL[@NEWREV]\n"
      "\n"
@@ -309,9 +309,7 @@ const svn_opt_subcommand_desc2_t svn_cl_
      " OLD-TGT and NEW-TGT and restrict the output to differences for those\n"
      " paths. OLD-TGT and NEW-TGT may be working copy paths or URL[@REV]. \n"
      " NEW-TGT defaults to OLD-TGT if not specified. -r N makes OLDREV default\n"
- " to N, -r N:M makes OLDREV default to N and NEWREV default to M,\n"
- " -c M makes OLDREV default to M-1 and NEWREV default to M.\n"
- " -c -M makes OLDREV default to M and NEWREV default to M-1.\n"
+ " to N, -r N:M makes OLDREV default to N and NEWREV default to M.\n"
      "\n"
      " 3. Shorthand for 'svn diff --old=OLD-URL[@OLDREV] --new=NEW-URL[@NEWREV]'\n"
      "\n"
@@ -807,7 +805,8 @@ main(int argc, const char *argv[])
   svn_cl__cmd_baton_t command_baton;
   svn_auth_baton_t *ab;
   svn_config_t *cfg;
-
+ svn_boolean_t used_change_arg = FALSE;
+
   /* Initialize the app. */
   if (svn_cmdline_init("svn", stderr) != EXIT_SUCCESS)
     return EXIT_FAILURE;
@@ -914,6 +913,13 @@ main(int argc, const char *argv[])
                    "can't specify -c twice, or both -c and -r"));
               return svn_cmdline_handle_exit_error(err, pool, "svn: ");
             }
+ if (opt_state.old_target)
+ {
+ err = svn_error_create
+ (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Can't specify -c with --old"));
+ return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+ }
           changeno = strtol(opt_arg, &end, 10);
           if (end == opt_arg || *end != '\0')
             {
@@ -943,6 +949,7 @@ main(int argc, const char *argv[])
             }
           opt_state.start_revision.kind = svn_opt_revision_number;
           opt_state.end_revision.kind = svn_opt_revision_number;
+ used_change_arg = TRUE;
         }
         break;
       case 'r':
@@ -1098,6 +1105,13 @@ main(int argc, const char *argv[])
         opt_state.editor_cmd = apr_pstrdup(pool, opt_arg);
         break;
       case svn_cl__old_cmd_opt:
+ if (used_change_arg)
+ {
+ err = svn_error_create
+ (SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Can't specify -c with --old"));
+ return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+ }
         opt_state.old_target = apr_pstrdup(pool, opt_arg);
         break;
       case svn_cl__new_cmd_opt:

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 16 00:56:07 2006

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