* Proposed changes to gnus-notifications.el
@ 2019-07-21 0:52 Basil L. Contovounesios
2019-07-21 8:01 ` Michael Albinus
2019-07-21 13:44 ` Lars Ingebrigtsen
0 siblings, 2 replies; 10+ messages in thread
From: Basil L. Contovounesios @ 2019-07-21 0:52 UTC (permalink / raw)
To: emacs-devel; +Cc: Julien Danjou, Lars Ingebrigtsen
[-- Attachment #1: Type: text/plain, Size: 134 bytes --]
The following patch for gnus-notifications.el enables lexical-binding
and simplifies and clarifies some of the code and docs. WDYT?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Refactor-gnus-notifications.el.patch --]
[-- Type: text/x-diff, Size: 13230 bytes --]
From 40e0cf058674d0791b5f8110aba8d9171d640c14 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sat, 20 Jul 2019 23:37:29 +0100
Subject: [PATCH] Refactor gnus-notifications.el
* lisp/gnus/gnus-notifications.el: Use lexical-binding. Pass
non-nil NOERROR to 'require' instead of wrapping in ignore-errors.
Add image.el as a dependency. Remove redundant :group tags.
(gnus-notifications-use-google-contacts): Default to nil if
google-contacts is not installed.
(gnus-notifications-use-gravatar, gnus-notifications-timeout)
(gnus-notifications-sent, gnus-notifications-id-to-msg): Clarify
docstring.
(gnus-notifications-notify, gnus-notifications-get-photo)
(gnus-notifications-get-photo-file): Simplify.
(gnus-notifications): Simplify to reduce indentation.
Call user option gnus-extract-address-components in place of
mail-extract-address-components.
---
lisp/gnus/gnus-notifications.el | 206 +++++++++++++++-----------------
1 file changed, 94 insertions(+), 112 deletions(-)
diff --git a/lisp/gnus/gnus-notifications.el b/lisp/gnus/gnus-notifications.el
index 3476164583..81e09d1fd5 100644
--- a/lisp/gnus/gnus-notifications.el
+++ b/lisp/gnus/gnus-notifications.el
@@ -1,4 +1,4 @@
-;; gnus-notifications.el -- Send notification on new message in Gnus
+;; gnus-notifications.el --- Notify of new Gnus messages -*- lexical-binding: t -*-
;; Copyright (C) 2012-2019 Free Software Foundation, Inc.
@@ -24,55 +24,54 @@
;; This implements notifications using `notifications-notify' on new
;; messages received.
-;; Use (add-hook 'gnus-after-getting-new-news-hook 'gnus-notifications)
+;; Use (add-hook 'gnus-after-getting-new-news-hook #'gnus-notifications)
;; to get notifications just after getting the new news.
;;; Code:
-(ignore-errors
- (require 'notifications))
+(require 'notifications)
(require 'gnus-sum)
(require 'gnus-group)
(require 'gnus-int)
(require 'gnus-art)
(require 'gnus-util)
-(ignore-errors
- (require 'google-contacts)) ; Optional
(require 'gnus-fun)
+(require 'image)
(defgroup gnus-notifications nil
- "Send notifications on new message in Gnus."
+ "Send notifications on new messages in Gnus."
:version "24.3"
:group 'gnus)
-(defcustom gnus-notifications-use-google-contacts t
- "Use Google Contacts to retrieve photo."
+(defcustom gnus-notifications-use-google-contacts
+ (and (require 'google-contacts nil t) t)
+ "Whether to retrieve sender avatars from Google Contacts.
+This requires the external package `google-contacts'."
:type 'boolean
- :group 'gnus-notifications)
+ :version "27.1")
(defcustom gnus-notifications-use-gravatar t
- "Use Gravatar to retrieve photo."
- :type 'boolean
- :group 'gnus-notifications)
+ "Whether to retrieve sender avatars from Gravatar."
+ :type 'boolean)
(defcustom gnus-notifications-minimum-level 1
"Minimum group level the message should have to be notified.
Any message in a group that has a greater value than this will
not get notifications."
- :type 'integer
- :group 'gnus-notifications)
+ :type 'integer)
(defcustom gnus-notifications-timeout nil
- "Timeout used for notifications sent via `notifications-notify'."
+ "Timeout used for notifications sent via `notifications-notify'.
+Value is either a duration in milliseconds or nil, which means to
+use the notification server's default timeout."
:type '(choice (const :tag "Server default" nil)
- (integer :tag "Milliseconds"))
- :group 'gnus-notifications)
+ (integer :tag "Milliseconds")))
(defvar gnus-notifications-sent nil
- "Notifications already sent.")
+ "Map group names to lists of sent notification IDs.")
(defvar gnus-notifications-id-to-msg nil
- "Map notifications ids to messages.")
+ "Map notification IDs to messages.")
(defun gnus-notifications-action (id key)
(let ((group-article (assoc id gnus-notifications-id-to-msg)))
@@ -90,57 +89,41 @@ gnus-notifications-action
(gnus-group-update-group group)))))))
(defun gnus-notifications-notify (from subject photo-file)
- "Send a notification about a new mail.
-Return a notification id if any, or t on success."
- (if (fboundp 'notifications-notify)
- (gnus-funcall-no-warning
- 'notifications-notify
- :title from
- :body subject
- :actions '("read" "Read" "mark-read" "Mark As Read")
- :on-action 'gnus-notifications-action
- :app-icon (gnus-funcall-no-warning
- 'image-search-load-path "gnus/gnus.png")
- :image-path photo-file
- :app-name "Gnus"
- :category "email.arrived"
- :timeout gnus-notifications-timeout)
- (message "New message from %s: %s" from subject)
- ;; Don't return an id
- t))
-
-(declare-function gravatar-retrieve-synchronously "gravatar.el"
- (mail-address))
+ "Send a notification about a new mail and return its ID."
+ (notifications-notify
+ :title from
+ :body subject
+ :actions '("read" "Read" "mark-read" "Mark As Read")
+ :on-action #'gnus-notifications-action
+ :app-icon (image-search-load-path "gnus/gnus.png")
+ :image-path photo-file
+ :app-name "Gnus"
+ :category "email.arrived"
+ :timeout gnus-notifications-timeout))
(defun gnus-notifications-get-photo (mail-address)
- "Get photo for mail address."
- (let ((google-photo (when (and gnus-notifications-use-google-contacts
- (fboundp 'google-contacts-get-photo))
- (ignore-errors
- (gnus-funcall-no-warning
- 'google-contacts-get-photo mail-address)))))
- (if google-photo
- google-photo
- (when gnus-notifications-use-gravatar
- (let ((gravatar (ignore-errors
- (gravatar-retrieve-synchronously mail-address))))
- (if (eq gravatar 'error)
- nil
- (plist-get (cdr gravatar) :data)))))))
+ "Return an avatar for MAIL-ADDRESS.
+Value is either a string of raw image data, or nil on failure."
+ (or (and gnus-notifications-use-google-contacts
+ (fboundp 'google-contacts-get-photo)
+ (ignore-errors
+ (google-contacts-get-photo mail-address)))
+ (let ((gravatar (and gnus-notifications-use-gravatar
+ (ignore-errors
+ (gravatar-retrieve-synchronously mail-address)))))
+ (and (eq (car-safe gravatar) 'image)
+ (image-property gravatar :data)))))
(defun gnus-notifications-get-photo-file (mail-address)
- "Get a temporary file with an image for MAIL-ADDRESS.
-You have to delete the temporary image yourself using
-`delete-image'.
+ "Return a temporary file name containing an image for MAIL-ADDRESS.
+Callers must themselves delete the file; it is not done
+automatically.
-Returns nil if no image found."
- (let ((photo (gnus-notifications-get-photo mail-address)))
+Returns nil if no image is found."
+ (let ((photo (gnus-notifications-get-photo mail-address))
+ (coding-system-for-write 'binary))
(when photo
- (let ((photo-file (make-temp-file "gnus-notifications-photo-"))
- (coding-system-for-write 'binary))
- (with-temp-file photo-file
- (insert photo))
- photo-file))))
+ (make-temp-file "gnus-notifications-photo-" nil nil photo))))
;;;###autoload
(defun gnus-notifications ()
@@ -151,53 +134,52 @@ gnus-notifications
This is typically a function to add in
`gnus-after-getting-new-news-hook'"
- (dolist (entry gnus-newsrc-alist)
- (let ((group (car entry)))
- ;; Check that the group level is less than
- ;; `gnus-notifications-minimum-level' and the group has unread
- ;; messages.
- (when (and (<= (gnus-group-level group) gnus-notifications-minimum-level)
- (let ((unread (gnus-group-unread group)))
- (and (numberp unread)
- (> unread 0))))
- ;; Each group should have an entry in the `gnus-notifications-sent'
- ;; alist. If not, we add one at this time.
- (let ((group-notifications (or (assoc group gnus-notifications-sent)
- ;; Nothing, add one and return it.
- (assoc group
- (add-to-list
- 'gnus-notifications-sent
- (cons group nil))))))
- (dolist (article (gnus-list-of-unread-articles group))
- ;; Check if the article already has been notified
- (unless (memq article (cdr group-notifications))
- (with-current-buffer nntp-server-buffer
- (gnus-request-head article group)
- (article-decode-encoded-words) ; to decode mail addresses, subjects, etc
- (let* ((address-components (mail-extract-address-components
- (or (mail-fetch-field "From") "")))
- (address (cadr address-components)))
- ;; Ignore mails from ourselves
- (unless (and gnus-ignored-from-addresses
- address
- (cond ((functionp gnus-ignored-from-addresses)
- (funcall gnus-ignored-from-addresses address))
- (t (string-match-p
- (gnus-ignored-from-addresses)
- address))))
- (let* ((photo-file (gnus-notifications-get-photo-file address))
- (notification-id (gnus-notifications-notify
- (or (car address-components) address)
- (mail-fetch-field "Subject")
- photo-file)))
- (when notification-id
- ;; Register that we did notify this message
- (setcdr group-notifications (cons article (cdr group-notifications)))
- (unless (eq notification-id t)
- ;; Register the notification id for later actions
- (add-to-list 'gnus-notifications-id-to-msg (list notification-id group article))))
- (when photo-file
- (delete-file photo-file)))))))))))))
+ (pcase-dolist (`(,group . ,_) gnus-newsrc-alist)
+ ;; Check that the group level is less than
+ ;; `gnus-notifications-minimum-level' and the group has unread
+ ;; messages.
+ (when (and (<= (gnus-group-level group) gnus-notifications-minimum-level)
+ (let ((unread (gnus-group-unread group)))
+ (and (numberp unread)
+ (> unread 0))))
+ ;; Each group should have an entry in the `gnus-notifications-sent'
+ ;; alist. If not, we add one at this time.
+ (let ((group-notifications
+ (or (assoc group gnus-notifications-sent)
+ ;; Nothing, add one and return it.
+ (assoc group (push (list group) gnus-notifications-sent)))))
+ (dolist (article (gnus-list-of-unread-articles group))
+ ;; Check if the article has already been notified.
+ (unless (memq article (cdr group-notifications))
+ (with-current-buffer nntp-server-buffer
+ (gnus-request-head article group)
+ ;; To decode mail addresses, subjects, etc.
+ (article-decode-encoded-words)
+ (let* ((address-components
+ (funcall gnus-extract-address-components
+ (or (mail-fetch-field "From") "")))
+ (address (cadr address-components)))
+ ;; Ignore mail from ourselves.
+ (unless (and gnus-ignored-from-addresses
+ (> (length address) 0)
+ (if (functionp gnus-ignored-from-addresses)
+ (funcall gnus-ignored-from-addresses address)
+ (string-match-p (gnus-ignored-from-addresses)
+ address)))
+ (let* ((photo-file (gnus-notifications-get-photo-file address))
+ (notification-id (gnus-notifications-notify
+ (or (car address-components) address)
+ (mail-fetch-field "Subject")
+ photo-file)))
+ (when notification-id
+ ;; Register that we did notify this message.
+ (push article (cdr group-notifications))
+ ;; Register the notification ID for later actions.
+ (setf (alist-get notification-id
+ gnus-notifications-id-to-msg)
+ (list group article)))
+ (when photo-file
+ (delete-file photo-file))))))))))))
(provide 'gnus-notifications)
--
2.20.1
[-- Attachment #3: Type: text/plain, Size: 20 bytes --]
Thanks,
--
Basil
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Proposed changes to gnus-notifications.el
2019-07-21 0:52 Proposed changes to gnus-notifications.el Basil L. Contovounesios
@ 2019-07-21 8:01 ` Michael Albinus
2019-07-21 9:40 ` Basil L. Contovounesios
2019-07-21 13:44 ` Lars Ingebrigtsen
1 sibling, 1 reply; 10+ messages in thread
From: Michael Albinus @ 2019-07-21 8:01 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: Julien Danjou, Lars Ingebrigtsen, emacs-devel
"Basil L. Contovounesios" <contovob@tcd.ie> writes:
Hi Basil,
> The following patch for gnus-notifications.el enables lexical-binding
> and simplifies and clarifies some of the code and docs. WDYT?
The patch seems to assume that notifications-notify works
everywhere. That's not the case, since it depends on D-Bus it runs only
for GNU/Linux systems.
> Thanks,
Best regards, Michael.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Proposed changes to gnus-notifications.el
2019-07-21 8:01 ` Michael Albinus
@ 2019-07-21 9:40 ` Basil L. Contovounesios
2019-07-21 10:40 ` Michael Albinus
2019-07-21 14:34 ` Proposed changes to gnus-notifications.el Eli Zaretskii
0 siblings, 2 replies; 10+ messages in thread
From: Basil L. Contovounesios @ 2019-07-21 9:40 UTC (permalink / raw)
To: Michael Albinus; +Cc: Julien Danjou, Lars Ingebrigtsen, emacs-devel
Michael Albinus <michael.albinus@gmx.de> writes:
> The patch seems to assume that notifications-notify works
> everywhere. That's not the case, since it depends on D-Bus it runs only
> for GNU/Linux systems.
Thank you for pointing this out. I had wondered about it while
preparing the patch but could not find a description of what happens
when notifications are not supported, other than the usual
"notifications-notify returns an integer ID".
Does notifications-notify return nil in this case? If so, I would like
to document this. If not, wouldn't this make sense?
What is the best way to determine whether notifications are supported by
the current Emacs instance? I skimmed through (info "(dbus) Top") and
(info "(elisp) Desktop Notifications"), as well as through the syms of
dbusbind.c, dbus.el, and notifications.el, but did not find any clear
answers.
Currently, gnus-notifications.el checks a) directly whether
notifications-notify is fboundp, and b) indirectly whether
notifications-notify returns nil. Even without notification support,
(a) should always be true after loading the library, right? So the
question is whether (b) is a sufficient condition.
PS Should gnus-notifications.el be extended to support
w32-notification-notify?
PPS Would it be possible/welcome to provide a common interface for both
notifications-notify and w32-notification-notify? At the moment it
sounds like programmers need to choose between the two depending on
system-type.
Thanks,
--
Basil
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Proposed changes to gnus-notifications.el
2019-07-21 9:40 ` Basil L. Contovounesios
@ 2019-07-21 10:40 ` Michael Albinus
2019-07-21 15:54 ` Basil L. Contovounesios
2019-07-21 15:55 ` Error handling in notifications-notify (was: Proposed changes to gnus-notifications.el) Basil L. Contovounesios
2019-07-21 14:34 ` Proposed changes to gnus-notifications.el Eli Zaretskii
1 sibling, 2 replies; 10+ messages in thread
From: Michael Albinus @ 2019-07-21 10:40 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: Julien Danjou, Lars Ingebrigtsen, emacs-devel
"Basil L. Contovounesios" <contovob@tcd.ie> writes:
Hi Basil,
> Michael Albinus <michael.albinus@gmx.de> writes:
>
>> The patch seems to assume that notifications-notify works
>> everywhere. That's not the case, since it depends on D-Bus it runs only
>> for GNU/Linux systems.
>
> Thank you for pointing this out. I had wondered about it while
> preparing the patch but could not find a description of what happens
> when notifications are not supported, other than the usual
> "notifications-notify returns an integer ID".
>
> Does notifications-notify return nil in this case? If so, I would like
> to document this. If not, wouldn't this make sense?
Likely yes. It uses with-demoted-errors, which should return nil indeed.
> What is the best way to determine whether notifications are supported by
> the current Emacs instance? I skimmed through (info "(dbus) Top") and
> (info "(elisp) Desktop Notifications"), as well as through the syms of
> dbusbind.c, dbus.el, and notifications.el, but did not find any clear
> answers.
(featurep 'dbusbind)
checks, whether Emacs is configured to use D-Bus. This does not
necessarily tells you that notifications are supported, but it gives you
a strong indication for this.
You could also wrap the call of notifications-notify by dbus-ignore-errors.
A better check would be to ping notifications-service, but this should
be done inside notifications.el, if needed.
> Currently, gnus-notifications.el checks a) directly whether
> notifications-notify is fboundp, and b) indirectly whether
> notifications-notify returns nil. Even without notification support,
> (a) should always be true after loading the library, right? So the
> question is whether (b) is a sufficient condition.
>
> PS Should gnus-notifications.el be extended to support
> w32-notification-notify?
>
> PPS Would it be possible/welcome to provide a common interface for both
> notifications-notify and w32-notification-notify? At the moment it
> sounds like programmers need to choose between the two depending on
> system-type.
In ELPA, there are several notification libraries. alert.el seems to be
the most advanced one, although it doesn't support w32-notification-notify.
> Thanks,
Best regards, Michael.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Proposed changes to gnus-notifications.el
2019-07-21 0:52 Proposed changes to gnus-notifications.el Basil L. Contovounesios
2019-07-21 8:01 ` Michael Albinus
@ 2019-07-21 13:44 ` Lars Ingebrigtsen
1 sibling, 0 replies; 10+ messages in thread
From: Lars Ingebrigtsen @ 2019-07-21 13:44 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: Julien Danjou, emacs-devel
"Basil L. Contovounesios" <contovob@tcd.ie> writes:
> The following patch for gnus-notifications.el enables lexical-binding
> and simplifies and clarifies some of the code and docs. WDYT?
Except for what Michael commented about checking for whether
notifications are supported, it looks good to me.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Proposed changes to gnus-notifications.el
2019-07-21 9:40 ` Basil L. Contovounesios
2019-07-21 10:40 ` Michael Albinus
@ 2019-07-21 14:34 ` Eli Zaretskii
1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2019-07-21 14:34 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: julien, larsi, michael.albinus, emacs-devel
> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Sun, 21 Jul 2019 10:40:03 +0100
> Cc: Julien Danjou <julien@danjou.info>, Lars Ingebrigtsen <larsi@gnus.org>,
> emacs-devel@gnu.org
>
> PPS Would it be possible/welcome to provide a common interface for both
> notifications-notify and w32-notification-notify?
It would definitely be welcome.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Proposed changes to gnus-notifications.el
2019-07-21 10:40 ` Michael Albinus
@ 2019-07-21 15:54 ` Basil L. Contovounesios
2019-07-21 17:16 ` Michael Albinus
2019-07-21 15:55 ` Error handling in notifications-notify (was: Proposed changes to gnus-notifications.el) Basil L. Contovounesios
1 sibling, 1 reply; 10+ messages in thread
From: Basil L. Contovounesios @ 2019-07-21 15:54 UTC (permalink / raw)
To: Michael Albinus; +Cc: Julien Danjou, Lars Ingebrigtsen, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1875 bytes --]
Michael Albinus <michael.albinus@gmx.de> writes:
> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Michael Albinus <michael.albinus@gmx.de> writes:
>>
>>> The patch seems to assume that notifications-notify works
>>> everywhere. That's not the case, since it depends on D-Bus it runs only
>>> for GNU/Linux systems.
>>
>> Thank you for pointing this out. I had wondered about it while
>> preparing the patch but could not find a description of what happens
>> when notifications are not supported, other than the usual
>> "notifications-notify returns an integer ID".
>>
>> Does notifications-notify return nil in this case? If so, I would like
>> to document this. If not, wouldn't this make sense?
>
> Likely yes. It uses with-demoted-errors, which should return nil
> indeed.
In that case I think the proposed patch already behaves as it should,
without changing existing behaviour. See the last few lines of the
function gnus-notifications:
(let* ((photo-file (gnus-notifications-get-photo-file address))
(notification-id (gnus-notifications-notify
(or (car address-components) address)
(mail-fetch-field "Subject")
photo-file)))
(when notification-id
;; Register that we did notify this message.
(push article (cdr group-notifications))
;; Register the notification ID for later actions.
(setf (alist-get notification-id
gnus-notifications-id-to-msg)
(list group article)))
...)
gnus-notifications-notify just calls notifications-notify, and the
resulting ID is not assumed to be non-nil. Is this acceptable? Or did
you mean something else when you said the patch assumes too much about
notifications-notify?
Here's the patch again, with an updated docstring for
gnus-notifications-notify:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Refactor-gnus-notifications.el.patch --]
[-- Type: text/x-diff, Size: 13370 bytes --]
From 81d1fda93f71152dd145500fefdfd0df1a40073f Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sat, 20 Jul 2019 23:37:29 +0100
Subject: [PATCH 1/3] Refactor gnus-notifications.el
For discussion, see the following thread:
https://lists.gnu.org/archive/html/emacs-devel/2019-07/msg00499.html
* lisp/gnus/gnus-notifications.el: Use lexical-binding. Pass
non-nil NOERROR to 'require' instead of wrapping in ignore-errors.
Add image.el as a dependency. Remove redundant :group tags.
(gnus-notifications-use-google-contacts): Default to nil if
google-contacts is not installed.
(gnus-notifications-use-gravatar, gnus-notifications-timeout)
(gnus-notifications-sent, gnus-notifications-id-to-msg): Clarify
docstring.
(gnus-notifications-notify, gnus-notifications-get-photo)
(gnus-notifications-get-photo-file): Simplify.
(gnus-notifications): Simplify to reduce indentation.
Call user option gnus-extract-address-components in place of
mail-extract-address-components.
---
lisp/gnus/gnus-notifications.el | 207 +++++++++++++++-----------------
1 file changed, 95 insertions(+), 112 deletions(-)
diff --git a/lisp/gnus/gnus-notifications.el b/lisp/gnus/gnus-notifications.el
index 3476164583..0dbbd9972e 100644
--- a/lisp/gnus/gnus-notifications.el
+++ b/lisp/gnus/gnus-notifications.el
@@ -1,4 +1,4 @@
-;; gnus-notifications.el -- Send notification on new message in Gnus
+;;; gnus-notifications.el --- Notify of new Gnus messages -*- lexical-binding: t -*-
;; Copyright (C) 2012-2019 Free Software Foundation, Inc.
@@ -24,55 +24,54 @@
;; This implements notifications using `notifications-notify' on new
;; messages received.
-;; Use (add-hook 'gnus-after-getting-new-news-hook 'gnus-notifications)
+;; Use (add-hook 'gnus-after-getting-new-news-hook #'gnus-notifications)
;; to get notifications just after getting the new news.
;;; Code:
-(ignore-errors
- (require 'notifications))
+(require 'notifications)
(require 'gnus-sum)
(require 'gnus-group)
(require 'gnus-int)
(require 'gnus-art)
(require 'gnus-util)
-(ignore-errors
- (require 'google-contacts)) ; Optional
(require 'gnus-fun)
+(require 'image)
(defgroup gnus-notifications nil
- "Send notifications on new message in Gnus."
+ "Send notifications on new messages in Gnus."
:version "24.3"
:group 'gnus)
-(defcustom gnus-notifications-use-google-contacts t
- "Use Google Contacts to retrieve photo."
+(defcustom gnus-notifications-use-google-contacts
+ (and (require 'google-contacts nil t) t)
+ "Whether to retrieve sender avatars from Google Contacts.
+This requires the external package `google-contacts'."
:type 'boolean
- :group 'gnus-notifications)
+ :version "27.1")
(defcustom gnus-notifications-use-gravatar t
- "Use Gravatar to retrieve photo."
- :type 'boolean
- :group 'gnus-notifications)
+ "Whether to retrieve sender avatars from Gravatar."
+ :type 'boolean)
(defcustom gnus-notifications-minimum-level 1
"Minimum group level the message should have to be notified.
Any message in a group that has a greater value than this will
not get notifications."
- :type 'integer
- :group 'gnus-notifications)
+ :type 'integer)
(defcustom gnus-notifications-timeout nil
- "Timeout used for notifications sent via `notifications-notify'."
+ "Timeout used for notifications sent via `notifications-notify'.
+Value is either a duration in milliseconds or nil, which means to
+use the notification server's default timeout."
:type '(choice (const :tag "Server default" nil)
- (integer :tag "Milliseconds"))
- :group 'gnus-notifications)
+ (integer :tag "Milliseconds")))
(defvar gnus-notifications-sent nil
- "Notifications already sent.")
+ "Map group names to lists of sent notification IDs.")
(defvar gnus-notifications-id-to-msg nil
- "Map notifications ids to messages.")
+ "Map notification IDs to messages.")
(defun gnus-notifications-action (id key)
(let ((group-article (assoc id gnus-notifications-id-to-msg)))
@@ -90,57 +89,42 @@ gnus-notifications-action
(gnus-group-update-group group)))))))
(defun gnus-notifications-notify (from subject photo-file)
- "Send a notification about a new mail.
-Return a notification id if any, or t on success."
- (if (fboundp 'notifications-notify)
- (gnus-funcall-no-warning
- 'notifications-notify
- :title from
- :body subject
- :actions '("read" "Read" "mark-read" "Mark As Read")
- :on-action 'gnus-notifications-action
- :app-icon (gnus-funcall-no-warning
- 'image-search-load-path "gnus/gnus.png")
- :image-path photo-file
- :app-name "Gnus"
- :category "email.arrived"
- :timeout gnus-notifications-timeout)
- (message "New message from %s: %s" from subject)
- ;; Don't return an id
- t))
-
-(declare-function gravatar-retrieve-synchronously "gravatar.el"
- (mail-address))
+ "Send a notification about a new mail and return its ID.
+Return nil on failure."
+ (notifications-notify
+ :title from
+ :body subject
+ :actions '("read" "Read" "mark-read" "Mark As Read")
+ :on-action #'gnus-notifications-action
+ :app-icon (image-search-load-path "gnus/gnus.png")
+ :image-path photo-file
+ :app-name "Gnus"
+ :category "email.arrived"
+ :timeout gnus-notifications-timeout))
(defun gnus-notifications-get-photo (mail-address)
- "Get photo for mail address."
- (let ((google-photo (when (and gnus-notifications-use-google-contacts
- (fboundp 'google-contacts-get-photo))
- (ignore-errors
- (gnus-funcall-no-warning
- 'google-contacts-get-photo mail-address)))))
- (if google-photo
- google-photo
- (when gnus-notifications-use-gravatar
- (let ((gravatar (ignore-errors
- (gravatar-retrieve-synchronously mail-address))))
- (if (eq gravatar 'error)
- nil
- (plist-get (cdr gravatar) :data)))))))
+ "Return an avatar for MAIL-ADDRESS.
+Value is either a string of raw image data, or nil on failure."
+ (or (and gnus-notifications-use-google-contacts
+ (fboundp 'google-contacts-get-photo)
+ (ignore-errors
+ (google-contacts-get-photo mail-address)))
+ (let ((gravatar (and gnus-notifications-use-gravatar
+ (ignore-errors
+ (gravatar-retrieve-synchronously mail-address)))))
+ (and (eq (car-safe gravatar) 'image)
+ (image-property gravatar :data)))))
(defun gnus-notifications-get-photo-file (mail-address)
- "Get a temporary file with an image for MAIL-ADDRESS.
-You have to delete the temporary image yourself using
-`delete-image'.
+ "Return a temporary file name containing an image for MAIL-ADDRESS.
+Callers must themselves delete the file; it is not done
+automatically.
-Returns nil if no image found."
- (let ((photo (gnus-notifications-get-photo mail-address)))
+Returns nil if no image is found."
+ (let ((photo (gnus-notifications-get-photo mail-address))
+ (coding-system-for-write 'binary))
(when photo
- (let ((photo-file (make-temp-file "gnus-notifications-photo-"))
- (coding-system-for-write 'binary))
- (with-temp-file photo-file
- (insert photo))
- photo-file))))
+ (make-temp-file "gnus-notifications-photo-" nil nil photo))))
;;;###autoload
(defun gnus-notifications ()
@@ -151,53 +135,52 @@ gnus-notifications
This is typically a function to add in
`gnus-after-getting-new-news-hook'"
- (dolist (entry gnus-newsrc-alist)
- (let ((group (car entry)))
- ;; Check that the group level is less than
- ;; `gnus-notifications-minimum-level' and the group has unread
- ;; messages.
- (when (and (<= (gnus-group-level group) gnus-notifications-minimum-level)
- (let ((unread (gnus-group-unread group)))
- (and (numberp unread)
- (> unread 0))))
- ;; Each group should have an entry in the `gnus-notifications-sent'
- ;; alist. If not, we add one at this time.
- (let ((group-notifications (or (assoc group gnus-notifications-sent)
- ;; Nothing, add one and return it.
- (assoc group
- (add-to-list
- 'gnus-notifications-sent
- (cons group nil))))))
- (dolist (article (gnus-list-of-unread-articles group))
- ;; Check if the article already has been notified
- (unless (memq article (cdr group-notifications))
- (with-current-buffer nntp-server-buffer
- (gnus-request-head article group)
- (article-decode-encoded-words) ; to decode mail addresses, subjects, etc
- (let* ((address-components (mail-extract-address-components
- (or (mail-fetch-field "From") "")))
- (address (cadr address-components)))
- ;; Ignore mails from ourselves
- (unless (and gnus-ignored-from-addresses
- address
- (cond ((functionp gnus-ignored-from-addresses)
- (funcall gnus-ignored-from-addresses address))
- (t (string-match-p
- (gnus-ignored-from-addresses)
- address))))
- (let* ((photo-file (gnus-notifications-get-photo-file address))
- (notification-id (gnus-notifications-notify
- (or (car address-components) address)
- (mail-fetch-field "Subject")
- photo-file)))
- (when notification-id
- ;; Register that we did notify this message
- (setcdr group-notifications (cons article (cdr group-notifications)))
- (unless (eq notification-id t)
- ;; Register the notification id for later actions
- (add-to-list 'gnus-notifications-id-to-msg (list notification-id group article))))
- (when photo-file
- (delete-file photo-file)))))))))))))
+ (pcase-dolist (`(,group . ,_) gnus-newsrc-alist)
+ ;; Check that the group level is less than
+ ;; `gnus-notifications-minimum-level' and the group has unread
+ ;; messages.
+ (when (and (<= (gnus-group-level group) gnus-notifications-minimum-level)
+ (let ((unread (gnus-group-unread group)))
+ (and (numberp unread)
+ (> unread 0))))
+ ;; Each group should have an entry in the `gnus-notifications-sent'
+ ;; alist. If not, we add one at this time.
+ (let ((group-notifications
+ (or (assoc group gnus-notifications-sent)
+ ;; Nothing, add one and return it.
+ (assoc group (push (list group) gnus-notifications-sent)))))
+ (dolist (article (gnus-list-of-unread-articles group))
+ ;; Check if the article has already been notified.
+ (unless (memq article (cdr group-notifications))
+ (with-current-buffer nntp-server-buffer
+ (gnus-request-head article group)
+ ;; To decode mail addresses, subjects, etc.
+ (article-decode-encoded-words)
+ (let* ((address-components
+ (funcall gnus-extract-address-components
+ (or (mail-fetch-field "From") "")))
+ (address (cadr address-components)))
+ ;; Ignore mail from ourselves.
+ (unless (and gnus-ignored-from-addresses
+ (> (length address) 0)
+ (if (functionp gnus-ignored-from-addresses)
+ (funcall gnus-ignored-from-addresses address)
+ (string-match-p (gnus-ignored-from-addresses)
+ address)))
+ (let* ((photo-file (gnus-notifications-get-photo-file address))
+ (notification-id (gnus-notifications-notify
+ (or (car address-components) address)
+ (mail-fetch-field "Subject")
+ photo-file)))
+ (when notification-id
+ ;; Register that we did notify this message.
+ (push article (cdr group-notifications))
+ ;; Register the notification ID for later actions.
+ (setf (alist-get notification-id
+ gnus-notifications-id-to-msg)
+ (list group article)))
+ (when photo-file
+ (delete-file photo-file))))))))))))
(provide 'gnus-notifications)
--
2.20.1
[-- Attachment #3: Type: text/plain, Size: 20 bytes --]
Thanks,
--
Basil
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Error handling in notifications-notify (was: Proposed changes to gnus-notifications.el)
2019-07-21 10:40 ` Michael Albinus
2019-07-21 15:54 ` Basil L. Contovounesios
@ 2019-07-21 15:55 ` Basil L. Contovounesios
2019-07-21 17:21 ` Error handling in notifications-notify Michael Albinus
1 sibling, 1 reply; 10+ messages in thread
From: Basil L. Contovounesios @ 2019-07-21 15:55 UTC (permalink / raw)
To: Michael Albinus; +Cc: Julien Danjou, Lars Ingebrigtsen, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]
Michael Albinus <michael.albinus@gmx.de> writes:
> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> What is the best way to determine whether notifications are supported by
>> the current Emacs instance? I skimmed through (info "(dbus) Top") and
>> (info "(elisp) Desktop Notifications"), as well as through the syms of
>> dbusbind.c, dbus.el, and notifications.el, but did not find any clear
>> answers.
>
> (featurep 'dbusbind)
>
> checks, whether Emacs is configured to use D-Bus. This does not
> necessarily tells you that notifications are supported, but it gives you
> a strong indication for this.
Thanks, I somehow missed this check littered all over the place.
> You could also wrap the call of notifications-notify by dbus-ignore-errors.
>
> A better check would be to ping notifications-service, but this should
> be done inside notifications.el, if needed.
Indeed, these two alternatives sound like the responsibility of
notifications.el, not its users. The patch I proposed does not change
existing behaviour of gnus-notifications.el in this regard.
However, I'm not sure what the best thing to do in notifications-notify
is. So far, it has returned nil for any type of error (unless
debug-on-error is non-nil), thanks to its use of with-demoted-errors.
Would it be too backward-incompatible to change this to
dbus-ignore-errors or pinging notifications-service, as you say?
If so, how's the following docfix instead?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Document-value-of-notifications-notify-on-failure.patch --]
[-- Type: text/x-diff, Size: 3717 bytes --]
From 72e45056209f79d1a944ae7810b831cfc62f9d49 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Sun, 21 Jul 2019 15:09:54 +0100
Subject: [PATCH 2/3] Document value of notifications-notify on failure
* doc/lispref/os.texi (Desktop Notifications):
* lisp/notifications.el (notifications-notify): Document return
value of notifications-notify on failure, e.g. when D-Bus support is
not available.
---
doc/lispref/os.texi | 25 ++++++++++++++-----------
lisp/notifications.el | 11 +++++++----
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index fef954eb7a..bc01f3722f 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -2644,22 +2644,25 @@ Desktop Notifications
Which parameters are accepted by the notification server can be
checked via @code{notifications-get-capabilities}.
-This function returns a notification id, an integer, which can be used
-to manipulate the notification item with
-@code{notifications-close-notification} or the @code{:replaces-id}
-argument of another @code{notifications-notify} call. For example:
+If successful, this function returns a notification ID, an integer,
+which can be used to manipulate the notification item with
+@code{notifications-close-notification} or as the @code{:replaces-id}
+argument of another @code{notifications-notify} call. Otherwise, if
+sending the notification failed, this function returns @code{nil}.
+This may happen when D-Bus or notification support is not available,
+for example.
+
+Here is an example demonstrating the use of action callbacks:
@example
@group
(defun my-on-action-function (id key)
(message "Message %d, key \"%s\" pressed" id key))
- @result{} my-on-action-function
@end group
@group
(defun my-on-close-function (id reason)
(message "Message %d, closed due to \"%s\"" id reason))
- @result{} my-on-close-function
@end group
@group
@@ -2667,15 +2670,15 @@ Desktop Notifications
:title "Title"
:body "This is <b>important</b>."
:actions '("Confirm" "I agree" "Refuse" "I disagree")
- :on-action 'my-on-action-function
- :on-close 'my-on-close-function)
+ :on-action #'my-on-action-function
+ :on-close #'my-on-close-function)
@result{} 22
@end group
@group
-A message window opens on the desktop. Press ``I agree''.
- @result{} Message 22, key "Confirm" pressed
- Message 22, closed due to "dismissed"
+A message window opens on the desktop. Press @samp{I agree}.
+ @print{} Message 22, key "Confirm" pressed
+ @print{} Message 22, closed due to "dismissed"
@end group
@end example
@end defun
diff --git a/lisp/notifications.el b/lisp/notifications.el
index 1d250e2d92..241f127b11 100644
--- a/lisp/notifications.el
+++ b/lisp/notifications.el
@@ -198,10 +198,13 @@ notifications-notify
Which parameters are accepted by the notification server can be
checked via `notifications-get-capabilities'.
-This function returns a notification id, an integer, which can be
-used to manipulate the notification item with
-`notifications-close-notification' or the `:replaces-id' argument
-of another `notifications-notify' call."
+If successful, this function returns a notification ID, an
+integer, which can be used to manipulate the notification item
+with `notifications-close-notification' or as the `:replaces-id'
+argument of another `notifications-notify' call. Otherwise, if
+sending the notification failed, this function returns nil. This
+may happen when D-Bus or notification support is not available,
+for example."
(with-demoted-errors
(let ((bus (or (plist-get params :bus) :session))
(title (plist-get params :title))
--
2.20.1
[-- Attachment #3: Type: text/plain, Size: 930 bytes --]
>> Currently, gnus-notifications.el checks a) directly whether
>> notifications-notify is fboundp, and b) indirectly whether
>> notifications-notify returns nil. Even without notification support,
>> (a) should always be true after loading the library, right? So the
>> question is whether (b) is a sufficient condition.
>>
>> PS Should gnus-notifications.el be extended to support
>> w32-notification-notify?
>>
>> PPS Would it be possible/welcome to provide a common interface for both
>> notifications-notify and w32-notification-notify? At the moment it
>> sounds like programmers need to choose between the two depending on
>> system-type.
>
> In ELPA, there are several notification libraries. alert.el seems to be
> the most advanced one, although it doesn't support w32-notification-notify.
Indeed, and it isn't in GNU ELPA either, though I seem to recall its
inclusion being discussed somewhere.
Thanks,
--
Basil
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Proposed changes to gnus-notifications.el
2019-07-21 15:54 ` Basil L. Contovounesios
@ 2019-07-21 17:16 ` Michael Albinus
0 siblings, 0 replies; 10+ messages in thread
From: Michael Albinus @ 2019-07-21 17:16 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: Julien Danjou, Lars Ingebrigtsen, emacs-devel
"Basil L. Contovounesios" <contovob@tcd.ie> writes:
> In that case I think the proposed patch already behaves as it should,
> without changing existing behaviour. See the last few lines of the
> function gnus-notifications:
>
> (let* ((photo-file (gnus-notifications-get-photo-file address))
> (notification-id (gnus-notifications-notify
> (or (car address-components) address)
> (mail-fetch-field "Subject")
> photo-file)))
> (when notification-id
> ;; Register that we did notify this message.
> (push article (cdr group-notifications))
> ;; Register the notification ID for later actions.
> (setf (alist-get notification-id
> gnus-notifications-id-to-msg)
> (list group article)))
> ...)
>
> gnus-notifications-notify just calls notifications-notify, and the
> resulting ID is not assumed to be non-nil. Is this acceptable? Or did
> you mean something else when you said the patch assumes too much about
> notifications-notify?
Likely, it's ok (I didn't test on non GNU/Linux). Maybe we shall say,
that this works only for GNU/Linux systems with compiled D-Bus support.
> Thanks,
Best regards, Michael.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Error handling in notifications-notify
2019-07-21 15:55 ` Error handling in notifications-notify (was: Proposed changes to gnus-notifications.el) Basil L. Contovounesios
@ 2019-07-21 17:21 ` Michael Albinus
0 siblings, 0 replies; 10+ messages in thread
From: Michael Albinus @ 2019-07-21 17:21 UTC (permalink / raw)
To: Basil L. Contovounesios; +Cc: Julien Danjou, Lars Ingebrigtsen, emacs-devel
"Basil L. Contovounesios" <contovob@tcd.ie> writes:
Hi Basil,
> However, I'm not sure what the best thing to do in notifications-notify
> is. So far, it has returned nil for any type of error (unless
> debug-on-error is non-nil), thanks to its use of with-demoted-errors.
>
> Would it be too backward-incompatible to change this to
> dbus-ignore-errors or pinging notifications-service, as you say?
>
> If so, how's the following docfix instead?
No objection from my side, but maybe Julien wants to comment about?
>> In ELPA, there are several notification libraries. alert.el seems to be
>> the most advanced one, although it doesn't support w32-notification-notify.
>
> Indeed, and it isn't in GNU ELPA either, though I seem to recall its
> inclusion being discussed somewhere.
Perhaps we shall ask John about. For moving it to GNU ELPA, and for
w32-notification-notify support.
> Thanks,
Best regards, Michael.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-07-21 17:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-21 0:52 Proposed changes to gnus-notifications.el Basil L. Contovounesios
2019-07-21 8:01 ` Michael Albinus
2019-07-21 9:40 ` Basil L. Contovounesios
2019-07-21 10:40 ` Michael Albinus
2019-07-21 15:54 ` Basil L. Contovounesios
2019-07-21 17:16 ` Michael Albinus
2019-07-21 15:55 ` Error handling in notifications-notify (was: Proposed changes to gnus-notifications.el) Basil L. Contovounesios
2019-07-21 17:21 ` Error handling in notifications-notify Michael Albinus
2019-07-21 14:34 ` Proposed changes to gnus-notifications.el Eli Zaretskii
2019-07-21 13:44 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).