std.path review: second update

[Lars T. Kyllingstad]

On Sat, 30 Jul 2011 03:47:55 +0800, KennyTM~ wrote:

> On Jul 30, 11 02:06, Lars T. Kyllingstad wrote:
>> Here's a new update based on your comments and requests.  Code and docs
>> are in the usual place,
>>
>>    https://github.com/kyllingstad/phobos/blob/std-path/std/path.d
>>    http://www.kyllingen.net/code/std-path/phobos-prerelease/
std_path.html
>>
>> Here are the highlights:
>>
>> * UNC paths, i.e. \\server\share\..., are now (hopefully) handled
>> correctly on Windows.  See the baseName(), dirName(), rootName(),
>> driveName(), pathSplitter(), etc. docs for examples.
>>
>> * Support for //foo/bar paths on POSIX has been removed.
>>
>> * rootName() is new.
>>
>> * pathSplitter() now returns a bidirectional range.
>>
>> * pathCharMatch() and fcmp() have been replaced by improved functions
>> filenameCharCmp() and filenameCmp(), respectively, with optional case
>> sensitivity.
>>
>> * joinPath() has been renamed to buildPath().
>>
>> * Added a function buildNormalizedPath(), which performs normalization
>> while joining the segments.
>>
>> * Redefined normalize() in terms of buildNormalizedPath().
>>
>> I am still unsure of the extent to which long UNC paths (i.e. \\?\...)
>> should be supported, if at all.  If anyone has anything to say on the
>> matter, please do. :)
>>
>> -Lars
> 
> Thanks for the nice work!
> 
> Comments:
> 
> - pathSplitter: empty, front, back could be const. Also, make the struct
> 'static struct'.

Will do.


> - hasDrive, isDriveRoot: the path
> 
>      "#:\x"
> 
> should not pass hasDrive. In Windows only /[a-zA-Z]/ are supported drive
> letters. '#:' may work but is not officially supported.

True, but then "#:" must be considered a directory name, which is not 
much better.  The module generally assumes that any path you pass to it 
is well-formed.  If not, the results are undefined.

The rationale for this is that you don't know whether a path is valid, 
i.e. well-formed AND correct (pointing to the right place in the file 
system), until you try to use it.  std.path can in principle verify well-
formedness, but since it cannot verify correctness, both may just as well 
be taken care of by the OS.


> - Bug 6390 (= 6337) has been fixed. Some CTFE tests can be enabled.

I know, but the fix hasn't been released yet.  I'd prefer if people 
didn't have to build the compiler from repo to test the module.  I'll be 
sure to enable the tests when DMD 2.055 has been released.


> - baseName, dirName "TODO: @safe pure nothrow": Mention which bugs are
> preventing this (std.conv.to and std.string.chomp?).

Will do.


> - buildPath, More than two path components: looks like a perfect
> candidate for  std.algorithm.reduce. And this should probably accept a
> range, but I'm not sure.

I'll look into using reduce.  I have considered letting the function take 
a general range, but variadic parameters likely cover the vast majority 
of use cases.  If there is a real need for a range version, however, I'd 
be happy to add it.  Otherwise, I'd rather avoid the API clutter.

 
> - CaseSensitive.osDefault: As I mentioned before, version(OSX) should
> set this to 'no'.

Sorry, forgot about that.  I'll fix it.


> - globMatch: only UTF-8 is supported?

Except for the name change (fnmatch -> globMatch), this function is not 
written by me.  It is taken from the current std.path, and is for all 
intents and purposes not part of my submission.

However, if it is trivial to redefine it in terms of general string 
types, I will do so.  I'll check.


> - extension: Note that the DDoc text of 'extension' is highlighted.

Thanks!

-Lars