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

Extend --no-history functionality #495

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

grmat
Copy link

@grmat grmat commented Jan 25, 2024

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 IMHO1:

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)

I've implemented this draft in the given PR.

Features/use cases:

  • choose whether group and all notifications should appear in history (not only single ones)
  • choose whether notifications should appear in history via dbus
  • activate actions not only when receiving them but also on timeout and/or dismissal
  • possibility to keep an indicator showing the current num of (missed) notifications without periodically polling mako

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

  • is extending the --no-history functionality even interesting?
  • is an API change even open to discussion?
  • general comments?

Footnotes

  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.

grmat added 5 commits January 23, 2024 19:36
Simplify usage of dismissal functions
- dismissing whole group
- dismissing all
- dbus API
- makoctl
@emersion
Copy link
Owner

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.

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