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

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

[Stephen Berman]

[-- 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.

bug#4820: marked as done (23.1.50; [PATCH] todo-mode.el)

[Emacs bug Tracking System]

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

Your message dated Fri, 30 Oct 2009 22:23:29 -0400
with message-id <k5hbtg9u8u.fsf@fencepost.gnu.org>
and subject line Re: bug#4820: 23.1.50; [PATCH] todo-mode.el
has caused the Emacs bug report #4820,
regarding 23.1.50; [PATCH] todo-mode.el
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com
immediately.)


-- 
4820: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4820
Emacs Bug Tracking System
Contact owner@emacsbugs.donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 9548 bytes --]

[-- Attachment #2.1.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.1.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.

[-- Attachment #3: Type: message/rfc822, Size: 1554 bytes --]

From: Glenn Morris <rgm@gnu.org>
To: 4820-done@emacsbugs.donarmstrong.com
Subject: Re: bug#4820: 23.1.50; [PATCH] todo-mode.el
Date: Fri, 30 Oct 2009 22:23:29 -0400
Message-ID: <k5hbtg9u8u.fsf@fencepost.gnu.org>


Thanks; applied.

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

[Stefan Monnier]

> 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

I've installed a different fix, which may not result in a clean error
message, but was needed for other reasons anyway.

> 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.

Thank you.  I believe the fix to save-restriction I just installed
should make this workaround unnecessary.


        Stefan