[PATCH] posix: fix split() for the case where the path is at the root of the filesystem

Remy Blank remy.blank at pobox.com
Sun Jan 6 02:02:10 CST 2013


Mads Kiilerich wrote:
> (This is the case that gave ('', 'file_at_root') without this patch. 
> That could perhaps be made more clear in the patch description.)

Will update.

>> +    >>> split('///multiple_leading_separators_at_root')
>> +    ('///', 'multiple_leading_separators_at_root')
> 
> That patch also changes this result from '//' to '///'. I am not sure 
> that is an improvement ... but probably not a problem.

It's what posixpath.split() does, and considering this is supposed to be
a faster drop-in replacement, I would say it's a good thing.

>> +    >>> split('')
>> +    ('', '')
> 
> I can not imagine a case where this is relevant, so it should perhaps 
> just be left undefined and untested.

Again, this just checks that the behavior is identical to
posixpath.split(). Maybe the doctest should be written more explicitly
as such, by iterating over a list of strings and asserting that both
split() and posixpath.split() return the same result?

> This is a clever way to make sure posix.py only is doctested on posix 
> systems ... but I tend to think it would be better to be more explicit.

Do you mean by adding a comment, or by repeating the import logic from
util.py?

if os.name == 'nt':
    import mercurial.windows
    doctest.testmod(mercurial.windows)
else:
    import mercurial.posix
    doctest.testmod(mercurial.posix)

> Somewhat related:
> We already use posixpath (= os.path on posix systems) on windows too. We 
> might soon want to use this posix.split on windows too, so it should 
> perhaps be made generally available in util somehow ...

I assume this would go into a separate patch?

-- Remy

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20130106/3987ce9e/attachment.pgp>


More information about the Mercurial-devel mailing list