Next in Review Queue: The New std.path

[Lars T. Kyllingstad]

On Thu, 14 Jul 2011 20:33:20 -0400, dsimcha wrote:

> ...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.

I agree.  (I didn't really want to keep them, but I wasn't sure whether 
anyone used them.)  If I don't hear any objections, I will remove 
currentDirSymbol and parentDirSymbol.

 
> 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.

I guess it depends on what is the most common case.  If "immutable 
without extension" is the most common case then it could be worth it.  
Otherwise it just adds code bloat for little gain.

 
> 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.

True.  As Jonathan points out, there is ongoing discussion about how best 
to do deprecation.  As has also been pointed out elsewhere, the old 
functions should be kept as they are, so behaviour doesn't silently 
change.  I'll add them back in, and include "scheduled for deprecation" 
pragma(msg)s where possible.


> 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.

I'm not sure I agree with this.  As I understand it, most of D should 
eventually be usable in CTFE.  (Don has made enormous progress in this 
regard.)  In most cases we therefore shouldn't have to change Phobos to 
make it CTFEable, it should Just Work.  On that note, it shouldn't be 
necessary to add CTFE-specific unittests either.

-Lars