New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dogfood term package from go-gh #6421
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this work! Left one question comment.
In the future do you invasion more of the iostreams
package getting moved over to the term
package?
func hasAlternateScreenBuffer(hasTrueColor bool) bool { | ||
// on Windows we just assume that alternate screen buffer is supported if we | ||
// enabled virtual terminal processing, which in turn enables truecolor | ||
return hasTrueColor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does windows not support os.Getenv("TERM")
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To my knowledge, Windows typically doesn't advertise terminal features via the TERM environment variable, but it does expose them programmatically through Console APIs. Hence, if we successfully enabled virtual terminal processing via these APIs, I think we can safely assume that alternate screen buffers are enabled. This is why the check isn't symmetrical with non-Windows platforms.
I felt that iostreams was large (had too many responsibilities) and I didn't want to port it to go-gh verbatim; that's why I cherry-picked parts of it that felt "stable" and reusable. Some features that didn't make it:
Of all the things listed, I only see pager & spinner support as potential next extractions, but I feel neither are pressing and that they have to be redesigned (code-wise) as they're being ported over. |
Fix setting up git credential helper on Windows
Features: - Support garage.github.com - Resolve ssh hostname aliases with `ssh -G` - Correctly measure terminal size when stdout is redirected
This switches IOStreams to use
pkg/term
from the go-gh library.term
does not handle most of what IOStreams did previously, so a lot of its logic still stays, but this cleans up overlapping functionality.TODO:
Fixes #6524