Robert Pluim wrote:
> Garrett Rooney writes:
> > issues@subversion.tigris.org wrote:
> >
> > >+It doesn't appear that the -w options is passed to diff as the
> > >+ documentation suggests.
> > >
> >
> > ok, this is like the 5th time someone has reported this "bug".
> >
> > would anyone object to me adding code to check if -x is used with the
> > built in diff lib so we can error out in that case? i mean we can't
> > possibly do what the user is expecting since we don't handle diff
> > options yet, and silently ignoring them seems to be causing a lot of
> > confusion.
>
> Sounds good to me (it's on my TODO list, right at the bottom ;-)
Absolutely. I was taking a look at this too, but as I'm not familiar
with the code it is taking me a while. The attached patch gives an
appropriate error message in the case of a local diff (no network
access). I haven't followed through the cases where net access is used,
but I am posting this unfinished work now because you may find the other
parts of the patch enlightening.
I would be glad to leave this to you (Garrett - or anyone really).
A possible further enhancement could be to silently ignore "-u" because
it behaves this way (unified diff) by default.
- Julian
Introduce some error handling for when an attempt is made to pass options to
the internal diff. This unfinished patch only addresses local diffs.
* subversion/libsvn_client/diff.c:
Display an error message if any arguments are passed to the internal diff
for local files, because it does not yet recognise any arguments and it
is confusing when it silently ignores them.
* subversion/clients/cmdline/main.c:
In the help for "-x", specify the effective default value and don't
mention
"GNU" because the option applies to any version of "diff". Just say
"diff",
expecting that this will apply to internal as well as external diff
in future.
Display any specific error message (as well as a usage message) where
previously in the case of an argument-parsing error it would only
display a
usage message.
Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 5486)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -285,6 +285,11 @@
{
svn_diff_t *diff;
+ /* Usage check: arguments are not (yet) valid for internal diff */
+ if (nargs > 0)
+ return svn_error_create (SVN_ERR_INCORRECT_PARAMS, NULL,
+ "internal 'diff' does not take options.");
+
SVN_ERR (svn_diff_file (&diff, tmpfile1, tmpfile2, subpool));
if (svn_diff_contains_diffs (diff))
{
===================================================================
--- subversion/clients/cmdline/main.c (revision 5486)
+++ subversion/clients/cmdline/main.c (working copy)
@@ -84,7 +84,8 @@
{"show-updates", 'u', 0, "display update information"},
{"username", svn_cl__auth_username_opt, 1, "specify a username ARG"},
{"password", svn_cl__auth_password_opt, 1, "specify a password ARG"},
- {"extensions", 'x', 1, "pass ARG as bundled options to GNU diff"},
+ {"extensions", 'x', 1,
+ "pass ARG as bundled options to diff [default: \"-u\"]"},
{"targets", svn_cl__targets_opt, 1,
"pass contents of file ARG as additional args"},
{"xml", svn_cl__xml_opt, 0, "output in xml"},
@@ -1092,11 +1093,11 @@
{
svn_error_t *tmp_err;
+ svn_handle_error (err, stderr, 0);
+
if (err->apr_err == SVN_ERR_CL_ARG_PARSING_ERROR)
svn_opt_subcommand_help (subcommand->name, svn_cl__cmd_table,
svn_cl__options, pool);
- else
- svn_handle_error (err, stderr, 0);
/* Tell the user about 'svn cleanup' if any error on the stack
was about locked working copies. */
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 28 01:45:51 2003