On Thu, Apr 21, 2011 at 10:00 AM, Julian Foad <julian.f...@wandisco.com> wrote: > This is a post-1.7 RFC.
I'm happy to make this a 1.7 RFC, since we're already rev'd the client API, and the question is coming up now. The effort shouldn't be that involved.[1] -Hyrum [1] Yes, I'm volunteering, and yes those are famous last words. > > 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. > > >
