Toqozz / wired-notify

Lightweight notification daemon with highly customizable layout blocks, written in Rust.
MIT License
606 stars 28 forks source link

Update timeout configuration to match spec #134

Closed bcspragu closed 9 months ago

bcspragu commented 9 months ago

Fixes #131 by changing behavior to:

I built and tested this locally (using notify-send and others), it works as expected

Also adds a new timeout_behavior = {Legacy, DBusSpec} config option, which defaults to the new behavior. The wiki should probably be updated after this lands as well.

Toqozz commented 9 months ago

Sorry, for some reason I thought timeout was a float, but it's actually a u32 in milliseconds.

I do think forever should actually mean forever, not just a big number, so I think we should change timeout to be an enum:

enum Timeout {
    Forever,
    Milliseconds(u32),
}

And then use that appropriately, similar to how you've done already. dbus.rs is probably a fine place for that.

In window.rs you'll probably need to make fuse a Timeout as well, and update it accordingly. I'm not sure what all that will look like yet -- your choice!

Also, we do want to have a new config option for the new behaviour, so users can go back to the old if they want. I think I'm OK with it being a breaking change, though.

Please let me know if you have any questions or thoughts!

bcspragu commented 9 months ago

That all sounds good, I'll give it a shot. Moving this to a draft in the mean time

bcspragu commented 9 months ago

All of the stuff from above should be done, let me know what you think!

Toqozz commented 9 months ago

Looks good! Also be sure to add the option to config.ron as well.

bcspragu commented 9 months ago

Looks good! Also be sure to add the option to config.ron as well.

Updated, with a bit of documentation.

Toqozz commented 9 months ago

Great, thank you for the pull request!