search

ocsigen / lwt

OCaml promises and concurrent I/O
https://ocsigen.org/lwt
MIT License
704 stars 172 forks source link

Have a separate set of `Lwt_process.pread` that also return the process status #989

Open bn-d opened 1 year ago

bn-d commented 1 year ago

Similar to https://github.com/ocsigen/lwt/issues/216 , but instead of handling the error for the user, return the status and let the user handle it.

ie

val pread : 
  command ->
  string Lwt.t * Unix.process_status Lwt.t

It will also be great to have some documentation around pread that says it will be raise exception on non-zero return.

BraulioVM commented 9 months ago

I would also like to have something like this in the library (and would even make the contribution if the maintainers agreed with the addition) but note that you can get what you want with little code:

let pread_with_status command =
  Lwt_process.with_process_in command @@ fun p ->
  Lwt.both
    (Lwt_io.read p#stdout)
    p#status
raphael-proust commented 8 months ago

@BraulioVM I think the addition would make sense. Note that I would want to keep backwards compatibility. So it can't be a change in the existing pread function. I'd suggest a new function pread_status (and pwrite_status too), or maybe a module WithProcessStatus which has a pread and pwrite function. That second approach is good if there are other existing functions which might benefit from the same generalisation.