On Thu, Apr 21, 2011 at 10:00 AM, Julian Foad <julian.foad_at_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.foad_at_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.
>
>
>
Received on 2011-04-21 20:12:03 CEST