add ReadQueryResponses method that will read ansi responses on windows #249
Conversation
This file contains 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
aymanbagabas
added a commit
to charmbracelet/lipgloss
that referenced
this pull request
Nov 8, 2024
This drops the dependency on `github.com/charmbracelet/x/input` and `github.com/erikgeiser/coninput` by manually parsing the terminal response to the background color query using `ansi`. Since we moved `x/input` to Bubble Tea, and the code is no longer maintained, we need a different approach to query the terminal for the background color. This is basically doing the same thing as before, but manually parsing the response instead of using the `x/input` package. Fixes: charmbracelet/x#248 Supersedes: charmbracelet/x#249
|
@aymanbagabas put together the actually correct fix for this in charmbracelet/lipgloss#429 |
aymanbagabas
added a commit
to charmbracelet/lipgloss
that referenced
this pull request
Nov 12, 2024
This drops the dependency on `github.com/charmbracelet/x/input` and `github.com/erikgeiser/coninput` by manually parsing the terminal response to the background color query using `ansi`. Since we moved `x/input` to Bubble Tea, and the code is no longer maintained, we need a different approach to query the terminal for the background color. This is basically doing the same thing as before, but manually parsing the response instead of using the `x/input` package. Fixes: charmbracelet/x#248 Supersedes: charmbracelet/x#249
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See #248
Non-windows systems have one way to read events from the terminal: Driver.ReadEvents calls Driver.readEvents, which pulls data from the CancellableReader and parses ANSI data into events.
Windows systems additionally have code that calls into the Windows ReadConsoleInput syscall, which can only read mouse and keyboard events, and will block until an event is received. This syscall cannot be cancelled or timed out.
Currently, there is no way to call the former (which works great!) on Windows. Bubbletea v2 seems to use code that does the latter from an input pump, and I assume it works great in that context. However, ReadEvents is used both for the bubbletea input pump and reading ANSI query responses in lipgloss v2. When used for the latter, the Windows code will hang because ReadConsoleInput isn't used for pulling ANSI query responses and can't be cancelled.
As a result, I would like to split the Driver API into two methods: ReadEvents, to be used from input pumps, and ReadQueryResponses, to be used from queryTerminal & equivalents.
I've verified that when plugged in to use the new method from windows terminal & standalone CMD terminals, lipgloss v2's
BackgroundColorwill successfully parse the primary DA response and immediately return a null background color. When used from the windows terminal preview version, lipgloss v2'sBackgroundColorwill actually return the correct background color.