master 14ae2101412: Fix and improve setting priority of todo-mode items (bug#64433)

[Stephen Berman]

branch: master
commit 14ae2101412d66576846d30ab32b4d5b0081383d
Author: Stephen Berman <stephen.berman@gmx.net>
Commit: Stephen Berman <stephen.berman@gmx.net>

    Fix and improve setting priority of todo-mode items (bug#64433)
    
    * lisp/calendar/todo-mode.el (todo-set-item-priority): Bugfixes:
    Prevent interactively setting item priority to its current
    priority in the same category and prompt user for a different
    priority (but allow using the same priority when item is moved to
    another category).  Ensure that the priority passed as a prefix
    argument is suitable: if it is not an integer between 1 and the
    highest item number, signal a user error.  New feature: Use the
    sequence of numbers of the category's items as the minibuffer
    history.
    
    * doc/misc/todo-mode.texi (Inserting New Items):
    (Reprioritizing Items): Document using the minibuffer history.
    
    * test/lisp/calendar/todo-mode-tests.el
    (todo-test-item-insertion-with-priority-1)
    (todo-test-item-insertion-with-priority-2)
    (todo-test-item-insertion-with-priority-3): New tests.
---
 doc/misc/todo-mode.texi               | 14 ++++++-
 lisp/calendar/todo-mode.el            | 69 ++++++++++++++++++++++++++---------
 test/lisp/calendar/todo-mode-tests.el | 65 +++++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+), 20 deletions(-)

diff --git a/doc/misc/todo-mode.texi b/doc/misc/todo-mode.texi
index 40e3056c659..62376195023 100644
--- a/doc/misc/todo-mode.texi
+++ b/doc/misc/todo-mode.texi
@@ -580,7 +580,14 @@ on every invocation of @code{todo-insert-item}.
 the highest or lowest priority in the category, if you do not
 explicitly assign it a priority on invoking @code{todo-insert-item}.
 By default, such new items are given highest priority, i.e., inserted
-at the top of the list.
+at the top of the list.  In addition, when setting an item's priority
+you can use the minibuffer history to quickly call up the lowest or
+highest priority number in the minibuffer by typing @kbd{M-p} or
+@kbd{M-n}, and you can scroll through all priority numbers for the
+current category with these keys.  For example, with the default
+setting of @code{todo-default-priority}, you can insert a new item as
+second to last in the category by typing @kbd{M-p M-p} at the prompt
+for setting the priority.
 
 @item
 @code{todo-always-add-time-string} is for including or omitting the
@@ -983,7 +990,10 @@ category, i.e., gives it third highest priority; all lower 
priority
 items are pushed down by one.  You can also pass the desired priority
 as a numeric prefix argument, e.g., @kbd{3 #} gives the item third
 highest priority without prompting.  (Prefix arguments have no effect
-with @kbd{r} or @kbd{l}.)
+with @kbd{r} or @kbd{l}.)  And you can type @kbd{M-p} and @kbd{M-n} in
+the minibuffer to scroll through all priority numbers for the current
+category.  If you mistakenly choose the item's current priority, you
+will be prompted to choose a different priority.
 @end table
 
 @node Moving and Deleting Items
diff --git a/lisp/calendar/todo-mode.el b/lisp/calendar/todo-mode.el
index 56b0943d303..ffb7b7168dd 100644
--- a/lisp/calendar/todo-mode.el
+++ b/lisp/calendar/todo-mode.el
@@ -2646,16 +2646,26 @@ meaning to raise or lower the item's priority by one."
                                 (save-excursion
                                   (re-search-forward regexp1 nil t)
                                   (match-string-no-properties 1)))))))
