matklad / xshell

Apache License 2.0
675 stars 28 forks source link

Get exit code of commands with inherited stdout/stderr #90

Open domenicquirl opened 2 months ago

domenicquirl commented 2 months ago

I have a use case where I'm running some subcommands of a CLI tool using xshell that may take some time and so would like to have their output appear immediately on the terminal where the CLI tool was invoked. As far as I understand, this is what Cmd::run will do by making stdout and stderr inherited. However, I also want to look at the exit code of the xshell Command to check why it failed (if it failed).

Currently, Cmd::run returns a Result with xshell::Error in the Err case, which is... very opaque (it basically just implements Error, so you can print it out). The contained ErrorKind does have a Variant CmdStatus { cmd: CmdData, status: ExitStatus } which you get if check_status fails, but there's a Doc comment on that enum that says "Note: this is intentionally not public.".

What I'm currently doing is calling Cmd::output instead to get the Output and the contained ExitStatus and then re-emitting the Output's stdout and stderr manually. This works, but has the downside of introducing a very noticeable delay between running the subcommand and seeing it's output, since in particular the output doesn't show up "live" but instead only appears when the subcommand has either succeeded or failed. But I don't know how to change that, since Cmd::output calls output_impl(true, true) directly.

domenicquirl commented 2 months ago

Perhaps one way to go about this could be to have Cmd::run return Result<ExitStatus> instead of Result<()>? This wouldn't really change anything about the default case of running with inherited IO streams and failing if the command fails (the ExitCode would still not be available in the Err case, but ? would propagate the execution failure), but when more granularity is needed it would allow using ignore_status to get an Ok(ExitStatus) which just says that the command was executed with some result, not that it returned 0.

This would let users explicitly opt in to the responsibility of checking for success via ignore_status and allow checking the result code, without impacting the (reasonable) default of run failing if the executed command fails.

domenicquirl commented 2 months ago

Just saw #89 and that that seems related. I didn't actually know about the From impl to convert to an std command! Might be worth noting that std

Defaults to inherit when used with spawn or status, and defaults to piped when used with output.

which is very similar to what xshell does, but its equivalent to run is status which does indeed return Result<ExitStatus>.