all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* multiple appointments bug
@ 2011-04-11  9:13 Ivan Kanis
  2011-04-12  8:03 ` Glenn Morris
  0 siblings, 1 reply; 2+ messages in thread
From: Ivan Kanis @ 2011-04-11  9:13 UTC (permalink / raw)
  To: Glen Norris; +Cc: emacs devel

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

Hi Glen,

There was a logic bug in the code I sent you. The symptom is a message
"App't in nil" in the modeline and some other nastiness. The last patch
fixes this problem. If people would like to try the new code I have
included the appt.el file. It's been tested on emacs 23.3.

Take care,
-- 
Ivan Kanis
http://kanis.fr

A great teacher is one who realizes that he himself is also a student
and whose goal is not dictate the answers, but to stimulate his
students creativity enough so that they go out and find the answers
themselves.
    -- Herbie Hancock 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-remove-full-check-and-mode-line-only.patch --]
[-- Type: text/x-diff, Size: 3213 bytes --]

From c4cfef7043ff51c48834d55cf0b163fb95f3f5d5 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Thu, 24 Mar 2011 18:32:23 +0100
Subject: [PATCH 01/14] remove full-check and mode-line-only

These two variables just make the code more complex than it should
be. The main interest is to have the display refresh at n mod
time. Remove appt-now-displayed since it's not used anymore.
---
 appt.el |   23 +++++------------------
 1 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/appt.el b/appt.el
index 4c318dc..e2b1158 100644
--- a/appt.el
+++ b/appt.el
@@ -217,9 +217,6 @@ Only used if `appt-display-mode-line' is non-nil.")
   "Time of day (mins since midnight) at which we last checked appointments.
 A nil value forces the diary file to be (re-)checked for appointments.")
 
-(defvar appt-now-displayed nil
-  "Non-nil when we have started notifying about a appointment that is near.")
-
 (defvar appt-display-count nil
   "Internal variable used to count number of consecutive reminders.")
 
@@ -314,17 +311,7 @@ displayed in a window:
   (let* ((min-to-app -1)
          (prev-appt-mode-string appt-mode-string)
          (prev-appt-display-count (or appt-display-count 0))
-         ;; Non-nil means do a full check for pending appointments and
-         ;; display in whatever ways the user has selected.  When no
-         ;; appointment is being displayed, we always do a full check.
-         (full-check
-          (or (not appt-now-displayed)
-              ;; This is true every appt-display-interval minutes.
-              (zerop (mod prev-appt-display-count appt-display-interval))))
-         ;; Non-nil means only update the interval displayed in the mode line.
-         (mode-line-only (unless full-check appt-now-displayed))
          now cur-comp-time appt-comp-time appt-warn-time)
-    (when (or full-check mode-line-only)
       (save-excursion
         ;; Convert current time to minutes after midnight (12.01am = 1).
         (setq now (decode-time)
@@ -402,10 +389,10 @@ displayed in a window:
           ;; appt-message-warning time.
           (when (and (<= min-to-app appt-warn-time)
                      (>= min-to-app 0))
-            (setq appt-now-displayed t
-                  appt-display-count (1+ prev-appt-display-count))
-            (unless mode-line-only
-              (appt-display-message (cadr (car appt-time-msg-list))
+            (setq appt-display-count (1+ prev-appt-display-count))
+            ;; This is true every appt-display-interval minutes.
+            (if (zerop (mod prev-appt-display-count appt-display-interval))
+                (appt-display-message (cadr (car appt-time-msg-list))
                                     min-to-app))
             (when appt-display-mode-line
               (setq appt-mode-string
@@ -426,7 +413,7 @@ displayed in a window:
                (force-mode-line-update t)
                ;; If the string now has a notification, redisplay right now.
                (if appt-mode-string
-                   (sit-for 0))))))))
+                   (sit-for 0)))))))
 
 (defun appt-disp-window (min-to-app new-time appt-msg)
   "Display appointment due in MIN-TO-APP (a string) minutes.
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-remove-prev-appt-display-count.patch --]
[-- Type: text/x-diff, Size: 2381 bytes --]

From bacf5ac866a12b6527a6144bd53e96781b563367 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Thu, 24 Mar 2011 18:39:43 +0100
Subject: [PATCH 02/14] remove prev-appt-display-count

using only global appt-display-count should be sufficient
---
 appt.el |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/appt.el b/appt.el
