Support IO capture from an already-started process #13374
Conversation
`capture_io/1,2,3` does not capture IO from a process that was already started. This change allows someone to pass a `:pid` option so that `Process.group_leader/2` is called on the target process rather than the calling process.
|
Thank you for the PR but unfortunately this is a bit trickier documentation wise. Capturing anything other than stdio is global, you don't need Given this, I believe the preferred API would be to pass the Thank you. |
|
I pushed a commit here: 73cc6e7 I tried to submit a PR to your fork but GitHub did not allow me to. We would be able to accept a PR that implements the API above. |
Sounds good, I'll implement as described. One thing about taking Do you anticipate that it will be useful or desirable for users to set a custom device |
The whole mechanism works because we make the named device point to another PID. If there is no name, there is no way to redirect. |
| @spec capture_io(atom() | String.t() | keyword(), (-> any())) :: String.t() | ||
| def capture_io(device_input_or_options, fun) | ||
| @spec capture_io(atom() | pid() | String.t() | keyword(), (-> any())) :: String.t() | ||
| def capture_io(device_pid_input_or_options, fun) |
There was a problem hiding this comment.
I was on the fence on whether to collapse these into fewer clauses. I can do that if you think it improves readability.
|
|
||
| original_gl = Process.group_leader() | ||
| {:ok, capture_gl} = StringIO.open(input, capture_prompt: prompt_config, encoding: encoding) | ||
| pid = if is_pid(device_or_pid), do: device_or_pid, else: self() |
There was a problem hiding this comment.
Originally had a pid_for private function for this, but it seemed excessive.
| defp map_dev(:stdio), do: :standard_io | ||
| defp map_dev(:stderr), do: :standard_error | ||
| defp map_dev(other), do: other | ||
|
|
||
| defp do_with_io(:standard_io, options, fun) do | ||
| defp do_with_io(device_or_pid, options, fun) |
There was a problem hiding this comment.
I would have a separate clause for this instead:
| defp do_with_io(device_or_pid, options, fun) | |
| defp do_with_io(:standard_io, options, fun), do: do_with_io(self(), options, fun) |
WDYT?
There was a problem hiding this comment.
Or maybe change this in map_dev. No worries, I will merge it and play with it locally.
|
💚 💙 💜 💛 ❤️ |
capture_io/1,2,3does not capture IO from a process that was already started. This change allows someone to pass a:pidoption so thatProcess.group_leader/2is called on the target process rather than the calling process.