Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bn-d opened this issue May 2, 2023 · 2 comments
Open

Comments

@bn-d
Copy link
Contributor

bn-d commented May 2, 2023

Similar to #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
Copy link

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
Copy link
Collaborator

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants