Next in Review Queue: The New std.path

[dsimcha]

...And now I'll do the honors and be the first to review.  Overall this 
looks solid and well documented but I have a few nitpicks and questions

1.  Do we really need currentDirSymbol and parentDirSymbol?  AFAIK 
they're standard across all operating systems that anyone cares about 
and everyone can easily remember them.  I have never used the 
equivalents in the old std.path and have never missed them.

2.  There's really no need to always allocate a new array on things like 
setExtension.  You could just append the extension in the case where the 
extension doesn't already exist and the string is immutable.  I'm not 
sure this is worth the extra complexity, though, given that setExtension 
will probably never be used in perf-critical code.

3.  All the stuff from the old std.path should be "scheduled for 
deprecation".  Deprecating stuff breaks people's build processes and 
should only be done after adequate warning.  I don't know a good way to 
implement this for enums and aliases, though, which I guess begs the 
question of whether DMD should support soft deprecation.

4.  This module might be useful at compile time.  As pointed out on this 
forum, Phobos should eventually include tests for CTFE-ability.  It 
would be nice if std.path had a few CTFE unit tests sprinkled in.

On 7/14/2011 8:20 PM, dsimcha wrote:
> Lars Kyllingstad's new and improved std.path module for Phobos is the
> next item up in the review queue. I've volunteered to be the review
> manager. Barring any objections, the review starts now and ends at the
> ends two weeks from now on July 28. This will be followed by a week of
> voting, ending August 4th.
>
> Code:
> https://github.com/kyllingstad/phobos/blob/std-path/std/path.d
>
> Docs:
> http://www.kyllingen.net/code/new-std-path/phobos-prerelease/std_path.html
>
> Author's comments -- please read before reviewing:
>
> First of all, note that I have rewritten almost everything from scratch.
> Mainly, this is because there are a lot of corner cases which aren't
> handled correctly by the current std.path, and in the end rewriting
> turned out to be easier than updating. (Check out the unittests; they
> cover all the cases I could think of.) The only functions I have kept
> unchanged from the current std.path are pathCharMatch (formerly
> fncharmatch), glob (formerly fnmatch) and expandTilde.
>
> Secondly, I hope you will find the new function names to be both more
> descriptive, more consistent and more in line with current Phobos
> conventions. They are the result of a (rather informal) round of
> discussion and voting in this forum a few months ago (which I hope you
> will consider before suggesting new names):
>
> http://www.digitalmars.com/d/archives/digitalmars/D/Proposal_for_std.path_replacement_131121.html
>
>
> Thirdly, I have applied @safe, pure and nothrow wherever possible.
> There were cases where this was not possible, either due to bugs in DMD
> (specifically, 5304, 5700 and 5798) or due to functions in other Phobos
> modules not being properly marked with these attributes. I have marked
> these with //TODO comments and will fix them as soon as possible.
>
> And finally, due to the deprecated: block at the bottom of the module,
> this version should be backwards compatible with the current std.path
> for a while yet.