[PATCH] Add notifications.el

[PATCH] Add notifications.el

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---

FYI I've signed the copyright assignment papers.

 lisp/ChangeLog            |    4 +
 lisp/net/notifications.el |  174 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 178 insertions(+), 0 deletions(-)
 create mode 100644 lisp/net/notifications.el

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index a33c43f..378449c 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,7 @@
+2010-06-05  Julien Danjou  <julien@danjou.info>
+
+	* net/notifications.el: New file.
+
 2010-06-04  Juanma Barranquero  <lekktu@gmail.com>

 	* subr.el (directory-sep-char): Move from fileio.c and make a defconst.
diff --git a/lisp/net/notifications.el b/lisp/net/notifications.el
new file mode 100644
index 0000000..aeeea31
--- /dev/null
+++ b/lisp/net/notifications.el
@@ -0,0 +1,174 @@
+;;; notifications.el --- Client interface to desktop notifications.
+
+;; Copyright (C) 2010 Free Software Foundation, Inc.
+
+;; Author: Julien Danjou <julien@danjou.info>
+;; Keywords: comm desktop notifications
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; This package provides an implementation of the Desktop Notifications
+;; <http://www.galago-project.org/specs/notification/>.
+
+;; In order to activate this package, you must add the following code
+;; into your .emacs:
+;;
+;;   (require 'notifications)
+
+;;; Code:
+
+;; Pacify byte-compiler.  D-Bus support in the Emacs core can be
+;; disabled with configuration option "--without-dbus".  Declare used
+;; subroutines and variables of `dbus' therefore.
+(declare-function dbus-call-method "dbusbind.c")
+
+(require 'dbus)
+
+(defconst notifications-application-name "Emacs"
+  "Default application name.")
+
+(defconst notifications-service "org.freedesktop.Notifications"
+  "D-Bus notifications service name.")
+
+(defconst notifications-path "/org/freedesktop/Notifications"
+  "D-Bus notifications service path.")
+
+(defconst notifications-interface "org.freedesktop.Notifications"
+  "D-Bus notifications service path.")
+
+(defconst notifications-notify-method "Notify"
+  "D-Bus notifications service path.")
+
+(defconst notifications-close-notification-method "CloseNotification"
+  "D-Bus notifications service path.")
+
+(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.
+ :app-name       The name of the application sending the notification.
+                 Default to `notifications-application-name'.
+ :replaces-id    The notification ID that this notification replaces.
+ :app-icon       The notification icon.
+ :action         A list of actions.
+ :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.
+ :urgency        The urgency level.
+                 Either `low', `normal' or `critical'.
+ :category       The type of notification this is.
+ :desktop-entry  This specifies the name of the desktop filename representing
+                 the calling program.
+ :image-data     This is a raw data image format which describes the width,
+                 height, rowstride, has alpha, bits per sample, channels and
+                 image data respectively.
+ :sound-file     The path to a sound file to play when the notification pops up.
+ :suppress-sound Causes the server to suppress playing any sounds, if it has
+                 that ability.
+ :x              Specifies the X location on the screen that the notification
+                 should point to. The \"y\" hint must also be specified.
+ :y              Specifies the Y location on the screen that the notification
+                 should point to. The \"x\" hint must also be specified.
+
+This function returns a notification id, an integer, which can be
+used to manipulate the notification item with
+`notifications-close'."
+  (let ((title (plist-get params :title))
+        (body (plist-get params :body))
+        (app-name (plist-get params :app-name))
+        (replaces-id (plist-get params :replaces-id))
+        (app-icon (plist-get params :app-icon))
+        (action (plist-get params :action))
+        (timeout (plist-get params :timeout))
+        ;; Hints
+        (hints '(:array))
+        (urgency (plist-get params :urgency))
+        (category (plist-get params :category))
+        (desktop-entry (plist-get params :desktop-entry))
+        (image-data (plist-get params :image-data))
+        (sound-file (plist-get params :sound-file))
+        (suppress-sound (plist-get params :suppress-sound))
+        (x (plist-get params :x))
+        (y (plist-get params :y)))
+    ;; Build hints array
+    (when urgency
+      (add-to-list 'hints `(:dict-entry
+                            "urgency"
+                            (:variant :byte ,(case urgency
+                                               ('low 0)
+                                               ('critical 2)
+                                               (t 1)))) t))
+    (when category
+      (add-to-list 'hints `(:dict-entry
+                            "category"
+                            (:variant :string ,category)) t))
+    (when desktop-entry
+      (add-to-list 'hints `(:dict-entry
+                            "desktop-entry"
+                            (:variant :string ,desktop-entry)) t))
+    (when image-data
+      (add-to-list 'hints `(:dict-entry
+                            "image_data"
+                            (:variant :struct ,image-data)) t))
+    (when sound-file
+      (add-to-list 'hints `(:dict-entry
+                            "sound-file"
+                            (:variant :string ,sound-file)) t))
+    (when suppress-sound
+      (add-to-list 'hints `(:dict-entry
+                            "suppress-sound"
+                            (:variant :boolean ,suppress-sound)) t))
+    (when x
+      (add-to-list 'hints `(:dict-entry "x" (:variant :int32 ,x)) t))
+    (when y
+      (add-to-list 'hints `(:dict-entry "y" (:variant :int32 ,y)) t))
+
+    ;; Call Notify method
+    (dbus-call-method :session
+                      notifications-service
+                      notifications-path
+                      notifications-interface
+                      notifications-notify-method
+                      :string (or app-name
+                                  notifications-application-name)
+                      :uint32 (or replaces-id 0)
+                      :string (if app-icon
+                                  (expand-file-name app-icon)
+                                "")
+                      :string (or title "")
+                      :string (or body "")
+                      `(:array ,@action)
+                      hints
+                      :int32 (or timeout -1))))
+
+(defun notifications-close-notification (id)
+  "Close a notification with identifier ID."
+  (dbus-call-method :session
+                    notifications-service
+                    notifications-path
+                    notifications-interface
+                    notifications-close-notification-method
+                    :int32 id))
+
+(provide 'notifications)
-- 
1.7.1

Re: [PATCH] Add notifications.el

[Michael Albinus]

Julien Danjou <julien@danjou.info> writes:

Hi,

> FYI I've signed the copyright assignment papers.

Looks nice. I have some comments, 'tho.

You should require 'cl when byte-compiling, due to the `case' macro.

> +(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")

> + :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)

> + :action         A list of actions.

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" ""))

> + :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.

> + :urgency        The urgency level.
> +                 Either `low', `normal' or `critical'.
> + :category       The type of notification this is.
> + :desktop-entry  This specifies the name of the desktop filename representing
> +                 the calling program.
> + :image-data     This is a raw data image format which describes the width,
> +                 height, rowstride, has alpha, bits per sample, channels and
> +                 image data respectively.
> + :sound-file     The path to a sound file to play when the notification pops up.
> + :suppress-sound Causes the server to suppress playing any sounds, if it has
> +                 that ability.
> + :x              Specifies the X location on the screen that the notification
> +                 should point to. The \"y\" hint must also be specified.
> + :y              Specifies the Y location on the screen that the notification
> +                 should point to. The \"x\" hint must also be specified.

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 also propose to add the handling of the "NotificationClosed" and
"ActionInvoked" signals. This could be done by adding a callback
function to `notifications-notify'.

Best regards, Michael.

Re: [PATCH] Add notifications.el

[Stefan Monnier]

>> +(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")

If these are really fundamentally mandatory, then it would be better to
turn them into normal parameters, like

  (notifications-notify title body &rest params)


-- Stefan

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 2619 bytes --]

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
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] Add notifications.el

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/ChangeLog            |    4 +
 lisp/net/notifications.el |  257 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 261 insertions(+), 0 deletions(-)
 create mode 100644 lisp/net/notifications.el

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index a33c43f..378449c 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,7 @@
+2010-06-05  Julien Danjou  <julien@danjou.info>
+
+	* net/notifications.el: New file.
+
 2010-06-04  Juanma Barranquero  <lekktu@gmail.com>
 
 	* subr.el (directory-sep-char): Move from fileio.c and make a defconst.
diff --git a/lisp/net/notifications.el b/lisp/net/notifications.el
new file mode 100644
index 0000000..d2f4624
--- /dev/null
+++ b/lisp/net/notifications.el
@@ -0,0 +1,257 @@
+;;; notifications.el --- Client interface to desktop notifications.
+
+;; Copyright (C) 2010 Free Software Foundation, Inc.
+
+;; Author: Julien Danjou <julien@danjou.info>
+;; Keywords: comm desktop notifications
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; This package provides an implementation of the Desktop Notifications
+;; <http://www.galago-project.org/specs/notification/>.
+
+;; In order to activate this package, you must add the following code
+;; into your .emacs:
+;;
+;;   (require 'notifications)
+
+;;; Code:
+(eval-when-compile
+  (require 'cl))
+
+;; Pacify byte-compiler.  D-Bus support in the Emacs core can be
+;; disabled with configuration option "--without-dbus".  Declare used
+;; subroutines and variables of `dbus' therefore.
+(declare-function dbus-call-method "dbusbind.c")
+
+(require 'dbus)
+
+(defconst notifications-application-name "Emacs"
+  "Default application name.")
+
+(defconst notifications-application-icon
+  (expand-file-name
+   "images/icons/hicolor/scalable/apps/emacs.svg"
+   data-directory)
+  "Default application icon.")
+
+(defconst notifications-service "org.freedesktop.Notifications"
+  "D-Bus notifications service name.")
+
+(defconst notifications-path "/org/freedesktop/Notifications"
+  "D-Bus notifications service path.")
+
+(defconst notifications-interface "org.freedesktop.Notifications"
+  "D-Bus notifications service path.")
+
+(defconst notifications-notify-method "Notify"
+  "D-Bus notifications service path.")
+
+(defconst notifications-close-notification-method "CloseNotification"
+  "D-Bus notifications service path.")
+
+(defconst notifications-action-signal "ActionInvoked"
+  "D-Bus notifications action signal.")
+
+(defconst notifications-closed-signal "NotificationClosed"
+  "D-Bus notifications closed signal.")
+
+(defconst notifications-closed-reason
+  '((1 expired)
+    (2 dismissed)
+    (3 close-notification)
+    (4 undefined))
+  "List of reasons why a notification has been closed.")
+
+(defvar notifications-on-action-map nil
+  "Mapping between notification and action callback functions.")
+
+(defvar notifications-on-close-map nil
+  "Mapping between notification and close callback functions.")
+
+(defun notifications-on-action-signal (id action)
+  (let ((entry (assoc id notifications-on-action-map)))
+    (when entry
+      (funcall (cadr entry) action)
+      (remove entry 'notifications-on-action-map))))
+
+(dbus-register-signal
+ :session
+ notifications-service
+ notifications-path
+ notifications-interface
+ notifications-action-signal
+ 'notifications-on-action-signal)
+
+(defun notifications-on-closed-signal (id reason)
+  (let ((entry (assoc id notifications-on-close-map)))
+    (when entry
+      (funcall (cadr entry) (cadr (assoc reason notifications-closed-reason)))
+      (remove entry 'notifications-on-close-map))))
+
+(dbus-register-signal
+ :session
+ notifications-service
+ notifications-path
+ notifications-interface
+ notifications-closed-signal
+ 'notifications-on-closed-signal)
+
+(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.
+ :app-name       The name of the application sending the notification.
+                 Default to `notifications-application-name'.
+ :replaces-id    The notification ID that this notification replaces.
+ :app-icon       The notification icon.
+ :actions        A list of actions in the form:
+                   (KEY TITLE KEY TITLE ...)
+                 where KEY and TITLE are both strings.
+                 The default action (usually invoked by clicking the notification)
+                 should have a key named \"default\". The name can be anything,
+                 though implementations are free not to display it.
+ :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.
+                 If 0, the notification never expires.
+                 Default value is -1.
+ :urgency        The urgency level.
+                 Either `low', `normal' or `critical'.
+ :category       The type of notification this is.
+ :desktop-entry  This specifies the name of the desktop filename representing
+                 the calling program.
+ :image-data     This is a raw data image format which describes the width,
+                 height, rowstride, has alpha, bits per sample, channels and
+                 image data respectively.
+ :sound-file     The path to a sound file to play when the notification pops up.
+ :suppress-sound Causes the server to suppress playing any sounds, if it has
+                 that ability.
+ :x              Specifies the X location on the screen that the notification
+                 should point to. The \"y\" hint must also be specified.
+ :y              Specifies the Y location on the screen that the notification
+                 should point to. The \"x\" hint must also be specified.
+ :on-action      Function to call when an action is invoked. The key of the
+                 action is passed as argument to the function.
+ :on-close       Function to call when the notification has been closed
+                 by timeout or by the user.
+                 The function receive the closing reason as argument:
+                   - `expired' if the notification has expired
+                   - `dismissed' if the notification was dismissed by the user
+                   - `close-notification' if the notification was closed
+                     by a call to CloseNotification
+
+This function returns a notification id, an integer, which can be
+used to manipulate the notification item with
+`notifications-close'."
+  (let ((title (plist-get params :title))
+        (body (plist-get params :body))
+        (app-name (plist-get params :app-name))
+        (replaces-id (plist-get params :replaces-id))
+        (app-icon (plist-get params :app-icon))
+        (actions (plist-get params :actions))
+        (timeout (plist-get params :timeout))
+        ;; Hints
+        (hints '())
+        (urgency (plist-get params :urgency))
+        (category (plist-get params :category))
+        (desktop-entry (plist-get params :desktop-entry))
+        (image-data (plist-get params :image-data))
+        (sound-file (plist-get params :sound-file))
+        (suppress-sound (plist-get params :suppress-sound))
+        (x (plist-get params :x))
+        (y (plist-get params :y))
+        id)
+    ;; Build hints array
+    (when urgency
+      (add-to-list 'hints `(:dict-entry
+                            "urgency"
+                            (:variant :byte ,(case urgency
+                                               ('low 0)
+                                               ('critical 2)
+                                               (t 1)))) t))
+    (when category
+      (add-to-list 'hints `(:dict-entry
+                            "category"
+                            (:variant :string ,category)) t))
+    (when desktop-entry
+      (add-to-list 'hints `(:dict-entry
+                            "desktop-entry"
+                            (:variant :string ,desktop-entry)) t))
+    (when image-data
+      (add-to-list 'hints `(:dict-entry
+                            "image_data"
+                            (:variant :struct ,image-data)) t))
+    (when sound-file
+      (add-to-list 'hints `(:dict-entry
+                            "sound-file"
+                            (:variant :string ,sound-file)) t))
+    (when suppress-sound
+      (add-to-list 'hints `(:dict-entry
+                            "suppress-sound"
+                            (:variant :boolean ,suppress-sound)) t))
+    (when x
+      (add-to-list 'hints `(:dict-entry "x" (:variant :int32 ,x)) t))
+    (when y
+      (add-to-list 'hints `(:dict-entry "y" (:variant :int32 ,y)) t))
+
+    ;; Call Notify method
+    (setq id
+          (dbus-call-method :session
+                            notifications-service
+                            notifications-path
+                            notifications-interface
+                            notifications-notify-method
+                            :string (or app-name
+                                        notifications-application-name)
+                            :uint32 (or replaces-id 0)
+                            :string (if app-icon
+                                        (expand-file-name app-icon)
+                                      notifications-application-icon)
+                            :string (or title "")
+                            :string (or body "")
+                            `(:array ,@actions)
+                            (or hints '(:array :signature "{sv}"))
+                            :int32 (or timeout -1)))
+
+    ;; Register close/action callback function
+    (let ((on-action (plist-get params :on-action))
+          (on-close (plist-get params :on-close)))
+      (when on-action
+        (add-to-list 'notifications-on-action-map (list id on-action)))
+      (when on-close
+        (add-to-list 'notifications-on-close-map (list id on-close))))
+
+    ;; Return notification id
+    id))
+
+(defun notifications-close-notification (id)
+  "Close a notification with identifier ID."
+  (dbus-call-method :session
+                    notifications-service
+                    notifications-path
+                    notifications-interface
+                    notifications-close-notification-method
+                    :int32 id))
+
+(provide 'notifications)
-- 
1.7.1

Re: [PATCH] Add notifications.el

[Michael Albinus]

Julien Danjou <julien@danjou.info> writes:

>>> +(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.

They are not mandatory functionally (there is no D-Bus error), but as
you say, the result is weird. I like Stefan's idea to make them mandatory
parameters; if somebody really wants to left them empty, she could pass "".

>>> + :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.

Grrr. I need new glasses!

>> 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.

Maybe you have disabled D-Bus error in one way or the other, I've got an
error. Did you run dbus-monitor in parallel, during tests?

>> 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. :-)

It is more than just bonus. Otherwise, :actions could not be handled
properly.

Thanks for adding.

Best regards, Michael.

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]

On Mon, Jun 07 2010, Michael Albinus wrote:

> They are not mandatory functionally (there is no D-Bus error), but as
> you say, the result is weird. I like Stefan's idea to make them mandatory
> parameters; if somebody really wants to left them empty, she could pass "".

If you really want, I will amend the patch to add at list `:title' as
mandatory. OTOH body is really optionnal, quoting specs[1]:

    ``If the body is omitted, just the summary is displayed.''

> Grrr. I need new glasses!

The fact you missed it also proves it might be so obvious at first
read. ;)

> Maybe you have disabled D-Bus error in one way or the other, I've got an
> error. Did you run dbus-monitor in parallel, during tests?

Yes, and I really saw nothing. I don't think I disabled any D-Bus
error. I just saw an empty array [].

[1]  http://www.galago-project.org/specs/notification/0.9/x81.html

Anyhow, thanks much for the review Michael.

Cheers,
-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add notifications.el

[Davis Herring]

>>> + :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.

You should be careful to allow the caller to specifically request no icon;
I see from the code that :app-icon nil won't do that; perhaps :app-icon ""
would?

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 352 bytes --]

On Tue, Jun 08 2010, Davis Herring wrote:

> You should be careful to allow the caller to specifically request no icon;
> I see from the code that :app-icon nil won't do that; perhaps :app-icon ""
> would?

No it won't neither.
I'll send a new version fixing that.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] Add notifications.el

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/ChangeLog            |    4 +
 lisp/net/notifications.el |  265 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 269 insertions(+), 0 deletions(-)
 create mode 100644 lisp/net/notifications.el

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index a33c43f..378449c 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,7 @@
+2010-06-05  Julien Danjou  <julien@danjou.info>
+
+	* net/notifications.el: New file.
+
 2010-06-04  Juanma Barranquero  <lekktu@gmail.com>
 
 	* subr.el (directory-sep-char): Move from fileio.c and make a defconst.
diff --git a/lisp/net/notifications.el b/lisp/net/notifications.el
new file mode 100644
index 0000000..a2147ed
--- /dev/null
+++ b/lisp/net/notifications.el
@@ -0,0 +1,265 @@
+;;; notifications.el --- Client interface to desktop notifications.
+
+;; Copyright (C) 2010 Free Software Foundation, Inc.
+
+;; Author: Julien Danjou <julien@danjou.info>
+;; Keywords: comm desktop notifications
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; This package provides an implementation of the Desktop Notifications
+;; <http://www.galago-project.org/specs/notification/>.
+
+;; In order to activate this package, you must add the following code
+;; into your .emacs:
+;;
+;;   (require 'notifications)
+
+;;; Code:
+(eval-when-compile
+  (require 'cl))
+
+;; Pacify byte-compiler.  D-Bus support in the Emacs core can be
+;; disabled with configuration option "--without-dbus".  Declare used
+;; subroutines and variables of `dbus' therefore.
+(declare-function dbus-call-method "dbusbind.c")
+
+(require 'dbus)
+
+(defconst notifications-application-name "Emacs"
+  "Default application name.")
+
+(defconst notifications-application-icon
+  (expand-file-name
+   "images/icons/hicolor/scalable/apps/emacs.svg"
+   data-directory)
+  "Default application icon.")
+
+(defconst notifications-service "org.freedesktop.Notifications"
+  "D-Bus notifications service name.")
+
+(defconst notifications-path "/org/freedesktop/Notifications"
+  "D-Bus notifications service path.")
+
+(defconst notifications-interface "org.freedesktop.Notifications"
+  "D-Bus notifications service path.")
+
+(defconst notifications-notify-method "Notify"
+  "D-Bus notifications service path.")
+
+(defconst notifications-close-notification-method "CloseNotification"
+  "D-Bus notifications service path.")
+
+(defconst notifications-action-signal "ActionInvoked"
+  "D-Bus notifications action signal.")
+
+(defconst notifications-closed-signal "NotificationClosed"
+  "D-Bus notifications closed signal.")
+
+(defconst notifications-closed-reason
+  '((1 expired)
+    (2 dismissed)
+    (3 close-notification)
+    (4 undefined))
+  "List of reasons why a notification has been closed.")
+
+(defvar notifications-on-action-map nil
+  "Mapping between notification and action callback functions.")
+
+(defvar notifications-on-close-map nil
+  "Mapping between notification and close callback functions.")
+
+(defun notifications-on-action-signal (id action)
+  (let ((entry (assoc id notifications-on-action-map)))
+    (when entry
+      (funcall (cadr entry) action)
+      (remove entry 'notifications-on-action-map))))
+
+(dbus-register-signal
+ :session
+ notifications-service
+ notifications-path
+ notifications-interface
+ notifications-action-signal
+ 'notifications-on-action-signal)
+
+(defun notifications-on-closed-signal (id reason)
+  (let ((entry (assoc id notifications-on-close-map)))
+    (when entry
+      (funcall (cadr entry) (cadr (assoc reason notifications-closed-reason)))
+      (remove entry 'notifications-on-close-map))))
+
+(dbus-register-signal
+ :session
+ notifications-service
+ notifications-path
+ notifications-interface
+ notifications-closed-signal
+ 'notifications-on-closed-signal)
+
+(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.
+ :app-name       The name of the application sending the notification.
+                 Default to `notifications-application-name'.
+ :replaces-id    The notification ID that this notification replaces.
+ :app-icon       The notification icon.
+                 Default is `notifications-application-icon'.
+                 Set to nil if you do not want any icon displayed.
+ :actions        A list of actions in the form:
+                   (KEY TITLE KEY TITLE ...)
+                 where KEY and TITLE are both strings.
+                 The default action (usually invoked by clicking the notification)
+                 should have a key named \"default\". The name can be anything,
+                 though implementations are free not to display it.
+ :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.
+                 If 0, the notification never expires.
+                 Default value is -1.
+ :urgency        The urgency level.
+                 Either `low', `normal' or `critical'.
+ :category       The type of notification this is.
+ :desktop-entry  This specifies the name of the desktop filename representing
+                 the calling program.
+ :image-data     This is a raw data image format which describes the width,
+                 height, rowstride, has alpha, bits per sample, channels and
+                 image data respectively.
+ :sound-file     The path to a sound file to play when the notification pops up.
+ :suppress-sound Causes the server to suppress playing any sounds, if it has
+                 that ability.
+ :x              Specifies the X location on the screen that the notification
+                 should point to. The \"y\" hint must also be specified.
+ :y              Specifies the Y location on the screen that the notification
+                 should point to. The \"x\" hint must also be specified.
+ :on-action      Function to call when an action is invoked. The key of the
+                 action is passed as argument to the function.
+ :on-close       Function to call when the notification has been closed
+                 by timeout or by the user.
+                 The function receive the closing reason as argument:
+                   - `expired' if the notification has expired
+                   - `dismissed' if the notification was dismissed by the user
+                   - `close-notification' if the notification was closed
+                     by a call to CloseNotification
+
+This function returns a notification id, an integer, which can be
+used to manipulate the notification item with
+`notifications-close'."
+  (let ((title (plist-get params :title))
+        (body (plist-get params :body))
+        (app-name (plist-get params :app-name))
+        (replaces-id (plist-get params :replaces-id))
+        (app-icon (plist-get params :app-icon))
+        (actions (plist-get params :actions))
+        (timeout (plist-get params :timeout))
+        ;; Hints
+        (hints '())
+        (urgency (plist-get params :urgency))
+        (category (plist-get params :category))
+        (desktop-entry (plist-get params :desktop-entry))
+        (image-data (plist-get params :image-data))
+        (sound-file (plist-get params :sound-file))
+        (suppress-sound (plist-get params :suppress-sound))
+        (x (plist-get params :x))
+        (y (plist-get params :y))
+        id)
+    ;; Build hints array
+    (when urgency
+      (add-to-list 'hints `(:dict-entry
+                            "urgency"
+                            (:variant :byte ,(case urgency
+                                               ('low 0)
+                                               ('critical 2)
+                                               (t 1)))) t))
+    (when category
+      (add-to-list 'hints `(:dict-entry
+                            "category"
+                            (:variant :string ,category)) t))
+    (when desktop-entry
+      (add-to-list 'hints `(:dict-entry
+                            "desktop-entry"
+                            (:variant :string ,desktop-entry)) t))
+    (when image-data
+      (add-to-list 'hints `(:dict-entry
+                            "image_data"
+                            (:variant :struct ,image-data)) t))
+    (when sound-file
+      (add-to-list 'hints `(:dict-entry
+                            "sound-file"
+                            (:variant :string ,sound-file)) t))
+    (when suppress-sound
+      (add-to-list 'hints `(:dict-entry
+                            "suppress-sound"
+                            (:variant :boolean ,suppress-sound)) t))
+    (when x
+      (add-to-list 'hints `(:dict-entry "x" (:variant :int32 ,x)) t))
+    (when y
+      (add-to-list 'hints `(:dict-entry "y" (:variant :int32 ,y)) t))
+
+    ;; Call Notify method
+    (setq id
+          (dbus-call-method :session
+                            notifications-service
+                            notifications-path
+                            notifications-interface
+                            notifications-notify-method
+                            :string (or app-name
+                                        notifications-application-name)
+                            :uint32 (or replaces-id 0)
+                            :string (if app-icon
+                                        (expand-file-name app-icon)
+                                      ;; If app-icon is nil because user
+                                      ;; requested it to be so, send the
+                                      ;; empty string
+                                      (if (plist-member params :app-icon)
+                                          ""
+                                        ;; Otherwise send the default icon path
+                                        notifications-application-icon))
+                            :string (or title "")
+                            :string (or body "")
+                            `(:array ,@actions)
+                            (or hints '(:array :signature "{sv}"))
+                            :int32 (or timeout -1)))
+
+    ;; Register close/action callback function
+    (let ((on-action (plist-get params :on-action))
+          (on-close (plist-get params :on-close)))
+      (when on-action
+        (add-to-list 'notifications-on-action-map (list id on-action)))
+      (when on-close
+        (add-to-list 'notifications-on-close-map (list id on-close))))
+
+    ;; Return notification id
+    id))
+
+(defun notifications-close-notification (id)
+  "Close a notification with identifier ID."
+  (dbus-call-method :session
+                    notifications-service
+                    notifications-path
+                    notifications-interface
+                    notifications-close-notification-method
+                    :int32 id))
+
+(provide 'notifications)
-- 
1.7.1

Re: [PATCH] Add notifications.el

[Michael Albinus]

Julien Danjou <julien@danjou.info> writes:

> Signed-off-by: Julien Danjou <julien@danjou.info>
> ---
>  lisp/ChangeLog            |    4 +
>  lisp/net/notifications.el |  265 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 269 insertions(+), 0 deletions(-)
>  create mode 100644 lisp/net/notifications.el

I would say it is OK. An entry for etc/NEWS is missing, that's it.

Stefan, Chong, would it be OK to add this package? If Julien has no
commit rights yet, I could do it.

Best regards, Michael.

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

On Tue, Jun 08 2010, Michael Albinus wrote:

> Stefan, Chong, would it be OK to add this package? If Julien has no
> commit rights yet, I could do it.

I don't.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add notifications.el

[Stefan Monnier]

> Stefan, Chong, would it be OK to add this package? If Julien has no
> commit rights yet, I could do it.

Please go ahead, thank you,


        Stefan

Re: [PATCH] Add notifications.el

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Stefan, Chong, would it be OK to add this package? If Julien has no
>> commit rights yet, I could do it.
>
> Please go ahead, thank you,

Done.

>         Stefan

Best regards, Michael.

Re: [PATCH] Add notifications.el

[Tassilo Horn]

Hi Julien,

I threw away my notification code and now use yours.  It's going pretty
well, except one thing I cannot figure out.  I want to use a :on-close
function, so that notifications I've explicitly clicked away don't
appear again. (They are triggered minutely by appt.el.)  But :on-close
functions only get the reason.  How do I get the id of the closed
notification?  Wouldn't it be better to pass the id to that function,
too?

And another thing: I think `notifications-notify' should error when
there is an unknown keyword.  I was nearly going mad when trying to
figure out why :replace-id didn't work.  It was, because the keyword is
:replaces-id. ;-)

Thanks for another brick in the desktop integration wall!
Tassilo

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On Wed, Jun 09 2010, Tassilo Horn wrote:

> I threw away my notification code and now use yours.  It's going pretty
> well, except one thing I cannot figure out.  I want to use a :on-close
> function, so that notifications I've explicitly clicked away don't
> appear again. (They are triggered minutely by appt.el.)  But :on-close
> functions only get the reason.  How do I get the id of the closed
> notification?  Wouldn't it be better to pass the id to that function,
> too?

It'd be possible sure, but I did not think it was worth it nor it would
have a use case. You can passe a lambda/closure to :on-close, can't you?

If you have an problematic example maybe we can figure out.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add notifications.el

[Tassilo Horn]

Hi!

> > How do I get the id of the closed notification?  Wouldn't it be
> > better to pass the id to that function, too?
> 
> It'd be possible sure, but I did not think it was worth it nor it
> would have a use case. You can passe a lambda/closure to :on-close,
> can't you?

Not really, cause the ID I need is the return value of the
`notifications-notify', so I cannot use it as parameter to that
function.  And elisp doesn't have closures anyway.

> If you have an problematic example maybe we can figure out.

Here's my code:

--8<---------------cut here---------------start------------->8---
(defvar th-notify-body-to-id-map (make-hash-table :test 'string=)
  "Maps BODY values of notifications to the last notification ID.
If ID is -1, then any further notifications with that body will
be skipped.")

(defun th-notify (&rest args)
  "Create a notification popup.
For ARGS, see `notifications-notify'.
If there was a notification with the same BODY before, that one
will be replaced by the current notification."
  (require 'notifications)
  (let* ((body (plist-get args :body))
         (replace-id (or (gethash body th-notify-body-to-id-map)
                         (plist-get args :replaces-id))))
    (unless (eql replace-id -1)
      (puthash
       body
       (apply 'notifications-notify
              (plist-put (plist-put args :replaces-id replace-id)
                         :timeout 0))
       th-notify-body-to-id-map))))
--8<---------------cut here---------------end--------------->8---

So what I want, is that if I delete a notification, there should be a
mapping from :body value to -1 in th-notify-body-to-id-map, so that no
new notifications are performed for that entry.

Bye,
Tassilo

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]

On Wed, Jun 09 2010, Tassilo Horn wrote:

> (defvar th-notify-body-to-id-map (make-hash-table :test 'string=)
>   "Maps BODY values of notifications to the last notification ID.
> If ID is -1, then any further notifications with that body will
> be skipped.")
>
> (defun th-notify (&rest args)
>   "Create a notification popup.
> For ARGS, see `notifications-notify'.
> If there was a notification with the same BODY before, that one
> will be replaced by the current notification."
>   (require 'notifications)
>   (let* ((body (plist-get args :body))
>          (replace-id (or (gethash body th-notify-body-to-id-map)
>                          (plist-get args :replaces-id))))
>     (unless (eql replace-id -1)
>       (puthash
>        body
>        (apply 'notifications-notify
>               (plist-put (plist-put args :replaces-id replace-id)
>                          :timeout 0))
>        th-notify-body-to-id-map))))

What about adding:

  :on-close
  `(lambda (reason)
      (when (eq reason 'dismissed)
         (puthash ,body -1 th-notify-body-to-id-map)))

in `args' ?

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add notifications.el

[Tassilo Horn]

On Wednesday 09 June 2010 21:47:47 Julien Danjou wrote:

Hi!

> What about adding:
> 
>   :on-close
>   `(lambda (reason)
>       (when (eq reason 'dismissed)
>          (puthash ,body -1 th-notify-body-to-id-map)))
> 
> in `args' ?

Well, this is perfect.  :-)

Thanks a lot,
Tassilo

Re: [PATCH] Add notifications.el

[Stefan Monnier]

> What about adding:

>   :on-close
>   `(lambda (reason)
>       (when (eq reason 'dismissed)
>          (puthash ,body -1 th-notify-body-to-id-map)))

> in `args' ?

Just so I understand better the trade-off: what would it take to pass
`id' to the on-close function?


        Stefan


PS: the process-filters and process-sentinels all could keep track of
the process to which they're connected, but we found it convenient to
pass the process object as an argument instead.  So based on that
precedent, it would not seem out-of-place to pass the `id' to the
on-close function.

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1.1: Type: text/plain, Size: 151 bytes --]

On Wed, Jun 09 2010, Stefan Monnier wrote:

> Just so I understand better the trade-off: what would it take to pass
> `id' to the on-close function?



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Passes-notification-id-as-argument-of-on-action-and-.patch --]
[-- Type: text/x-diff, Size: 2421 bytes --]

From 56b9ff8b24c5d762190701e7bf2bb6cc0e4937a8 Mon Sep 17 00:00:00 2001
From: Julien Danjou <julien@danjou.info>
Date: Wed, 9 Jun 2010 22:57:18 +0200
Subject: [PATCH] Passes notification id as argument of on-action and on-close functions

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/net/notifications.el |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/lisp/net/notifications.el b/lisp/net/notifications.el
index a2147ed..8d897eb 100644
--- a/lisp/net/notifications.el
+++ b/lisp/net/notifications.el
@@ -87,7 +87,7 @@
 (defun notifications-on-action-signal (id action)
   (let ((entry (assoc id notifications-on-action-map)))
     (when entry
-      (funcall (cadr entry) action)
+      (funcall (cadr entry) id action)
       (remove entry 'notifications-on-action-map))))
 
 (dbus-register-signal
@@ -101,7 +101,7 @@
 (defun notifications-on-closed-signal (id reason)
   (let ((entry (assoc id notifications-on-close-map)))
     (when entry
-      (funcall (cadr entry) (cadr (assoc reason notifications-closed-reason)))
+      (funcall (cadr entry) id (cadr (assoc reason notifications-closed-reason)))
       (remove entry 'notifications-on-close-map))))
 
 (dbus-register-signal
@@ -153,11 +153,13 @@ Various PARAMS can be set:
                  should point to. The \"y\" hint must also be specified.
  :y              Specifies the Y location on the screen that the notification
                  should point to. The \"x\" hint must also be specified.
- :on-action      Function to call when an action is invoked. The key of the
-                 action is passed as argument to the function.
+ :on-action      Function to call when an action is invoked.
+                 The notification id and the key of the action are passed
+                 as arguments to the function.
  :on-close       Function to call when the notification has been closed
                  by timeout or by the user.
-                 The function receive the closing reason as argument:
+                 The function receive the notification id and the closing
+                 reason as arguments:
                    - `expired' if the notification has expired
                    - `dismissed' if the notification was dismissed by the user
                    - `close-notification' if the notification was closed
-- 
1.7.1


[-- Attachment #1.3: Type: text/plain, Size: 79 bytes --]


-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add notifications.el

[Stefan Monnier]

>> Just so I understand better the trade-off: what would it take to pass
>> `id' to the on-close function?
[... sample patch ...]

So it seems it's very straightforward.  Now, not having used anything
closely (or even remotely) related, I don't have a good feel for whether
that would be very useful.  So if someone else could help us make
a choice, that would be good.
I'd lean towards adding the `id' argument since it is useful to make the
call "unambiguous" and also because that `id' is otherwise "difficult"
to get.


        Stefan

Re: [PATCH] Add notifications.el

[Tassilo Horn]

On Thursday 10 June 2010 02:26:12 Stefan Monnier wrote:

> >> Just so I understand better the trade-off: what would it take to pass
> >> `id' to the on-close function?
> [... sample patch ...]
> 
> So it seems it's very straightforward.  Now, not having used anything
> closely (or even remotely) related, I don't have a good feel for
> whether that would be very useful.  So if someone else could help us
> make a choice, that would be good.

At least for my usecase, the backquoted lambda is even better than
having the id passed to the function.  In the latter case, I'd have to
search the hashtable for a value.

But I'm sure passing the ID to the function is good, too.  It seems to
me that the current approach is a good fit if the caller of
`notifications-notify' wants to control what to do on actions or close.
If you want to create a more generic wrapper around
`notifications-notify' in which you want to implement some default
behavior (like don't show dismissed notifications in the future), then
passing the ID would be more flexible, because there are no requirements
on the caller.

Currently I only have one caller (org-mode events triggering appt.el
appointments), so there's no immediate benefit here.  But I'm also in
favour of doing that interface change now, so long as only few people
are using this facility.

Bye,
Tassilo

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 577 bytes --]

On Thu, Jun 10 2010, Tassilo Horn wrote:

> Currently I only have one caller (org-mode events triggering appt.el
> appointments), so there's no immediate benefit here.  But I'm also in
> favour of doing that interface change now, so long as only few people
> are using this facility.

IMHO it sounds reasonable.

It seems to clear to me now that people may want to identify the
notification by id to use with their close/actions function, so let's
make that change until it's too late.

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add notifications.el

[Michael Albinus]

Julien Danjou <julien@danjou.info> writes:

> It seems to clear to me now that people may want to identify the
> notification by id to use with their close/actions function, so let's
> make that change until it's too late.

Your patch looks reasonable to me. If you have a ChangeLog entry, I'm
willing to commit it.

(And maybe you have also docstrings for `notifications-on-action-signal'
and `notifications-on-closed-signal')

Best regards, Michael.

[PATCH] Passes notification id as argument of on-action and on-close functions

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/ChangeLog            |    6 ++++++
 lisp/net/notifications.el |   16 +++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index a21517b..07a36ce 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,9 @@
+2010-06-10  Julien Danjou  <julien@danjou.info>
+
+	* net/notifications.el (notifications-on-action-signal)
+	(notifications-on-closed-signal): Pass notification id as first
+	argument to the callback functions. Add docstrings.
+
 2010-06-10  Glenn Morris  <rgm@gnu.org>
 
 	* emacs-lisp/authors.el (authors-ignored-files)
diff --git a/lisp/net/notifications.el b/lisp/net/notifications.el
index ef246c7..4b619a4 100644
--- a/lisp/net/notifications.el
+++ b/lisp/net/notifications.el
@@ -86,9 +86,11 @@
   "Mapping between notification and close callback functions.")
 
 (defun notifications-on-action-signal (id action)
+  "Dispatch notifications action signals to callback functions
+from `notifications-on-action-map'."
   (let ((entry (assoc id notifications-on-action-map)))
     (when entry
-      (funcall (cadr entry) action)
+      (funcall (cadr entry) id action)
       (remove entry 'notifications-on-action-map))))
 
 (dbus-register-signal
@@ -100,9 +102,11 @@
  'notifications-on-action-signal)
 
 (defun notifications-on-closed-signal (id reason)
+  "Dispatch closed notification signals to callback functions
+from `notifications-on-closed-map'."
   (let ((entry (assoc id notifications-on-close-map)))
     (when entry
-      (funcall (cadr entry) (cadr (assoc reason notifications-closed-reason)))
+      (funcall (cadr entry) id (cadr (assoc reason notifications-closed-reason)))
       (remove entry 'notifications-on-close-map))))
 
 (dbus-register-signal
@@ -155,11 +159,13 @@ Various PARAMS can be set:
                  should point to.  The \"y\" hint must also be specified.
  :y              Specifies the Y location on the screen that the notification
                  should point to.  The \"x\" hint must also be specified.
- :on-action      Function to call when an action is invoked.  The key of the
-                 action is passed as argument to the function.
+ :on-action      Function to call when an action is invoked.
+                 The notification id and the key of the action are passed
+                 as arguments to the function.
  :on-close       Function to call when the notification has been closed
                  by timeout or by the user.
-                 The function receives the closing reason as argument:
+                 The function receive the notification id and the closing
+                 reason as arguments:
                    - `expired' if the notification has expired
                    - `dismissed' if the notification was dismissed by the user
                    - `close-notification' if the notification was closed
-- 
1.7.1

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Michael Albinus]

Julien Danjou <julien@danjou.info> writes:

> Signed-off-by: Julien Danjou <julien@danjou.info>
> ---
>  lisp/ChangeLog            |    6 ++++++
>  lisp/net/notifications.el |   16 +++++++++++-----
>  2 files changed, 17 insertions(+), 5 deletions(-)

Committed. I've also moved the file to lisp/, because it seems to be the
better location than lisp/net.

Best regards, Michael.

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On Thu, Jun 10 2010, Michael Albinus wrote:

> Committed. I've also moved the file to lisp/, because it seems to be the
> better location than lisp/net.

Well, I've put here because it depends on dbus which is net.
I don't know why dbus is in net either, after all. It's not like it
supports network.:-)

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Michael Albinus]

Julien Danjou <julien@danjou.info> writes:

>> Committed. I've also moved the file to lisp/, because it seems to be the
>> better location than lisp/net.
>
> Well, I've put here because it depends on dbus which is net.
> I don't know why dbus is in net either, after all. It's not like it
> supports network.:-)

We had a similar discussion weeks ago, when I have added secrets.el. See
<http://thread.gmane.org/gmane.emacs.devel/121762/focus=121793>.

Best regards, Michael.

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Juri Linkov]

>>> Committed. I've also moved the file to lisp/, because it seems to be the
>>> better location than lisp/net.
>>
>> Well, I've put here because it depends on dbus which is net.
>> I don't know why dbus is in net either, after all. It's not like it
>> supports network.:-)
>
> We had a similar discussion weeks ago, when I have added secrets.el. See
> <http://thread.gmane.org/gmane.emacs.devel/121762/focus=121793>.

IIRC the consensus was that lisp/net should contain files
with the keyword "comm" that has the description
"communications, networking, remote access to files".

If notifications.el doesn't belong to the category "communications",
then please remove the keyword "comm".  Otherwise, the right place for it
is lisp/net.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Michael Albinus]

Juri Linkov <juri@jurta.org> writes:

> IIRC the consensus was that lisp/net should contain files
> with the keyword "comm" that has the description
> "communications, networking, remote access to files".

The question is, whether we understand "communications" also as "desktop
communication between applications". Then, secrets.el still belongs to
lisp/net.

> If notifications.el doesn't belong to the category "communications",
> then please remove the keyword "comm".  Otherwise, the right place for it
> is lisp/net.

I agree. It isn't "communications", the keyword "comm" could be removed.

Best regards, Michael.

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Juri Linkov]

>> IIRC the consensus was that lisp/net should contain files
>> with the keyword "comm" that has the description
>> "communications, networking, remote access to files".
>
> The question is, whether we understand "communications" also as "desktop
> communication between applications".

Why not?  Communication is a process of transferring information
from one entity to another.

>> If notifications.el doesn't belong to the category "communications",
>> then please remove the keyword "comm".  Otherwise, the right place for it
>> is lisp/net.
>
> I agree. It isn't "communications", the keyword "comm" could be removed.

I disagree that it isn't "communications" ;-) I think the right place for it
is lisp/net because it's very closely related to other communication package
dbus.el.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Michael Albinus]

Juri Linkov <juri@jurta.org> writes:

> I disagree that it isn't "communications" ;-) I think the right place for it
> is lisp/net because it's very closely related to other communication package
> dbus.el.

notifications.el is about communication. Not with another application,
but with the user in front of the monitor. If this counts, every package
using `message' would belong to the category "communications".

And just because it uses D-Bus ... battery.el reads /proc/acpi/battery
or /sys/class/power_supply in order to get the information; it does not
belong to "communications". If it would use the D-Bus interface
org.freedesktop.UPowerDevice instead, would this classify it for
"communications"?

Best regards, Michael.

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Stefan Monnier]

> Why not?  Communication is a process of transferring information
> from one entity to another.

Then bytecomp.el should be in lisp/net because it deals with
communicating info between a .el and a .elc file.
And VC also belongs there, because it communicates info between
different buffers as well as between Emacs and some external VCS.
... should I add more examples?


        Stefan

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Juri Linkov]

> Then bytecomp.el should be in lisp/net because it deals with
> communicating info between a .el and a .elc file.

I remember the goal was to move files out of the lisp directory
into subdirectories.  Among existing subdirectories, "net" is
the most suitable for notifications.el.  If not, we could
create a new subdirectory for files related to it.

> And VC also belongs there, because it communicates info between
> different buffers as well as between Emacs and some external VCS.

Speaking of VC, why not to create a subdirectory for VC files?

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [PATCH] Add notifications.el

[Jan Moringen]

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 &lt;b&gt;dsf&lt;/b&gt;"
   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] Passes notification id as argument of on-action and on-close functions

[Stefan Monnier]

>> And VC also belongs there, because it communicates info between
>> different buffers as well as between Emacs and some external VCS.
> Speaking of VC, why not to create a subdirectory for VC files?

We'd need to find a unifying theme that can attract at least 20 files to
be worthwhile.  Maybe something that could include VC, diff-mode, ediff,
log-view, ...


        Stefan

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Daniel Pittman]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> And VC also belongs there, because it communicates info between
>>> different buffers as well as between Emacs and some external VCS.
>> Speaking of VC, why not to create a subdirectory for VC files?
>
> We'd need to find a unifying theme that can attract at least 20 files to
> be worthwhile.  Maybe something that could include VC, diff-mode, ediff,
> log-view, ...

Wouldn't it fit squarely into the category of "user interface", or perhaps
"user interaction", along with battery.el, custom, the various frame support
things like dframe, and other UI-focused code?

        Daniel

-- 
✣ Daniel Pittman            ✉ daniel@rimspace.net            ☎ +61 401 155 707
               ♽ made with 100 percent post-consumer electrons

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]

On Fri, Jun 11 2010, Jan Moringen wrote:

>      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.

Yes, that'd be a good idea. And yes xml-escape-string is good for that.

>      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.

Seems weird to me. Which version do you have?

I've implemented version 0.9 of the spec here; there's still version 0.3
online, and even in that old version there's a reason in the
ClosedNotification signal.

So if this is real you might have a really really really old version?

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add notifications.el

[Jan Moringen]

Hi Julien.

> >      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.
> 
> Seems weird to me. Which version do you have?

In the packaged NEWS file it says:

version 0.4.4 (27-February-2007)

> I've implemented version 0.9 of the spec here; there's still version 0.3
> online, and even in that old version there's a reason in the
> ClosedNotification signal.

Maybe it's a bug in the notification daemon? It happened for every
notification I produced from Emacs on this machine. The notifications
also varied significantly. So in case of an error it wouldn't exactly be
a corner case which makes this possibility seem unlikely.

> So if this is real you might have a really really really old version?

Doesn't seem /that/ old to me.

Kind regards,
Jan

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Juri Linkov]

>> Speaking of VC, why not to create a subdirectory for VC files?
>
> We'd need to find a unifying theme that can attract at least 20 files to
> be worthwhile.  Maybe something that could include VC, diff-mode, ediff,
> log-view, ...

A "vc" subdirectory for the category "version control" could include
37 files:

add-log.el cvs-status.el diff.el diff-mode.el ediff-diff.el ediff.el
ediff-help.el ediff-hook.el ediff-init.el ediff-merg.el ediff-mult.el
ediff-ptch.el ediff-util.el ediff-vers.el ediff-wind.el log-edit.el
log-view.el pcvs-defs.el pcvs.el pcvs-info.el pcvs-parse.el pcvs-util.el
vc-annotate.el vc-arch.el vc-bzr.el vc-cvs.el vc-dav.el vc-dir.el
vc-dispatcher.el vc.el vc-git.el vc-hg.el vc-hooks.el vc-mtn.el
vc-rcs.el vc-sccs.el vc-svn.el

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Juri Linkov]

> Wouldn't it fit squarely into the category of "user interface", or perhaps
> "user interaction", along with battery.el, custom, the various frame support
> things like dframe, and other UI-focused code?

Maybe "user communications" with a "comm" subdirectory?  But such categories
are too fuzzy to define.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [PATCH] Add notifications.el

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

On Fri, Jun 11 2010, Jan Moringen wrote:

> In the packaged NEWS file it says:
>
> version 0.4.4 (27-February-2007)

It's not that old, I've got the latest one which is 0.4.5.

> Maybe it's a bug in the notification daemon? It happened for every
> notification I produced from Emacs on this machine. The notifications
> also varied significantly. So in case of an error it wouldn't exactly be
> a corner case which makes this possibility seem unlikely.

Does it happend with the ones made from notify-send?

Anyhow, nothing allows you to work-around the bug by putting reason
&optional. Anybody can emit a wrong formatted signal on the dbus, even
with dbus-send. Based on that, any D-Bus bad formatted signal/method
call will raise an error.

Bad that's like calling a Lisp function with requires 2 arguments with
only 1. It raises an error because you call it badly. I don't see why we
wouldn't do the thing even if the function is called via D-Bus.

But yes if, you don't want to know about bugged implementation, you use
&optional in every dbus signal/method function you will use (even
outside notifications.el).

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] Add notifications.el

[Jan Moringen]

> > Maybe it's a bug in the notification daemon? It happened for every
> > notification I produced from Emacs on this machine. The notifications
> > also varied significantly. So in case of an error it wouldn't exactly be
> > a corner case which makes this possibility seem unlikely.
> 
> Does it happend with the ones made from notify-send?

I don't have notify-send on this system, but here is an example where
evolution caused the notification:

method call sender=:1.764 -> dest=org.freedesktop.Notifications
path=/org/freedesktop/Notifications;
interface=org.freedesktop.Notifications; member=Notify
   string "evolution-mail-notification"
   uint32 0
   string "mail-unread"
   string "New email"
   string "You have received 1 new message
in Inbox."
   array [
   ]
   array [
      dict entry(
         string "y"
         variant             int32 1188
      )
      dict entry(
         string "xdisplay"
         variant             string ":0.0"
      )
      dict entry(
         string "urgency"
         variant             byte 1
      )
      dict entry(
         string "x"
         variant             int32 1313
      )
   ]
   int32 -1
method return sender=:1.19 -> dest=:1.764 reply_serial=1011
   uint32 258
signal sender=:1.19 -> dest=:1.764 path=/org/freedesktop/Notifications;
interface=org.freedesktop.Notifications; member=NotificationClosed
   uint32 258

The NotificationClosed signal has no reason, just like with Emacs.

> Anyhow, nothing allows you to work-around the bug by putting reason
> &optional. Anybody can emit a wrong formatted signal on the dbus, even
> with dbus-send. Based on that, any D-Bus bad formatted signal/method
> call will raise an error.
> 
> Bad that's like calling a Lisp function with requires 2 arguments with
> only 1. It raises an error because you call it badly. I don't see why we
> wouldn't do the thing even if the function is called via D-Bus.
> 
> But yes if, you don't want to know about bugged implementation, you use
> &optional in every dbus signal/method function you will use (even
> outside notifications.el).

I agree, working around other people's bugs is undesirable. However,
since this system is a Ubuntu LTS version, the problem could potentially
affect many users.

Kind regards,
Jan

Re: [PATCH] Add notifications.el

[Michael Albinus]

Julien Danjou <julien@danjou.info> writes:

>
>> Maybe it's a bug in the notification daemon? It happened for every
>> notification I produced from Emacs on this machine. The notifications
>> also varied significantly. So in case of an error it wouldn't exactly be
>> a corner case which makes this possibility seem unlikely.
>
> Does it happend with the ones made from notify-send?
>
> Anyhow, nothing allows you to work-around the bug by putting reason
> &optional. Anybody can emit a wrong formatted signal on the dbus, even
> with dbus-send. Based on that, any D-Bus bad formatted signal/method
> call will raise an error.

Jan cannot work around this problem in his own code, because
`notifications-on-closed-signal' would already fail. A common trick is
to declare it as

  (defun notifications-on-closed-signal (&rest args) ...)

Then you can walk through `args' in your code, and set `id' and `reason'
as possible.

Best regards, Michael.

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Stefan Monnier]

>> We'd need to find a unifying theme that can attract at least 20 files to
>> be worthwhile.  Maybe something that could include VC, diff-mode, ediff,
>> log-view, ...
> Wouldn't it fit squarely into the category of "user interface", or perhaps
> "user interaction", along with battery.el, custom, the various frame support
> things like dframe, and other UI-focused code?

I was thinking of something like "version management".
I don't think VC fits in well into "UI", since most of its files have
nothing to do with the UI, but only to interfacing with
a particular backend.


        Stefan

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Stefan Monnier]

>>> Speaking of VC, why not to create a subdirectory for VC files?
>> We'd need to find a unifying theme that can attract at least 20 files to
>> be worthwhile.  Maybe something that could include VC, diff-mode, ediff,
>> log-view, ...

> A "vc" subdirectory for the category "version control" could include
> 37 files:

> add-log.el cvs-status.el diff.el diff-mode.el ediff-diff.el ediff.el
> ediff-help.el ediff-hook.el ediff-init.el ediff-merg.el ediff-mult.el
> ediff-ptch.el ediff-util.el ediff-vers.el ediff-wind.el log-edit.el
> log-view.el pcvs-defs.el pcvs.el pcvs-info.el pcvs-parse.el pcvs-util.el
> vc-annotate.el vc-arch.el vc-bzr.el vc-cvs.el vc-dav.el vc-dir.el
> vc-dispatcher.el vc.el vc-git.el vc-hg.el vc-hooks.el vc-mtn.el
> vc-rcs.el vc-sccs.el vc-svn.el

That looks pretty good to me.  Any objection?


        Stefan

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> A "vc" subdirectory for the category "version control" could include
>> 37 files:
>
>> add-log.el cvs-status.el diff.el diff-mode.el ediff-diff.el ediff.el
>> ediff-help.el ediff-hook.el ediff-init.el ediff-merg.el ediff-mult.el
>> ediff-ptch.el ediff-util.el ediff-vers.el ediff-wind.el log-edit.el
>> log-view.el pcvs-defs.el pcvs.el pcvs-info.el pcvs-parse.el pcvs-util.el
>> vc-annotate.el vc-arch.el vc-bzr.el vc-cvs.el vc-dav.el vc-dir.el
>> vc-dispatcher.el vc.el vc-git.el vc-hg.el vc-hooks.el vc-mtn.el
>> vc-rcs.el vc-sccs.el vc-svn.el
>
> That looks pretty good to me.  Any objection?

Not from me.  I've been looking forward to something like this for a
while.

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Stefan Monnier]

>>> A "vc" subdirectory for the category "version control" could include
>>> 37 files:
>> 
>>> add-log.el cvs-status.el diff.el diff-mode.el ediff-diff.el ediff.el
>>> ediff-help.el ediff-hook.el ediff-init.el ediff-merg.el ediff-mult.el
>>> ediff-ptch.el ediff-util.el ediff-vers.el ediff-wind.el log-edit.el
>>> log-view.el pcvs-defs.el pcvs.el pcvs-info.el pcvs-parse.el pcvs-util.el
>>> vc-annotate.el vc-arch.el vc-bzr.el vc-cvs.el vc-dav.el vc-dir.el
>>> vc-dispatcher.el vc.el vc-git.el vc-hg.el vc-hooks.el vc-mtn.el
>>> vc-rcs.el vc-sccs.el vc-svn.el
>> 
>> That looks pretty good to me.  Any objection?

> Not from me.  I've been looking forward to something like this for a
> while.

Jury, please go ahead (and please put emerge.el in there as well).


        Stefan

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Juri Linkov]

>>>> A "vc" subdirectory for the category "version control" could include
>>>> 37 files:
>>>
>>>> add-log.el cvs-status.el diff.el diff-mode.el ediff-diff.el ediff.el
>>>> ediff-help.el ediff-hook.el ediff-init.el ediff-merg.el ediff-mult.el
>>>> ediff-ptch.el ediff-util.el ediff-vers.el ediff-wind.el log-edit.el
>>>> log-view.el pcvs-defs.el pcvs.el pcvs-info.el pcvs-parse.el pcvs-util.el
>>>> vc-annotate.el vc-arch.el vc-bzr.el vc-cvs.el vc-dav.el vc-dir.el
>>>> vc-dispatcher.el vc.el vc-git.el vc-hg.el vc-hooks.el vc-mtn.el
>>>> vc-rcs.el vc-sccs.el vc-svn.el
>>>
>>> That looks pretty good to me.  Any objection?
>
>> Not from me.  I've been looking forward to something like this for a
>> while.
>
> Jury, please go ahead (and please put emerge.el in there as well).

Done.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Dan Nicolaescu]

Juri Linkov <juri@jurta.org> writes:

>>>>> A "vc" subdirectory for the category "version control" could include
>>>>> 37 files:
>>>>
>>>>> add-log.el cvs-status.el diff.el diff-mode.el ediff-diff.el ediff.el
>>>>> ediff-help.el ediff-hook.el ediff-init.el ediff-merg.el ediff-mult.el
>>>>> ediff-ptch.el ediff-util.el ediff-vers.el ediff-wind.el log-edit.el
>>>>> log-view.el pcvs-defs.el pcvs.el pcvs-info.el pcvs-parse.el pcvs-util.el
>>>>> vc-annotate.el vc-arch.el vc-bzr.el vc-cvs.el vc-dav.el vc-dir.el
>>>>> vc-dispatcher.el vc.el vc-git.el vc-hg.el vc-hooks.el vc-mtn.el
>>>>> vc-rcs.el vc-sccs.el vc-svn.el
>>>>
>>>> That looks pretty good to me.  Any objection?
>>
>>> Not from me.  I've been looking forward to something like this for a
>>> while.
>>
>> Jury, please go ahead (and please put emerge.el in there as well).
>
> Done.

How about compare-w.el ?

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Juri Linkov]

> How about compare-w.el ?

I could move compare-w.el too after fixing the bootstrapping problems.
I thought that Makefiles don't list Lisp source files anymore,
but there is a loadup dependence on "hook" files.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Stefan Monnier]

> How about compare-w.el ?

Can be moved as well,


        Stefan

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Juri Linkov]

>> How about compare-w.el ?
>
> Can be moved as well,

Done.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [PATCH] Passes notification id as argument of on-action and on-close functions

[Stefan Monnier]

>>> How about compare-w.el ?
>> Can be moved as well,
> Done.

Thank you,


        Stefan