This is a post-1.7 RFC. Most libsvn_client APIs allow the caller to throw either URLs or working copy paths at the API and then it just does the right thing. But does this paradigm make sense for APIs such as this one?
svn_client_propset4() operates on either WC paths or URLs. Although its purpose is the same either way in general terms, the differences are substantial, and in particular the parameters needed by the API are substantially disjoint. The differences are highlighted by its doc string which looks like this in outline: /** * Set @a propname to @a propval on each (const char *) target in @a * targets. The targets must be either all working copy paths or all URLs. * A @a propval of @c NULL will delete the property. * * If @a targets are URLs: * * - Immediately attempt to commit the property change in the repository, * using the authentication baton in @a ctx and @a * ctx->log_msg_func3/@a ctx->log_msg_baton3. * * - If the property has changed on any target since revision * @a base_revision_for_url (which must not be #SVN_INVALID_REVNUM), no * change will be made and an error will be returned. * * - If non-NULL, @a revprop_table is a hash table holding additional, * custom revision properties (...) to be set on the new revision. ... * * - If @a commit_callback is non-NULL, then for each successful commit, * call @a commit_callback with @a commit_baton ... * * - @a depth must be #svn_depth_empty. @a changelists is ignored. * * If @a targets are working copy paths: * * - If @a depth is #svn_depth_empty, set the property on each member of * @a targets only; if #svn_depth_files, ...; if #svn_depth_immediates, * ...; if #svn_depth_infinity, ... * * - @a changelists is ... used as a restrictive filter on items whose * properties are set ... * * - @a base_revision_for_url must be #SVN_INVALID_REVNUM. @a revprop_table, * @a commit_callback and @a commit_baton are ignored. * * If @a propname is an svn-controlled property (i.e. prefixed with * #SVN_PROP_PREFIX), then the caller is responsible for ensuring that * the value is UTF8-encoded and uses LF line-endings. * * If @a skip_checks is TRUE, do no validity checking. But if @a * skip_checks is FALSE, and @a propname is not a valid property for @a * targets, return an error ... * * If @a ctx->cancel_func is non-NULL, invoke it passing @a * ctx->cancel_baton at various places during the operation. */ Notice that apart from the basic inputs (propname, propval, targets) and cancellation, there are about four parameters that are relevant just for URLs, two that are relevant just for WC paths, and only one interesting parameter (skip_checks) that is common to both. There are currently three callers in "svn": "propset" and "propdel" only support using WC paths, while "propedit" supports using both forms of target. Thoughts? - Julian On Wed, 2011-04-20, Hyrum K Wright wrote: > On Wed, Apr 20, 2011 at 9:50 AM, Julian Foad <julian.f...@wandisco.com> wrote: > > And noticing that several of the parameters are applicable to only one > > kind of targets, this also makes me think the API would be better split > > into two: one for URLs and one for WC paths. Thoughts? > > It sounds reasonable, but I'm not sure how that jives with the overall > philosophy of libsvn_client. Most libsvn_client APIs allow the caller > to throw either URLs or working copy paths at the API and then it just > does the right thing. However, in this context, where the various > operations are quite different, it may make sense to divorce the two > APIs. > > I'm interested to hear what others may think.
