Some tweaks and updates for the next release

[garyb]

Resolves #14, will resolve #5.

Wanted to get a few things in before we tag a new version 😄

[cryogenian]

How could this be used? "./" < "/" < "../foo" < "/foo/" < "/foo" 😕

[garyb]

It's so you can use paths in dictionaries and the like, the ordering isn't meaningful.

[cryogenian]

Ah! I see. I don't know if it makes sense but why not something like this?

instance ordPath :: Ord (Path a b s) where 
  compare p1 p2 = compare (unsafePrintPath p1) (unsafePrintPath p2) 

[garyb]

I'm allergic to strings 😆. In all seriousness, I think that would be less efficient, as the escaper runs for every path segment when you convert it to a string vs just checking the structure.

[cryogenian]

Ok 😄

[garyb]

Ok, this is ready to review now. As well as fixing the linked issues and adding some new basic instances there's a new function:

-- | Marks an absolute path as sandboxed without sandboxing to a particular
-- | location.
sandboxAbs :: forall b. Path Abs b Unsandboxed -> Path Abs b Sandboxed

as we end up using a coercion to that effect all the time. I did consider making it Path a b Unsandboxed -> Path Abs b Sandboxed so it could turn any path into an absolute sandboxed path, but I'm not sure if that's a bit more questionable?

Also updated the formatting to be more consistent with how we do it everywhere else.

Also fixed some links in the docs to a non-existant examples file.

[jdegoes]

@garyb That function is not safe and defeats the point of sandboxing. You can change the signature to return Maybe (Path Abs b Sandboxed) and then return Nothing in the case where the parent path node (..) appears in the path.

[garyb]

@jdegoes you mean the version of sandboxAbs that works with any path? The current version doesn't have that problem, as you can't do that in an Abs path, can you?

If so, yeah, that's why I said it was questionable. I was going to handle Rel cases by doing something like p relativeTo rootDir with a fromMaybe rootDir, etc. but it's already possible to easily do that without resorting to hacks, but making an Abs path sandboxed and still Abs is pretty tedious currently.

[jdegoes]

you mean the version of sandboxAbs that works with any path? The current version doesn't have that problem, as you can't do that in an Abs path, can you?

An absolute path may contain relative entries. Meaning that the signature of sandboxAbs is incorrect, in the sense you can produce a "sandboxed" path which attempts to access a directory higher than the root directory (which does not make any sense).

[garyb]

Ah ok, I see now.

[garyb]

I've removed the offending sandboxAbs.

I think fixing #5 should fix slamdata/slamdata#928 since that seems to be arising due to dir "." not being escaped, so when it is printed and re-parsed it is read as currentDir.

[garyb]

We'll have to release this as v2.0.0 as it didn't make the v1.0.0 cut - making the left/right stuff consistent means the API has a breaking change. Possibly changing things to escape properly could be considered a breaking change too, given the behaviour above with dir ".".

[garyb]

Any objections to me merging this as it is @jdegoes?

[jdegoes]

Looks good. 👍