Notifications

[edfloreshz]

Implements a subscribe function that returns a stream that yields a new mode every time a theme change is detected in the OS.

#[tokio::main]
async fn main() -> anyhow::Result<()> {
    while let Some(event) = dark_light::subscribe().await?.next().await {
        if let Event::ThemeChanged(mode) = event {
            println!("System theme changed: {:?}", mode);
        }
    }

    Ok(())
}

Closes #3

[BlackHoleFox]

This is just a driveby review from someone who wants to use this crate in a project, disregard what you or the crate owner would like to:

[BlackHoleFox]

Is this a busy loop?

[edfloreshz]

Yes, I'm still figuring out a way to replace this loop with something better, suggestions are welcome.

[edfloreshz]

The Linux implementation should be good to go, macOS and Windows still need some work.

@frewsxcv could you help me test these changes?

[d2weber]

I just pointed out a few things noted. Sidenote: I authored the #152 and #153 that landed in ashpd to return the stream (also because I was interested in color scheme changes). Note that while testing I encountered duplicate change messages that you might want to mitigate.

[d2weber]

You might also want to check for the freedesktop.portal color theme by blocking on this async fn https://docs.rs/ashpd/latest/ashpd/desktop/settings/struct.Settings.html#method.color_scheme

[edfloreshz]

Is there a way to do this without locking the crate to a specific runtime?

[d2weber]

Not sure which would be the right way to do that. Maybe using block_on from async_io and switch to tokois if the tokio feature is enabled (that's the way, zbus offers it's blocking APIs).

[edfloreshz]

Thanks, I'll implement more runtimes it once we test with tokio.

[edfloreshz]

I encountered this as well using KDE Plasma, oddly I could not replicate this when switching the color scheme using the terminal.

[d2weber]

I encountered this as well using KDE Plasma, oddly I could not replicate this when switching the color scheme using the terminal.

For me there are duplicates also when I use gsettings set org.gnome.desktop.interface color-scheme \'default\'

[edfloreshz]

I tried with Color Scheme Simulator and I wasn't able to replicate the issue. This may be related either to how DEs handle the color scheme change or ashpd.

[bilelmoussaoui]

It is not related to ashpd, it is a portal backend issue. As they listen to changes happening on the host and forward them through the portal, the signal ends up being emitted twice

[edfloreshz]

I haven't tested on macOS but Windows and Linux are working fine.

The current implementation is using tokio but we could implement other runtimes in the future.

@frewsxcv Would you mind taking a look?

[d2weber]

I Just took a quick look on this.

I'm thinking about, what a ThemeWatcher really is, and I believe it really is just a Stream. So the most obvious interface to me would be similar to ashpd's: a function returning a stream. I don't know how difficult it is to implement for platforms that are not supported by ashpd. An advantage might be, that the only dependency needed would be stream_utils.

[edfloreshz]

@d2weber is this what you had in mind?

use std::task::Poll;

use futures::{stream, Stream};

use crate::{detect, Mode};

#[derive(Debug)]
pub enum Event<T> {
    ThemeChanged(T),
    Waiting,
}

pub fn subscribe() -> impl Stream<Item = Event<Mode>> + Send {
    let mut last_mode = detect();

    stream::poll_fn(move |_| -> Poll<Option<Event<Mode>>> {
        let current_mode = detect();

        if current_mode != last_mode {
            last_mode = current_mode;
            Poll::Ready(Some(Event::ThemeChanged(current_mode)))
        } else {
            Poll::Ready(Some(Event::Waiting))
        }
    })
}

This is how you would consume it.

let mut stream = subscribe();

while let Some(event) = stream.next().await {
    if let Event::ThemeChanged(mode) = event {
        println!("System theme changed: {:?}", mode);
    }
}

I was returning an Option<Mode> but I wrapped it in a custom enum to make it clear what was happening.

[edfloreshz]

I re-implemented this to use futures and Stream as it makes more sense that way.

If someone could help me review before merging I'd appreciate it.

[BlackHoleFox]

saw that a review was requested of me the other day, can probably get to this sometime during the week if that works?

Definitely think a Stream is the right idea for this :)

[jacksongoode]

@edfloreshz Was there a reason tokio was added as a dependency here? It seems it's only being used in the example?

Also curious why zbus was bumped down?

[edfloreshz]

Yeah, it was added as a dev-dependency to showcase the example.

As for zbus, that might've been an oversight on my end.

[nickolasclarke]

@edfloreshz the latest build is from April 3, but this was merged 10 days later. Could you cut a new release?

[edfloreshz]

Made an issue to keep track of this: #38

[d2weber]

Also curious why zbus was bumped down?

Zbus has become a transient dependency through ashpd