Re: Gnus and new mail notification

[Philipp Haselwarter]

looks useful!

On Sat, 25 Dec 2010 00:33:12 +0100, Yuri D'Elia <wavexx@users.sf.net> said:

---8<---[snipped 23 lines]---8<---

YD> [1] I know there's already a gnus-notify.el script, but I wasn't
YD> able to come up with a better name.

Please try harder, there are already at least two with that name:

http://www.mail-archive.com/gnu-emacs-sources@gnu.org/msg01615.html
http://www.emacswiki.org/emacs/gnus-notify.el

and you do something different (*desktop notification* instead of *mode
line* stuff)!


While I was testing you lib, I discovered that
`gnus-newsrc-alist'-elements `g' can contain 3 different type elements
at (nth 2 g) (ie `gnus-info-read'):
- `nil'
- a list, like '(1 . 140)
- a list of lists, like '((1 . 9))

Obviously, a cdar on a list (eg. (cdar '(1 . 140))) is invalid, which
can result in problems in `gnus-notify-check'.

So here's a patch to work around that data inconsistency:

--- a/gnus-notify.el    2010-12-25 04:28:54.600753814 +0100
+++ b/gnus-notify.el    2010-12-25 04:29:07.473993965 +0100
@@ -110,7 +110,10 @@
   (interactive)
   (let ( (updated-groups '()) )
     (dolist (g gnus-newsrc-alist)
-      (let ( (read (cdar (gnus-info-read g))) )
+      (let ( (read (or
+                    (and (listp (car (gnus-info-read g)))
+                         (cdar (gnus-info-read g)))
+                    (cdr (gnus-info-read g)))) )
        (when read
          (let* ( (name (gnus-info-group g))
                  (unread (gnus-group-unread (car g)))

Furthermore, I saw that you directly added hooks at the end; referring
to the "Emacs Lisp Coding Conventions" at

http://www.gnu.org/software/emacs/elisp/html_node/Coding-Conventions.html
,----
| Simply loading a package should not change Emacs's editing
| behavior. Include a command or commands to enable and disable the
| feature, or to invoke it.
| 
| This convention is mandatory for any file that includes custom
| definitions. If fixing such a file to follow this convention requires an
| incompatible change, go ahead and make the incompatible change; don't
| postpone it. 
`----


You can simply wrap a function around the adding/removal of the hooks:

--- b/gnus-notify.el    2010-12-25 04:29:07.473993965 +0100
+++ c/gnus-notify.el    2010-12-25 05:13:46.467275657 +0100
@@ -135,7 +135,17 @@
 
 
 ;; Hooks into gnus
-(add-hook 'gnus-after-getting-new-news-hook 'gnus-notify-check)
-(add-hook 'gnus-started-hook 'gnus-notify-check)
+;;;###autoload
+(defun gnus-notify-on ()
+  "Add hooks for `gnus-notify-check'"
+  (interactive)
+  (add-hook 'gnus-after-getting-new-news-hook 'gnus-notify-check)
+  (add-hook 'gnus-started-hook 'gnus-notify-check))
+
+(defun gnus-notify-off ()
+  "Remove hooks for `gnus-notify-check'"
+  (interactive)
+  (remove-hook 'gnus-after-getting-new-news-hook 'gnus-notify-check)
+  (remove-hook 'gnus-started-hook 'gnus-notify-check))
 
 (provide 'gnus-notify)


And I noticed that you put spaces around the enclosing parentheses of
your let-assignments, which I found a bit weird, so here's a whitespace
patch :p

--- c/gnus-notify.el    2010-12-25 04:29:14.390610600 +0100
+++ d/gnus-notify.el    2010-12-25 04:35:41.004238410 +0100
@@ -108,18 +108,18 @@
 
 (defun gnus-notify-check (&rest ignored)
   (interactive)
-  (let ( (updated-groups '()) )
+  (let ((updated-groups '()))
     (dolist (g gnus-newsrc-alist)
-      (let ( (read (or
+      (let ((read (or
                     (and (listp (car (gnus-info-read g)))
                          (cdar (gnus-info-read g)))
-                    (cdr (gnus-info-read g)))) )
+                    (cdr (gnus-info-read g)))))
        (when read
-         (let* ( (name (gnus-info-group g))
+         (let* ((name (gnus-info-group g))
                  (unread (gnus-group-unread (car g)))
                  (count (+ read unread))
                  (old-count (cdr (assoc name gnus-notify-counts)))
-                 (notify (gnus-group-find-parameter name 'group-notify)) )
+                 (notify (gnus-group-find-parameter name 'group-notify)))
            (when (or
                    (and (eq gnus-notify-mode 'gnus-notify-all-except) (not 
notify))
                    (and (eq gnus-notify-mode 'gnus-notify-explicit) notify))

(don't take this one too seriously ;)


How about making it a global minor mode?


-- 
Philipp Haselwarter