GNU bug report logs - #2085
23.0.60; todo-add-category bugs + patch

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 2085 in the body.
You can then email your comments to 2085 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Stephen Berman <stephen.berman <at> gmx.net>
To: emacs-pretest-bug <at> gnu.org
Subject: 23.0.60; todo-add-category bugs + patch
Date: Tue, 27 Jan 2009 16:34:12 +0100

[Message part 1 (text/plain, inline)]

The following bug, reported in
http://lists.gnu.org/archive/html/bug-gnu-emacs/2001-12/msg00198.html, still
exists in CVS:

From: Klaus Zeitler <kzeitler <at> lucent.com>
Subject: todo-mode error "todo-category-select: Search failed: ..." 
To: bug-gnu-emacs <at> gnu.org
Date: Fri, 7 Dec 2001 15:17:51 +0100 (MET)

> 1. start emacs with options -q --no-site-file
> 2. M-x todo-insert-item "entry1"
>    specify a category e.g. "cat1"
> 3. type s and q to save and quit
> 4. M-x todo-show
>    shows the item in category "Todo" instead of category "cat1"
>    and now entering "+" i.e. todo-forward-category results in error:
>    
>    todo-category-select: Search failed: "^\\*/\\* --- cat1$"
> 
> the todo-do file contains:
> 
> -*- mode: todo; todo-categories: ("Todo" "cat1" "Todo" "Todo"); -*-
> */* --- Todo
> */* 2001-12-07 15:08 kzeitler: entry1
> --- End
> */* ---------------------------------------------------------------------------

(The report implicitly assumes there is no Todo file ~/.todo-do.  Step 3 and
invoking todo-show in step 4 are actually unnecessary.)  In
http://lists.gnu.org/archive/html/bug-gnu-emacs/2002-01/msg00543.html I posted
a patch which fixed this bug by writing todo-file-do to file during
todo-initial-setup, but the patch was not committed to CVS.  However, it turns
out that patch was at best incomplete, since it does not prevent the following
buggy situations:

(i) If there is no Todo file, calling todo-show after todo-add-category
overwrites the effect of the latter in the displayed Todo buffer, though not
in todo-categories:

1. emacs -Q, no ~/.todo-do
2. M-x todo-add-category cat1 RET
3. M-x todo-show (without my patch: RET; with my patch: Todo RET)
4. +
=> Search failed: "^\\*/\\* --- cat1$"
The todo-do file contains:

-*- mode: todo; todo-categories: ("Todo" "cat1"); -*-
*/* --- Todo
--- End
*/* ---------------------------------------------------------------------------

(ii) If there is a Todo file, calling todo-show after todo-add-category
overwrites todo-categories, though not the category structure inserted by
todo-add-category into the Todo file -- but the new category cannot be
displayed in Todo buffer:

1. Let the Todo file ~/.todo-do consist of the following:

-*- mode: todo; todo-categories: ("Todo"); -*-
*/* --- Todo
--- End
*/* ---------------------------------------------------------------------------

2. emacs -Q
3. In *scratch*, type `M-x todo-add-category cat1 RET', answer "yes" to apply
the local variables list and accept the existing value ("Todo") of
todo-categories. 
4. Type `M-x todo-show'
=> The only category accessible by the Todo mode commands is "Todo";
~/.todo-file-do nows consists of this:

-*- mode: todo; todo-categories: ("Todo"); -*-
*/* --- cat1
--- End
*/* ---------------------------------------------------------------------------
*/* --- Todo
--- End
*/* ---------------------------------------------------------------------------

(iii) If there is no Todo file but there is a non-empty buffer ".todo-do"
whose content does not conform to the structure imposed by Todo mode,
todo-add-category will nevertheless add category structure to the top of this
buffer, leaving the existing non-Todo mode content in place.  This content is
not accessible from Todo mode commands, so it may be innocuous to ignore it.
However, todo-show explicitly prevents a similar situation, and it is easy
enough to make todo-add-category to so as well.

The attached patch contains changes to todo-add-category and todo-show to fix
these bugs (as well as the OP bug).  The patch also contains fixes for two
further problem areas with todo-add-category that I'm aware of:

(iv) When invoked interactively, todo-add-category does not leave the user in
a Todo mode buffer displaying the newly added category.  If called from
outside of Todo mode, the Todo mode buffer is not even selected.  If called
from within Todo mode, it leaves the Todo mode buffer widened, showing all the
otherwise hidden structure of Todo mode.  This does not prevent using Todo
mode commands, but it does look like a buggy user interface.

(v) todo-add-category is too permissive in what names it allows for new
categories.  It will add categories with:

