[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add notifications.el
From: |
Julien Danjou |
Subject: |
Re: [PATCH] Add notifications.el |
Date: |
Mon, 07 Jun 2010 17:17:34 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) |
On Mon, Jun 07 2010, Michael Albinus wrote:
> You should require 'cl when byte-compiling, due to the `case' macro.
Thanks, added.
>> +(defun notifications-notify (&rest params)
>> + "Send notification via D-Bus using the Freedesktop notification protocol.
>> +Various PARAMS can be set:
>> +
>> + :title The notification title.
>> + :body The notification body text.
>
> You might mention, that these are mandatory parameters. Check the effect of
>
> (notifications-notify :urgency "low")
They are not mandatory, even if it's weird. It works fine here, even if
the text is blank.
>> + :app-icon The notification icon.
>
> If not given, you could apply a default value. A good value might be
>
> (expand-file-name "images/icons/hicolor/scalable/apps/emacs.svg"
> data-directory)
Good idea, thanks. Added.
> I would call this parameter :actions, because you allow several actions
> in that list. Or you allow multiple :action parameters, and concat them.
>
> I would also be more descriptive, with examples.
>
> And I would explain, that the action identifier "default" has a special
> meaning. Try for effect
>
> (notifications-notify :title "title" :body "body" :urgency "low"
> :action '("default" ""))
Enhanced in that way.
>> + :timeout The timeout time in milliseconds since the display
>> + of the notification at which the notification should
>> + automatically close.
>> + If -1, the notification's expiration time is dependent
>> + on the notification server's settings, and may vary for
>> + the type of notification (default).
>> + If 0, the notification never expires.
>
> You might mention, that -1 is the default value.
Well, it is mentionned but since you did not saw that, I modified the help.
> All of them are hints. If none of them is given, you get a D-Bus error, try
>
> (notifications-notify)
>
> This is because you initialize your `hints' variable as '(:array). If no
> hint is given, you cannot pass it to `dbus-call-method' as such. You
> must pass '(:array :signature "{sv}") as empty hint.
I did not have any error, but well, I fixed it anyway.
> I also propose to add the handling of the "NotificationClosed" and
> "ActionInvoked" signals. This could be done by adding a callback
> function to `notifications-notify'.
That's clearly bonus, but added. :-)
Feel free to comment, I'm still new at this.
--
Julien Danjou
// ᐰ <address@hidden> http://julien.danjou.info
pgpH1JV9lINoa.pgp
Description: PGP signature
- [PATCH] Add notifications.el, Julien Danjou, 2010/06/05
- Re: [PATCH] Add notifications.el, Michael Albinus, 2010/06/07
- Re: [PATCH] Add notifications.el, Stefan Monnier, 2010/06/07
- Re: [PATCH] Add notifications.el,
Julien Danjou <=
- Re: [PATCH] Add notifications.el, Michael Albinus, 2010/06/07
- Re: [PATCH] Add notifications.el, Julien Danjou, 2010/06/07
- [PATCH] Add notifications.el, Julien Danjou, 2010/06/07
- Re: [PATCH] Add notifications.el, Davis Herring, 2010/06/07
- Re: [PATCH] Add notifications.el, Julien Danjou, 2010/06/08
- [PATCH] Add notifications.el, Julien Danjou, 2010/06/08
- Re: [PATCH] Add notifications.el, Michael Albinus, 2010/06/08
- Re: [PATCH] Add notifications.el, Julien Danjou, 2010/06/08
- Re: [PATCH] Add notifications.el, Jan Moringen, 2010/06/10
- Re: [PATCH] Add notifications.el, Julien Danjou, 2010/06/11