You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@newton.ch.collab.net> on 2002/11/26 15:56:03 UTC
Re: svn_path_is_url()
David Mankin <ma...@ants.com> writes:
> The comment in svn_path.h isn't any more help; it appears to be just
> plain wrong:
> /* Compare PATH to an array of const char * URL SCHEMES (like "file",
> "http", etc.) to determine if PATH looks like a URL. If so, return
> the matching scheme used by PATH, else return NULL. Returned
> values point to the allocations in SCHEMES. */
> svn_boolean_t svn_path_is_url (const char *path);
That comment is pretty confusing yeah. It shouldn't talk about the
internal implementation, just say that it returns true iff PATH is a
url in a known scheme.
> Anyone have any comments? I'm inclined to fix the comment in
> svn_path.h, and then fix the test case so it thinks ://blah/blah is
> invalid.
Seems like a good plan to me. Anyone know a reason why "://foo" is a
valid url?
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn_path_is_url()
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Karl Fogel <kf...@newton.ch.collab.net> writes:
> The function should behave "correctly", and the doc string should
> describe what the function does. Right now, it sounds like neither of
> these is the case :-).
Mike Pilato's tracked this down. It's basically a bad doc string and
a bad test printf, but the implementation of svn_path_is_url() itself
is fine. Details coming from Mike...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn_path_is_url()
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
David Mankin <ma...@ants.com> writes:
> Beyond that, the implementation just looks for :// with no : or /
> before it, it doesn't even check any list of schemes at all! Should
> it be changed to match the docstring, or should the docstring be
> changed to match the function?
The function should behave "correctly", and the doc string should
describe what the function does. Right now, it sounds like neither of
these is the case :-).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn_path_is_url()
Posted by cm...@collab.net.
David Mankin <ma...@ants.com> writes:
> On Tuesday, November 26, 2002, at 07:56 AM, Karl Fogel wrote:
>
> > David Mankin <ma...@ants.com> writes:
> >> The comment in svn_path.h isn't any more help; it appears to be just
> >> plain wrong:
> >> /* Compare PATH to an array of const char * URL SCHEMES (like "file",
> >> "http", etc.) to determine if PATH looks like a URL. If so,
> >> return
> >> the matching scheme used by PATH, else return NULL. Returned
> >> values point to the allocations in SCHEMES. */
> >> svn_boolean_t svn_path_is_url (const char *path);
> >
> > That comment is pretty confusing yeah. It shouldn't talk about the
> > internal implementation, just say that it returns true iff PATH is a
> > url in a known scheme.
The docstring is wrong, and should be changed, but doesn't talk about
the internal implementation. I think the function might have
originally taken a list of valid schemes (such as the list returned by
the RA layer when asked what URL types it supports) and validated a
string against those. That's not an implementation detail. It's a
description of the inputs and outputs of the function.
> Beyond that, the implementation just looks for :// with no : or /
> before it, it doesn't even check any list of schemes at all! Should
> it be changed to match the docstring, or should the docstring be
> changed to match the function?
As I said, the docstring is just wrong. Changing it now.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn_path_is_url()
Posted by David Mankin <ma...@ants.com>.
On Tuesday, November 26, 2002, at 07:56 AM, Karl Fogel wrote:
> David Mankin <ma...@ants.com> writes:
>> The comment in svn_path.h isn't any more help; it appears to be just
>> plain wrong:
>> /* Compare PATH to an array of const char * URL SCHEMES (like "file",
>> "http", etc.) to determine if PATH looks like a URL. If so,
>> return
>> the matching scheme used by PATH, else return NULL. Returned
>> values point to the allocations in SCHEMES. */
>> svn_boolean_t svn_path_is_url (const char *path);
>
> That comment is pretty confusing yeah. It shouldn't talk about the
> internal implementation, just say that it returns true iff PATH is a
> url in a known scheme.
Beyond that, the implementation just looks for :// with no : or /
before it, it doesn't even check any list of schemes at all! Should it
be changed to match the docstring, or should the docstring be changed
to match the function?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org