dbuenzli / fpath Goto Github PK

@dsheets noted that some of the functions do change directoryness in their result. For example he writes on the fact that normalize never returns a path with a trailing slash:

This doesn't seem right. In particular, why should normalization change the value of a path's 'this directory' which effectively changes the 'type' of a path? I don't want to have to check for a directory-like path, remember that, normalize, and then re-instate directory-ness.

The documentation of the module should expose a better conceptual vocabulary for a path and its "type", then functions should be reviewed with that in mind and clarified w.r.t. possible changes in their type.

Related to #3.

Currently detecting file path extensions (Fpath.{get_ext,has_ext,ext_exists,rem_ext,split_ext}) on directory paths (e.g. "hey.ho/"), results in having no extension. In other words these functions always operate on the path's last segment which is "" for directories.

However Fpath.{add_ext,set_ext} will add extensions to the empty path segment, resulting in dot files being created. E.g. Fpath.set_ext "png" (v "hey.ho/") = (v "hey.ho/.png"). This is clearly unsatisfactory.

We should either:

1. Make Fpath.{add_ext,set_ext} be the identity on directory paths.
2. Make all extension processing work on the last non-empty segment, doing nothing on root paths like "/" (these paths have no last non-empty segment).

In the model exposed by 1) it means that if you wish to operate on extensions you have to make sure to convert your path to a file path, possibly reinstating directoryness after having processed the extension which is a bit annoying.

In the model exposed by 2) you can do so regardless of directoryness which seems more user friendly API-wise and doesn't seem to pose any kind of conceptual problem.

Both 1) and 2) would behave the same on "/" since Fpath.dir_to_file (v "/") = (v "/")).

• We'd like to be able to test the handling of Windows file paths on Unix (so that we don't have too much catch-up to do when we decide our application needs to work on Windows).
• We'd like to use Fpath to represent abstract paths such as paths within a git project and have a concrete syntax that uses / as the separator without surprises when moving to another OS. We have test suites that use paths written as strings such as "a/b" and these tests can't run on Windows even though they're not used to access actual files.

Possible solution:

• Provide a module Fpath_Posix equivalent to the Fpath module on POSIX systems.
• Provide a module Fpath_Windows equivalent to the Fpath module on Windows systems.

Alternative solution:

• Add optional arguments to the parsing and printing functions:

type syntax = Posix | Windows
val to_string : ?syntax:syntax -> t -> string
val v : ?syntax:syntax -> string -> t
...

The alternative solution is more error-prone for the user but may have the advantage of not duplicating (or triplicating) the documentation.

In terms of implementation, both seem relatively straightforward.

Good backward compatibility is achieved in both cases.

What I want:

• the ability to access all the infix operators (/, //, etc.) directly with open Fpath.Operators;
• not putting the other names in scope like open Fpath does.

Usage:

open Fpath.Operators

...

let abs_dir ?(cwd = Unix.getcwd () |> Fpath.v) path =
  cwd // path |> Fpath.parent

If I'm not missing an operator, the signature of the submodule would be:

module Operators : sig
  val (/) : t -> string -> t
  val (//) : t -> Fpath.t -> t
  val (+) : t -> ext -> t
  val (-+) : t -> ext -> t
end

I like the long name Operators because it makes it clear to the reader of the code that this is probably where the exotic operators come from. Since it's only meant to be used as open Fpath.Operators or let open Fpath.Operators in, it's not that frequent and doesn't need to be extremely short.

On a related note, we're experimenting with the prefix operator !! which would make !!file an alias for (Fpath.to_string file). I can make a dedicated issue or PR once I'm convinced it's a good idea.

Along the lines of #1 it would be useful to support POSIX and Windows path renderings for applications which may communicate across platforms.

The issue here is similar to #5.

For example testing for equality against these constants is unreliable since e.g. cur_dir e.g. could be represented by either "." or "./".

It seems better to introduce is_cur_dir and is_par_dir for testing and keep Fpath.{par_dir,cur_dir} as path segment constants, i.e. change their type to a string.

List.exists (fun ext -> Fpath.has_ext ext p) exts

Is a little bit painful to write. Just missing a name that fits.

When windows = true, Fpath does this:

# let d, b = split_base (v "A:B:C") in to_string (d // b);;
- : string = "B:C"
# let d, b = split_base (v "\\\\server\\share\\foo:bar") in to_string (d // b);;
- : string = "foo:bar"

(I think colons are invalid in segments of Windows paths, so the issue might be that v allows such silly paths rather than that split_base and // give a strange result)

See dbuenzli/bos#31

I apologise if that's not the right place for this question but I wonder why Fpath.of_string restricts the type of its result:

val of_string : string -> (t, [`Msg of string]) Result.result

instead of

val of_string : string -> (t, [> `Msg of string]) Result.result

Because of that I have to use double coercion in my code where I have other variants in the error side of the result.

Currently no path can be the empty string, Path.(to_string (v "")) = ".". This had a number of advantages at the API works but I can't remember why exactly so this should be reviewed, also w.r.t. to the semantics of paths in POSIX.

I was surprised to find that rem_prefix on two equal paths returned None - but makes sense as an edgecase of not being able to construct Fpath.v "". Though I don't think it feels nice that is_prefix isn't consistent with rem_prefix - so an idea could be to return more information from rem_prefix, e.g.:

type rem_prefix = [
  | `Prefix of t
  | `Equal
]
val rem_prefix : t -> t -> rem_prefix option

Fpath.root (which is Fpath.v "/") is certainly going to be wrongly used as stop condition when one loops up in a file hierarchy.

Formally the stop condition should be Fpath.(equal (parent p) p). But it seems better to add at least a Fpath.is_root function that checks if a path is a root directory, doing the right thing on paths with volumes (windows or //).

At some point.