2 files changed, 203 insertions, 120 deletions

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 900b38b9d6b..95e932acded 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,26 @@
+2013-02-04  Stephen Berman  <stephen.berman@gmx.net>
+
+        * calendar/todos.el: Bug fixes involving or displaying done items.
+        (todos-done-separator): Make length of long separator one less
+        than window-width, in order to avoid following empty line.
+        (todos-forward-item, todos-backward-item): Make empty line below
+        last todo item accessible only by invoking without a numerical
+        prefix argument and make the latter invocations move only to items.
+        (todos-insert-item): Set todos-current-todos-file after selecting
+        buffer, in order not to set global value.  If only done items are
+        shown when this command is invoked, toggle to show todo items to
+        avoid insertion into done items section.  When done items are
+        shown, protect from cancelling before setting item priority: this
+        also needs recentering to display all items.
+        (todos-set-item-priority): Only call on (not done) todo items.
+        Check that done items are visible not because file is widened.
+        Use only `any' argument of called-interactively-p.  Make sure that
+        giving an undone item lowest priority restores it to the todo
+        section.
+        (todos-item-done): If done items are visible on invoking this
+        command, keep them visible afterwards.  Stop looking for marked
+        items on reaching the empty line below the last todo item.
+
 2013-02-02  Stephen Berman  <stephen.berman@gmx.net>
 
         * calendar/todos.el: Several small fixes and improvements.
diff --git a/lisp/calendar/todos.el b/lisp/calendar/todos.el
index 1e8f10b0552..ce7fa961b48 100644
--- a/lisp/calendar/todos.el
+++ b/lisp/calendar/todos.el
@@ -1150,7 +1150,11 @@ number as its value."
   "Return string used as value of variable `todos-done-separator'."
   (let ((sep todos-done-separator-string))
     (propertize (if (= 1 (length sep))
-                    (make-string (window-width) (string-to-char sep))
+                    ;; If separator's length is window-width, an
+                    ;; indented empty line appears between the
+                    ;; separator and the first done item.
+                    ;; FIXME: should this be customizable?
+                    (make-string (1- (window-width)) (string-to-char sep))
                   todos-done-separator-string)
                 'face 'todos-done-sep)))
 
@@ -1522,6 +1526,14 @@ The final element is \"*\", indicating an unspecified month.")
     (todos-item-start)
     (looking-at todos-done-string-start)))
 
+;; (defun todos-done-item-section-p ()
+;;   "Return non-nil if point is in category's done items section."
+;;   (save-excursion
+;;     (or (re-search-backward (concat "^" (regexp-quote todos-category-done))
+;;                          nil t)
+;;      (progn (goto-char (point-min))
+;;             (looking-at todos-done-string-start)))))
+
 (defun todos-prefix-overlay ()
   "Return this item's prefix overlay."
   (let* ((lbp (line-beginning-position))
@@ -3522,7 +3534,17 @@ CAT; this is used in Todos Categories mode."
 (defun todos-forward-item (&optional count)
   "Move point down to start of item with next lower priority.
 With positive numerical prefix COUNT, move point COUNT items
-downward."
+downward.
+
+If the category's done items are hidden, this command also moves
+point to the empty line below the last todo item from any higher
+item in the category, i.e., when invoked with or without a prefix
+argument.  If the category's done items are visible, this command
+called with a prefix argument only moves point to a lower item,
+e.g., with point on the last todo item and called with prefix 1,
+it moves point to the first done item; but if called with point
+on the last todo item without a prefix argument, it moves point
+the the empty line above the done items separator."
   (interactive "P")
   ;; It's not worth the trouble to allow prefix arg value < 1, since we have
   ;; the corresponding command.
@@ -3538,9 +3560,9 @@ downward."
       ;; space above todos-done-separator, since that is a legitimate place to
       ;; insert an item.  But skip this space if count > 1, since that should
       ;; only stop on an item.
-      (when (and not-done (todos-done-item-p))
-        (if (or (not count) (= count 1))
-            (re-search-backward "^$" start t))))))
+      (when (and not-done (todos-done-item-p) (not count))
+        ;; (if (or (not count) (= count 1))
+            (re-search-backward "^$" start t)))));)
     ;; FIXME: The preceding sexp is insufficient when buffer is not narrowed,
     ;; since there could be no done items in this category, so the search puts
     ;; us on first todo item of next category.  Does this ever happen?  If so:
