Codecov Report

Attention: Patch coverage is 62.50000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 65.93%. Comparing base (f556044) to head (3b29cae).
Report is 2 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master    #1348      +/-   ##
==========================================
- Coverage   65.95%   65.93%   -0.02%     
==========================================
  Files          50       50              
  Lines        8209     8213       +4     
  Branches      962      962              
==========================================
+ Hits         5414     5415       +1     
- Misses       2795     2798       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

I like the idea.

I was wondering if there was a nice way to call closeNotification from dunstctl or the like. Maybe we can add an optional argument to dunstctl close?

edit: I actually made it in #1351

I suggest to implement a more generic NotificationList that takes a bool parameter for displayed. This way we can query all the current notifications (otherwise the one neither displayed nor in history would be inaccessible).
this would allow for example autocompletion of dunstctl close with the id (and surely other nice things).

I don't think, that NotificationList bool would make good API as the meaning of the parameter is not immediately clear. Given that NotificationListHistory already exists and this one adds NotificationListDisplayed adding another NotificationListQueued or NotificationListWaiting seems preferable from my side.

Maybe for the future one could think about implementing NotificationList [waiting/queued|displayed|history], but at the moment NotificationListHistory is already published and thus it might be used. Changing it now would be a breaking change.

I don't think, that NotificationList bool would make good API as the meaning of the parameter is not immediately clear.

Well the parameter name can be specified in the xml. Maybe for a clearer name something like NotificationListCurrent to differentiate from the history one?

But creating another method is alright. So we would just need to combine the two where needed

Maybe for a clearer name something like NotificationListCurrent to differentiate from the history one?

TBH, I don't get this proposal. NotificationListDisplayed states that it's listing notifications displayed. For NotificationListCurrent I would expect the union of displayed and waiting notifications being returned.

I also don't understand why there would be a need to differentiate from NotificationListHistory. We then would end up with NotificationListDisplayed, NotificationListWaiting, and NotificationListHistory, which form a kind of self-describing API stating what they're about to return.

But creating another method is alright. So we would just need to combine the two where needed

For both use cases described in #1242 and #1346 just the displayed notifications are of interest and thus NotificationListWaiting (to use the term used in the code already) would not be strictly needed (though, I'd add it anyhow as it's just a couple of lines).

But creating another method is alright

It was just an idea. You can go ahead with displayed and waiting as you said