index e2b1158..8a2919e 100644
--- a/appt.el
+++ b/appt.el
@@ -217,7 +217,7 @@ Only used if `appt-display-mode-line' is non-nil.")
   "Time of day (mins since midnight) at which we last checked appointments.
 A nil value forces the diary file to be (re-)checked for appointments.")
 
-(defvar appt-display-count nil
+(defvar appt-display-count 0
   "Internal variable used to count number of consecutive reminders.")
 
 (defvar appt-timer nil
@@ -310,7 +310,6 @@ displayed in a window:
   (interactive "P")                     ; so people can force updates
   (let* ((min-to-app -1)
          (prev-appt-mode-string appt-mode-string)
-         (prev-appt-display-count (or appt-display-count 0))
          now cur-comp-time appt-comp-time appt-warn-time)
       (save-excursion
         ;; Convert current time to minutes after midnight (12.01am = 1).
@@ -389,11 +388,11 @@ displayed in a window:
           ;; appt-message-warning time.
           (when (and (<= min-to-app appt-warn-time)
                      (>= min-to-app 0))
-            (setq appt-display-count (1+ prev-appt-display-count))
             ;; This is true every appt-display-interval minutes.
             (if (zerop (mod prev-appt-display-count appt-display-interval))
                 (appt-display-message (cadr (car appt-time-msg-list))
                                     min-to-app))
+            (setq appt-display-count (1+ appt-display-count))
             (when appt-display-mode-line
               (setq appt-mode-string
                     (concat " " (propertize
@@ -404,7 +403,7 @@ displayed in a window:
             ;; appointment on the next cycle.
             (if (zerop min-to-app)
                 (setq appt-time-msg-list (cdr appt-time-msg-list)
-                      appt-display-count nil))))
+                      appt-display-count 0))))
         ;; If we have changed the mode line string, redisplay all mode lines.
         (and appt-display-mode-line
              (not (string-equal appt-mode-string
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-plop-now-and-cur-comp-time-in-the-let-statement.patch --]
[-- Type: text/x-diff, Size: 1251 bytes --]

From 0da2c2a482b71a119326490d22184e933c19629e Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Thu, 24 Mar 2011 18:42:52 +0100
Subject: [PATCH 03/14] plop now and cur-comp-time in the let statement

---
 appt.el |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/appt.el b/appt.el
index 8a2919e..3f8493b 100644
--- a/appt.el
+++ b/appt.el
@@ -310,11 +310,11 @@ displayed in a window:
   (interactive "P")                     ; so people can force updates
   (let* ((min-to-app -1)
          (prev-appt-mode-string appt-mode-string)
-         now cur-comp-time appt-comp-time appt-warn-time)
+         ;; Convert current time to minutes after midnight (12.01am = 1).
+         (now (decode-time))
+         (cur-comp-time (+ (* 60 (nth 2 now)) (nth 1 now)))
+         appt-comp-time appt-warn-time)
       (save-excursion
-        ;; Convert current time to minutes after midnight (12.01am = 1).
-        (setq now (decode-time)
-              cur-comp-time (+ (* 60 (nth 2 now)) (nth 1 now)))
         ;; At first check in any day, update appointments to today's list.
         (if (or force                      ; eg initialize, diary save
                 (null appt-prev-comp-time) ; first check
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-move-diary-update-to-its-own-function-appt-check-dia.patch --]
[-- Type: text/x-diff, Size: 5519 bytes --]

From 7353db6f568d0b32d2aaf72d58160e62a830ba25 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Thu, 24 Mar 2011 18:53:37 +0100
Subject: [PATCH 04/14] move diary update to its own function appt-check-diary

---
 appt.el |   84 +++++++++++++++++++++++++++++++++------------------------------
 1 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/appt.el b/appt.el
index 3f8493b..9531797 100644
--- a/appt.el
+++ b/appt.el
@@ -315,46 +315,7 @@ displayed in a window:
          (cur-comp-time (+ (* 60 (nth 2 now)) (nth 1 now)))
          appt-comp-time appt-warn-time)
       (save-excursion
-        ;; At first check in any day, update appointments to today's list.
-        (if (or force                      ; eg initialize, diary save
-                (null appt-prev-comp-time) ; first check
-                (< cur-comp-time appt-prev-comp-time)) ; new day
-            (ignore-errors
-              (if appt-display-diary
-                  (let ((diary-hook
-                         (if (assoc 'appt-make-list diary-hook)
-                             diary-hook
-                           (cons 'appt-make-list diary-hook))))
-                    (diary))
-                (let* ((diary-display-function 'appt-make-list)
-                       (d-buff (find-buffer-visiting diary-file))
-                       (selective
-                        (if d-buff    ; diary buffer exists
-                            (with-current-buffer d-buff
-                              diary-selective-display)))
-                       d-buff2)
-                  ;; Not displaying the diary, so we can ignore
-                  ;; diary-number-of-entries.  Since appt.el only
-                  ;; works on a daily basis, no need for more entries.
-                  ;; FIXME why not using diary-list-entries with
-                  ;; non-nil LIST-ONLY?
-                  (diary 1)
-                  ;; If the diary buffer existed before this command,
-                  ;; restore its display state.  Otherwise, kill it.
-                  (and (setq d-buff2 (find-buffer-visiting diary-file))
-                       (if d-buff
-                           (or selective
-                               (with-current-buffer d-buff2
-                                 (if diary-selective-display
-                                     ;; diary-show-all-entries displays
-                                     ;; the diary buffer.
-                                     (diary-unhide-everything))))
-                         ;; FIXME does not kill any included diary files.
-                         ;; The real issue is that (diary) should not
-                         ;; have the side effect of visiting all the
-                         ;; diary files.  It is not really appt.el's job to
-                         ;; clean up this mess...
-                         (kill-buffer d-buff2)))))))
+        (appt-check-diary force cur-comp-time)
         (setq appt-prev-comp-time cur-comp-time
               appt-mode-string nil
               appt-display-count nil)
@@ -414,6 +375,49 @@ displayed in a window:
                (if appt-mode-string
                    (sit-for 0)))))))
 
+(defun appt-check-diary (force cur-comp-time)
+  "Update appointments to today's list."
+  ;; At first check in any day
+  (if (or force                      ; eg initialize, diary save
+          (null appt-prev-comp-time) ; first check
+          (< cur-comp-time appt-prev-comp-time)) ; new day
+      (ignore-errors
+        (if appt-display-diary
+            (let ((diary-hook
+                   (if (assoc 'appt-make-list diary-hook)
+                       diary-hook
+                     (cons 'appt-make-list diary-hook))))
+              (diary))
+          (let* ((diary-display-function 'appt-make-list)
+                 (d-buff (find-buffer-visiting diary-file))
+                 (selective
+                  (if d-buff    ; diary buffer exists
+                      (with-current-buffer d-buff
+                        diary-selective-display)))
+                 d-buff2)
+            ;; Not displaying the diary, so we can ignore
+            ;; diary-number-of-entries.  Since appt.el only
+            ;; works on a daily basis, no need for more entries.
+            ;; FIXME why not using diary-list-entries with
+            ;; non-nil LIST-ONLY?
+            (diary 1)
+            ;; If the diary buffer existed before this command,
+            ;; restore its display state.  Otherwise, kill it.
+            (and (setq d-buff2 (find-buffer-visiting diary-file))
+                 (if d-buff
+                     (or selective
+                         (with-current-buffer d-buff2
+                           (if diary-selective-display
+                               ;; diary-show-all-entries displays
+                               ;; the diary buffer.
+                               (diary-unhide-everything))))
+                   ;; FIXME does not kill any included diary files.
+                   ;; The real issue is that (diary) should not
+                   ;; have the side effect of visiting all the
+                   ;; diary files.  It is not really appt.el's job to
+                   ;; clean up this mess...
+                   (kill-buffer d-buff2)))))))
+
 (defun appt-disp-window (min-to-app new-time appt-msg)
   "Display appointment due in MIN-TO-APP (a string) minutes.
 NEW-TIME is a string giving the date.  Displays the appointment
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0005-appt-issue-message-is-obsolete-so-don-t-use-it-anymo.patch --]
[-- Type: text/x-diff, Size: 925 bytes --]

From bac7910464bf27cf63c541f85f3fd3ee6cf30e38 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Thu, 24 Mar 2011 18:56:14 +0100
Subject: [PATCH 05/14] appt-issue-message is obsolete so don't use it anymore

---
 appt.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/appt.el b/appt.el
index 9531797..8cb011f 100644
--- a/appt.el
+++ b/appt.el
@@ -322,7 +322,7 @@ displayed in a window:
         ;; If there are entries in the list, and the user wants a
         ;; message issued, get the first time off of the list and
         ;; calculate the number of minutes until the appointment.
-        (when (and appt-issue-message appt-time-msg-list)
+        (when appt-time-msg-list
           (setq appt-comp-time (caar (car appt-time-msg-list))
                 appt-warn-time (or (nth 3 (car appt-time-msg-list))
                                    appt-message-warning-time)
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #7: 0006-don-t-initialize-min-to-app-to-1-there-is-no-good-re.patch --]
[-- Type: text/x-diff, Size: 1120 bytes --]

From 8c909e82ab3dc93fc0302a92985fd633d3f0e5b2 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Thu, 24 Mar 2011 18:59:24 +0100
Subject: [PATCH 06/14] don't initialize min-to-app to -1, there is no good reason

---
 appt.el |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/appt.el b/appt.el
index 8cb011f..f6ff5c1 100644
--- a/appt.el
+++ b/appt.el
@@ -308,12 +308,11 @@ displayed in a window:
 `appt-delete-window-function'
         Function called to remove appointment window and buffer."
   (interactive "P")                     ; so people can force updates
