On Mon, May 26, 2008 at 1:14 AM, Daniel Shahaf <d.s_at_daniel.shahaf.co.il> wrote:
> Troy Curtis Jr wrote on Sun, 25 May 2008 at 21:45 -0500:
>> On Sun, May 25, 2008 at 8:39 PM, Karl Fogel <kfogel_at_red-bean.com> wrote:
>> > "Troy Curtis Jr" <troycurtisjr_at_gmail.com> writes:
>> >> You know after thinking about it I could "fix" this behavior. I'm
>> >> working on my revised patch right now and I think the trailing slash
>> >> workaround would work if I returned "@" for no peg revision found
>> >> instead of "" in my new fangled svn_opt__split_arg_at_peg_revision()
>> >> function. This would mean that every single target coming out of the
>> >> svn_client_args_to_target_array() would have a peg revision, but some
>> >> of them would be blank.
>> >
>> > What's the connection betweening return "@" instead of "" and making
>> > trailing slashes prevent peg-revision interpretation?
>> >
>> > I'd hate to see an improvement to our peg revision interpretation affect
>> > APIs that way. Why can't we say "anything appearing before the last
>> > slash in a path is not a peg revision" without having the API effect you
>> > describe above?
>> >
>>
>
> Maybe append an "@" to the canonicalized path only if the latter contains
> an "@"? It is not necessary otherwise.
>
> Daniel
>
Sure, that's true but you'd have to put a bit of extra logic in there
and it only saves you a character. Though I suppose if later
functions print out error messages that have a trailing '@' in the
paths that the user doesn't expect it could be somewhat confusing. If
you wanted to only stick a trailing @ on when it mattered than the
logic would need to try to re-parse a peg revision on the already
parsed (and canonicalized) path. If a peg revision is "found" on the
second pass then the '@' is needed, otherwise it is not. Oh actually
that won't work because canonicalization happens somewhere else so the
svn_opt__split_arg_at_peg_revision() function could only check for the
existence of '@', not anything more intelligent than that.
Troy
>> The issue is where the canonicalization happens and when the peg
>> revision is actually parsed. When the paths are canonicalization
>> happens the peg revision is split off, the path is canonicallized and
>> the peg is stuck back on for later processing. Unfortunately a
>> trailing slash is removed during canonicallization, so during the
>> later peg processing it is not there to 'escape' the non-peg '@' sign.
>>
>> For all later interpretation a trailing '@' symbol and no @ symbol at
>> all mean the same thing, no specified peg revision. By having
>> svn_opt__split_arg_at_peg_revision() return "@" for an empty peg
>> revision (instead of the empty string) you can always get the desired
>> behavior. In the case of a trailing slash on a target, you are
>> effectively replacing the trailing slash "escaping" method (which is
>> not canonical) with the "real" trailing '@' escaping method. But you
>> do guarantee that all targets will have an '@' specifier, but most of
>> them will not have anything following them.
>>
>> Taking the some of the paths from the test_args_to_target_array() function in
>> subversion/tests/libsvn_client/client-test.c and adding a couple more:
>>
>> Current behavior:
>> "." ""
>> "._at_BASE" "@BASE"
>> "foo///bar" "foo/bar"
>> "http://a//b////" "http://a/b"
>> "http://a///b@27" "http://a/b@27"
>> "http://a/b//@COMMITTED" "http://a/b@COMMITTED"
>> "foo@///bar" "foo@/bar"
>> "foo_at_HEAD///bar" "foo_at_HEAD/bar"
>> Adding:
>> "svn+ssh://svn@10.0.1.1" "svn+ssh://svn@10.0.1.1"
>> "svn+ssh://svn@10.0.1.1/" "svn+ssh://svn@10.0.1.1"
>> "svn+ssh://svn@10.0.1.1@" "svn+ssh://svn@10.0.1.1@"
>>
>> Behavior if we always use '@' for no peg specified:
>> "." "@"
>> "._at_BASE" "@BASE"
>> "foo///bar" "foo/bar@"
>> "http://a//b////" "http://a/b@"
>> "http://a///b@27" "http://a/b@27"
>> "http://a/b//@COMMITTED" "http://a/b@COMMITTED"
>> "foo///@bar_at_HEAD" "foo/@bar_at_HEAD"
>> "foo@///bar" "foo@/bar@"
>> "foo_at_HEAD///bar" "foo_at_HEAD/bar@"
>> Adding:
>> "svn+ssh://svn@10.0.1.1" "svn+ssh://svn@10.0.1.1"
>> "svn+ssh://svn@10.0.1.1/" "svn+ssh://svn@10.0.1.1@"
>> "svn+ssh://svn@10.0.1.1@" "svn+ssh://svn@10.0.1.1@"
>>
>> All of the downstream APIs don't have to change and in fact don't have to know
>> about this modification. A trailing '@' IS a valid peg revision. If it is
>> there the APIs can take it. If not, it is handled just the same, unspecified.
>>
>> This isn't really a change in our specification at the APIs, just in how we
>> interpret the user specified args. It is probably valid for a user to believe
>> a trailing slash should tell Subversion that the characters before it are a
>> target and NOT a peg revision, despite any '@' symbols that may be in it.
>>
>> Troy
>>
>>
>
--
"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-26 12:50:34 CEST