Karl Fogel wrote:
>Looks good -- really good, you've hit the Right Solution IMHO.
>
>On the question of whether normalization should be done by
>libsvn_client or by its callers, I think I agree with you: since
>anyone can call any of the SVN libraries, there just has to be a rule
>that those libraries take normalized paths. Probably should document
>this at the tops of all the svn public header files.
>
>One thought about the interface:
>
> /* Convert PATH from the local style to the canonical internal style. */
> void svn_path_normalize (svn_stringbuf_t *path);
>
> /* Convert PATH from the canonical internal style to the local style. */
> void svn_path_denormalize (svn_stringbuf_t *path);
>
>Real-world conversions are always from "/" to "\" or vice-versa,
>right? That is, the total length of the string never changes, or at
>least never gets longer -- maybe normalization will remove trailing
>slashes and/or fold repeated slashes down to one, but that can only
>make the string shorter, never longer.
>
Eeek. Don't like that, it's too much of a hack.
>Since the path never lengthens, why not use plain `char *' and return
>the number of bytes by which the string was shortened?
>
> /* Convert PATH from the local style to the canonical internal
> style, and return the number of bytes by which PATH was
> shortened. */
> int svn_path_normalize (char *path);
>
> /* Convert PATH from the canonical internal style to the local
> style, and return the number of bytes by which PATH was
> shortened. */
> int svn_path_denormalize (char *path);
>
>Then callers will never have a type-conversion issue. Callers with
>plain strings will just pass them and ignore the return value; callers
>with stringbufs or whatever can pass foo->data and use the return
>value to adjust foo->len (more likely, we'll write wrappers to do this
>for them, of course).
>
>Oh... I see the problem: all the svn_path_*() functions currently take
>stringbufs anyway, so it would be gratuitously inconsistent to have
>the core normalization/denormalization routines take char *s, and then
>have everyone else calling wrappers of those routines. Hmmm. Well,
>do whatever you think best, then; someday we may get rid of all this
>svn_stringblah_t crap and rewrite the path library, among many other
>things.
>
Exactly. And use apr-util's URI parsing stuff so that paths and URLs
never mix again.
>By the way, IMHO normalization should both remove trailing slashes,
>and fold repeated slashes to a single slash. Not sure that has to be
>part of this change, though.
>
Well, ...normalize calls svn_path_canonicalize, which is supposed to do
all of that.
>Anyway, looks great! I *love* it when code gets smaller... :-)
>
>I assume that adding the normalization/denormalization calls to the
>command-line client is intended to be part of the same commit, and
>that you were just waiting to have the Windows build working so you
>could test it there? Anyway, would be good to include that in the
>commit, of course.
>
No; I meant to commit this change first, then add the conversion calls
in later commits. Unix should be unaffected, while the only difference
on Win would be that paths with \'s aren't accepted until those calls
are added. I think that amount of "breakage" on Win32 is acceptable,
compared with having to make the path even bigger.
>Thanks for doing this,
>-Karl
>
>Branko �ibej <brane@xbc.nu> writes:
>
>>I finally managed to create a patch for this issue. It's a bit big, so
>>I'd like to ask someone to give it a quick review, and test the
>>changes on Unix for me -- I don't have a Unix machine handy for SVN
>>development at the moment.
>>
>>Basically, this patch does the following:
>>
>> * Removes all traces of svn_path_style from the code
>> * Make Subversion use / as the directory separatore internally
>> * Adds two new functions, svn_path_normalize and
>> svn_path_denormalize (he he, love those names) that convert
>> between the local and internal path syntax. These functions aren't
>> used anywhere yet, but are intended to be used by the client
>> program (*not* libsvn_client) to hammer incoming paths into the
>> internal form and outgoing paths (e.g., those coming from trace
>> editors) into the local form.
>>
>>I suspect everything should work as before on Unix.
>>
>>I'm not sure about Win32 yet -- as long as you don't try to feed it \
>>separators, up and st definitely work (better than before), and ci
>>should, barring problems in APR, which I'll fix as I come to them.
>>
>>If everything is O.K. on Unix, I'll try to revive the tests on Win32
>>and commit this in about a week -- the sooner the better, as it's a
>>bit hard to maintain a patch of this size. The next step is to add
>>calls to svn_path_(de)normalize in the appopriate places.
>>
>
--
Brane �ibej <brane_at_xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:56 2006