Next in Review Queue: The New std.path

[Lars T. Kyllingstad]

On Thu, 14 Jul 2011 21:00:26 -0400, bearophile wrote:

> Just few comments to the source code. I have not used it yet.
> 
> version (Windows) private alias Signed!size_t ssize_t;
> 
> Why not just ptrdiff_t ?

I find ssize_t (which is standard on POSIX) to be much more descriptive 
of its purpose.

http://en.wikipedia.org/wiki/Size_t


> In theory this too is (in C[] path) but I presume you have found
> troubles: private C[] chompDirSeparators(C)(C[] path)  @safe pure
> nothrow

The problem is that path will then be const(C[]), and it becomes 
impossible to return a slice of it as C[].  The correct solution is to 
use inout, and I intend to do so once inout is correctly implemented in 
DMD.


> So here you have had to use Unqual
> immutable(Unqual!C1)[] setExtension(C1, C2)(in C1[] path, in C2[] ext)
> immutable(Unqual!C1)[] defaultExtension(C1, C2)(in C1[] path, in C2[]
> ext)
> 
> Instead of Unqual isn't it nicer to use a Deconst!() template?

Hmm, I guess you're right.  "shared" shouldn't be stripped, for instance.


> // Detects whether the given types are all string types of the same
> width private template Strings...)  if (Strings.length > 0)
> 
> I think that a CTFE function with a static foreach is
> better/faster/simpler than the recursive compatibleStrings() template.

Maybe, but I also find it uglier in use due to the extra parentheses.

   if (compatibleStrings!(T, U, V)())

Personally, I don't think it's worth changing it.


> unittest
> {
> 
> I suggest to add a comment that tells what function/class/struct is
> tested in this unittest (this is a poor's man named unittest).

I don't think that is necessary.  Looking at the unittests in this 
module, I think it is rather obvious what is being tested -- not to 
mention that the unittests always follow directly after the function 
being tested.

-Lars