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 2008/03/26 02:55:54 UTC

Re: svn commit: r30050 - in trunk/subversion: include svn tests/cmdline

hwright@tigris.org writes:
> --- trunk/subversion/include/svn_error_codes.h (r30049)
> +++ trunk/subversion/include/svn_error_codes.h (r30050)
> @@ -193,6 +193,11 @@ SVN_ERROR_START
>               SVN_ERR_BAD_CATEGORY_START + 8,
>               "Bogus UUID")
>  
> +  /** @since New in 1.5. */
> +  SVN_ERRDEF(SVN_ERR_BAD_PATH,
> +             SVN_ERR_BAD_CATEGORY_START + 9,
> +             "Bogus path")
> +
>    /* xml errors */
>  
>    SVN_ERRDEF(SVN_ERR_XML_ATTRIB_NOT_FOUND,
>
> --- trunk/subversion/svn/commit-cmd.c (r30049)
> +++ trunk/subversion/svn/commit-cmd.c (r30050)
> @@ -50,11 +50,22 @@ svn_cl__commit(apr_getopt_t *os,
>    svn_config_t *cfg;
>    svn_boolean_t no_unlock = FALSE;
>    svn_commit_info_t *commit_info = NULL;
> +  int i;
>  
>    SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
>                                                        opt_state->targets, 
>                                                        pool));
>  
> +  /* Check that no targets are URLs */
> +  for (i = 0; i < targets->nelts; i++)
> +    {
> +      const char *target = APR_ARRAY_IDX(targets, i, const char *);
> +      if (svn_path_is_url(target))
> +        return svn_error_create(SVN_ERR_BAD_PATH, NULL,
> +                                "Must give local path (not URL) as the "
> +                                "target of a commit");
> +    }
> +
>    /* Add "." if user passed 0 arguments. */
>    svn_opt_push_implicit_dot_target(targets, pool);

Wouldn't the existing error code SVN_ERR_WC_BAD_PATH be fine for this?

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r30050 - in trunk/subversion: include svn tests/cmdline

Posted by Karl Fogel <kf...@red-bean.com>.
"Erik Huelsmann" <eh...@gmail.com> writes:
>> In fact, the best thing to do would be to return that error and document
>> the circumstances under which that happens, I guess.
>
> Interesting read in that context:
> http://dlweinreb.wordpress.com/2008/03/24/what-conditions-exceptions-are-really-about/

Oooooh, erm.  The relationship between what that essay proposes and how
Subversion works today (and could work in the future) are so complex
that I can't even begin to start a thread on it :-).  I'm glad I read
it, though.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r30050 - in trunk/subversion: include svn tests/cmdline

Posted by Erik Huelsmann <eh...@gmail.com>.
On 3/26/08, Karl Fogel <kf...@red-bean.com> wrote:
> "Hyrum K. Wright" <hy...@mail.utexas.edu> writes:
> > I s'pose so.  I just didn't know if it was appropriate to throw an
> > SVN_ERR_WC_* error from within the client.
>
> Yes, I think it is.  libsvn_client is, among other things, a wrapper
> around libsvn_wc, and there are surely other instances where it throws
> an SVN_ERR_WC_foo already.
>
> In fact, the best thing to do would be to return that error and document
> the circumstances under which that happens, I guess.

Interesting read in that context:
http://dlweinreb.wordpress.com/2008/03/24/what-conditions-exceptions-are-really-about/

Bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r30050 - in trunk/subversion: include svn tests/cmdline

Posted by Karl Fogel <kf...@red-bean.com>.
"Hyrum K. Wright" <hy...@mail.utexas.edu> writes:
> I s'pose so.  I just didn't know if it was appropriate to throw an
> SVN_ERR_WC_* error from within the client.

Yes, I think it is.  libsvn_client is, among other things, a wrapper
around libsvn_wc, and there are surely other instances where it throws
an SVN_ERR_WC_foo already.

In fact, the best thing to do would be to return that error and document
the circumstances under which that happens, I guess.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r30050 - in trunk/subversion: include svn tests/cmdline

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Karl Fogel wrote:
> hwright@tigris.org writes:
>> --- trunk/subversion/include/svn_error_codes.h (r30049)
>> +++ trunk/subversion/include/svn_error_codes.h (r30050)
>> @@ -193,6 +193,11 @@ SVN_ERROR_START
>>               SVN_ERR_BAD_CATEGORY_START + 8,
>>               "Bogus UUID")
>>  
>> +  /** @since New in 1.5. */
>> +  SVN_ERRDEF(SVN_ERR_BAD_PATH,
>> +             SVN_ERR_BAD_CATEGORY_START + 9,
>> +             "Bogus path")
>> +
>>    /* xml errors */
>>  
>>    SVN_ERRDEF(SVN_ERR_XML_ATTRIB_NOT_FOUND,
>>
>> --- trunk/subversion/svn/commit-cmd.c (r30049)
>> +++ trunk/subversion/svn/commit-cmd.c (r30050)
>> @@ -50,11 +50,22 @@ svn_cl__commit(apr_getopt_t *os,
>>    svn_config_t *cfg;
>>    svn_boolean_t no_unlock = FALSE;
>>    svn_commit_info_t *commit_info = NULL;
>> +  int i;
>>  
>>    SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
>>                                                        opt_state->targets, 
>>                                                        pool));
>>  
>> +  /* Check that no targets are URLs */
>> +  for (i = 0; i < targets->nelts; i++)
>> +    {
>> +      const char *target = APR_ARRAY_IDX(targets, i, const char *);
>> +      if (svn_path_is_url(target))
>> +        return svn_error_create(SVN_ERR_BAD_PATH, NULL,
>> +                                "Must give local path (not URL) as the "
>> +                                "target of a commit");
>> +    }
>> +
>>    /* Add "." if user passed 0 arguments. */
>>    svn_opt_push_implicit_dot_target(targets, pool);
> 
> Wouldn't the existing error code SVN_ERR_WC_BAD_PATH be fine for this?

I s'pose so.  I just didn't know if it was appropriate to throw an 
SVN_ERR_WC_* error from within the client.

-Hyrum