dbuenzli / bos Goto Github PK

It was designed for assemblage. The selection of symbolic extension may feel a little bit weird for a general purpose library.

OTOH it's nice to have it in assemblage. Maybe we can devise a scheme with a polymorphic extension converter function.

Review String module API and fork it away.

val writef : Bos.path -> ('a, Format.formatter, unit, unit Bos.OS.result) format4 -> 'a

/cc @Drup

At the moment we have for example:

    val with_ic : Fpath.t -> (in_channel -> 'a -> 'b OS.result) -> 'a -> 'b OS.result

But if the bracketee has a richer result type it forces conversion to OS.result which is ('a, [Msg of string]) result`. This is better:

    val with_ic : Fpath.t -> (in_channel -> 'a -> 'b) -> 'a -> 'b OS.result

With a bracketee that results this will nest results which can then either be joined if that result happens to be OS.results or combined appropriately by the client to a tagged union or errors if he wishes so.

A call to Unix.unlink on a directory returns EISDIR on Linux (it seems to be EPERM on osx), so the function here fails. Maybe we should rather properly stat the thing before, it seems osx is collapsing different errors in a single one.

Devise good path predicates combinators to use with #3.

It's not using Path.t.

Currently it returns elements prepended with the dirname (which, it seems to me, is the good default) but we may still want only the base name so maybe add an optional ?base argument defaulting to [false].

This is currently not the case:

# OS.Cmd.(run_out Cmd.(v "printf" % "\\n") |> out_lines);;  
- : (string list * Bos.OS.Cmd.run_status, _[> Rresult.R.msg ]) Bos.OS.result
= Result.Ok ([""], (<abstr>, `Exited 0))

The function is an ad-hoc remain from topkg and earlier scripts for watermarking sources in my packages with version information. Review with Pat module in mind, though the variable syntax scheme is not the same (it uses a scheme unlikely to occur in OCaml sources). Still want something along these lines for quick substitutions in files.

The current set is unsatisfactory. We need pipelines and stdio redirection.

See samoht/assemblage#149

It can be used in a variety of other contexts and offers a much more decent Filename module. But people may not like the Unix dependency or the rest provided by bos.

The only problem is that I do not want to expose it as yet another namespace and squatting the toplevel Path name is bad. Fpath, Filepath, Fspath could be options.

We'd also loose the nice toplevel path name you can use in signatures whenever you open Bos which will make signatures a little more busy. But I can get over that.

To tokenize a string into a Cmd.t value. Basically whitespace separated tokens and allow quoting for tokens with spaces.

and implement equal.

It's nice to have info-level low level logging in OS.Cmd combinators when scripting (akin to set -v) but we could simply provide a hook, and fill it with Logs in another package that can be depended upon for "scripting" (an idea already hinted in #28).

Other places were logging is being used are:

1. Internally OS.Dir.user () at debug level to report any failure in determining the user directory.
2. At the API level for reporting file hierarchy fold errors (and used as the default value of OS.Path.fold's err argument).
3. At the API level for reporting environment variable parsing errors.

A little bit undecided since I still quite like 2+3 as it makes good error reports by default without the client having to care.

Windows needs to know whether you link to a directory or a file. See ocaml/ocaml#462

This should be done here. Here is a blog post that explains how to do this properly for Sys.command (which uses system(3)) on windows.

Currently these brackets have the following signature:

with_inf : (Pervasives.in_channel -> 'a -> 'b Bos.OS.result) -> Bos.path ->
    'a -> 'b Bos.OS.result

This fits the order argument order provided by the function f (file (* opened as in_channel *)) v but in practice it doesn't real well:

OS.File.with_inf slurp db_file ()

Though maybe less convenient for currying I think putting the file name as a first argument would read better:

OS.File.with_inf db_file slurp ()

alternatively other names could be found so that the current order doesn't make it cumbersome to read.

See the hints given by @fdopen here.

This is annoying for bos.setup since we can no longer use module aliases.

The implementation introduced in 8d15b14 is vulnerable to that kind of attack.

val split_ext : ?multi:bool -> path -> path * ext

This is Path.(rem_ext ?multi p, ext ?multi p)

Having different units from now_s seems "pointless", see mirage/mirage-clock#1 (comment)

Doesn't match correctly $ escapes.

Both for generic and string ``Errors`

So that #require "bos.script" avoids boilerplate opens. Formally at the moment this would be equal to bos.top.

One of the problem is that not having the opens breaks merlin.

The current name doesn't read well with the named argument:

OS.Dir.fold_contents ~over:`Files add [] dir

we are not folding the contents over files. This:

OS.Dir.contents_fold ~over:`Files add [] dir

should be better.

basically like to_string but always use /. What do we do with the path volume ?

Maybe this is just

String.concat ~sep:"/" (Path.segs p)

Simply make the whole thing using Unix and forget about trying to hack around with Sys.

maybe it would be better to store them as strings. The conversion from lists to strings and vice-versa may kill us in certain usage scenarios: system calls do take strings but we provide higher-level abstractions of them on path values, each time you apply a high-level abstraction a String.concat occurs. Related to #5.

The bracketed function should always be 'a -> 'b. If it returns a result type compatible with the bracketer it can simply be R.joined. Otherwise the client may not be able to distinguish between errors made by the bracket boilerplate and the function itself or may want to return a richer error type than the one provided by the bracketer.

For example the current OS.Dir.with_current should be changed:

    val with_current :
      Fpath.t -> ('a -> ('b, 'e) result) -> 'a ->
      ('b, 'e) result

seems a little bit fucked up w.r.t. to turn into Rresult.Error if undefined. E.g. lookup for host_arch assemblage config variable.

Make sure those are convenient, as secure as possible and that there are no leftovers on program termination.

Currently we have:

val ext_is : ext -> path -> bool
val has_ext : ?multi:bool -> path -> bool

Still always don't remember which one is which. ext_is should become has_ext which matches Haskell's Filesystem.Path.hasExtension. Also its arguments should be reversed so that we can write List.exists (Path.has_ext p) exts to check against multiple extensions.

has_ext should be renamed to something else or maybe dropped altogether, after all this is simply Path.ext ?multi p <> "".

Platform independent path handling is mostly done, there may be a few remaining things to settle down regarding trailing slashes in Path.{normalize,rooted,relativize}.

Other than that at a certain point OS.Path needs to be nailed down together with #11, something that is missing and should be there is tilde expansion and an easy way to substitute env variables, to be defined in conjunction with Pat.format and Bos_unix.Env.vars.

Ptime_os will cater for this.

Support for parsing and overriding existing values. Extract the right combinators to be able to implement what is done in the assemblage driver api here (interaction with command line opts).

Basically it seems we have absence/presence of variable, if presence parsing, if parsing succeeds possibly overrides another value (e.g. command line defined).

String.make_unique_in returns an option, that's insane (at least on a 64-bit platform) and just annoying for the programmer. I think I only did this to be able to use the Fmt.pp_doomed formatter in assemblage.

Replace that None by raising Invalid_argument and document the unlikely case were it may be raised (most likely a programming error).

That binds to realpath(3) on POSIX. On Windows @bobatkey suggests using GetFullPathName.

We could drop the pp prefix from the function names and maybe rename the module to Pp (though the latter is a little bit ugly to read).

Currently logs opam-like but if everybody does this there's no way to trace the source of the logging program e.g. if the program is used in a larger script.

The opam-like may be nice for more involved programs, so it should be somehow kept, however the default should be make your program a good command line citizen and this means prefixing your messages by the utility name. See e.g. http://www.gnu.org/prep/standards/standards.html#Errors, but sadly it does not define a convention for e.g. distinguishing error messages from warnings. Any standard I may be missing ?