bug#4820: 23.1.50; [PATCH] todo-mode.el

[Stephen Berman <stephen.berman@gmx.net>]

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

Here are two bugs in todo-mode.el; a patch is attached.

1. emacs -Q with no existing file as the value of todo-file-do
2. M-x todo-show
3. Type `e' (todo-edit-item)
=> Args out of range: 60, 61

This is because there is no todo item, so when todo-edit-item calls
todo-item-string, todo-item-start returns (point-min), which is 61, and
todo-item-end returns (1- (line-beginning-position)), i.e. 60, which
raises the args-out-of-range error on buffer-substring in
todo-item-string.  The attached fix to todo-edit-item checks whether
there is a todo item to edit and throws an error if not.  This is
consistent with other Todo mode commands, e.g. todo-{raise, lower,
delete, file}-item.

1. emacs -Q
2. M-x todo-show
3. Type `i' and at the prompt a todo item, e.g "test", then RET to
   display the item in the TODO buffer in the category "Todo"; note that
   narrowing is in effect, as required by Todo mode.
4. Type `t' to display the Todo top priorities list in a window below
   the window displaying the TODO buffer.
5. In the TODO buffer type `E' on the item to put it in Todo Edit mode
   in the buffer *TODO Edit*.
6. Type C-x k to kill *TODO Edit* and return to the TODO buffer, which
   remains unchanged.