-  (let* ((min-to-app -1)
-         (prev-appt-mode-string appt-mode-string)
+  (let* ((prev-appt-mode-string appt-mode-string)
          ;; Convert current time to minutes after midnight (12.01am = 1).
          (now (decode-time))
          (cur-comp-time (+ (* 60 (nth 2 now)) (nth 1 now)))
-         appt-comp-time appt-warn-time)
+         appt-comp-time appt-warn-time min-to-app)
       (save-excursion
         (appt-check-diary force cur-comp-time)
         (setq appt-prev-comp-time cur-comp-time
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #8: 0007-move-appt-prev-comp-time-to-the-bottom-of-appt-check.patch --]
[-- Type: text/x-diff, Size: 1380 bytes --]

From 5df9dfd98fce372b89bbb9f77462d93b745cee84 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Thu, 24 Mar 2011 19:05:10 +0100
Subject: [PATCH 07/14] move appt-prev-comp-time to the bottom of appt-check-diary

This variable is referenced only in this function so it makes sense to
put it there. I have also fixed that function as it was missing a
parentheses.
---
 appt.el |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/appt.el b/appt.el
index f6ff5c1..51cc5e1 100644
--- a/appt.el
+++ b/appt.el
@@ -315,8 +315,7 @@ displayed in a window:
          appt-comp-time appt-warn-time min-to-app)
       (save-excursion
         (appt-check-diary force cur-comp-time)
-        (setq appt-prev-comp-time cur-comp-time
-              appt-mode-string nil
+        (setq appt-mode-string nil
               appt-display-count nil)
         ;; If there are entries in the list, and the user wants a
         ;; message issued, get the first time off of the list and
@@ -416,6 +415,7 @@ displayed in a window:
                    ;; diary files.  It is not really appt.el's job to
                    ;; clean up this mess...
                    (kill-buffer d-buff2)))))))
