-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Use theme based on color scheme #7541
Conversation
zbus = { version = "3.13" } | ||
zvariant = { version = "3.13" } |
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.
This pulls in hundreds of dependencies just to read a single value. Could this be done by a wrapper script on the user side that's executed when the editor starts?
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.
Yes that's quite unfortunate, this is the impact on the binary size: 26MB > 30MB. I'm going to add functionality that watches the dbus as well so a wrapper script won't cut it.
Will look into using a more lightweight way, maybe the dbus
crate or using the dbus cli tools.
As someone who's invested in this sort of behaviour [1], I'm curious about this feature! I primarily use macOS, and as far as I can see, the proposed solution so far wouldn't support macOS? If that's true, I'm happy to do some thinking about how this could feature could be supported for macOS. [1] https://github.com/jesse-c/AppearanceNotifier |
Concerns like this is why I personally belive something like this does not belong in core. There is no standard solution/its highly platform specific and all that platform-specific code pulls in a huge number of dependencies. If this could be implemented in a couple lines of code plus a simple wrapper script that could be ok but I don't think that's the case. Something like this could be a plugin (which could reasonably be a platform-specific so you have one plugin for mac and one for linux) once we have ar plugin system. Until then you could implement a rimple daemon to symlinks a different config file depending on whether dark/light theme is active and send a signal (pkill -USR1 hx) to reload the config. |
That makes sense, @pascalkuthe!
For macOS, if with performance, and other points, allowed, it can be done with a timer that runs every
That sounds very reasonable.
This is what I have at the moment! [1] I was happy when I discovered this recently, haha. |
As an alternative, most major xterm-compatible support the OSC 11 escape sequence to query the current background colour and correctly reply with the RGB value of the current background colour. From this value one can derive the luminance and determine whether the background colour is light or dark. This approach works with raw IO, ie stdlib only without any additional dependencies, and detects the appropriate colour theme even if the terminal itself does not honour the desktop dark mode settings (some people do use a dark terminal even if the overall desktop appearance is light). It does not allow monitoring (though helix could also just poll the background colour at certain intervals), but a simple command to change the background colour could help here. I believe that this is how (neo)vim detects the terminal background colour. |
@Siilwyn You may be interested in https://github.com/udoprog/tokio-dbus which only depends on |
closing this one out as I don't see us adding this to core (see discussion above). Thank you for contributing! |
Start at solving #2158, I hope this can eventually land in master, otherwise I'm happy to share this branch for others to use.
theme_light
in favor of using the "normal"theme
when the color scheme is: not found, has no preference or is set to prefer light. This simplifies things a bit and matches more with most distros that don't communicate prefer light when dark mode is off.