-
-
Notifications
You must be signed in to change notification settings - Fork 300
gtk: support NixOS #2107
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
base: master
Are you sure you want to change the base?
gtk: support NixOS #2107
Conversation
This currently only supports non-libadwaita apps because libadwaita overrides the theme name [1] for compatibility reasons. [1]: https://gitlab.gnome.org/GNOME/libadwaita/-/blob/43192bc3e2870a142e57f971b00b60d69abc9492/src/adw-style-manager.c#L492-495
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.
Before this is merged, it seems like a good idea to split
libadwaitainto a separate target, so that users can disable the overlay if none of their apps need it.
I feel that it should also be disabled by default, as rebuilding all libadwaita pacakges can be extremely compute and time consuming.
side note: I feel the same about gtksourceview
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 makes sense. I opened #2112 with some more ideas.
Although note that this patch is independent of the theme used, so it should be available from the nix-community binary cache, at least for packages that are used in one of our testbeds.
| && config.stylix.targets ? gtk | ||
| && config.stylix.targets.gtk.enable |
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.
| && config.stylix.targets ? gtk | |
| && config.stylix.targets.gtk.enable | |
| && config.stylix.targets.gtk.enable or false |
| extension = ".css"; | ||
| }; | ||
|
|
||
| theme = |
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 seem unnecessarily to have in a let in block, why not just inline it?
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'm planning to move it to a separate file so that we can reuse it for Home Manager.
Later, I believe the Home Manager module could be refactored to use the same theme package I've created here.
This branch (work in progress) adds support for GTK at the NixOS level.
Currently the theme works on non-
libadwaitaapps, and onlibadwaitaapps when$GTK_THEMEis set. I've added a patch to allowlibadwaitato load the theme by default, but I've not had time to test this yet, since it causes most apps on my system to be built from source. The patch is necessary since$GTK_THEMEis meant for debugging purposes, so it takes priority over Home Manager and any imperative settings.The reason why
libadwaitadoes not load themes by default is because they create additional work for upstream maintainers who never intended for their app to be themed. As a general note, we might want to add a warning in our README about this, asking users to disable Stylix before sending any bug reports upstream.Before this is merged, it seems like a good idea to split
libadwaitainto a separate target, so that users can disable the overlay if none of their apps need it.Later, I believe the Home Manager module could be refactored to use the same theme package I've created here.