+  (setq appt-prev-comp-time cur-comp-time))
 
 (defun appt-disp-window (min-to-app new-time appt-msg)
   "Display appointment due in MIN-TO-APP (a string) minutes.
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #9: 0008-always-refresh-the-mode-line.patch --]
[-- Type: text/x-diff, Size: 1939 bytes --]

From d17b6cfd2b6369afe5e4f6032787d871492c59a0 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Thu, 24 Mar 2011 19:18:02 +0100
Subject: [PATCH 08/14] always refresh the mode line

The operation is not expensive every 60s. Don't pass t to
force-mode-line-update since we are only updating the mode
line. Remove prev-appt-mode-string since it's not needed anymore.
---
 appt.el |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/appt.el b/appt.el
index 51cc5e1..a940831 100644
--- a/appt.el
+++ b/appt.el
@@ -308,9 +308,8 @@ displayed in a window:
 `appt-delete-window-function'
         Function called to remove appointment window and buffer."
   (interactive "P")                     ; so people can force updates
-  (let* ((prev-appt-mode-string appt-mode-string)
+  (let* ((now (decode-time))
          ;; Convert current time to minutes after midnight (12.01am = 1).
-         (now (decode-time))
          (cur-comp-time (+ (* 60 (nth 2 now)) (nth 1 now)))
          appt-comp-time appt-warn-time min-to-app)
       (save-excursion
@@ -363,15 +362,9 @@ displayed in a window:
             (if (zerop min-to-app)
                 (setq appt-time-msg-list (cdr appt-time-msg-list)
                       appt-display-count 0))))
-        ;; If we have changed the mode line string, redisplay all mode lines.
-        (and appt-display-mode-line
-             (not (string-equal appt-mode-string
-                                prev-appt-mode-string))
-             (progn
-               (force-mode-line-update t)
-               ;; If the string now has a notification, redisplay right now.
-               (if appt-mode-string
-                   (sit-for 0)))))))
+        ;; Redisplay all mode lines.
+        (when appt-display-mode-line
+          (force-mode-line-update)))))
 
 (defun appt-check-diary (force cur-comp-time)
   "Update appointments to today's list."
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #10: 0009-fix-stray-prev-appt-display-count-don-t-set-app-disp.patch --]
[-- Type: text/x-diff, Size: 1425 bytes --]

From 247dcc5e0bb0414b72feac1b4012db18f334b8da Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Thu, 24 Mar 2011 19:24:13 +0100
Subject: [PATCH 09/14] fix stray prev-appt-display-count, don't set app-display-count to nil

---
 appt.el |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/appt.el b/appt.el
index a940831..48184ea 100644
--- a/appt.el
+++ b/appt.el
@@ -314,8 +314,7 @@ displayed in a window:
          appt-comp-time appt-warn-time min-to-app)
       (save-excursion
         (appt-check-diary force cur-comp-time)
-        (setq appt-mode-string nil
-              appt-display-count nil)
+        (setq appt-mode-string nil)
         ;; If there are entries in the list, and the user wants a
         ;; message issued, get the first time off of the list and
         ;; calculate the number of minutes until the appointment.
@@ -347,7 +346,7 @@ displayed in a window:
           (when (and (<= min-to-app appt-warn-time)
                      (>= min-to-app 0))
             ;; This is true every appt-display-interval minutes.
-            (if (zerop (mod prev-appt-display-count appt-display-interval))
+            (if (zerop (mod appt-display-count appt-display-interval))
                 (appt-display-message (cadr (car appt-time-msg-list))
                                     min-to-app))
             (setq appt-display-count (1+ appt-display-count))
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #11: 0010-precede-all-local-variable-in-appt-check-with-ac.patch --]
[-- Type: text/x-diff, Size: 4840 bytes --]

From 0d6589584bc8059f3de85bdf5c2be6e8583c56c0 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Sat, 26 Mar 2011 15:55:04 +0100
Subject: [PATCH 10/14] precede all local variable in appt-check with ac-

This helps distinguishing local variables from global variables. There
is no code change, just substitution of variables name.
---
 appt.el |   38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/appt.el b/appt.el
index 48184ea..17473d2 100644
--- a/appt.el
+++ b/appt.el
@@ -308,57 +308,57 @@ displayed in a window:
 `appt-delete-window-function'
         Function called to remove appointment window and buffer."
   (interactive "P")                     ; so people can force updates
-  (let* ((now (decode-time))
+  (let* ((ac-now (decode-time))
          ;; Convert current time to minutes after midnight (12.01am = 1).
-         (cur-comp-time (+ (* 60 (nth 2 now)) (nth 1 now)))
-         appt-comp-time appt-warn-time min-to-app)
+         (ac-cur-time (+ (* 60 (nth 2 ac-now)) (nth 1 ac-now)))
+         ac-appt-time ac-warn-time ac-min-to-app)
       (save-excursion
-        (appt-check-diary force cur-comp-time)
+        (appt-check-diary force ac-cur-time)
         (setq appt-mode-string nil)
         ;; If there are entries in the list, and the user wants a
         ;; message issued, get the first time off of the list and
         ;; calculate the number of minutes until the appointment.
         (when appt-time-msg-list
-          (setq appt-comp-time (caar (car appt-time-msg-list))
-                appt-warn-time (or (nth 3 (car appt-time-msg-list))
+          (setq ac-appt-time (caar (car appt-time-msg-list))
+                ac-warn-time (or (nth 3 (car appt-time-msg-list))
                                    appt-message-warning-time)
-                min-to-app (- appt-comp-time cur-comp-time))
+                ac-min-to-app (- ac-appt-time ac-cur-time))
           (while (and appt-time-msg-list
-                      (< appt-comp-time cur-comp-time))
+                      (< ac-appt-time ac-cur-time))
             (setq appt-time-msg-list (cdr appt-time-msg-list))
             (if appt-time-msg-list
-                (setq appt-comp-time (caar (car appt-time-msg-list)))))
+                (setq ac-appt-time (caar (car appt-time-msg-list)))))
           ;; If we have an appointment between midnight and
-          ;; `appt-warn-time' minutes after midnight, we
+          ;; `ac-warn-time' minutes after midnight, we
           ;; must begin to issue a message before midnight.  Midnight
           ;; is considered 0 minutes and 11:59pm is 1439
           ;; minutes.  Therefore we must recalculate the minutes to
           ;; appointment variable.  It is equal to the number of
           ;; minutes before midnight plus the number of minutes after
           ;; midnight our appointment is.
-          (if (and (< appt-comp-time appt-warn-time)
-                   (> (+ cur-comp-time appt-warn-time)
+          (if (and (< ac-appt-time ac-warn-time)
+                   (> (+ ac-cur-time ac-warn-time)
                       appt-max-time))
-              (setq min-to-app (+ (- (1+ appt-max-time) cur-comp-time)
-                                  appt-comp-time)))
+              (setq ac-min-to-app (+ (- (1+ appt-max-time) ac-cur-time)
+                                  ac-appt-time)))
           ;; Issue warning if the appointment time is within
           ;; appt-message-warning time.
-          (when (and (<= min-to-app appt-warn-time)
-                     (>= min-to-app 0))
+          (when (and (<= ac-min-to-app ac-warn-time)
+                     (>= ac-min-to-app 0))
             ;; This is true every appt-display-interval minutes.
             (if (zerop (mod appt-display-count appt-display-interval))
                 (appt-display-message (cadr (car appt-time-msg-list))
-                                    min-to-app))
+                                    ac-min-to-app))
             (setq appt-display-count (1+ appt-display-count))
             (when appt-display-mode-line
               (setq appt-mode-string
                     (concat " " (propertize
-                                 (format "App't in %s min." min-to-app)
+                                 (format "App't in %s min." ac-min-to-app)
                                  'face 'mode-line-emphasis))))
             ;; When an appointment is reached, delete it from the
             ;; list.  Reset the count to 0 in case we display another
             ;; appointment on the next cycle.
-            (if (zerop min-to-app)
+            (if (zerop ac-min-to-app)
                 (setq appt-time-msg-list (cdr appt-time-msg-list)
                       appt-display-count 0))))
         ;; Redisplay all mode lines.
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #12: 0011-copy-appt-time-msg-list-in-a-local-variabl-ac-appt-l.patch --]
[-- Type: text/x-diff, Size: 4179 bytes --]

From 0d204807d8c7866f661d65810d32c8ca9e4d5f00 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Sat, 26 Mar 2011 16:46:35 +0100
Subject: [PATCH 11/14] copy appt-time-msg-list in a local variabl ac-appt-list

This is in preparation to using this variable do handle multiple
appointment. Clean up obscure code at the beginning of the
function. The strategy is to remove all appointment prior to the
current time. We then setup all necessary local variable in one
setq. There is no need to remove appointment from the list at the end
of the routine since that's taken care of at the beginning.
---
 appt.el |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/appt.el b/appt.el
index 17473d2..9bae717 100644
--- a/appt.el
+++ b/appt.el
@@ -311,23 +311,25 @@ displayed in a window:
   (let* ((ac-now (decode-time))
          ;; Convert current time to minutes after midnight (12.01am = 1).
          (ac-cur-time (+ (* 60 (nth 2 ac-now)) (nth 1 ac-now)))
+         (ac-appt-list appt-time-msg-list)
          ac-appt-time ac-warn-time ac-min-to-app)
       (save-excursion
         (appt-check-diary force ac-cur-time)
-        (setq appt-mode-string nil)
+        ;; Discard appointments previous to current time
+        (while (and ac-appt-list
+                    (setq ac-appt-time (caar (car ac-appt-list)))
+                    (< ac-appt-time ac-cur-time))
+          (setq ac-appt-list (cdr ac-appt-list)))
+        (setq appt-time-msg-list ac-appt-list)
         ;; If there are entries in the list, and the user wants a
         ;; message issued, get the first time off of the list and
         ;; calculate the number of minutes until the appointment.
-        (when appt-time-msg-list
-          (setq ac-appt-time (caar (car appt-time-msg-list))
-                ac-warn-time (or (nth 3 (car appt-time-msg-list))
-                                   appt-message-warning-time)
-                ac-min-to-app (- ac-appt-time ac-cur-time))
-          (while (and appt-time-msg-list
-                      (< ac-appt-time ac-cur-time))
-            (setq appt-time-msg-list (cdr appt-time-msg-list))
-            (if appt-time-msg-list
-                (setq ac-appt-time (caar (car appt-time-msg-list)))))
+        (when ac-appt-list
+          (setq ac-warn-time (or (nth 3 (car ac-appt-list))
+                                 appt-message-warning-time)
+                ac-min-to-app (- ac-appt-time ac-cur-time)
+                ac-appt-time (caar (car ac-appt-list))
+                appt-mode-string nil)
           ;; If we have an appointment between midnight and
           ;; `ac-warn-time' minutes after midnight, we
           ;; must begin to issue a message before midnight.  Midnight
@@ -347,7 +349,7 @@ displayed in a window:
                      (>= ac-min-to-app 0))
             ;; This is true every appt-display-interval minutes.
             (if (zerop (mod appt-display-count appt-display-interval))
-                (appt-display-message (cadr (car appt-time-msg-list))
+                (appt-display-message (cadr (car ac-appt-list))
                                     ac-min-to-app))
             (setq appt-display-count (1+ appt-display-count))
             (when appt-display-mode-line
@@ -355,12 +357,10 @@ displayed in a window:
                     (concat " " (propertize
                                  (format "App't in %s min." ac-min-to-app)
                                  'face 'mode-line-emphasis))))
-            ;; When an appointment is reached, delete it from the
-            ;; list.  Reset the count to 0 in case we display another
-            ;; appointment on the next cycle.
+            ;; When an appointment is reached reset the count to 0 in
+            ;; case we display another appointment on the next cycle.
             (if (zerop ac-min-to-app)
-                (setq appt-time-msg-list (cdr appt-time-msg-list)
-                      appt-display-count 0))))
+                (setq appt-display-count 0))))
         ;; Redisplay all mode lines.
         (when appt-display-mode-line
           (force-mode-line-update)))))
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #13: 0012-implement-multiple-appointments.patch --]
[-- Type: text/x-diff, Size: 10269 bytes --]

From cf17d3430324cf43d77e3260b4f7dabe6b005860 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Sun, 27 Mar 2011 17:59:07 +0200
Subject: [PATCH 12/14] implement multiple appointments

Change strategy in appt-check : first remove old appointments, go
through all appointments and pick relevant appointments, last display
mode line and message. Add a new variable appt-display-multiple that
is set to t by default. I think setting it to t is a good idea to
avoid missing appointments. This can happen when a second appointment
has a warning time that precedes the first.

Add a new variable appt-disp-window-multiple-function that takes the
function to display multiple appointments. Add the function
appt-disp-multiple-window that does the actual displaying. Add
function appt-display-multiple-message that is called from
appt-check. Note that the minutes passed is an integer instead of a
string for the single appointment version. I just feel the conversion
is kind of silly.
---
 appt.el |  150 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 125 insertions(+), 25 deletions(-)

diff --git a/appt.el b/appt.el
index 9bae717..1a9e9f3 100644
--- a/appt.el
+++ b/appt.el
@@ -164,13 +164,24 @@ This will occur at midnight when the appointment list is updated."
 
 (defcustom appt-disp-window-function 'appt-disp-window
   "Function called to display appointment window.
-Only relevant if reminders are being displayed in a window.
-It should take three string arguments: the number of minutes till
-the appointment, the current time, and the text of the appointment."
+It's called when `appt-display-multiple' is set to nil. Only
+relevant if reminders are being displayed in a window.  It should
+take three string arguments: the number of minutes till the
+appointment, the current time, and the text of the appointment."
   :type '(choice (const appt-disp-window)
                  function)
   :group 'appt)
 
+(defcustom appt-disp-window-multiple-function 'appt-disp-multiple-window
+  "Function called to display appointment window.
+It's called when `appt-display-multiple' is set to t. Only
+relevant if reminders are being displayed in a window. It takes
+two argument the first is the number of minutes until the first
+appointment the second contain a list of the appointments."
+  :type '(choice (const appt-disp-multiple-window)
+                 function)
+  :group 'appt)
+
 (defcustom appt-delete-window-function 'appt-delete-window
   "Function called to remove appointment window and buffer.
 Only relevant if reminders are being displayed in a window."
@@ -178,6 +189,10 @@ Only relevant if reminders are being displayed in a window."
                  function)
   :group 'appt)
 
+(defcustom appt-display-multiple t
+  "Non-nil means display multiple appointment."
+  :type 'boolean
+  :group 'appt)
 
 ;;; Internal variables below this point.
 
@@ -252,6 +267,26 @@ The variable `appt-audible' controls the audible reminder."
           ((eq appt-display-format 'echo)
            (message "%s" string)))))
 
+(defun appt-display-multiple-message (appt-list mins)
+  "Display a list of due appointments, first appointment in MINS minutes.
+The list APPT-LIST contains the list of appointments.
+The format of the visible reminder is controlled by `appt-display-format'.
+The variable `appt-audible' controls the audible reminder."
+  ;; Let-binding for backwards compatibility.  Remove when obsolete
+  ;; vars appt-msg-window and appt-visible are dropped.
+  (let ((appt-display-format
+         (if (eq appt-display-format 'ignore)
+             (cond (appt-msg-window 'window)
+                   (appt-visible 'echo))
+           appt-display-format)))
+    (if appt-audible (beep 1))
+    (cond ((eq appt-display-format 'window)
+           (funcall appt-disp-window-multiple-function mins appt-list)
+           (run-at-time (format "%d sec" appt-display-duration)
+                        nil
+                        appt-delete-window-function))
+          ((eq appt-display-format 'echo)
+           (message "%s" (mapconcat 'identity (nreverse appt-list) " "))))))
 
 (defvar diary-selective-display)
 
@@ -312,24 +347,24 @@ displayed in a window:
          ;; Convert current time to minutes after midnight (12.01am = 1).
          (ac-cur-time (+ (* 60 (nth 2 ac-now)) (nth 1 ac-now)))
          (ac-appt-list appt-time-msg-list)
-         ac-appt-time ac-warn-time ac-min-to-app)
+         (ac-first nil)
+         ac-appt-time ac-warn-time ac-min-to-app ac-msg ac-msg-list)
       (save-excursion
         (appt-check-diary force ac-cur-time)
         ;; Discard appointments previous to current time
         (while (and ac-appt-list
-                    (setq ac-appt-time (caar (car ac-appt-list)))
-                    (< ac-appt-time ac-cur-time))
+                    (< (caar (car ac-appt-list)) ac-cur-time))
           (setq ac-appt-list (cdr ac-appt-list)))
-        (setq appt-time-msg-list ac-appt-list)
-        ;; If there are entries in the list, and the user wants a
-        ;; message issued, get the first time off of the list and
-        ;; calculate the number of minutes until the appointment.
-        (when ac-appt-list
+        (setq appt-time-msg-list ac-appt-list
+              appt-mode-string nil
+              appt-display-count (1+ appt-display-count))
+
+        (while ac-appt-list
           (setq ac-warn-time (or (nth 3 (car ac-appt-list))
                                  appt-message-warning-time)
-                ac-min-to-app (- ac-appt-time ac-cur-time)
                 ac-appt-time (caar (car ac-appt-list))
-                appt-mode-string nil)
+                ac-min-to-app (- ac-appt-time ac-cur-time))
+
           ;; If we have an appointment between midnight and
           ;; `ac-warn-time' minutes after midnight, we
           ;; must begin to issue a message before midnight.  Midnight
@@ -347,23 +382,28 @@ displayed in a window:
           ;; appt-message-warning time.
           (when (and (<= ac-min-to-app ac-warn-time)
                      (>= ac-min-to-app 0))
-            ;; This is true every appt-display-interval minutes.
-            (if (zerop (mod appt-display-count appt-display-interval))
-                (appt-display-message (cadr (car ac-appt-list))
-                                    ac-min-to-app))
-            (setq appt-display-count (1+ appt-display-count))
-            (when appt-display-mode-line
-              (setq appt-mode-string
-                    (concat " " (propertize
-                                 (format "App't in %s min." ac-min-to-app)
-                                 'face 'mode-line-emphasis))))
+                (setq ac-msg (cadr (car ac-appt-list)))
+                (add-to-list 'ac-msg-list ac-msg)
+                (when (not ac-first)
+                    (setq ac-first (list ac-min-to-app ac-msg)))
             ;; When an appointment is reached reset the count to 0 in
             ;; case we display another appointment on the next cycle.
             (if (zerop ac-min-to-app)
-                (setq appt-display-count 0))))
+                (setq appt-display-count 0)))
+          (setq ac-appt-list (cdr ac-appt-list)))
+        (when (and ac-first
+                   (zerop (mod appt-display-count appt-display-interval))
+            (if appt-display-multiple
+                (appt-display-multiple-message ac-msg-list (car ac-first))
+              (appt-display-message (cadr ac-first) (car ac-first)))))
         ;; Redisplay all mode lines.
         (when appt-display-mode-line
-          (force-mode-line-update)))))
+          (when ac-first
+            (setq appt-mode-string
+                  (concat " " (propertize
+                               (format "App't in %s min." (car ac-first))
+                               'face 'mode-line-emphasis)))))
+          (force-mode-line-update))))
 
 (defun appt-check-diary (force cur-comp-time)
   "Update appointments to today's list."
@@ -450,6 +490,25 @@ message APPT-MSG in a separate buffer."
     (raise-frame (selected-frame))
     (select-window this-window)))
 
