std.path review: update

[Lars T. Kyllingstad]

On Mon, 18 Jul 2011 14:51:06 -0400, Steven Schveighoffer wrote:

> On Mon, 18 Jul 2011 14:25:57 -0400, Lars T. Kyllingstad
> <public at kyllingen.nospamnet> wrote:
> 
>> On Mon, 18 Jul 2011 13:16:29 -0400, Steven Schveighoffer wrote:
> 
>>> In driveName:
>>>
>>> Should std.path handle uunc paths?  i.e. \\servername\share\path  (I
>>> think if it does, it should specify \\servername\share as the drive)
>>
>> Yes, std.path is supposed to support UNC paths.  For instance, the
>> following works now:
>>
>>   assert (equal(pathSplitter(`\\foo\bar\baz`), [`\\foo`, "bar",
>>   "baz"]));
>>
>> I guess you would rather have that
>>
>>   assert (equal(pathSplitter(`\\foo\bar\baz`), [`\\foo\bar`, "baz"]));
>>
>> then?  I am not very familiar with Windows network shares; is \\foo
>> never a valid path on its own?
> 
> It is and it isn't.

Well, that certainly cleared things up. ;)


> It's *not* a normal directory, because only shares
> can be in that directory.  In other words, the point at which a UNC path
> turns into normal directory structure is after the share name.
> 
> An easy way to compare is, you can only map drive letters to shares, not
> to servers.

Then driveName() should probably return the full share path.  But, of the 
following asserts, which should pass?

    assert (pathSplitter(`\\foo\bar\baz`).front == `\\foo\bar`);
    assert (pathSplitter(`\\foo\bar\baz`).front == `\\foo`);

    assert (baseName(`\\foo\bar`) == `\\foo\bar`);
    assert (baseName(`\\foo\bar`) == "bar");

    assert (dirName(`\\foo\bar`) == `\\foo\bar`);
    assert (dirName(`\\foo\bar`) == `\\foo`);

Note that if you replace `\\foo\bar` with `c:\` in the above, the first 
assert in each pair will pass.  Same with "/" on POSIX.  Basically, that 
choice corresponds to treating `\\foo\bar` as a filesystem root.


>> As I understand it, some POSIX systems also mount network drives using
>> similar paths.  Does anyone know whether "//foo" is a valid path on
>> these systems, or does it have to bee "//foo/bar"?
> 
> Typically, linux uses URL's, i.e. smb://server/share   URL parsing is
> probably not in std.path's charter.
> 
> However, I have used a command like:
> 
> mount -t cifs //server/share /mnt/serverfiles
> 
> But this is only in very special contexts.  In general I don't think
> //foo should be considered a server path on Posix systems.

I actually got a request on the Phobos list that std.path should support 
such paths.  Furthermore, the POSIX stardard explicitly mentions "//" 
paths (though it basically says it is implementation-defined whether to 
bother dealing with them).


>>> joinPath:
>>>
>>> Does this normalize the paths?  For example:
>>>
>>> joinPath("/home/steves", "../lars") => /home/steves/../lars or
>>> /home/lars ?
>>>
>>> If so, the docs should reflect that.  If not, maybe it should :)  If
>>> it doesn't, at least the docs should state that it doesn't.
>>
>> No, it doesn't, and I don't think it should.  It is better to let the
>> user choose whether they want the overhead of normalization by calling
>> normalize() explicitly.  I will specify this in the docs.
> 
> In fact, if you do not normalize during the join, it's *more* overhead
> to normalize afterwards.  If normalization is done while joining, then
> you only build one string.  There's no need to build a non-normalized
> string, then build a normalized string based on that.
> 
> Plus the data is only iterated once.
> 
> I think it's at least worth an option, but I'm not going to hold back my
> vote based on this :)

If it doesn't turn out to be a huge undertaking, I think I'll replace 
joinPath() with a function buildPath() that takes an input range of path 
segments and joins them together, with optional normalization.  Then, 
normalize(path) can be implemented as:

    buildPath(pathSplitter(path));

Does that sound sensible?


>>> pathSplitter:
>>>
>>> I think this should be a bi-directional range (no technical limitation
>>> I can think of).
>>
>> It is more of a complexity vs. benefit thing, but as you are the second
>> person to ask for this, I will look into it.  A convincing use case
>> would be nice, though. :)
> 
> Well a path is more like a stack than a queue.  You are usually
> operating more on the back side of it.  To provide back and popBack
> makes a lot of sense to me.  For example, to implement the command cd
> ../foo, you need to popBack the topmost directory.

Ok, I'll see what I can do about it. :)


>>> fcmp:
>>> "On Windows, fcmp is an alias for std.string.icmp, which yields a case
>>> insensitive comparison. On POSIX, it is an alias for
>>> std.algorithm.cmp, i.e. a case sensitive comparison."
>>>
>>> What about comparing c:/foo with c:\foo?  This isn't going to be equal
>>> with icmp.
>>
>> I am a bit unsure what to do about the comparison functions (fcmp,
>> pathCharMatch and globMatch).  Aside from the issue with directory
>> separators it is, as was pointed out by someone else, entirely possible
>> to mount case-sensitive file systems on Windows and case-insensitive
>> file systems on POSIX.  (The latter is not uncommon on OSX, I believe.)
>>  I am open to suggestions.
> 
> It's definitely something to think about.  At the very least, I think
> the default file system case sensitivity should be mapped to a certain
> function.  It doesn't hurt to expose the opposite sensitivity as an
> alternate (you need to implement both anyway).  A template with all
> options defaulted for the current OS makes good sense I think. 
> Actually, expanding/renaming pathCharMatch provides a perfect way to
> default these:
> 
> e.g.:
> version(Windows)
> {
>     enum defaultOSSensitivity = false;
>     enum defaultOSDirSeps = `\/`;
> }
> else version(Posix)
> {
>     enum defaultOSSensitivity = true;
>     enum defaultOSDirSeps = "/";
> }
> 
> // replaces pathCharMatch
> int pathCharCmp(bool caseSensitive = defaultOSSensitivity, string
> dirseps = defaultOSDirSeps)(dchar a, dchar b);
> 
> int fcmp(alias pred = "pathCharCmp(a, b)", S1, S2)(S1 filename1, S2
> filename2);
> 
> Anyone who wants to do alternate comparisons is free to do so using
> other options from pathCharCmp.

Good idea.  I'll probably implement something like that.


>>> expandTilde:
>>>
>>> I've commented on expandTilde from the other posts, but if it is kept
>>> a posix-only function, the documentation should reflect that.
>>
>> It does; look at the "Returns" section.  Perhaps it should be moved to
>> a more prominent location?
> 
> Yes.  It should say (Posix-only).  I believe technically that it should
> fail to compile on Windows if it does not map to a "home" directory
> there.  Note that as named, it's possible to confuse with expanding the
> DOS 8.3 name of a file, i.e. Progra~1

I agree.  I'll put it inside a version(Posix) block.

-Lars