[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add notifications.el
From: |
Jan Moringen |
Subject: |
Re: [PATCH] Add notifications.el |
Date: |
Fri, 11 Jun 2010 01:16:13 +0200 |
Hi Julien,
thank you for this patch.
I have two suggestions:
1. Maybe the docstring should mention that :body and action name
strings can contain markup like <b>bold</b> and that the
notification does not display a string if it contains invalid
markup like <illegal>text</illegal>. I think,
`xml-escape-string' can be used to safely display strings that
may contain invalid markup.
2. Older versions of libnotify seem to use a slightly different
interface: it seems like the signal for closing notifications
does not include a reason (see output of dbus-monitor [1]). I
suggest making the reason argument in the -on-closed handler
optional.
What do you think?
Kind regards,
Jan
[1] dbus-monitor output:
method call sender=:1.1293 -> dest=org.freedesktop.Notifications
path=/org/freedesktop/Notifications;
interface=org.freedesktop.Notifications; member=Notify
string "Emacs"
uint32 0
string
"/homes/jmoringe/opt/emacs/share/emacs/24.0.50/etc/images/icons/hicolor/scalable/apps/emacs.svg"
string "Bla <b>dsf</b> [bla]"
string "bla Bla <b>dsf</b>"
array [
string ":subscribe"
string "subscribe"
]
array [
]
int32 -1
method return sender=:1.19 -> dest=:1.1293 reply_serial=6
uint32 245
signal sender=:1.19 -> dest=:1.1293 path=/org/freedesktop/Notifications;
interface=org.freedesktop.Notifications; member=NotificationClosed
uint32 245
- Re: [PATCH] Add notifications.el, (continued)
- Re: [PATCH] Add notifications.el, Stefan Monnier, 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 <=
- Re: [PATCH] Add notifications.el, Julien Danjou, 2010/06/11
- Message not available
- Re: [PATCH] Add notifications.el, Jan Moringen, 2010/06/11
- Re: [PATCH] Add notifications.el, Julien Danjou, 2010/06/11
- Re: [PATCH] Add notifications.el, Michael Albinus, 2010/06/11
- Re: [PATCH] Add notifications.el, Jan Moringen, 2010/06/11
- Re: [PATCH] Add notifications.el, Stefan Monnier, 2010/06/08
- Re: [PATCH] Add notifications.el, Michael Albinus, 2010/06/09
Re: [PATCH] Add notifications.el, Tassilo Horn, 2010/06/09