Re: [PATCH] Add variable wl-biff-check-idle-delay, because with wl-biff-use-idle-timer Wanderlust checks mail only once

On Mon, 12 Dec 2016 09:42:27 +0100,
Kazuhiro Ito wrote:
> 
> > > > I wrote the patch to fix the below two problems.  But because I don't
> > > > use wl-biff and it is very unstable in my environment, the patch is
> > > > not much tested.
> > > 
> > > Thank you! I will try the patch and report any problems.
> > 
> > The patch seems to work well after adding an "(unwind-protect ...)":
> > 
> 
> I can't test it much.  But it seems not to have bad effects, so I
> committed it into the repository with minor fix.
> Please test.

Unfortunately it appears to have problems after all. They did not show
up before, but now that I changed my config a little and moved to the
latest WL it seems there are two issues:


1. With the patch, WL enters a tight loop when wl-biff-use-idle-timer
is set. The call to (wl-biff-start) immediately calls
wl-biff-event-handler again, because the idle time exceeds
wl-biff-check-interval. The patch does set a second timer for when
Emacs remains idle, but that handler is never called when Emacs
becomes active again.

End result is that WL starts checking for mail in a tight loop without
regard to wl-biff-check-interval or wl-biff-check-delay.

Below is a revised patch. The only simple and robust solution I found
is to simply not rely on the idle timer at all. Mainly because there
is no way to know when Emacs becomes active again without extra state
tracking.

Instead the patch uses wl-check-idle-delay to check whether Emacs has
been idle for long enough to check mail. If nil, Emacs will check mail
whether it is idle or not. The wl-biff-use-idle-timer var is still
there, but I think it should be deprecated/removed as it seems
confusing and not very useful.

In summary, with this patch:

- wl-biff-check-interval controls the frequency for checking mail
- wl-biff-check-idle-delay controls how long Emacs needs to be idle
  before checking mail (to avoid interrupting the user)
- wl-biff-use-idle-timer is still supported, but only triggers once,
  wl-biff-check-interval seconds after Emacs becomes idle


2. For some reason there now appears to be a large memory leak in
wl-check-folders. I don't know why, yet.


Regards
Pascal


diff -U2 a/wanderlust-20161217.2220/wl-util.el wanderlust-20161227.2220/wl-util.el
--- a/wanderlust-20161217.2220/wl-util.el	2017-01-03 15:55:17.178545207 +0100
+++ wanderlust-20161227.2220/wl-util.el	2017-01-03 22:15:12.626907962 +0100
@@ -777,19 +777,20 @@
 	  (put 'wl-biff 'timers
 	       (if wl-biff-use-idle-timer
-		   (list (run-with-idle-timer
-			  wl-biff-check-interval nil 'wl-biff-event-handler))
-		 (list (run-at-time wl-biff-check-interval nil
-				    'wl-biff-launch-handler)))))
+                   (list (run-with-idle-timer
+                          wl-biff-check-interval nil 'wl-biff-event-handler))
+                 (list (run-at-time wl-biff-check-interval nil
+				    'wl-biff-event-handler)))))
       (message "No folder is specified for biff")))
 
-
-  (defun wl-biff-launch-handler ()
-    (put 'wl-biff 'timers
-	 (run-with-idle-timer wl-biff-check-delay nil 'wl-biff-event-handler)))
-
-  (defun wl-biff-event-handler ()
-      ;; PAKURing from FSF:time.el
-      (unwind-protect
-          (progn
+  (defun wl-biff-event-handler ()
+    (unwind-protect
+        (let ((idle (and (fboundp 'current-idle-time) ;; Available on
+                                                      ;; Emacs 22 or
+                                                      ;; later.
+                         (current-idle-time))))
+          (when (or (null wl-biff-check-idle-delay)
+                    (and idle
+                         (> (float-time idle) wl-biff-check-idle-delay)))
             (condition-case signal
                 (wl-biff-check-folders)
@@ -797,20 +798,7 @@
                (message "wl-biff: %s (%s)" (car signal) (cdr signal))))
             ;; Do redisplay right now, if no input pending.
-            (sit-for 0))
-        (wl-biff-stop)
-        (wl-biff-start)
-        (let ((idle (and wl-biff-use-idle-timer
-                         ;; Available on Emacs 22 or later.
-                         (fboundp 'current-idle-time)
-                         (current-idle-time))))
-          (when idle
-            ;; Run idle timer for the case Emacs keeps idle.
-            (put 'wl-biff 'timers
-                 (cons (run-with-idle-timer
-                        (+ wl-biff-check-interval (float-time idle))
-                        nil 'wl-biff-event-handler)
-                       (get 'wl-biff 'timers))))))
-      )))
-
+            (sit-for 0)))
+      (wl-biff-stop)
+      (wl-biff-start)))))
 
 (defsubst wl-biff-notify (new-mails notify-minibuf)
diff -U2 a/wanderlust-20161217.2220/wl-vars.el wanderlust-20161227.2220/wl-vars.el
--- a/wanderlust-20161217.2220/wl-vars.el	2017-01-03 15:55:17.178545207 +0100
+++ wanderlust-20161227.2220/wl-vars.el	2017-01-03 21:54:05.635819188 +0100
@@ -2252,12 +2252,16 @@
   :group 'wl-setting)
 
-(defcustom wl-biff-check-delay 0
-  "After interval specified by `wl-biff-check-interval', automatically checking new mail will start when Emacs keeps idle longer than specified seconds by this varaible.
-It has no effect on XEmacs or for the case which `wl-biff-use-idle-timer' is non-nil."
+(defcustom wl-biff-check-idle-delay nil
+  "Number of seconds that Emacs has to be idle before checking
+for new mail. If nil, Emacs will check for new mail every
+`wl-biff-check-interval' seconds, regardless of whether Emacs is
+idle or not."
   :type 'number
   :group 'wl-setting)
 
 (defcustom wl-biff-use-idle-timer nil
-  "Non-nil means that Emacs will not use normal timer for wl-biff."
+  "Non-nil means that Emacs will use the idle timer to check for
+new mail. The idle timer fires only once after Emacs becomes
+idle, after `wl-biff-check-interval' seconds."
   :type 'boolean
   :group 'wl-setting)