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.
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.
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.
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.
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.
---------------------------------------------------------------------
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:55 2006