Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'expiring' criteria #56

Merged
merged 4 commits into from
Jul 3, 2018
Merged

Conversation

vilhalmer
Copy link
Collaborator

A criteria to match whether a notification will ever disappear on its own.

This allows you to set ignore-timeout + default-timeout on non-expiring notifications in general, rather than having to match on applications that tend to send them. This effectively prevents non-expiring notifications entirely:

[!expiring]
ignore-timeout=true
default-timeout=5000

Obviously you can do other things with it as well, but this is the most immediately useful application.

This also adds a field to mako_notification to track the requested timeout, as opposed to what we decided to give it. This was necessary for the value to be present at the time criteria are applied. This means that you can only match based on the timeout that the notifier requested, not what you have overridden it to be in other criteria sections using ignore-timeout. There's no way to "fix" this without introducing multiple stages of criteria matching, which is a road I really don't want to go down. This case is a bit unique, as ignore-timeout is the only option which has the effect of changing something provided by the notifier.

I don't see this as a big issue, as if you are overriding the timeout for something specific, you can also just set the other style options in that criteria section.

vilhalmer added 2 commits July 3, 2018 16:46
This also adds a field to mako_notification to track the requested
timeout, as opposed to what we decided to give it. This was required for
the value to be present at the time criteria are applied.

Probably the most useful thing you can do with this is set
ignore-timeout + default-timeout to prevent anything from sending you
permanent notifications.
dbus/xdg.c Outdated
@@ -227,7 +228,8 @@ static int handle_notify(sd_bus_message *msg, void *data,
return -1;
}

if (expire_timeout < 0 || notif->style.ignore_timeout) {
int32_t expire_timeout;
if (notif->requested_timeout < 0 || notif->style.ignore_timeout) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this condition isn't satisfied, expire_timeout will be uninitialized. We should probably set it to requested_timeout.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, good catch. My test notification wasn't specifying one.

Copy link
Owner

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this is a nice addition.

This means that you can only match based on the timeout that the notifier requested, not what you have overridden it to be in other criteria sections using ignore-timeout.

Yeah i think that's fine.

@emersion emersion merged commit f496e7c into emersion:master Jul 3, 2018
@emersion
Copy link
Owner

emersion commented Jul 3, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants