Add support for use_stdio option to System.shell and Mix.Shell.cmd #13580
Conversation
lib/elixir/lib/system.ex
Outdated
| @@ -1179,6 +1183,11 @@ defmodule System do | |||
| defp cmd_opts([{:stderr_to_stdout, false} | t], opts, into, line), | |||
| do: cmd_opts(t, opts, into, line) | |||
|
|
|||
| defp cmd_opts([{:use_stdio, false} | t], opts, into, line) do | |||
| opts = opts -- [:use_stdio, :stderr_to_stdout] | |||
There was a problem hiding this comment.
We cannot remove stderr_to_stdout after because there is no guarantee it was set before. For example, someone can do use_stdio: false, stderr_to_stdout: true and now it won't be removed. We should also support use_stdio: true and we can add a test for that.
There was a problem hiding this comment.
I've come up with something. However not very happy with the tests - they're on unix only for one - but obviously detaching the IO means we don't capture it anymore so it logs a bit on the console.
There was a problem hiding this comment.
(note about the use_stdio: false, stderr_to_stdout: true case, we could just forward that to erlang and it'll raise, even though I added an extra check at the end for that to log our own message)
expected:
~r/cannot use `stderr_to_stdout: true` and `use_stdio: false`/
actual:
"errors were found at the given arguments:\n\n * 2nd argument: invalid option in list\n"
arguably it's less clear though
|
💚 💙 💜 💛 ❤️ |
See #13531