std.path review: second update

[torhu]

On 01.08.2011 20:06, Lars T. Kyllingstad wrote:
> On Mon, 01 Aug 2011 17:24:14 +0000, Jonathan M Davis wrote:
>
>>>  Here's a new update based on your comments and requests. Code and docs
>>>  are in the usual place,
>>>  [...]
>>  Well, my previous comments on function names still stand - both in terms
>>  of names using Ext instead of Extension and Sep instead of sep -
>
> Since the feedback has been minimal since my last update, I take it
> people are more or less happy with the module's functionality now.
> (Unless they're just tired of the whole discussion, of course. ;))
>
> That means it is time to let the bikeshedding begin.  Since we have been
> unable to reach a consensus in previous discussions, and there are now
> only three days left of the review, I would really like to get a good
> sample of the general opinion here.
>
> Folks, please state your preferences in terms of function names.  I'll
> try to put personal bias aside and compose a naming scheme that is both
> internally consistent and consistent with the majority opinion.
>
>
>>  and also with regards to making non-property functions verbs (e.g.
>>  absolutePath and relativePath).
>
> I'd be happy to change it, but I'm at loss for good alternatives.  I seem
> to remember you suggesting makeAbsolute and makeRelative, but not being
> 100% happy with them yourself.  Any other suggestions?

I tend to like the current naming scheme.  I tried taking the list of 
functions and turning them into verbs, resulting in names like 
toAbsolute, ensureExtension, getBaseName, etc.  It makes it a bit easier 
to tell what's a function and what's not.  But given just a little bit 
of context, that's usually pretty clear.  Then those "to" and "get" 
prefixes start to feel like more like noise.

Leaving out prefixes in some cases leaves more room for making the names 
descriptive.  I guess that's another way of seeing it, so I thought I'd 
mention it.  It makes it a bit harder to get an overview of the module 
by reading the symbol list at the top of the doc page, though.

>>  Aside from that, wasn't it discussed that extensions should include the
>>  . in them in order to cover the case where a file ends in a .? Otherwise
>>
>>  assert(extension("file.") == extension("file"));
>>
>>  So, all of the stuff dealing with extensions should probably include the
>>  dot in the extension. It looks like both the current std.path and your
>>  std.path handle it by using "" for dot and null for nothing, but
>>  distinguishing between null and the empty string like that isn't
>>  necessarily a good idea, and your documentation doesn't make the
>>  distinction clear, unlike the documentation for the current std.path.
>>
>>  So, at minimum, the docs should be clearer about null vs empty when it
>>  comes to the extension (and there should be tests for it), but it would
>>  arguably be better to just include the . in the extension.
>
> I'm slowly coming around to the idea of including the dot.  Unless I hear
> any loud protests I will probably do so.

I mentioned this in the other thread, but I'm hard pressed to find an 
actual use case for it.  The only thing I can think of is that with some 
Windows applications specifying a file name that ends with a dot will 
suppress the addition of a default extension.  Notepad is an example, it 
adds .txt by default.  I'm not sure if that's really relevant, though.

On Windows, I believe it's generally true that a file name that ends 
with a dot is not considered to have an extension.  You can't normally 
create a file name ending with a dot, IIRC, fopen() will just ignore the 
dot and create the file without it.  In light of that, I think the 
current behavior of extension() makes perfect sense.  I the rare case 
that a file name ending with a dot exists on the file system, it could 
easily fool naive user code that would likely never have seen such a 
file name in testing:

---
if (extension(fname).length == 0) {
     writeln("Missing extension, adding default!);
     setExtension(fname, "xyz");
}
---

Returning null for the special case of a name ending with a dot seems 
reasonable, I'm fine with that.  A slightly obscure solution for a 
slightly obscure problem.

I think setExtension could be smarter, same goes for defaultExtension. 
This is what I think they should behave like:

assert(setExtension("foo", "bar")   == "foo.bar");
assert(setExtension("foo", ".bar")  == "foo.bar");
assert(setExtension("foo.", "bar")  == "foo.bar");
assert(setExtension("foo.", ".bar") == "foo.bar");

This is what .NET does.  It's treating extensions not just like 
substrings, but recognizing that there is syntax and conventions 
associated with them.  Which is what you want 90% of the time.

Just my pragmatic, Windows-sentric view of things :)