On 5/17/06, Julian Foad <julianfoad@btopenworld.com> wrote:
> Garrett Rooney wrote:
> > + if ((peg_revision->kind == svn_opt_revision_base
> > + || peg_revision->kind == svn_opt_revision_committed
> > + || peg_revision->kind == svn_opt_revision_previous)
> > + && targets->nelts == 1)
>
> I feel the logic for validating and handling a peg ought to be common among
> many commands, and factored out. If you could have a bit of a look for such
> commonality, that would be great.
>
> Note also that "@PREV" is not a valid peg specifier: "local/path@PREV" would
> imply looking up "local_path" in a revision which does not exist in the local
> file system (and "URL@PREV" is obviously invalid). Similarly, we can decree
> that "@COMMITTED" is not valid (even though it identifies a tree in which the
> item was at the same location as it is now). Only "@BASE" and (implied)
> "@WORKING" are valid pegs on a local path. I've been meaning to write up a
> specification for peg revisions that states this. This might simplify your
> conditional expression.
I'm not convinced @PREV or @COMMITTED are invalid. It's just a matter
of converting them to a url/rev pair and then doing exactly what we
would do with any other peg revision argument...
Regardless, I do agree that this stuff could be consolidated, but I
think it can wait. The patch I committed is what is minimally needed
to fix this problem, and can easily be merged into 1.4.x without much
danger. A more wide reaching change to consolidate the handling of
this stuff is probably warented, but seems like a it risks changing
behavior unexpectedly somewhat late in the game for 1.4.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed May 17 22:04:46 2006