7. In the TODO buffer type `t' again.
=> Now narrowing is no longer in effect in the TODO buffer.

This is because todo-top-priorities, although it calls widen within
save-restriction, subsequently calls set-buffer, with the result, after
the call to make-indirect-buffer in todo-edit-multiline (step 5 above),
that narrowing is not restored.  The causal chain here is not quite
clear to me: in (elisp)Current Buffer there is a warning to use
set-buffer within save-current-buffer or save-excursion to guarantee
restoration, and in fact in todo-top-priorities set-buffer is within
save-excursion, but that is outside of the save-restriction.  If the
order of save-excursion and save-restriction is switched, then narrowing
is correctly restored after step 7 above; however, this order is
discouraged in (elisp)Narrowing.  But putting save-current-buffer
between save-restriction and set-buffer also restores narrowing, so this
is what the attached patch does.  (I don't know why the failure to
restore narrowing is conditioned by make-indirect-buffer; I asked about
this on emacs-devel but have not yet gotten any response.)  In addition,
the use of save-excursion appears to be gratuitous here: point is
already moved by the preceding call to todo-show, and subsequent code
concerns the temporary top priorities buffer.  But moving todo-show
inside save-excursion does prevent point from being relocated, so I made
this change as well in the patch.


2009-10-28  Stephen Berman  <stephen.berman@gmx.net>

	* calendar/todo-mode.el (todo-edit-item): Signal an error if there
	is no item to edit. (Bug#XXXX)
	(todo-top-priorities): Restore point and restore narrowing in Todo
	buffer. (Bug#XXXX)


[-- Attachment #2: todo-mode.el patch --]
[-- Type: text/plain, Size: 4322 bytes --]

*** emacs/lisp/calendar/todo-mode.el.~1.76.~	2009-10-28 11:07:53.000000000 +0100
--- emacs/lisp/calendar/todo-mode.el	2009-10-28 12:55:58.000000000 +0100
***************
*** 505,518 ****
  (defun todo-edit-item ()
    "Edit current TODO list entry."
    (interactive)
!   (let ((item (todo-item-string)))
!     (if (todo-string-multiline-p item)
!         (todo-edit-multiline)
!       (let ((new (read-from-minibuffer "Edit: " item)))
!         (todo-remove-item)
!         (insert new "\n")
!         (todo-backward-item)
!         (message "")))))
  (defalias 'todo-cmd-edit 'todo-edit-item)
  
  (defun todo-edit-multiline ()
--- 505,520 ----
  (defun todo-edit-item ()
    "Edit current TODO list entry."
    (interactive)
!   (if (< (point-min) (point-max))
!       (let ((item (todo-item-string)))
! 	(if (todo-string-multiline-p item)
! 	    (todo-edit-multiline)
! 	  (let ((new (read-from-minibuffer "Edit: " item)))
! 	    (todo-remove-item)
! 	    (insert new "\n")
! 	    (todo-backward-item)
! 	    (message ""))))
!     (error "No TODO list entry to edit")))
  (defalias 'todo-cmd-edit 'todo-edit-item)
  
  (defun todo-edit-multiline ()
***************
*** 745,778 ****
                        (regexp-quote todo-prefix) " " todo-category-sep "\n")
              (concat todo-category-end "\n"))))
          beg end)
-     (todo-show)
      (save-excursion
        (save-restriction
!         (widen)
!         (copy-to-buffer todo-print-buffer-name (point-min) (point-max))
!         (set-buffer todo-print-buffer-name)
!         (goto-char (point-min))
!         (when (re-search-forward (regexp-quote todo-header) nil t)
! 	  (beginning-of-line 1)
! 	  (delete-region (point) (line-end-position)))
!         (while (re-search-forward       ;Find category start
!                 (regexp-quote (concat todo-prefix todo-category-beg))
!                 nil t)
!           (setq beg (+ (line-end-position) 1)) ;Start of first entry.
!           (re-search-forward cat-end nil t)
!           (setq end (match-beginning 0))
!           (replace-match todo-category-break)
!           (narrow-to-region beg end)    ;In case we have too few entries.
!           (goto-char (point-min))
!           (if (zerop nof-priorities)      ;Traverse entries.
!               (goto-char end)            ;All entries
!             (todo-forward-item nof-priorities))
!           (setq beg (point))
!           (delete-region beg end)
!           (widen))
!         (and (looking-at "\f") (replace-match "")) ;Remove trailing form-feed.
!         (goto-char (point-min))         ;Due to display buffer
!         ))
      ;; Could have used switch-to-buffer as it has a norecord argument,
      ;; which is nice when we are called from e.g. todo-print.
      ;; Else we could have used pop-to-buffer.
--- 747,781 ----
                        (regexp-quote todo-prefix) " " todo-category-sep "\n")
              (concat todo-category-end "\n"))))
          beg end)
      (save-excursion
+       (todo-show)
        (save-restriction
! 	(save-current-buffer
! 	  (widen)
! 	  (copy-to-buffer todo-print-buffer-name (point-min) (point-max))
! 	  (set-buffer todo-print-buffer-name)
! 	  (goto-char (point-min))
! 	  (when (re-search-forward (regexp-quote todo-header) nil t)
! 	    (beginning-of-line 1)
! 	    (delete-region (point) (line-end-position)))
! 	  (while (re-search-forward       ;Find category start
! 		  (regexp-quote (concat todo-prefix todo-category-beg))
! 		  nil t)
! 	    (setq beg (+ (line-end-position) 1)) ;Start of first entry.
! 	    (re-search-forward cat-end nil t)
! 	    (setq end (match-beginning 0))
! 	    (replace-match todo-category-break)
! 	    (narrow-to-region beg end)    ;In case we have too few entries.
! 	    (goto-char (point-min))
! 	    (if (zerop nof-priorities)      ;Traverse entries.
! 		(goto-char end)            ;All entries
! 	      (todo-forward-item nof-priorities))
! 	    (setq beg (point))
! 	    (delete-region beg end)
! 	    (widen))
! 	  (and (looking-at "\f") (replace-match "")) ;Remove trailing form-feed.
! 	  (goto-char (point-min))         ;Due to display buffer
! 	  )))
      ;; Could have used switch-to-buffer as it has a norecord argument,
      ;; which is nice when we are called from e.g. todo-print.
      ;; Else we could have used pop-to-buffer.