+(defun appt-disp-multiple-window (min-to-app appt-list)
+  "Display list of APPT-LIST."
+  (let ((this-window (selected-window)))
+    (with-current-buffer (get-buffer-create appt-buffer-name)
+      (setq buffer-read-only nil
+            buffer-undo-list t)
+      (erase-buffer)
+      (insert (mapconcat 'identity (nreverse appt-list) "\n")))
+    (pop-to-buffer appt-buffer-name)
+    (fit-window-to-buffer)
+    (calendar-set-mode-line
+     (format " Appointment %s."
+             (if (= min-to-app 0) "now"
+               (format "in %d minute%s" min-to-app
+                       (if (= min-to-app 1) "" "s")))))
+
+    (raise-frame (selected-frame))
+    (select-window this-window)))
+
 (defun appt-delete-window ()
   "Function called to undisplay appointment messages.
 Usually just deletes the appointment buffer."
@@ -705,6 +764,47 @@ ARG is positive, otherwise off."
       (appt-check t))))
 
 
+;;; Test functions
+;; two helper functions for tests
+;; (defun appt-test-add (min-from-now msg warntime)
+;;   (appt-add
+;;    (format-time-string
+;;     "%H:%M"
+;;     (let ((time (current-time)))
+;;       (cons (nth 0 time)
+;;             (+ (* 60 min-from-now) (nth 1 time))))) msg warntime))
+
+;; (defun appt-test-init ()
+;;   (setq appt-display-count 0)
+;;   (setq appt-display-interval 1)
+;;   (setq appt-time-msg-list nil))
+
+;; appointment in a minute
+;; (progn
+;;   (appt-test-init)
+;;   (add-appt 1 "one minute" 1))
+
+;; two appointment one after the other, it should display:
+;; 1 first
+;; 2 first and second
+;; 3 second
+;; (progn
+;;   (appt-test-init)
+;;   (add-appt 1 "first" 1)
+;;   (add-appt 2 "second" 1))
+
+;; typical shadowing problem solved by multiple appointments. The
+;; second appointment has warning time that precedes the first. I have
+;; actually missed an appointment because of this! It should display:
+;; 1 second
+;; 2 first and second
+;; 3 first and second
+;; 4 second
+;; (progn
+;;   (appt-test-init)
+;;   (add-appt 2 "first" 1)
+;;   (add-appt 3 "second" 3))
+
 (provide 'appt)
 
 ;; arch-tag: bf5791c4-8921-499e-a26f-772b1788d347
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #14: 0013-move-appointment-checking-code-in-a-function.patch --]
[-- Type: text/x-diff, Size: 5659 bytes --]

From f58eba27aebbcfb0bbf5385a9efe7a5f3b32f289 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@tao>
Date: Mon, 28 Mar 2011 19:54:09 +0200
Subject: [PATCH 13/14] move appointment checking code in a function

This makes function appt-check a reasonable size and I have added some
comments as well.
---
 appt.el |   78 +++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/appt.el b/appt.el
index 1a9e9f3..c3cc429 100644
--- a/appt.el
+++ b/appt.el
@@ -348,55 +348,39 @@ displayed in a window:
          (ac-cur-time (+ (* 60 (nth 2 ac-now)) (nth 1 ac-now)))
          (ac-appt-list appt-time-msg-list)
          (ac-first nil)
-         ac-appt-time ac-warn-time ac-min-to-app ac-msg ac-msg-list)
+         ac-msg ac-msg-list ac-min-to-app)
       (save-excursion
         (appt-check-diary force ac-cur-time)
+
         ;; Discard appointments previous to current time
         (while (and ac-appt-list
                     (< (caar (car ac-appt-list)) ac-cur-time))
           (setq ac-appt-list (cdr ac-appt-list)))
+
+        ;; Update global list and few other things
         (setq appt-time-msg-list ac-appt-list
               appt-mode-string nil
               appt-display-count (1+ appt-display-count))
 
+        ;; Now walk the remaining list checking for appointments
         (while ac-appt-list
-          (setq ac-warn-time (or (nth 3 (car ac-appt-list))
-                                 appt-message-warning-time)
-                ac-appt-time (caar (car ac-appt-list))
-                ac-min-to-app (- ac-appt-time ac-cur-time))
-
-          ;; If we have an appointment between midnight and
-          ;; `ac-warn-time' minutes after midnight, we
-          ;; must begin to issue a message before midnight.  Midnight
-          ;; is considered 0 minutes and 11:59pm is 1439
-          ;; minutes.  Therefore we must recalculate the minutes to
-          ;; appointment variable.  It is equal to the number of
-          ;; minutes before midnight plus the number of minutes after
-          ;; midnight our appointment is.
-          (if (and (< ac-appt-time ac-warn-time)
-                   (> (+ ac-cur-time ac-warn-time)
-                      appt-max-time))
-              (setq ac-min-to-app (+ (- (1+ appt-max-time) ac-cur-time)
-                                  ac-appt-time)))
-          ;; Issue warning if the appointment time is within
-          ;; appt-message-warning time.
-          (when (and (<= ac-min-to-app ac-warn-time)
-                     (>= ac-min-to-app 0))
-                (setq ac-msg (cadr (car ac-appt-list)))
-                (add-to-list 'ac-msg-list ac-msg)
-                (when (not ac-first)
-                    (setq ac-first (list ac-min-to-app ac-msg)))
-            ;; When an appointment is reached reset the count to 0 in
-            ;; case we display another appointment on the next cycle.
-            (if (zerop ac-min-to-app)
-                (setq appt-display-count 0)))
+          (setq ac-min-to-app
+                (appt-check-appointment (car ac-appt-list) ac-cur-time))
+          (when ac-min-to-app
+            (setq ac-msg (cadr (car ac-appt-list)))
+            (add-to-list 'ac-msg-list ac-msg))
+            (when (not ac-first)
+              (setq ac-first (list ac-min-to-app ac-msg)))
           (setq ac-appt-list (cdr ac-appt-list)))
+
+        ;; Display messages
         (when (and ac-first
                    (zerop (mod appt-display-count appt-display-interval))
             (if appt-display-multiple
                 (appt-display-multiple-message ac-msg-list (car ac-first))
               (appt-display-message (cadr ac-first) (car ac-first)))))
-        ;; Redisplay all mode lines.
+
+        ;; Display mode line
         (when appt-display-mode-line
           (when ac-first
             (setq appt-mode-string
@@ -405,6 +389,36 @@ displayed in a window:
                                'face 'mode-line-emphasis)))))
           (force-mode-line-update))))
 
+(defun appt-check-appointment (appt cur-time)
+  "Check if apppointment needs to be displayed.
+Returns integer of minute till the appointment is due or nil"
+  (let* ((aca-warn-time (or (nth 3 appt)
+                           appt-message-warning-time))
+         (aca-appt-time (caar appt))
+         (aca-min-to-app (- aca-appt-time cur-time)))
+    
+    ;; If we have an appointment between midnight and
+    ;; `aca-warn-time' minutes after midnight, we
+    ;; must begin to issue a message before midnight.  Midnight
+    ;; is considered 0 minutes and 11:59pm is 1439
+    ;; minutes.  Therefore we must recalculate the minutes to
+    ;; appointment variable.  It is equal to the number of
+    ;; minutes before midnight plus the number of minutes after
+    ;; midnight our appointment is.
+    (when (and (< aca-appt-time aca-warn-time)
+               (> (+ cur-time aca-warn-time)
+                  appt-max-time))
+      (setq aca-min-to-app (+ (- (1+ appt-max-time) cur-time)
+                             aca-appt-time)))
+    ;; Issue warning if the appointment time is within
+    ;; appt-message-warning time.
+    (when (and (<= aca-min-to-app aca-warn-time)
+               (>= aca-min-to-app 0))
+      ;; When an appointment is reached reset the count to 0 in
+      ;; case we display another appointment on the next cycle.
+      (when (zerop aca-min-to-app)
+        (setq appt-display-count 0)) aca-min-to-app)))
+                              
 (defun appt-check-diary (force cur-comp-time)
   "Update appointments to today's list."
   ;; At first check in any day
-- 
1.7.3.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #15: 0014-fix-logic-bug.patch --]
[-- Type: text/x-diff, Size: 892 bytes --]

From 830d6dc5dc758aaa80a9aad08423daf8b060f026 Mon Sep 17 00:00:00 2001
From: Ivan Kanis <ivan@mac.local>
Date: Sun, 10 Apr 2011 11:06:01 +0200
Subject: [PATCH 14/14] fix logic bug

---
 appt.el |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/appt.el b/appt.el
index c3cc429..b86d657 100644
--- a/appt.el
+++ b/appt.el
@@ -368,9 +368,9 @@ displayed in a window:
                 (appt-check-appointment (car ac-appt-list) ac-cur-time))
           (when ac-min-to-app
             (setq ac-msg (cadr (car ac-appt-list)))
-            (add-to-list 'ac-msg-list ac-msg))
+            (add-to-list 'ac-msg-list ac-msg)
             (when (not ac-first)
-              (setq ac-first (list ac-min-to-app ac-msg)))
+              (setq ac-first (list ac-min-to-app ac-msg))))
           (setq ac-appt-list (cdr ac-appt-list)))
 
         ;; Display messages
-- 
1.7.3.2


[-- Attachment #16: appt.el --]
[-- Type: application/emacs-lisp, Size: 34670 bytes --]

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: multiple appointments bug
  2011-04-11  9:13 multiple appointments bug Ivan Kanis
@ 2011-04-12  8:03 ` Glenn Morris
  0 siblings, 0 replies; 2+ messages in thread
From: Glenn Morris @ 2011-04-12  8:03 UTC (permalink / raw)
  To: Ivan Kanis; +Cc: emacs devel


Please send any future mail on this to the relevant bug report rather
than to emacs-devel, thanks.



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-04-12  8:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-11  9:13 multiple appointments bug Ivan Kanis
2011-04-12  8:03 ` Glenn Morris

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.