Fix "missing method Fd" crash on Windows#6200
Merged
Conversation
This ensures that `IOStreams.Out` always keeps the original `Fd()` value even if it's wrapped as a Colorable stream for Windows in cases when enabling virtual terminal processing has failed.
we do it automatically
samcoe
approved these changes
Sep 6, 2022
Comment on lines
+284
to
+285
| var bodyWriter io.Writer = opts.IO.Out | ||
| var headersWriter io.Writer = opts.IO.Out |
Contributor
There was a problem hiding this comment.
Is this necessary because the Go compiler does not recognize that the fileWriter interface implements the io.Writer interface?
Contributor
Author
There was a problem hiding this comment.
No, it's because without an explicit type, bodyWriter would become the exact same type as IOStreams.Out, which is fileWriter. Then, later in this function I want to reassign bodyWriter to another Writer which doesn't necessarily have the Fd() method, which isn't needed anyway.
Basically, I'm just narrowing down the interface for the purpose of the rest of the code in this function.
| } | ||
| return false | ||
| func isCygwinTerminal(fd uintptr) bool { | ||
| return isatty.IsCygwinTerminal(fd) |
Contributor
There was a problem hiding this comment.
I like that you made this and terminalSize simpler.
This was referenced Sep 6, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This ensures that
IOStreams.Outalways keeps the originalFd()value even if it's wrapped as a Colorable stream for Windows in cases when enabling virtual terminal processing has failed.Fixes #6147