cli / go-gh

A Go module for interacting with gh and the GitHub API from the command line.
https://pkg.go.dev/github.com/cli/go-gh/v2
MIT License
322 stars 45 forks source link

Fix non-Windows terminals assumed to always support 256-color #81

Closed mislav closed 1 year ago

mislav commented 1 year ago

Backports https://github.com/cli/cli/pull/6356

heaths commented 1 year ago

Wouldn't this completely break alternate screen buffers on *nix then? There are other ways of detecting whether VT sequences are supported:

See https://github.com/cli/cli/pull/6356#issuecomment-1276773296 for more information.

heaths commented 1 year ago

I submitted https://github.com/cli/cli/pull/6435 to restore behavior without impacting this PR, since the change was only to restore the original behavior without assumptions of VT support this change inferred. This PR would actually be a good fix, but long-term support for various color or other VT support might be good to add to go-gh. See https://github.com/cli/cli/pull/6435#issuecomment-1277776665 for possibilities.

mislav commented 1 year ago

@heaths I don't see how this can break alternate screen buffers on *nix. There is still an issue with that in the CLI repo, I agree, but we will resolve that in that repo.

heaths commented 1 year ago

Agreed. Please see my latest comment.

heaths commented 1 year ago

To clarify my comment about VT support, I was suggesting maybe something in this module could define, but perhaps that would best be in one of @mattn's many term modules, if it isn't already (I couldn't find one).

mattn commented 1 year ago

Can I help you? If you can explain how to reproduce the problem, I can fix that.

heaths commented 1 year ago

Mainly, with all the terminal modules you have, was wondering if any of them might be a good place to put better VT support detection into. See https://github.com/cli/cli/pull/6435#issuecomment-1277776665 for some thoughts. We've also been wrestling with some of these ideas for Windows Terminal as well. If you think one of your modules might be appropriate, maybe you or I could open an issue and discuss more there.