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:
julianf...@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
