On Wed, May 28, 2008 at 12:56 PM, Julian Foad
<julianfoad_at_btopenworld.com> wrote:
> Julian Foad wrote:
>>
>> Karl Fogel wrote:
>>
>>> Julian Foad <julianfoad_at_btopenworld.com> writes:
>>>
>>>> It sounds to me like what's broken is the idea that we can validly stick
>>>> the
>>>> "@PEG" back on to a path or URL again after parsing and canonicalising
>>>> it.
>>>>
>>>> Do you think we can get rid of all the code that relies on that, and
>>>> deprecate the functions that do it and any functions that currently
>>>> take "<canonical path or URL>@PEG" as input? Once we've parsed the
>>>> syntax, keep the results separate. That sounds like the Right
>>>> Thing. (The alternative, if we insist on glueing the two parts back
>>>> together, is to define how we're going to do it in a way that doesn't
>>>> lose information.)
>>>
>>> Public APIs would still take
>>>
>>> <canonical path or URL>@PEG
>>>
>>> right?
>>
>> No. I was suggesting deprecating all functions that currently do so.
>> Haven't looked to see how feasible that would be, though. It might mean most
>> of libsvn_client.
>
> I took a look. This is completely localised within subversion/svn/; no
> public library functions are affected. That makes it rather easy, I think.
>
> Basically I'm talking about changing svn_client__args_to_target_array() to
> return an array of (const char *path, svn_opt_revision_t peg) pairs.
>
> Troy, I recommended against making this change while you were working on the
> relative-URL patch, but now, as a separate improvement, it looks like it
> would be great... Would you care to have a go at this?
>
> - Julian
>
My first thought was "no thank you, I've already looked into that and
its too much for me". But your comment about it being limited to
subversion/svn caught my interest, so I looked at it again. Sure
enough, you were right, who would've thought :)
I'm not sure what I was thinking before, I seemed to think that the
peg revs were processed quite deep, and I have comments in this thread
that imply as much. But it really isn't like that. Perhaps it was
because a couple of other people were asking for additional things in
that returned structure that actually WERE processed deep in the API?
I don't remember exactly.
So on further consideration I've decided that yes, I'd like to give it a shot.
There are several APIs that expect arrays of targets, but they
invariably deal with working paths only or are modification commands
that can affect HEAD only (i.e. svn_client_mkdir(),
svn_client_delete(), etc) (thus not needing pegs, right?). I guess I
will also need to make sure that no peg revisions were specified for
those types of commands, and give an appropriate error message if one
is found.
This was brought up last time and so I'll bring it up again, should I
create a brand-new structure to return (like svn_client_target_t for
instance), or should I try to use the existing
svn_client_copy_source_t. The name on that current one would be
generally misleading of course. Should I create my new
svn_client_target_t structure and also rev svn_client_copy4() to use
it instead of it's special purpose svn_client_copy_source_t?
Thoughts?
>
>> Or, to develop the idea a bit further, the more precise problem is
>> thinking that the "@PEG" is optional in such APIs: you can't pass an
>> arbitrary <canonical path or URL> without a peg to such an API and guarantee
>> that it will be interpreted as not having a peg. Therefore another solution
>> is to deprecate those APIs that take
>>
>> <canonical path or URL>[@PEG]
>>
>> and replace them with ones that take
>>
>> <canonical path or URL>@[PEG]
>>
>> where the peg specifier field (starting with '@') has to be there even if
>> there is no peg as such.
>>
>> But if we're considering doing that, I think we'd want to take the
>> opportunity to separate the (optional) peg into a separate argument, because
>> that's cleaner, given that it will already have been parsed once (before
>> canonicalisation) anyway.
>>
>> (BTW, when I say "canonicalisation" in this thread, I really mean
>> everything we do to arguments, which includes character-coding conversions
>> and auto-escaping as well.)
>>
>>> Then we have the problem of one public API calling another one.
>>
>> Er... I don't follow. I guess you mean how would a deprecated function
>> taking, say, "THING[@PEG]", convert its argument to a form suitable for
>> passing to a different API taking, say, "THING@[PEG]". In the cases where
>> such a conversion is defined, I don't know why it would be a problem. In the
>> cases where it's ambiguous, like the one that started this thread
>> ("http://name@10.0.0.1") then we won't be any worse off than we are now.
>>
>>> Or are you proposing that users of our APIs would call some public
>>> function to separate THING[@PEG] into a { THING, [PEG] } structure,
>>> and then all the other public APIs would take that structure?
>>
>> Yes, something more like that. This peg-separating operation needs to be
>> done before or at the same time as "canonicalising" the raw arguments, so
>> probably during the *_args_to_target_array*() functions.
>>
>> - Julian
>
>
--
"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-29 05:45:12 CEST