You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Hyrum K. Wright" <hy...@byu.edu> on 2005/11/10 03:37:55 UTC
[PATCH] Change ARG_PARSING_ERR to INSUFFICIENT_ARGS
In many places in the command line client application, we return a
SVN_ERR_CL_ARG_PARSING_ERR when we try to run a command that needs
arguments. One notable except is the merge command, which returns a
SVN_ERR_CL_INSUFFICIENT_ARGS error instead. This more closely matches
the nature of the error, we haven't errored because we can't parse the
arguments, we've errored because there are an insufficient number of them.
Because this error occurs when the user hasn't fed us enough arguments,
the generic error message has also been changed to suggest they run help
for more information.
Also, the method of checking the number of arguments was a little
inconsistent between commands, so I've fixed that as well.
This patch complements Julian Foad's patch[1], because it causes a
semi-useful error message to be displayed, instead of running the
command help when no arguments are passed. These error messages should
probably be touched up to reflect the individual nature of the commands,
but that can be done in another patch.
-Hyrum
[1] http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=108158
[[[
Replace some occurrences of SVN_ERR_CL_ARG_PARSING_ERR in favor of
SVN_ERR_CL_INSUFFICIENT_ARGS. INSUFFICIENT_ARGS better matches the
meaning of what we were using ARG_PARSING_ERR for. Also, update the
generic error message to suggest running help.
* subversion/include/svn_error_codes.h
(SVN_ERR_CL_INSUFFICIENT_ARGS): Add suggestion to run svn help.
* subversion/libsvn_subr/opt.c
(svn_opt_parse_num_args): Replace SVN_ERR_CL_ARG_PARSING_ERR in favor
of SVN_ERR_CL_INSUFFICIENT_ARGS.
[in subversion/clients/cmdline]
* checkout-cmd.c
(svn_cl__checkout): Replace SVN_ERR_CL_ARG_PARSING_ERR in favor
of SVN_ERR_CL_INSUFFICIENT_ARGS, and update a comment.
* import-cmd.c
(svn_cl__import): Split the conditional checking for arguments and
replace SVN_ERR_CL_ARG_PARSING_ERR in favor of
SVN_ERR_CL_INSUFFICIENT_ARGS. This is consistent with svn_cl__export.
* mkdir-cmd.c (svn_cl__mkdir),
cat-cmd.c (svn_cl__cat),
revert-cmd.c (svn_cl__revert),
blame-cmd.c (svn_cl__blame),
resolved-cmd.c (svn_cl__resolved),
add-cmd.c (svn_cl__add),
switch-cmd.c (svn_cl__switch),
lock-cmd.c (svn_cl__lock),
unlock-cmd.c (svn_cl__unlock): Replace SVN_ERR_CL_ARG_PARSING_ERR in
favor of SVN_ERR_CL_INSUFFICIENT_ARGS.
]]]
Re: [PATCH] Change ARG_PARSING_ERR to INSUFFICIENT_ARGS
Posted by Julian Foad <ju...@btopenworld.com>.
Hyrum K. Wright wrote:
> Julian Foad wrote:
>
>>I've fixed up the log message and added everything I mentioned in the
>>attached patch.
>
> Thanks, I'm still getting used to Subversion's patch and log message
> format. I look forward to this being committed.
OK, it's committed in r17336. Thanks for the work.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Change ARG_PARSING_ERR to INSUFFICIENT_ARGS
Posted by "Hyrum K. Wright" <hy...@byu.edu>.
Julian Foad wrote:
> I've fixed up the log message and added everything I mentioned in the
> attached patch.
Thanks, I'm still getting used to Subversion's patch and log message
format. I look forward to this being committed.
-Hyrum
Re: [PATCH] Change ARG_PARSING_ERR to INSUFFICIENT_ARGS
Posted by Julian Foad <ju...@btopenworld.com>.
I want to commit this. Do I have any supporters or objectors? It goes
hand-in-hand with my patch
<http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=108158> to stop
printing help as well as or instead of an error message. The present patch
goes some way towards that same goal.
Hyrum K. Wright wrote:
> In many places in the command line client application, we return a
> SVN_ERR_CL_ARG_PARSING_ERR when we try to run a command that needs
> arguments. One notable except is the merge command, which returns a
> SVN_ERR_CL_INSUFFICIENT_ARGS error instead. This more closely matches
> the nature of the error, we haven't errored because we can't parse the
> arguments, we've errored because there are an insufficient number of them.
Excellent. Thanks for this.
> Because this error occurs when the user hasn't fed us enough arguments,
> the generic error message has also been changed to suggest they run help
> for more information.
I wasn't sure at first that I liked that change. There are a lot of errors
where we could suggest reading the help. What's special about this one? Well,
it is the one that results if no arguments are given, and that is likely to be
a common action of new people who need help, so OK, it does make some sense to
add the suggestion just to this particular error message. I've tweaked the
message slightly.
> Also, the method of checking the number of arguments was a little
> inconsistent between commands, so I've fixed that as well.
Well, partly; you changed "nelts < 1" to "! nelts" in one place, but not where
it is accompanied by a "nelts > 2". But that's OK: it's a bit more consistent
than it was.
> This patch complements Julian Foad's patch[1], because it causes a
> semi-useful error message to be displayed, instead of running the
> command help when no arguments are passed. These error messages should
> probably be touched up to reflect the individual nature of the commands,
> but that can be done in another patch.
OK. Actually, I can't see any need to provide messages more specific than "Not
enough arguments".
> [[[
> Replace some occurrences of SVN_ERR_CL_ARG_PARSING_ERR in favor of
> SVN_ERR_CL_INSUFFICIENT_ARGS. INSUFFICIENT_ARGS better matches the
> meaning of what we were using ARG_PARSING_ERR for. Also, update the
> generic error message to suggest running help.
>
> * subversion/include/svn_error_codes.h
> (SVN_ERR_CL_INSUFFICIENT_ARGS): Add suggestion to run svn help.
>
> * subversion/libsvn_subr/opt.c
> (svn_opt_parse_num_args): Replace SVN_ERR_CL_ARG_PARSING_ERR in favor
> of SVN_ERR_CL_INSUFFICIENT_ARGS.
>
> [in subversion/clients/cmdline]
>
> * checkout-cmd.c
> (svn_cl__checkout): Replace SVN_ERR_CL_ARG_PARSING_ERR in favor
> of SVN_ERR_CL_INSUFFICIENT_ARGS, and update a comment.
"merge" had exactly the same, so I've changed it too. (Actually I deleted the
comments because the new versions of them don't say anything more than the code
says.)
> * import-cmd.c
> (svn_cl__import): Split the conditional checking for arguments and
> replace SVN_ERR_CL_ARG_PARSING_ERR in favor of
> SVN_ERR_CL_INSUFFICIENT_ARGS. This is consistent with svn_cl__export.
Actually it was "export" where you made this change and "import" with which it
was consistent.
Also, the same applies to "copy", "move" and "svn_cl__switch".
> * mkdir-cmd.c (svn_cl__mkdir),
> cat-cmd.c (svn_cl__cat),
> revert-cmd.c (svn_cl__revert),
> blame-cmd.c (svn_cl__blame),
> resolved-cmd.c (svn_cl__resolved),
> add-cmd.c (svn_cl__add),
> switch-cmd.c (svn_cl__switch),
Actually the function you changed in switch-cmd.c was "rewrite_urls".
> lock-cmd.c (svn_cl__lock),
> unlock-cmd.c (svn_cl__unlock): Replace SVN_ERR_CL_ARG_PARSING_ERR in
> favor of SVN_ERR_CL_INSUFFICIENT_ARGS.
Some files were missing from that list.
> ]]]
I've fixed up the log message and added everything I mentioned in the attached
patch.
- Julian