dbuenzli / bos

Basic OS interaction for OCaml
http://erratique.ch/software/bos
ISC License
63 stars 16 forks source link

Make good file hierarchy folding functions #3

Closed dbuenzli closed 8 years ago

dbuenzli commented 9 years ago

It's not using Path.t.

dbuenzli commented 9 years ago

Here's a proposal. Error management both default and custom is tricky and I'm not sure that this is the final design. But it seems like a first good try, feedback welcome.

Devising a few combinators to succinctly derive Path.t -> bool result functions will unleash the power of this OS.Dir.fold function. Will also make it available in OS.Path to directly fold over a list of paths (i.e. a forest).

dbuenzli commented 9 years ago

Also I wonder if the default value of the over argument shouldn't be Files` rather thanAny`.

dbuenzli commented 8 years ago

I think this is nailed down now. Other than naming tweaks the last commit adds the dotfiles argument that controls whether dotfiles are considered both for elements and traverse, this avoids having to write `Sat predicates for these if we are only interested in non-dot files and directories (dotfiles defaults to false).

I'd still be interested in feedback from repo watchers, here's the documentation.

samoht commented 8 years ago

however this may not be the case due to OS races.

which kind of race? if a file is deleted and a directory with the same name replace it?

The rest looks good, just not sure about exposing an 'a to log_error really makes sense. Couldn't just be:

type fold_error = Fpath.t -> Rresult.R.msg-> (unit, Rresult.R.msg) Bos.OS.result 

maybe you did it that way to let the client just pass the result without having to worry about ok/error?

dbuenzli commented 8 years ago

Thanks.

which kind of race? if a file is deleted and a directory with the same name replace it?

Yes, suppose the fold function detected that a given path p is a directory, now it needs to call your predicate function with p to determine if it must be traversed. By the time this has happened another process can have deleted p or changed it to a regular file.

The rest looks good, just not sure about exposing an 'a to log_error really makes sense. Couldn't just be: [...]

It could but then you can't use handy combinators you may have on result value Error cases (e.g. Logs.on_error_msg, what the default functions does --- still not sure if I want bos to depend on Logs though).

maybe you did it that way to let the client just pass the result without having to worry about ok/error?

This you can't since you are given an alpha Ok and you need to return a unit Ok.

dbuenzli commented 8 years ago

Also made path the first argument of the folding function to make it compatible with Fpath.Set.add and the future List.cons.