* 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 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
* 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
* 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: 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
* 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 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
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 external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.