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