-
Notifications
You must be signed in to change notification settings - Fork 740
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
Automatically detect terminal background color #4674
base: main
Are you sure you want to change the base?
Conversation
I would like to get some feedback on the approach I have taken to add support for this. I have only tested this with "Kitty" on Mac OS, more testing is needed. Should the auto light/dark detection be configurable? And maybe off by default? |
54902b2
to
b5d55f9
Compare
Add support for automatically detecting the background color of the terminal using operating system command escape sequence. The response is then parsed and used to determine if the background is light or dark.
b5d55f9
to
2192423
Compare
@willmcgugan - can you take a look at this MR to check if I am heading in the right direction? |
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.
What happens on terminals that don't support this?
If you have a Mac, you might want to test terminal.app
@@ -31,6 +31,10 @@ | |||
"""Sequence received when the terminal receives focus.""" | |||
FOCUSOUT: Final[str] = "\x1b[O" | |||
"""Sequence received when focus is lost from the terminal.""" | |||
BG_COLOR: Final[str] = "\x1b]11;rgb:" |
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.
We have a policy in Textual of no abbreviations, so BG should be BACKGROUND.
I think this would be better done as a regex.
Do you have a reference to the docs? Can you be certain it is sent in a two hex characters?
perceived_brightness = 0.2126 * r + 0.7152 * g + 0.0722 * b | ||
return perceived_brightness < 128 | ||
|
||
async def _on_background_color(self, event: events.BackgroundColor) -> None: |
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.
I wouldn't want to do this automatically, unless the dev explicitly requests this behaviour.
Sending the message is probably fine, until we have a mechanism for the dev to request automatic light / dark mode.
@@ -248,6 +248,7 @@ def on_terminal_resize(signum, stack) -> None: | |||
self.flush() | |||
self._key_thread = Thread(target=self._run_input_thread) | |||
send_size_event() | |||
self.write("\x1b]11;?\x07") # Detect background color |
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.
AFAIK some terminals respond with nothing when they see a sequence they don't recognize/support, i.e. if you send requests <supported 1>;<unsupported 2>;<supported 3>;
the responses will be <response 1>;<response 3>
only. If anything expects order guarantee for requests it may end up waiting forever.
Is this case handled (I didn't do an in-depth review, just asking)?
Add support for automatically detecting the background color of the terminal using operating system command escape sequence. The response is then parsed and used to determine if the background is light or dark.
fixes #2419
Please review the following checklist.