@@ -3557,7 +3579,14 @@ downward."
 (defun todos-backward-item (&optional count)
   "Move point up to start of item with next higher priority.
 With positive numerical prefix COUNT, move point COUNT items
-upward."
+upward.
+
+If the category's done items are visible, this command called
+with a prefix argument only moves point to a higher item, e.g.,
+with point on the first done item and called with prefix 1, it
+moves to the last todo item; but if called with point on the
+first done item without a prefix argument, it moves point the the
+empty line above the done items separator."
   (interactive "P")
   ;; Avoid moving to bob if on the first item but not at bob.
   (when (> (line-number-at-pos) 1)
@@ -3575,7 +3604,8 @@ upward."
         ;; todos-done-separator, since that is a legitimate place to insert an
         ;; item.  But skip this space if count > 1, since that should only
         ;; stop on an item.
-        (when (and done (not (todos-done-item-p)) (or (not count) (= count 1))
+        (when (and done (not (todos-done-item-p)) (not count)
+                                        ;(or (not count) (= count 1))
                    (not (equal (buffer-name) todos-regexp-items-buffer)))
           (re-search-forward (concat "^" (regexp-quote todos-category-done))
                              nil t)
@@ -4550,12 +4580,17 @@ the priority is not given by HERE but by prompting."
                                    (substring (current-time-string) 11 16)))))
         (setq todos-date-from-calendar nil)
         (find-file-noselect file 'nowarn)
-        (setq todos-current-todos-file file)
         (set-window-buffer (selected-window)
                            ;; If current category was nil till now, on
                            ;; entering Todos mode here it will be set to
                            ;; file's first category.
                            (set-buffer (find-buffer-visiting file)))
+        (setq todos-current-todos-file file)
+        ;; If only done items are displayed in category, toggle to
+        ;; todo items.
+        (when (and (goto-char (point-min))
+                   (looking-at todos-done-string-start))
+          (todos-show-done-only))
         (unless todos-global-current-todos-file
           (setq todos-global-current-todos-file todos-current-todos-file))
         ;; These are not needed here, since they are called in
@@ -4595,10 +4630,17 @@ the priority is not given by HERE but by prompting."
             ;;  (todos-insert-with-overlays new-item)
             ;;   )
             ;; (todos-set-item-priority new-item (todos-current-category) t))
-            (todos-set-item-priority new-item cat t)
-            ;; If item is inserted at end of category, make sure the
-            ;; items above it are displayed in the window.
-            (recenter))
+            (unwind-protect
+                (todos-set-item-priority new-item cat t)
+              ;; In (at least) two circumstances, point may be at eob
+              ;; and eob at window-start, so that that the todo items
+              ;; are out of view: (i) if item is inserted at end of
+              ;; category, (ii) if only done items are shown, this is
+              ;; (above) programmatically toggled to show todo items,
+              ;; and user cancels before setting new item's
+              ;; priority.  To make sure the todo items are displayed
+              ;; in the window, force recentering.
+              (recenter)))
           (todos-update-count 'todo 1)
           (if (or diary todos-include-in-diary) (todos-update-count 'diary 1))
           (todos-update-categories-sexp))))))
