On Fri, May 23, 2008 at 12:15 PM, Karl Fogel <kfogel_at_red-bean.com> wrote:
> Troy, FWIW, I think Julian's approach is best too.
>
> -Karl
>
There's big discussion right now at work about peer reviewing and how
we might integrate it in our work flow. There is lots of resistance.
But my experience with this relative url patch has really showed me
the benefit of doing them. And that's exactly what all this has been,
discussion of several implementations and the best one emerges!
Troy
> "Troy Curtis Jr" <troycurtisjr_at_gmail.com> writes:
>>> From the point of view of parsing command-line syntax, I think removing the
>>> peg revision specifier comes logically before canonicalising the rest of the
>>> argument. Therefore, may I recommend not making the
>>> svn_opt__arg_canonicalize_path()/_url() functions "cope with" a peg
>>> revision, but leave them as they were, to operate on just a path or URL.
>>> Instead, make your split_arg_at_peg_revision() function semi-public (maybe
>>> called "svn_opt__arg_split_at_peg") and use it where previously the
>>> *_args_to_target_array*() and other functions did their own parsing.
>>>
>>> If your split_arg_at_peg_revision() will return the peg string with the "@"
>>> still on it, then it could represent "no peg" with either NULL or the empty
>>> string. I suggest it should never return NULL but rather the empty string
>>> when there is no peg, so that callers can simply re-concatenate the peg
>>> without checking for NULL. This seems a reasonable way to go for now, given
>>> that (as you point out) some client functions still want to add the peg back
>>> on and then take it off again later. In the long term, the clean way to
>>> organise it would be for this initial splitting to pass the peg back
>>> separately (without its "@") and for the path and the peg to be passed
>>> around separately thereafter. But there's no need to go into that now.
>>>
>>> That seems to make it all simpler. Does that make sense and is there
>>> anything else I could help with? It would be great to see an update when you
>>> have time. Thanks.
>>
>> That would certainly work, I was just trying to keep from having to
>> put another implementation in the relative url parsing block of the
>> code. I think that is an excellent compromise between my patch and
>> Karl's suggestions. I'll work up the patch and send it in when I get
>> a chance.
>
--
"Beware of spyware. If you can, use the Firefox browser." - USA Today
Download now at http://getfirefox.com
Registered Linux User #354814 ( http://counter.li.org/)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-24 04:17:23 CEST