(a) a name identical to that of an existing category:
1. emacs -Q, no ~/.todo-do
3. Type `M-x todo-show' to get the initial category names "Todo", then type
`s' to save the Todo file. 
4. Type `I item1' to insert "item1" into category "Todo".
5. Type `M-x todo-add-category Todo RET
=>
-*- mode: todo; todo-categories: ("Todo" "Todo"); -*-
*/* --- Todo
--- End
*/* ---------------------------------------------------------------------------
*/* --- Todo
*/* 2009-01-15 21:44 steve: item1
--- End
*/* ---------------------------------------------------------------------------

Now it is not possible by using the commands provided by Todo mode to access
the Todo entry "item1", because its category is "masked" by the new
identically named category.

(b) a name that is the empty string, by typing `M-x todo-add-category RET'.
The main problem with this is that you cannot enter the empty string in
response to a completing-read prompt: just typing RET returns the default
value, typing `C-q RET' return ^M.  So a category with this name is
inaccessible from todo-insert-item and todo-jump-to-category.

(c) a name that is a string of white space.  Such a name, unlike the empty
string, is accessible from all Todo mode commands (with completing-read by
typing e.g. `C-q SPC'); however, it is visually indistinguishable from a
category whose name is the empty string and is liable to cause confusion.
AFAICS there is no advantage to being able to have such category names.


2009-01-19  Stephen Berman  <stephen.berman <at> gmx.net>

	* calendar/todo-mode.el (todo-add-category): Change the interactive
	spec.  Signal an error if the Todo file is non-empty but contains
	no category.  Reject category names that could induce bugs and
	confusion.  Call todo-mode if the Todo file is new and unsaved.
	Simplify handling of local variables cookie.  Properly display the
	newly added category in Todo mode.
	(todo-show): Call todo-initial-setup only if there is neither a 
        Todo file nor a corresponding unsaved buffer.

[todo-add-category.diff (text/x-patch, inline)]

*** emacs/lisp/calendar/todo-mode.el.~1.72.~	2009-01-09 11:48:58.000000000 +0100
--- emacs/lisp/calendar/todo-mode.el	2009-01-21 00:39:08.000000000 +0100
***************
*** 527,554 ****
      (narrow-to-region (todo-item-start) (todo-item-end))))
  
  ;;;###autoload
! (defun todo-add-category (cat)
    "Add new category CAT to the TODO list."
!   (interactive "sCategory: ")
!   (save-window-excursion
!     (setq todo-categories (cons cat todo-categories))
!     (find-file todo-file-do)
!     (widen)
!     (goto-char (point-min))
!     (let ((posn (search-forward "-*- mode: todo; " 17 t)))
!       (if posn
!           (progn
!             (goto-char posn)
!             (kill-line))
!         (insert "-*- mode: todo; \n")
!         (forward-char -1)))
!     (insert (format "todo-categories: %S; -*-" todo-categories))
!     (forward-char 1)
!     (insert (format "%s%s%s\n%s\n%s %s\n"
!                     todo-prefix todo-category-beg cat
!                     todo-category-end
!                     todo-prefix todo-category-sep)))
!   0)
  
  ;;;###autoload
  (defun todo-add-item-non-interactively (new-item category)
--- 527,571 ----
      (narrow-to-region (todo-item-start) (todo-item-end))))
  
  ;;;###autoload
! (defun todo-add-category (&optional cat)
    "Add new category CAT to the TODO list."
!   (interactive)
!   (let ((buf (find-file-noselect todo-file-do t))
! 	(prompt "Category: "))
!     (unless (zerop (buffer-size buf))
!       (and (null todo-categories)
! 	   (null todo-cats)
! 	   (error "Error in %s: File is non-empty but contains no category" 
! 		  todo-file-do)))
!     (unless cat (setq cat (read-from-minibuffer prompt)))
!     (with-current-buffer buf
!       ;; reject names that could induce bugs and confusion
!       (while (and (cond ((string= "" cat)
! 			 (setq prompt "Enter a non-empty category name: "))
! 			((string-match "\\`\\s-+\\'" cat)
! 			 (setq prompt "Enter a category name that is not only white space: "))
! 			((member cat todo-categories)
! 			 (setq prompt "Enter a non-existing category name: ")))
! 		  (setq cat (read-from-minibuffer prompt))))
!       ;; initialize a newly created Todo buffer for Todo mode
!       (unless (file-exists-p todo-file-do) (todo-mode))
!       (setq todo-categories (cons cat todo-categories))
!       (widen)
!       (goto-char (point-min))
!       (if (search-forward "-*- mode: todo; " 17 t)
! 	  (kill-line)
! 	(insert "-*- mode: todo; \n")
! 	(forward-char -1))
!       (insert (format "todo-categories: %S; -*-" todo-categories))
!       (forward-char 1)
!       (insert (format "%s%s%s\n%s\n%s %s\n"
! 		      todo-prefix todo-category-beg cat
! 		      todo-category-end
! 		      todo-prefix todo-category-sep))
!       (if (interactive-p)
! 	  ;; properly display the newly added category
! 	  (progn (setq todo-category-number 0) (todo-show))
! 	0))))
  
  ;;;###autoload
  (defun todo-add-item-non-interactively (new-item category)
***************
*** 929,935 ****
  (defun todo-show ()
    "Show TODO list."
    (interactive)
!   (if (file-exists-p todo-file-do)
        (find-file todo-file-do)
      (todo-initial-setup))
    (if (null todo-categories)
--- 946,957 ----
  (defun todo-show ()
    "Show TODO list."
    (interactive)
!   ;; Call todo-initial-setup only if there is neither a Todo file nor
!   ;; a corresponding unsaved buffer.
!   (if (or (file-exists-p todo-file-do)
! 	  (let* ((buf (get-buffer (file-name-nondirectory todo-file-do)))
! 		 (bufname (buffer-file-name buf)))
! 	    (equal (expand-file-name todo-file-do) bufname)))
        (find-file todo-file-do)
      (todo-initial-setup))
    (if (null todo-categories)

Message #10 received at 2085-done <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Chong Yidong <cyd <at> stupidchicken.com>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 2085-done <at> debbugs.gnu.org
Subject: Re: 23.0.60; todo-add-category bugs + patch
Date: Tue, 27 Jan 2009 22:59:23 -0500

> 2009-01-19  Stephen Berman  <stephen.berman <at> gmx.net>
>
> * calendar/todo-mode.el (todo-add-category): Change the interactive
> spec.  Signal an error if the Todo file is non-empty but contains no
> category.  Reject category names that could induce bugs and confusion.
> Call todo-mode if the Todo file is new and unsaved.  Simplify handling
> of local variables cookie.  Properly display the newly added category
> in Todo mode.
> (todo-show): Call todo-initial-setup only if there is
> neither a Todo file nor a corresponding unsaved buffer.

Applied, thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.