@@ -5152,115 +5194,125 @@ items in this category."
 (defun todos-set-item-priority (&optional item cat new arg)
   "Prompt for and set ITEM's priority in CATegory.
 
-Interactively, ITEM defaults to the item at point, CAT to the
-current category in Todos mode, and the priority is a number
-between 1 and the number of items in the category.
-Non-interactively, non-nil NEW means ITEM is a new item and the
-lowest priority is one more than the number of items in CAT.
+Interactively, ITEM is the todo item at point, CAT is the current
+category, and the priority is a number between 1 and the number
+of items in the category.  Non-interactively, non-nil NEW means
+ITEM is a new item and the lowest priority is one more than the
+number of items in CAT.
 
 The new priority is set either interactively by prompt or by a
 numerical prefix argument, or noninteractively by argument ARG,
 whose value can be either of the symbols `raise' or `lower',
 meaning to raise or lower the item's priority by one."
-  (interactive)                         ; Prefix arg?
-  (let* ((item (or item (todos-item-string)))
-         (marked (todos-marked-item-p))
-         (cat (or cat (cond ((eq major-mode 'todos-mode)
-                             (todos-current-category))
-                            ((eq major-mode 'todos-filtered-items-mode)
-                             (let* ((regexp1
-                                     (concat todos-date-string-start
-                                             todos-date-pattern
-                                             "\\( " diary-time-regexp "\\)?"
-                                             (regexp-quote todos-nondiary-end)
-                                             "?\\(?1: \\[\\(.+:\\)?.+\\]\\)")))
-                               (save-excursion
-                                 (re-search-forward regexp1 nil t)
-                                 (match-string-no-properties 1)))))))
-         curnum
-         (todo (cond ((or (eq arg 'raise) (eq arg 'lower)
-                          (eq major-mode 'todos-filtered-items-mode))
-                      (save-excursion
-                        (let ((curstart (todos-item-start))
-                              (count 0))
-                          (goto-char (point-min))
-                          (while (looking-at todos-item-start)
-                            (setq count (1+ count))
-                            (when (= (point) curstart) (setq curnum count))
-                            (todos-forward-item))
-                          count)))
-                     ((eq major-mode 'todos-mode)
-                      (todos-get-count 'todo cat))))
-         (maxnum (if new (1+ todo) todo))
-         (prompt (format "Set item priority (1-%d): " maxnum))
-         (priority (cond ((numberp current-prefix-arg)
-                          current-prefix-arg)
-                         ((and (eq arg 'raise) (>= curnum 1))
-                          (1- curnum))
-                         ((and (eq arg 'lower) (<= curnum maxnum))
-                          (1+ curnum))))
-         candidate
-         buffer-read-only)
-    (unless (and priority
-                 (or (and (eq arg 'raise) (zerop priority))
-                     (and (eq arg 'lower) (> priority maxnum))))
-      ;; When moving item to another category, show the category before
-      ;; prompting for its priority.
-      (unless (or arg (called-interactively-p t))
-        (todos-category-number cat)
-        ;; If done items in category are visible, keep them visible.
-        (let (done)
-          (save-excursion
-            (goto-char (point-min))
-            (setq done (re-search-forward todos-done-string-start nil t)))
-          (let ((todos-show-with-done done))
-            (todos-category-select))))
-      ;; Prompt for priority only when the category has at least one todo item.
-      (when (> maxnum 1)
-        (while (not priority)
-          (setq candidate (read-number prompt))
-          (setq prompt (when (or (< candidate 1) (> candidate maxnum))
-                         (format "Priority must be an integer between 1 and %d.\n"
-                                 maxnum)))
-          (unless prompt (setq priority candidate))))
-      ;; In Top Priorities buffer, an item's priority can be changed
-      ;; wrt items in another category, but not wrt items in the same
-      ;; category.
-      (when (eq major-mode 'todos-filtered-items-mode)
-        (let* ((regexp2 (concat todos-date-string-start todos-date-pattern
-                                "\\( " diary-time-regexp "\\)?"
-                                (regexp-quote todos-nondiary-end)
-                                "?\\(?1:" (regexp-quote cat) "\\)"))
-               (end (cond ((< curnum priority)
-                           (save-excursion (todos-item-end)))
-                          ((> curnum priority)
-                           (save-excursion (todos-item-start)))))
-               (match (save-excursion
-                        (cond ((< curnum priority)
-                               (todos-forward-item (1+ (- priority curnum)))
-                               (when (re-search-backward regexp2 end t)
-                                 (match-string-no-properties 1)))
-                              ((> curnum priority)
-                               (todos-backward-item (- curnum priority))
-                               (when (re-search-forward regexp2 end t)
-                                 (match-string-no-properties 1)))))))
-          (when match
-            (error (concat "Cannot reprioritize items from the same "
-                           "category in this mode, only in Todos mode")))))
-      ;; Interactively or with non-nil ARG, relocate the item within its
-      ;; category.
-      (when (or arg (called-interactively-p))
-        (todos-remove-item))
-      (goto-char (point-min))
-      (when priority
-        (unless (= priority 1)
-          (todos-forward-item (1- priority))))
-      (todos-insert-with-overlays item)
-      ;; If item was marked, restore the mark.
-      (and marked
-           (let* ((ov (todos-prefix-overlay))
-                  (pref (overlay-get ov 'before-string)))
-             (overlay-put ov 'before-string (concat todos-item-mark pref)))))))
+  (interactive)                         ;FIXME: Prefix arg?
+  (unless (and (called-interactively-p 'any)
+               (or (todos-done-item-p) (looking-at "^$")))
+    (let* ((item (or item (todos-item-string)))
+           (marked (todos-marked-item-p))
+           (cat (or cat (cond ((eq major-mode 'todos-mode)
+                               (todos-current-category))
+                              ((eq major-mode 'todos-filtered-items-mode)
+                               (let* ((regexp1
+                                       (concat todos-date-string-start
+                                               todos-date-pattern
+                                               "\\( " diary-time-regexp "\\)?"
+                                               (regexp-quote todos-nondiary-end)
+                                               "?\\(?1: \\[\\(.+:\\)?.+\\]\\)")))
+                                 (save-excursion
+                                   (re-search-forward regexp1 nil t)
+                                   (match-string-no-properties 1)))))))
+           curnum
+           (todo (cond ((or (eq arg 'raise) (eq arg 'lower)
+                            (eq major-mode 'todos-filtered-items-mode))
+                        (save-excursion
+                          (let ((curstart (todos-item-start))
+                                (count 0))
+                            (goto-char (point-min))
+                            (while (looking-at todos-item-start)
+                              (setq count (1+ count))
+                              (when (= (point) curstart) (setq curnum count))
+                              (todos-forward-item))
+                            count)))
+                       ((eq major-mode 'todos-mode)
+                        (todos-get-count 'todo cat))))
+           (maxnum (if new (1+ todo) todo))
+           (prompt (format "Set item priority (1-%d): " maxnum))
+           (priority (cond ((numberp current-prefix-arg)
+                            current-prefix-arg)
+                           ((and (eq arg 'raise) (>= curnum 1))
+                            (1- curnum))
+                           ((and (eq arg 'lower) (<= curnum maxnum))
+                            (1+ curnum))))
+           candidate
+           buffer-read-only)
+      (unless (and priority
+                   (or (and (eq arg 'raise) (zerop priority))
+                       (and (eq arg 'lower) (> priority maxnum))))
+        ;; When moving item to another category, show the category before
+        ;; prompting for its priority.
+        (unless (or arg (called-interactively-p 'any))
+          (todos-category-number cat)
+          ;; If done items in category are visible, keep them visible.
+          (let ((done todos-show-with-done))
+            (when (> (buffer-size) (- (point-max) (point-min)))
+              (save-excursion
+                (goto-char (point-min))
+                (setq done (re-search-forward todos-done-string-start nil t))))
+            (let ((todos-show-with-done done))
+              (todos-category-select)))))
+        ;; Prompt for priority only when the category has at least one todo item.
+        (when (> maxnum 1)
+          (while (not priority)
+            (setq candidate (read-number prompt))
+            (setq prompt (when (or (< candidate 1) (> candidate maxnum))
+                           (format "Priority must be an integer between 1 and %d.\n"
+                                   maxnum)))
+            (unless prompt (setq priority candidate))))
+        ;; In Top Priorities buffer, an item's priority can be changed
+        ;; wrt items in another category, but not wrt items in the same
+        ;; category.
+        (when (eq major-mode 'todos-filtered-items-mode)
+          (let* ((regexp2 (concat todos-date-string-start todos-date-pattern
+                                  "\\( " diary-time-regexp "\\)?"
+                                  (regexp-quote todos-nondiary-end)
+                                  "?\\(?1:" (regexp-quote cat) "\\)"))
+                 (end (cond ((< curnum priority)
+                             (save-excursion (todos-item-end)))
+                            ((> curnum priority)
+                             (save-excursion (todos-item-start)))))
+                 (match (save-excursion
+                          (cond ((< curnum priority)
+                                 (todos-forward-item (1+ (- priority curnum)))
+                                 (when (re-search-backward regexp2 end t)
+                                   (match-string-no-properties 1)))
+                                ((> curnum priority)
+                                 (todos-backward-item (- curnum priority))
+                                 (when (re-search-forward regexp2 end t)
+                                   (match-string-no-properties 1)))))))
+            (when match
+              (error (concat "Cannot reprioritize items from the same "
+                             "category in this mode, only in Todos mode")))))
+        ;; Interactively or with non-nil ARG, relocate the item within its
+        ;; category.
+        (when (or arg (called-interactively-p 'any))
+          (todos-remove-item))
+        (goto-char (point-min))
+        (when priority
+          (unless (= priority 1)
+            (todos-forward-item (1- priority))
+            ;; When called from todos-item-undo and the highest priority
+            ;; is chosen, this advances point to the first done item, so
+            ;; move it up to the empty line above the done items
+            ;; separator.
+            (when (looking-back (concat "^"
+                                        (regexp-quote todos-category-done) "\n"))
+              (todos-backward-item))))
+        (todos-insert-with-overlays item)
+        ;; If item was marked, restore the mark.
+        (and marked
+             (let* ((ov (todos-prefix-overlay))
+                    (pref (overlay-get ov 'before-string)))
+               (overlay-put ov 'before-string (concat todos-item-mark pref))))))))
 
 (defun todos-raise-item-priority ()
   "Raise priority of current item by moving it up by one item."
@@ -5427,7 +5479,7 @@ section in the category moved to."
             (goto-char omark))))))))
 
 (defun todos-item-done (&optional arg)
-  "Tag at least one item in this category as done and hide it.
+  "Tag a todo item in this category as done and relocate it.
 
 With prefix argument ARG prompt for a comment and append it to
 the done item; this is only possible if there are no marked
@@ -5435,7 +5487,9 @@ items.  If there are marked items, tag all of these with
 `todos-done-string' plus the current date and, if
 `todos-always-add-time-string' is non-nil, the current time;
 otherwise, just tag the item at point.  Items tagged as done are
-relocated to the category's (by default hidden) done section."
+relocated to the category's (by default hidden) done section.  If
+done items are visible on invoking this command, they remain
+visible."
   (interactive "P")
   (let* ((cat (todos-current-category))
          (marked (assoc cat todos-categories-with-marks)))
@@ -5451,11 +5505,16 @@ relocated to the category's (by default hidden) done section."
              (comment (and arg (not marked) (read-string "Enter a comment: ")))
              (item-count 0)
              (diary-count 0)
+             (show-done (save-excursion
+                          (goto-char (point-min))
+                          (re-search-forward todos-done-string-start nil t)))
              item done-item
              (buffer-read-only))
         (and marked (goto-char (point-min)))
         (catch 'done
-          (while (not (eobp))
+          ;; Stop looping when we hit the empty line below the last
+          ;; todo item (this is eobp if only done items are hidden).
+          (while (not (looking-at "^$")) ;(not (eobp))
             (if (or (not marked) (and marked (todos-marked-item-p)))
                 (progn
                   (setq item (todos-item-string))
@@ -5488,7 +5547,8 @@ relocated to the category's (by default hidden) done section."
         (todos-update-count 'done item-count)
         (todos-update-count 'diary (- diary-count))
         (todos-update-categories-sexp)
-        (save-excursion (todos-category-select))))))
+        (let ((todos-show-with-done show-done))
+          (todos-category-select))))))
 
 (defun todos-done-item-add-edit-or-delete-comment (&optional arg)
   "Add a comment to this done item or edit an existing comment.