GNU bug report logs - #2086
23.0.60; Todo mode bug with completing-read + 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 2086 in the body.
You can then email your comments to 2086 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 mode bug with completing-read + patch
Date: Tue, 27 Jan 2009 16:48:11 +0100

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

GNU Emacs 23.0.60.30 (i686-pc-linux-gnu, GTK+ Version 2.14.4) of
2009-01-21 on escher

1. emacs -Q
2. Type `M-x todo-show', and in the Todo mode buffer type `s' to save
~/.todo-do, then type the following sequence of Todo mode commands and input:
`I item1 RET i item2 RET Newcat RET'.  Widen the buffer and you see the
following (modulo the time-stamp headers):

-*- mode: todo; todo-categories: ("Newcat" "Todo"); -*-
*/* --- Newcat
*/* 2009-01-14 13:48 steve: item2
--- End
*/* ---------------------------------------------------------------------------
*/* --- Todo
*/* 2009-01-14 13:48 steve: item1
--- End
*/* ---------------------------------------------------------------------------
   
3. Now type `M-x customize-option RET history-delete-duplicates RET', toggle
   the value to `on (non-nil)' and save for the current session.
4. Type `M-x todo-show'.
5. In the Todo mode buffer, with category "Newcat" displayed, type `j' and at
   the prompt, type `Todo', to jump the the category "Todo".
=> Now category "Todo" is displayed, but it is empty.
6. Widen the Todo mode buffer, and this is what it now looks like:

-*- mode: todo; todo-categories: ("Todo" "Newcat"); -*-
*/* --- Todo
--- End
*/* ---------------------------------------------------------------------------
*/* --- Newcat
*/* 2009-01-14 13:48 steve: item2
--- End
*/* ---------------------------------------------------------------------------
*/* --- Todo
*/* 2009-01-14 13:48 steve: item1
--- End
*/* ---------------------------------------------------------------------------

As a second example, after step 4 above do the following:
5'. While in category "Newcat", type `i item3 RET Todo RET' to insert "item3".
=> Now category "Todo" is displayed, but it contains only item3, not also
item1.
6. Widen the Todo mode buffer, and this is what it now looks like:

