Fix #11444: remote breaks with stdout redirection #11550
Conversation
`setConsoleMode` should do nothing if the handle is not a terminal. The proposed change is [exactly what `golang.org/x/term/IsTerminal()` does on Windows](https://cs.opensource.google/go/x/term/+/6886f2df:term_windows.go). [NO TESTS NEEDED] Signed-off-by: Anton Tykhyy <[email protected]>
|
Preliminary LGTM, but someone more familiar with Windows than me should review |
|
LGTM too, but definitely want a head nod from @jwhonce |
| @@ -25,7 +25,7 @@ func setConsoleMode(handle windows.Handle, flags uint32) error { | |||
| var mode uint32 | |||
| err := windows.GetConsoleMode(handle, &mode) | |||
| if err != nil { | |||
| return err | |||
| return nil // not a terminal | |||
There was a problem hiding this comment.
Shouldn't we check for specific error here, like NotATerminal or something like rhat?
There was a problem hiding this comment.
Golang standard library doesn't think it's worth the bother, and I agree. The only thing about the handle the code here is interested in is whether it makes sense to call SetConsoleMode on it to turn on VT mode. If standard handles are terminal handles but there is something else wrong with them that makes GetConsoleMode return an error, the terminal will just remain in dumb mode and the error will come up a bit later when trying to read or write to them. IMO this is reasonable behavior.
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: atykhyy, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
github-actions
bot
added
the
locked - please file new issue/PR
Fixes #11444.
setConsoleModeshould do nothing if the handle is not a terminal. The proposed change is exactly whatgolang.org/x/term/IsTerminal()does on Windows.