You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/12/10 18:51:57 UTC
Re: svn commit: r1044028 - in /subversion/trunk/subversion:
libsvn_client/ svn/
I don't really mind having the input checks both at the client library
and in the cmdline client --- though, of course, having them just in the
former should be sufficient --- but this time it's straight code
duplication:
julianfoad@apache.org wrote on Thu, Dec 09, 2010 at 16:42:43 -0000:
> Author: julianfoad
> Date: Thu Dec 9 16:42:42 2010
> New Revision: 1044028
>
> URL: http://svn.apache.org/viewvc?rev=1044028&view=rev
> Log:
> Factor out some code that checks whether the target types are homogeneous.
> Move it into two new functions, one each for command line and client API.
>
Exhibit A:
> Modified: subversion/trunk/subversion/libsvn_client/util.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/util.c?rev=1044028&r1=1044027&r2=1044028&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/util.c (original)
> +++ subversion/trunk/subversion/libsvn_client/util.c Thu Dec 9 16:42:42 2010
> @@ -320,3 +320,26 @@ svn_cl__rev_default_to_peg(const svn_opt
> return peg_revision;
> return revision;
> }
> +
> +svn_error_t *
> +svn_client__assert_homogeneous_target_type(const apr_array_header_t *targets)
> +{
> + svn_boolean_t wc_present = FALSE, url_present = FALSE;
> + int i;
> +
> + for (i = 0; i < targets->nelts; ++i)
> + {
> + const char *target = APR_ARRAY_IDX(targets, i, const char *);
> + if (! svn_path_is_url(target))
> + wc_present = TRUE;
> + else
> + url_present = TRUE;
> + }
> +
> + if (url_present && wc_present)
> + return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
> + _("Cannot mix repository and working copy "
> + "targets"));
> +
> + return SVN_NO_ERROR;
> +}
>
Exhibit B:
> Modified: subversion/trunk/subversion/svn/util.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/util.c?rev=1044028&r1=1044027&r2=1044028&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svn/util.c (original)
> +++ subversion/trunk/subversion/svn/util.c Thu Dec 9 16:42:42 2010
> @@ -1345,3 +1345,26 @@ svn_cl__opt_parse_path(svn_opt_revision_
>
> return SVN_NO_ERROR;
> }
> +
> +svn_error_t *
> +svn_cl__assert_homogeneous_target_type(const apr_array_header_t *targets)
> +{
> + svn_boolean_t wc_present = FALSE, url_present = FALSE;
> + int i;
> +
> + for (i = 0; i < targets->nelts; ++i)
> + {
> + const char *target = APR_ARRAY_IDX(targets, i, const char *);
> + if (! svn_path_is_url(target))
> + wc_present = TRUE;
> + else
> + url_present = TRUE;
> + }
> +
> + if (url_present && wc_present)
> + return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Cannot mix repository and working copy "
> + "targets"));
> +
> + return SVN_NO_ERROR;
> +}
>
>
I see no good reason for this duplication, and it should be easy to
eliminate it.
Daniel
Re: svn commit: r1044028 - in /subversion/trunk/subversion: libsvn_client/
svn/
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 12/10/2010 01:51 PM, Daniel Shahaf wrote:
> I don't really mind having the input checks both at the client library
> and in the cmdline client --- though, of course, having them just in the
> former should be sufficient --- but this time it's straight code
> duplication:
I'm on this.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1044028 - in /subversion/trunk/subversion: libsvn_client/
svn/
Posted by Branko Čibej <br...@xbc.nu>.
On 10.12.2010 20:31, Noorul Islam K M wrote:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
>> I don't really mind having the input checks both at the client library
>> and in the cmdline client --- though, of course, having them just in the
>> former should be sufficient --- but this time it's straight code
>> duplication:
>>
> Even though both the message are same, the error type is different. In
> command line we use SVN_ERR_CL_ARG_PARSING_ERROR and in client API
> SVN_ERR_ILLEGAL_TARGET. The reason to check both at command line and
> client API is that, this will protect users who directly use API calls.
>
> Thanks and Regards
> Noorul
That code should be only in libsvn_client, IMHO. If the client wants to
wrap the generic error with its own specific one, it can; but there's no
need for the code to be in more than one place.
-- Brane
Re: svn commit: r1044028 - in /subversion/trunk/subversion:
libsvn_client/ svn/
Posted by Julian Foad <ju...@wandisco.com>.
On Fri, 2010-12-10, C. Michael Pilato wrote:
> On 12/10/2010 02:31 PM, Noorul Islam K M wrote:
> > Daniel Shahaf <d....@daniel.shahaf.name> writes:
> >
> >> I don't really mind having the input checks both at the client library
> >> and in the cmdline client --- though, of course, having them just in the
> >> former should be sufficient --- but this time it's straight code
> >> duplication:
FWIW, my thought on committing this was that it's less duplication than
what was there before.
> > Even though both the message are same, the error type is different. In
> > command line we use SVN_ERR_CL_ARG_PARSING_ERROR and in client API
> > SVN_ERR_ILLEGAL_TARGET. The reason to check both at command line and
> > client API is that, this will protect users who directly use API calls.
>
> r1044486 should address the duplication while preserving the distinct error
> codes.
Thanks, Mike, for completing the de-duplification.
- Julian
Re: svn commit: r1044028 - in /subversion/trunk/subversion: libsvn_client/
svn/
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 12/10/2010 02:31 PM, Noorul Islam K M wrote:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
>> I don't really mind having the input checks both at the client library
>> and in the cmdline client --- though, of course, having them just in the
>> former should be sufficient --- but this time it's straight code
>> duplication:
>>
>
> Even though both the message are same, the error type is different. In
> command line we use SVN_ERR_CL_ARG_PARSING_ERROR and in client API
> SVN_ERR_ILLEGAL_TARGET. The reason to check both at command line and
> client API is that, this will protect users who directly use API calls.
r1044486 should address the duplication while preserving the distinct error
codes.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1044028 - in /subversion/trunk/subversion: libsvn_client/ svn/
Posted by Noorul Islam K M <no...@collab.net>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
> I don't really mind having the input checks both at the client library
> and in the cmdline client --- though, of course, having them just in the
> former should be sufficient --- but this time it's straight code
> duplication:
>
Even though both the message are same, the error type is different. In
command line we use SVN_ERR_CL_ARG_PARSING_ERROR and in client API
SVN_ERR_ILLEGAL_TARGET. The reason to check both at command line and
client API is that, this will protect users who directly use API calls.
Thanks and Regards
Noorul