Freedesktop: use ashpd for both sync & async implementations #42
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()))
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
There was a problem hiding this comment.
Let's continue the conversation in #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 frewsxcv#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.
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.