emersion / mako

A lightweight Wayland notification daemon
https://wayland.emersion.fr/mako
MIT License
2.17k stars 137 forks source link

Extend --no-history functionality #495

Open grmat opened 7 months ago

grmat commented 7 months ago

Hi, the recently introduced dismiss --no-history feature is very helpful to me. However, afaics, it's only usable when clicking the notification itself, not through the dbus API (or I'm blind) - and therefore neither with makoctl.

I tried to get familiar with the codebase to add a parameter for dbus calls and it seems a bit weird to me. For my elaboration, I'd like to exclude DismissAllNotifications(), as this one is self-explanatory. For the rest, there are currently 3 function signatures for 4 situations. If there's an ID given, DismissNotification() is used to dismiss whole groups as well, not DismissGroupNotifications(). I drafted a proposal of what would be easier to understand IMHO[^1]:

ID given? Whole group? Current Proposal
0 0 DismissLastNotification() DismissLastNotification(false)
0 1 DismissGroupNotifications() DismissLastNotification(true)
1 0 DismissNotification(id, false) DismissNotification(id, false)
1 1 DismissNotification(id, true) DismissNotification(id, true)

[^1]: of course, with 2 params it would also be possible to handle all those cases in one function (e.g. id=0 for last), but I'm not sure if this would go too far.

I've implemented this draft in the given PR.

Features/use cases:

I'm aware of the implications an API change could entail and I haven't been involved in this project. I don't claim to say that change would make sense here, I'd just like to ask

emersion commented 4 months ago

Hi! Sorry for the delay.

Yeah, the current D-Bus API is pretty clunky. I wonder if we should just have a single Dismiss method which takes a dict of options (that way, we can extend it later more easily). Seems like this is a somewhat common D-Bus pattern?

I think it's fine to make breaking changes to our internal D-Bus API.