-          curnum
+          (count 1)
+          (curnum (save-excursion
+                    (let ((curstart
+                           ;; If point is in done items section or not on an
+                           ;; item, use position of first todo item to avoid
+                           ;; the while-loop.
+                           (or (and (not (todo-done-item-section-p))
+                                    (todo-item-start))
+                               (point-min))))
+                      (goto-char (point-min))
+                      (while (/= (point) curstart)
+                        (setq count (1+ count))
+                        (todo-forward-item))
+                      count)))
           (todo (cond ((or (memq arg '(raise lower))
                            (eq major-mode 'todo-filtered-items-mode))
                        (save-excursion
-                         (let ((curstart (todo-item-start))
-                               (count 0))
-                           (goto-char (point-min))
+                         (let ((count curnum))
                            (while (looking-at todo-item-start)
                              (setq count (1+ count))
-                             (when (= (point) curstart) (setq curnum count))
                              (todo-forward-item))
                            count)))
                       ((eq major-mode 'todo-mode)
@@ -2667,11 +2677,16 @@ meaning to raise or lower the item's priority by one."
                           ((and (eq arg 'raise) (>= curnum 1))
                            (1- curnum))
                           ((and (eq arg 'lower) (<= curnum maxnum))
-                           (1+ curnum))))
-          candidate)
+                           (1+ curnum)))))
+      (and (called-interactively-p 'any)
+          priority  ; Check further only if arg or prefix arg was passed.
+          (or (< priority 1) (> priority maxnum))
+          (user-error (format "Priority must be an integer between 1 and %d"
+                              maxnum)))
       (unless (and priority
+                  (/= priority curnum)
                   (or (and (eq arg 'raise) (zerop priority))
-                      (and (eq arg 'lower) (> priority maxnum))))
+                      (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))
@@ -2687,16 +2702,34 @@ meaning to raise or lower the item's priority by one."
              ;; while setting priority.
              (save-excursion (todo-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
-                                        (if (eq todo-default-priority 'first)
-                                            1 maxnum)))
-           (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))))
+       ;; todo item or when passing the current priority as prefix arg.
+       (when (and (or (not priority) (= priority curnum))
+                  (> maxnum 1))
+          (let* ((read-number-history (mapcar #'number-to-string
+                                              (if (eq todo-default-priority
+                                                     'first)
+                                                  (number-sequence maxnum 1 -1)
+                                               (number-sequence 1 maxnum))))
+                 (history-add-new-input nil)
+                (candidate (or priority
+                               (read-number prompt
+                                            (if (eq todo-default-priority
+                                                    'first)
+                                                1 maxnum))))
+                (success nil))
+           (while (not success)
+              (setq prompt
+                    (cond
+                    ((and (= candidate curnum)
+                          ;; Allow same priority in a different category
+                          ;; (only possible when called non-interactively).
+                          (called-interactively-p 'any))
+                     "New priority must be different from current priority: ")
+                    (t (when (or (< candidate 1) (> candidate maxnum))
+                         (format "Priority must be an integer between 1 and 
%d: "
+                                 maxnum)))))
+             (when prompt (setq candidate (read-number prompt)))
+              (unless prompt (setq priority candidate success t)))))
        ;; In Top Priorities buffer, an item's priority can be changed
        ;; wrt items in another category, but not wrt items in the same
        ;; category.
diff --git a/test/lisp/calendar/todo-mode-tests.el 
b/test/lisp/calendar/todo-mode-tests.el
index 8d4ea69e9eb..3b49dd56b69 100644
--- a/test/lisp/calendar/todo-mode-tests.el
+++ b/test/lisp/calendar/todo-mode-tests.el
@@ -934,5 +934,70 @@ since all non-initial item lines must begin with 
whitespace."
      (insert (concat "\n" item1))
      (should-error (todo-edit-quit) :type 'user-error))))
 
+(ert-deftest todo-test-item-insertion-with-priority-1 ()
+  "Test inserting new item when point is not on a todo item.
+When point is on the empty line at the end of the todo items
+section, insertion with priority setting should succeed."
+  (with-todo-test
+   (todo-test--show 1)
+   (goto-char (point-max))
+   ;; Now point should not be on a todo item.
+   (should-not (todo-item-start))
+   (let ((item "Point was on empty line at end of todo items section."))
+     (todo-test--insert-item item 1)
+     ;; Move point to item that was just inserted.
+     (goto-char (point-min))
+     (re-search-forward (concat todo-date-string-start todo-date-pattern
+                               (regexp-quote todo-nondiary-end) " ")
+                       (pos-eol) t)
+     (should (looking-at (regexp-quote item))))))
+
+(ert-deftest todo-test-item-insertion-with-priority-2 ()
+  "Test inserting new item when point is not on a todo item.
+When point is on the empty line at the end of the done items
+section, insertion with priority setting should succeed."
+  (with-todo-test
+   (todo-test--show 1)
+   (goto-char (point-max))
+   ;; See comment about recentering in todo-test-raise-lower-priority.
+   (set-window-buffer nil (current-buffer))
+   (todo-toggle-view-done-items)
+   (todo-next-item)
+   (goto-char (point-max))
+   ;; Now point should be at end of done items section, so not be on a
+   ;; todo item.
+   (should (todo-done-item-section-p))
+   (should-not (todo-item-start))
+   (let ((item "Point was on empty line at end of done items section."))
+     (todo-test--insert-item item 1)
+     ;; Move point to item that was just inserted.
+     (goto-char (point-min))
+     (re-search-forward (concat todo-date-string-start todo-date-pattern
+                               (regexp-quote todo-nondiary-end) " ")
+                       (pos-eol) t)
+     (should (looking-at (regexp-quote item))))))
+
+(ert-deftest todo-test-item-insertion-with-priority-3 ()
+  "Test inserting new item when point is not on a todo item.
+When point is on a done item, insertion with priority setting
+should succeed."
+  (with-todo-test
+   (todo-test--show 1)
+   (goto-char (point-max))
+   ;; See comment about recentering in todo-test-raise-lower-priority.
+   (set-window-buffer nil (current-buffer))
+   (todo-toggle-view-done-items)
+   (todo-next-item)
+   ;; Now point should be on first done item.
+   (should (and (todo-item-start) (todo-done-item-section-p)))
+   (let ((item "Point was on a done item."))
+     (todo-test--insert-item item 1)
+     ;; Move point to item that was just inserted.
+     (goto-char (point-min))
+     (re-search-forward (concat todo-date-string-start todo-date-pattern
+                               (regexp-quote todo-nondiary-end) " ")
+                       (pos-eol) t)
+     (should (looking-at (regexp-quote item))))))
+
 (provide 'todo-mode-tests)
 ;;; todo-mode-tests.el ends here