Freedesktop: use ashpd for both sync & async implementations#42
Freedesktop: use ashpd for both sync & async implementations#42Be-ing merged 4 commits intorust-dark-light:mainfrom
Conversation
Be-ing
force-pushed
the
xdg_cleanup
branch
10 times, most recently
from
01d0852 to
1d18ab0
Compare
src/platforms/freedesktop/notify.rs
Outdated
| .receive_color_scheme_changed() | ||
| .await? | ||
| .map(Mode::from) | ||
| .boxed()) |
There was a problem hiding this comment.
I want to prepend the initial value to this stream so the downstream application can get the current state on startup. I used future::stream::once(initial_value()) but that caused the notify example to continually print the initial value instead of printing it once then waiting... suggestions?
This doesn't need to be done in this PR considering the main branch doesn't do that either.
There was a problem hiding this comment.
I think you can use the stream![] macro to construct a Stream, perhaps that could be a way.
There was a problem hiding this comment.
but that caused the notify example to continually print the initial value instead of printing it once then waiting... suggestions?
This is because of a bug in the notify example fixed by #43.
Something like this should work
Ok(stream::once(inital_value()).chain(Settings::new()
.await?
.receive_color_scheme_changed()
.await?
.map(Mode::from)
.boxed()))
edfloreshz
left a comment
edfloreshz
left a comment
There was a problem hiding this comment.
Some tiny details but overall it looks good 👍🏼
src/platforms/freedesktop/notify.rs
Outdated
| .receive_color_scheme_changed() | ||
| .await? | ||
| .map(Mode::from) | ||
| .boxed()) |
There was a problem hiding this comment.
I think you can use the stream![] macro to construct a Stream, perhaps that could be a way.
src/platforms/freedesktop/mod.rs
Outdated
| .unwrap() | ||
| .color_scheme() | ||
| .await | ||
| .unwrap() |
There was a problem hiding this comment.
Calling unwrap() here might fail on some platforms, we might want to return Result<Mode>, use ? and handle it in detect().
There was a problem hiding this comment.
That would require changing the public API of detect. I don't want to change public APIs in this PR, just the implementations.
There was a problem hiding this comment.
I didn't mean change detect signature, just initial_value's and then match it in detect.
There was a problem hiding this comment.
I'm not sure bubbling up the error would be helpful unless it's given to the user. But I'm also wondering... do we need to keep the sync API? #45
setting up the XDG Desktop Portal on CI is not easy
Before, the sync and async APIs had completely separate implementations. When Linux support was first added in rust-dark-light#6 the XDG Desktop Portal color-scheme API was new and not yet widely supported, so fallbacks were needed. Now, the portal API is more widely supported so the fallbacks are not needed. In my testing, the portal works on: KDE Plasma GNOME Cinnamon Xfce MATE does not currently support the portal API. Selecting MATE's GTK themes from Xfce's settings also does not change what xdg-desktop-portal-gtk reports. I am not sure, but I think that may be due to missing metadata in MATE's GTK themes. The old implementation never worked on MATE anyway because MATE's dark themes don't have "dark" in their name, so there's no regression removing it.
This allows downstream applications to respond to the system light/dark mode preference on application startup while continuing to react to changes of the preference.
3 participants
Before, the sync and async APIs had completely separate implementations. When Linux support was first added in
#6 the XDG Desktop Portal color-scheme API was new and not yet widely supported, so fallbacks were needed. Now, the portal API is more widely supported so the fallbacks are not needed. In my testing, the portal works on:
KDE Plasma
GNOME
Cinnamon
Xfce
MATE does not currently support the portal API. Selecting MATE's GTK themes from Xfce's settings also does not
change what xdg-desktop-portal-gtk reports. I am not sure, but I think that may be due to missing metadata in MATE's
GTK themes. The old implementation never worked on MATE anyway because MATE's dark themes don't have "dark" in
their name, so there's no regression removing it.