fix(spin): preserve color output when --show-output is given
#850
Conversation
andreynering
requested review from
raphamorim
and removed request for
a team
andreynering
force-pushed
the
spin-preserve-color-output-show-output-flag
branch
from
f7ffddc to
9eaccca
Compare
andreynering
force-pushed
the
spin-preserve-color-output-show-output-flag
branch
2 times, most recently
from
7588eef to
b93a075
Compare
There was a problem hiding this comment.
code lgtm, I was just gonna ask about windows but pinged @andreynering instead
andreynering
force-pushed
the
spin-preserve-color-output-show-output-flag
branch
from
b93a075 to
da77e9a
Compare
| if err = stdoutPty.Start(executing); err != nil { | ||
| return errorMsg(err) | ||
| } | ||
| _ = xpty.WaitProcess(context.Background(), executing) |
There was a problem hiding this comment.
Just an FYI, we don't need to use xpty.WaitProcess here, we can call executing.Wait() directly. That's what it does under the hood.
There was a problem hiding this comment.
@aymanbagabas I decided to keep as is so the code is more prepared to work on Windows if we decide to try again in the future. WDYT?
| if isTerminal && runtime.GOOS == "windows" { | ||
| executing.Stdout = io.MultiWriter(&bothbuf, &outbuf) | ||
| executing.Stderr = io.MultiWriter(&bothbuf, &errbuf) | ||
| _ = executing.Run() |
There was a problem hiding this comment.
After some headache, me and @aymanbagabas decided to keep the old behavior on Windows for now.
(Git Bash was freezing for some reason, although the other terminals worked correctly).
We can reconsider in the future if we find a reliable way to make it work on Windows.
andreynering
force-pushed
the
spin-preserve-color-output-show-output-flag
branch
from
da77e9a to
f66c0ba
Compare
Fixes #305
Running the command inside a PTY (using this package) to ensure colors and general formatting are preserved.