Add support for ColorScheme detection on Linux with the GTK backend.#517
Add support for ColorScheme detection on Linux with the GTK backend.#517tosage05 wants to merge 1 commit intomoreSwift:mainfrom
Conversation
stackotter
left a comment
stackotter
left a comment
There was a problem hiding this comment.
Thanks for tackling this! This will be a nice QoL improvement for Linux users. I've requested some changes, and I'll test locally next
| guard | ||
| let proxy = g_dbus_proxy_new_for_bus_sync( | ||
| G_BUS_TYPE_SESSION, | ||
| .init(0), |
There was a problem hiding this comment.
I think you may be able to use G_DBUS_PROXY_FLAGS_NONE here? Otherwise use an explicit initialiser such as gint(0) or guint(0) depending on the required underlying type
| let proxy = g_dbus_proxy_new_for_bus_sync( | ||
| G_BUS_TYPE_SESSION, | ||
| .init(0), | ||
| nil, | ||
| "org.freedesktop.portal.Desktop", | ||
| "/org/freedesktop/portal/desktop", | ||
| "org.freedesktop.portal.Settings", | ||
| nil, | ||
| nil | ||
| ) |
There was a problem hiding this comment.
Can we instead initialise this proxy once when initialising the backend and then reuse it as necessary?
| nil, | ||
| nil | ||
| ) | ||
| else { return .light } |
There was a problem hiding this comment.
As a formatting thing, we generally put the else body on its own line if the guard condition got its own line;
guard
...
else {
...
}
| guard | ||
| let v = | ||
| ("('org.freedesktop.appearance', 'color-scheme')".withCString { | ||
| g_variant_parse(nil, $0, nil, nil, nil) |
There was a problem hiding this comment.
I believe that you can pass the string as an argument directly and Swift should do the necessary conversion to a c string (because the parameter is a const char *)
| proxy, | ||
| "Read", | ||
| v, | ||
| .init(0), |
There was a problem hiding this comment.
Is it possible to use G_DBUS_CALL_FLAGS_NONE here?
| "Read", | ||
| v, | ||
| .init(0), | ||
| .init(-1), |
There was a problem hiding this comment.
Explicitly cast this to gint instead of using .init (just a style choice I've made for the code base)
|
|
||
| let colorSchemeId = g_variant_get_uint32(final) | ||
|
|
||
| if colorSchemeId == 1 { |
There was a problem hiding this comment.
Link to https://github.com/flatpak/xdg-desktop-portal/blob/fae2c09a530eeef30af8d6401870fd5bae5d3989/data/org.freedesktop.portal.Settings.xml#L38 in a comment so that it's easy for people to look up these values
| private func getPreferredColorScheme() -> ColorScheme { | ||
| #if os(Linux) | ||
| guard | ||
| let proxy = g_dbus_proxy_new_for_bus_sync( |
There was a problem hiding this comment.
Would you feel comfortable refactoring these two proxy implementations (proxy.call_sync and setting the signal handler) into a wrapper class in the Gtk target? I think it'd be great to have this bundled up in a reusable way so that we can easily fetch other settings too in future. For reference I'd recommend looking at the existing TextBuffer wrapper because it has an init, and a signal handler example.
2 participants
Fixes #386
Does so by making use of
org.freedesktop.appearance->color-schemethrough dbus (method supported by most desktop environments), both at launch and while the scui app is running.