From 14ae2101412d66576846d30ab32b4d5b0081383d Mon Sep 17 00:00:00 2001 From: Stephen Berman Date: Mon, 3 Jul 2023 14:19:41 +0200 Subject: [PATCH] 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 -- 2.39.2