Changed stdin reads in _get_terminal_color to non-blocking #25
Conversation
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
Set stdin to non-blocking before reading the OSC response in _get_terminal_color to avoid freezes due to unexpected response formats.
Added try block to catch exceptions that might occur.
Contributor
Author
|
While this works fine on my local machine, I just realized that in GitHub Codespaces it leaks characters to the terminal. |
Owner
|
@saschatx just tested this locally and it fixes a similar but with iTerm, I think we can ship and then maybe do another fix for codespaces? 😊 |
Contributor
Author
|
@patrick91 Looks like the latest commit I pushed fixed the issue. From my point of view, this can be merged. |
Owner
|
thank you @saschatx! |
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.
In this pull request, I try to address the issue I mentioned in #24
The Issue
On certain (rare) occasions, the OSC response containing the terminal color that is parsed in _get_terminal_color might have an unexpected format. In these cases, the current implementation might freeze.
I was not able to reliably reproduce the issue, however it's possible to simulate the issue by replacing the expected "end of OSC response" character in line 163 by an incorrect character, e.g. "!". Then, the function should always freeze, if the OSC response is less than 50 characters long.
The solution
My suggested solution is to set sys.stdin to perform non-blocking reads. In my tests, that resolved the issue.
I have to admit that I am not an expert when it comes to these Python functions. I'm not sure if the try-except block is required (it wasn't in any of my tests, but a quick Google search revealed that these exception might be or have been raised here).