You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2007/08/09 15:55:44 UTC
Re: svn commit: r26007 - in trunk/subversion: include svn
kameshj@tigris.org writes:
> Log:
> Disallow 'switch --relocate' with --depth, since it is not a valid usecase.
>
> * subversion/include/svn_error_codes.h
> (SVN_ERR_CL_OBSCURE_ARGS): New error code.
>
> * subversion/svn/main.c
> (main) Check for combination of '--relocate' and '--depth' argument and report
> an error
>
> Patch by: Senthil Kumaran <se...@collab.net>
Nice catch, Senthil (and Kamesh).
Some comments:
> --- trunk/subversion/include/svn_error_codes.h (original)
> +++ trunk/subversion/include/svn_error_codes.h Thu Aug 9 03:35:17 2007
> @@ -1115,6 +1115,10 @@
> SVN_ERR_CL_CATEGORY_START + 9,
> "A log message was given where none was necessary")
>
> + SVN_ERRDEF(SVN_ERR_CL_OBSCURE_ARGS,
> + SVN_ERR_CL_CATEGORY_START + 10,
> + "The arguments are obscure and does not imply a valid usecase")
> +
> SVN_ERROR_END
"Obscure" isn't really the right word here, as it means hidden or
mysterious. "Invalid" is probably what we want, so maybe:
SVN_ERRDEF(SVN_ERR_CL_INVALID_COMBINATION,
SVN_ERR_CL_CATEGORY_START + 10,
"The requested option or argument combination is invalid")
Then in the code:
> Modified: trunk/subversion/svn/main.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/main.c?pathrev=26007&r1=26006&r2=26007
> ==============================================================================
> --- trunk/subversion/svn/main.c (original)
> +++ trunk/subversion/svn/main.c Thu Aug 9 03:35:17 2007
> @@ -1222,6 +1222,13 @@
> (svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> _("Error converting depth "
> "from locale to UTF8")), pool, "svn: ");
> + if (opt_state.relocate)
> + {
> + err = svn_error_create(SVN_ERR_CL_OBSCURE_ARGS, NULL,
> + _("--relocate and --depth are obscure "
> + "combination of arguments"));
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + }
> /* ### TODO(sd): Use svn_depth_from_word() here? That could work
> ### as long as that function continues to return
> ### svn_depth_unknown for unrecognized words, but there's
> @@ -1294,6 +1301,13 @@
> opt_state.ignore_externals = TRUE;
> break;
> case svn_cl__relocate_opt:
> + if (opt_state.depth != svn_depth_unknown)
> + {
> + err = svn_error_create(SVN_ERR_CL_OBSCURE_ARGS, NULL,
> + _("--depth and --relocate are obscure "
> + "combination of arguments"));
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + }
> opt_state.relocate = TRUE;
> break;
> case 'x':
This results in code duplication, because we check the same condition
in two places: the condition of both options being passed.
The usual way to handle this sort of situation is to put a single
check, after all the option/argument processing. For example, the way
we handle "dash_F_arg" later in the file.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26007 - in trunk/subversion: include svn
Posted by Kamesh Jayachandran <ka...@collab.net>.
> This results in code duplication, because we check the same condition
> in two places: the condition of both options being passed.
>
> The usual way to handle this sort of situation is to put a single
> check, after all the option/argument processing. For example, the way
> we handle "dash_F_arg" later in the file.
>
> -Karl
>
Thanks Karl for taking note of it.
Fixed it in r26030.
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org