-*- mode: todo; todo-categories: ("Todo" "Newcat"); -*-
*/* --- Todo
*/* 2009-01-14 13:48 steve: item3
--- End
*/* ---------------------------------------------------------------------------
*/* --- Newcat
*/* 2009-01-14 13:48 steve: item2
--- End
*/* ---------------------------------------------------------------------------
*/* --- Todo
*/* 2009-01-14 13:48 steve: item1
--- End
*/* ---------------------------------------------------------------------------


The problem in both cases is due, I believe, to the failure to make a copy of
todo-categories before calling completing-read and to restore it afterwards.
This is because, when history-delete-duplicates is non-nil, completing-read
(via read_minibuf) destructively deletes the list element used as the
completion and conses it on to the front, yielding a reordered list.  The fix
I propose, in the attached patch, is a wrapper for completing-read that copies
and restores todo-categories as required.  The wrapper is used instead of
completing-read by todo-insert-item and todo-jump-to-category.  (The resulting
modularization also implements one to the "things to do" mentioned in the
todo-mode.el commentary.)

I also think the completing-read code in todo-mode.el should be simplified by
replacing todo-category-alist.  The doc string of the latter says: "Generate
an alist for use in `completing-read' from `todo-categories'."  AFAICS there
is no need for this structure.  The members of the alist, which constitutes
the COLLECTION argument of completing-read, are just the single element lists
of each category name.  This provides no more information than the value of
todo-categories itself, which as a list of strings is suitable for the
COLLECTION argument.  (Was there a time when completing-read took an alist but
not a list of strings as the COLLECTION argument?)  The wrapper uses
todo-categories instead of todo-category-alist, so the patch eliminates the
latter function from todo-mode.el.

Finally, the patch makes the code of todo-insert-item conform to its doc
string.  The latter says: "With a prefix argument solicit the category,
otherwise use the current category."  But the code does just the opposite.  I
assume the desired behavior is as stated in the doc string.  (In
http://lists.gnu.org/archive/html/bug-gnu-emacs/2002-01/msg00543.html there
was a patch that contained a fix for this, but it was not committed to CVS.)


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

	* calendar/todo-mode.el (todo-category-alist): Delete.
	(todo-completing-read): New function.
	(todo-insert-item, todo-jump-to-category): Use it.
        (todo-insert-item): Make the use of the prefix argument
	conform to the doc string. 
	

[todo-completing-read.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 10:36:15.000000000 +0100
***************
*** 596,610 ****
  			      "New TODO entry: "
  			      (if todo-entry-prefix-function
  				  (funcall todo-entry-prefix-function)))))
- 	   (categories todo-categories)
- 	   (history (cons 'categories (1+ todo-category-number)))
  	   (current-category (nth todo-category-number todo-categories))
! 	   (category
! 	    (if arg
! 		current-category
! 	      (completing-read (concat "Category [" current-category "]: ")
! 			       (todo-category-alist) nil nil nil
! 			       history current-category))))
        (todo-add-item-non-interactively new-item category))))
  
  (defalias 'todo-cmd-inst 'todo-insert-item)
--- 596,603 ----
  			      "New TODO entry: "
  			      (if todo-entry-prefix-function
  				  (funcall todo-entry-prefix-function)))))
  	   (current-category (nth todo-category-number todo-categories))
! 	   (category (if arg (todo-completing-read) current-category)))
        (todo-add-item-non-interactively new-item category))))
  
  (defalias 'todo-cmd-inst 'todo-insert-item)
***************
*** 801,812 ****
  (defun todo-jump-to-category ()
    "Jump to a category.  Default is previous category."
    (interactive)
!   (let* ((categories todo-categories)
!          (history (cons 'categories (1+ todo-category-number)))
! 	 (default (nth todo-category-number todo-categories))
! 	 (category (completing-read
!                     (concat "Category [" default "]: ")
!                     (todo-category-alist) nil nil nil history default)))
      (if (string= "" category)
          (setq category (nth todo-category-number todo-categories)))
      (setq todo-category-number
--- 794,800 ----
  (defun todo-jump-to-category ()
    "Jump to a category.  Default is previous category."
    (interactive)
!   (let ((category (todo-completing-read)))
      (if (string= "" category)
          (setq category (nth todo-category-number todo-categories)))
      (setq todo-category-number
***************
*** 861,869 ****
    "Return non-nil if STRING spans several lines."
    (> (todo-string-count-lines string) 1))
  
! (defun todo-category-alist ()
!   "Generate an alist for use in `completing-read' from `todo-categories'."
!   (mapcar #'list todo-categories))
  
  ;; ---------------------------------------------------------------------------
  
--- 849,867 ----
    "Return non-nil if STRING spans several lines."
    (> (todo-string-count-lines string) 1))
  
! (defun todo-completing-read ()
!   "Return a category name, with completion, for use in Todo mode."
!   ;; make a copy of todo-categories in case history-delete-duplicates is
!   ;; non-nil, which makes completing-read alter todo-categories
!   (let* ((categories (copy-sequence todo-categories))
!          (history (cons 'todo-categories (1+ todo-category-number)))
! 	 (default (nth todo-category-number todo-categories))
! 	 (category (completing-read
!                     (concat "Category [" default "]: ")
!                     todo-categories nil nil nil history default)))
!     ;; restore the original value of todo-categories
!     (setq todo-categories categories)
!     category))
  
  ;; ---------------------------------------------------------------------------
  

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

From: Chong Yidong <cyd <at> stupidchicken.com>
To: Stephen Berman <stephen.berman <at> gmx.net>
Cc: 2086-done <at> debbugs.gnu.org
Subject: Re: 23.0.60; Todo mode bug with completing-read + patch
Date: Tue, 27 Jan 2009 23:00:37 -0500

> The fix I propose, in the attached patch, is a wrapper for
> completing-read that copies and restores todo-categories as required.
> The wrapper is used instead of completing-read by todo-insert-item and
> todo-jump-to-category.  (The resulting modularization also implements
> one to the "things to do" mentioned in the todo-mode.el commentary.)

Applied, thanks.

Previous Next

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