Add struct Path to std.path

[dymk]

This adds a struct, struct Path, to std.path, which exposes a much more palatable interface to path string manipulation.

Rationale for adding:

• Adds a (more) platform independent abstraction for path strings.
• Path provides a type safe way to pass, compare, and manipulate arbitrary path strings.
• It wraps over the functions defined in std.path, so behavior of methods on Path are, in most cases, identical to their corresponding module function.

Some examples of how Path can be used:

Equality Testing:

Path p1 = `foo/bar/baz`; 
assert(p1 == `zaz/../foo/bar/baz`);
Path p2 = `foo/bar/./baz`;
assert(p1 == p2); 

Joining with other paths

Path p1 = `foo/bar`;
Path p2 = `baz`;
string s3 = `quix`;
assert(Path.build(p1, p2, p3) == `foo/bar/baz/quix`;
assert(p1.join(p2, p3) == `foo/bar/baz/quix`);

Implicit conversion to a normalized path when passed as a string argument

void doSomething(string path) {
  writeln(path);
}

Path p1 = `/path/../to/./some/file`;
p1.doSomething(); //Prints "path/some/file"

Nice OO-style functions that make sense to have on the concept of a path

Path p1 = `/abs/path`;
Path p2 = `rel/path`;

assert(p1.isAbsolute());
assert(p2.isRelative());
assert(p2.toAbsolute(`/abs/base`) == `/abs/base/rel/path`);
assert(p1.toRelative(`/abs`) == `path`);

---

Plus, lots of other goodies. I encourage you to take a look at the unittests to get a feel for what the wrapper can do. If I overlooked something important, please let me know! I'd love for this to make it into the standard library. Forgive me if I haven't done a great job with the documentation; this was the first time I've had to use ddoc.

[jmdavis]

Well, thank for the work, but we already discussed this in the past and decided that it was not worth having an object to handle paths. std.path uses strings, and that works fine.

However, for future notice, I would point out some implementation details that would be needed were we to merge your work into Phobos.

1. Don't put braces on the same line as other code. You're mixing K&R bracing and BSD-Allman bracing. Phobos uses BSD-Allman, so braces go on their own line.

2. When using version, make sure that you use else if for each subsequent version as well else an else with a static assert. e.g.

   version(Posix)
   {
       //POSIX Version
   }
   else version(Windows)
   {
       //Windows Version
   }
   else
       static assert(0, "Unsupported OS");
   
   That way, you make sure that each OS is covered and you get a nice error message when the code gets compiled with an OS that wasn't planned for.
   
   I would also point out that there's no point (as far as I can see anyway) to disable `Path`'s `init` property. It just makes the type less useful, because it can't be used in anywhere as many circumstances.
   
   But I'm going to close this, because we discussed this previously and determined that this approach was not the way that we wanted to go. Thanks for your effort though.
   

[alexrp]

Where was this discussed?

[dymk]

@alexrp, I started off with a struct with some method prototypes, and it became pretty easy to fill them in as much of it was just a basic wrapper around std.path. I suppose I should make a post on d.D, but it didn't occur to me to do that for a commit.

[dymk]

@jmdavis, Can you point me to the discussion about why Path should not be an object? I can't really see how it'd hurt std.path with its addition, as it's only meant to be a high level wrapper around std.path functions, not depreciate them.

[jmdavis]

I believe that it was this thread here: http://forum.dlang.org/post/4C91FF15.5070508@nifty.com

Andrei in particular seemed against the idea. So, if you want something like this, I think that it's going to need to be discussed in the newsgroup first. Personally, given that we already have a whole module operating on paths as strings, I think that it would be a bad idea to add a struct to handle them. None of the rest of Phobos is going to handle them with that struct, so having it would go against how the rest of the library works and make it so that some code out there uses the string and some code uses the struct. I think that it's far cleaner for the std lib to just have the one way. 3rd party libraries are obviously free to do as they please, but as we already decided that std.path would operate on strings, I think that we should continue with that.

[dymk]

@jmdavis Well, I'm glad that the community is discussing it. I've made another commit that deals with the code style issues you mentioned, however they won't show up in this commit as its been closed.

[JakobOvrum]

Seems unnecessary to use this attribute style when there is only one affected symbol, and it's not like Path is likely to have more fields in the future.

At any rate, it's not common D style, including in Phobos.

[JakobOvrum]

I know this might not get anywhere, but I decided to review it anyway.

Not mentioned in line comments: this needs @safe, pure and nothrow annotations where appropriate.

[JakobOvrum]

I think this was closed prematurely. He's taken it to the NewsGroup and is still dedicated to working on the implementation.

Can we please not close pull requests unilaterally minutes after they were opened?

[alexrp]

Let us reopen this and see where the NG discussion and the improvement of this patch goes and then make a decision.

[jmdavis]

There's no reason to leave a pull request open if it's clearly not acceptable, and it was my understanding that we had already decided against this.

Given the discussion in the newsgroup, we can reopen it, but there will have to be a clear consensus on it among the Phobos devs (or at least Andrei needs to be convinced) before we consider pulling it. And I'm still very much against this idea. We already went with a string-based solution, and I think that it's a big mistake to have two solutions like this.

[alexrp]

Well, look at it like this: This is an improvement over the string-based solution. For good reasons, we can't remove all the string function because that would break endless amounts of code.

Just because something was voted against in the past doesn't mean that it should never be considered again. Sometimes, in retrospect, something that seemed like a bad idea could actually be the good idea.

[alexrp]

cc @kyllingstad

[alexrp]

Most of these is* functions should be @property.

[alexrp]

All functions here probably also need to be marked inout. I'm not entirely sure, though. @JakobOvrum?

[JakobOvrum]

Looks like they should be const, but inout is not needed because there is no mutable indirection in the return types, afaics.

[JakobOvrum]

There's no reason to leave a pull request open if it's clearly not acceptable, and it was my understanding that we had already decided against this.

Why not give it some time? What may be clear to one person is not so clear to someone else. At any rate, it wasn't clear to the requester, and you closed it without any conversation. If you had just pointed to the old discussions and summarized the consensus at the time without closing the pull request right away, the requester could have closed it if he, as a result, had become disinterested in pursuing it. When a contributor with commit access closes a pull request, the requester cannot reopen, which seems unnecessarily unilateral.

Surely we want an environment where contributions are given a fair chance as to encourage the contributor. There's no harm in leaving a pull request open for a few days.

[andralex]

I hate to be the one who needs to decide here, as there are pros and cons going either way. FWIW the whole discussion would have been very different if we didn't have a string-based API. But as things are, the notion that we have two APIs one of which is a near-trivial wrapper over the other seems overengineered and creates a dangerous precedent.

Let's let the forum discussion simmer a bit more.

[dymk]

@JakobOvrum

Not mentioned in line comments: this needs @safe, pure and nothrow annotations where appropriate.

I thought that the compiler inferred those attributes now? Perhaps not.

[jmdavis]

@andralex Well, you can leave it to others to decide if you want, but my vote is definitely no - not when we already have a string-based API for paths.

@dymk

I thought that the compiler inferred those attributes now? Perhaps not.

It does so for templates. Separate compilation makes it so that it won't work for normal functions. Neither Path nor its functions are templatized, so there will be no attribute inference. Also, if all of the types involved are known so that a function can always have a particular attribute on it without limiting anything, then the attribute really should be explicit. That way, we have a better guarantee that the function will keep having those attributes. But for stuff templated on type where the type could be user-defined, inference is required, because whether the function can have a particular attribute or not depends on the type.

[kyllingstad]

Thanks for notifying me about this, @alexrp. I have joined the forum discussion rather than post here.

[ghost]

Since this will likely not be merged that easily (it's been 5 months), I think it's best to attempt a DIP first.

[CyberShadow]

Path p1 = `/path/../to/./some/file`;
p1.doSomething(); //Prints "path/some/file"

Um, is that a typo? It's silently converting an absolute path to a relative one.

[ghost]

No update after 9 months. Closing due to inactivity.