Re: Peg Revision Syntax

Hyrum K. Wright wrote:
> C. Michael Pilato wrote:
>> Hyrum K. Wright wrote:
>>> Ben Collins-Sussman wrote:
>>>> Actually, it's already documented in the svnbook:
>>>>
>>>> http://svnbook.red-bean.com/nightly/en/svn.advanced.pegrevs.html
>>>>
>>>> Notice the sidebar text there: "Remember that even when you don't
>>>> explicitly supply a peg-revision, it's still present. It defaults to
>>>> BASE for working copy items, and to HEAD for URLs."
>>>>
>>>> On 1/3/07, Giovanni Bajo <rasky@develer.com> wrote:
>>>>> Ben Collins-Sussman wrote:
>>>>>
>>>>>> Let me generalize: my understanding is that for *any* command:
>>>>>>
>>>>>> * URLs pegrevs default to HEAD, unless specified by @N
>>>>>> * wc-path pegrevs default to BASE, unless specified by @N
>>>>>> * operational revs default to pegrev, unless specified by -rN
>>>>> This has been discussed in the past:
>>>>> http://svn.haxx.se/dev/archive-2005-11/1303.shtml
>>>>>
>>>>> I would like if it could be eventually set in stone and documented (in
>>>>> the FAQ
>>>>> maybe). I personally value more consistency among different svn commands
>>>>> rather than endless debates on the "best" defaults :)
>>> Thanks for the pointers. Given this information, where should the
>>> defaulting should occur? For 'svn cat' we do it in libsvn_client,
>>> whereas for 'svn log' we do it in the client application itself. It
>>> doesn't appear to be happening in any single location.
>>>
>>> Once we decide on the right layer to default, I'll just move all the
>>> defaulting behavior there, write a helper function and call it good.
>> The libsvn_client APIs should implement the defaulting behavior
>> themselves, subject (of course) to being overridden by callers who
>> provide non-unspecified svn_opt_revision_t's. Why make every client do
>> this work?
>
> That makes sense. Given Ben's generalization above, would something
> like the following work? It just seems so simple, that I'm concerned
> that I'm overlooking something. :)
>
> Index: subversion/libsvn_client/revisions.c
> ===================================================================
> --- subversion/libsvn_client/revisions.c (revision 22879)
> +++ subversion/libsvn_client/revisions.c (working copy)
> @@ -140,3 +140,24 @@
> else
> return TRUE;
> }
> +
> +
> +svn_error_t *
> +svn_client__resolve_revisions(svn_opt_revision_t *peg_rev,
> + svn_opt_revision_t *op_rev,
> + svn_boolean_t is_url)
> +{
> + if (peg_rev->kind == svn_opt_revision_unspecified)
> + {
> + if (is_url)
> + peg_rev->kind = svn_opt_revision_head;
> + else
> + peg_rev->kind = svn_opt_revision_base;
> + }

This looks good.

> +
> + if (op_rev->kind == svn_opt_revision_unspecified)
> + op_rev->kind = peg_rev->kind;

But not this. If the op_rev->kind isn't specified, we *do* need to take
the kind from the peg_rev. But depending on peg_rev's kind, might we
also need to copy its ->value field, too?

What about this?

  if (op_rev->kind == svn_opt_revision_unspecified)
    *op_rev = *peg_rev;

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand