bug#20832: 25.0.50; todo-show accidentally deleted my todo file.

[Stephen Berman]

On Wed, 17 Jun 2015 07:39:59 +0200 Nicolas Richard <youngfrog@members.fsf.org> 
wrote:

> I managed to lose my todo-mode.el todo file. I don't really know how
> this happened, I guess it was a combination of showing the todo file
> while in the minibuffer and hitting C-g, but I noticed it too late, and
> then could not reproduce.

Thanks for the report.  I can reproduce something that seems like what
you describe; here's the recipe:

0. emacs -Q
   M-x global-set-key RET C-c t RET todo-show RET
1. Type `M-x' to enter the minibuffer. 
2. If I now try to call todo-show by again typing `M-x', it fails with
   the error "Command attempted to use minibuffer while in minibuffer".
   However, todo-show does not require minibuffer input, so when I type
   `C-c t' it succeeds and displays the current todo category in the
   minibuffer.
3. Now I cannot exit the minibuffer by typing `C-g', no matter how many
   times.  However, `ESC ESC ESC' (keyboard-escape-quit) does exit the
   minibuffer (by calling abort-recursive-edit), and switching to the
   buffer that was visiting the todo file, it is indeed modified and
   empty (it just displays the overlays of the item numbers) and no
   longer in todo-mode but instead minibuffer-inactive-mode.  (I think
   this is due to read_minibuf_unwind in minibuf.c, which "is called on
   exiting minibuffer, whether normally or not" and deliberately erases
   the minibuffer (but I could not find a call-chain from
   Fabort_recursive_edit to read_minibuf_unwind).)
4. Although the former todo-mode buffer is empty, the todo file is not,
   so killing the buffer leaves the file unchanged.  In addition, by
   typing `C-x C-q' to make the buffer writeable, you can undo the
   deletion and restore the buffer contents.
5. However, invoking todo-show in the empty former todo-mode buffer is
   fatal: it prompts for a Todo category, and supplying one overwrites
   the todo file, while canceling with `C-g' deletes the file (needed
   when invoking todo-add-file and canceling before adding the first
   category, in order to avoid having a todo file with an illegitimate
   format).

Can you remember if this is what happened to you?  Note that even after
the todo file is overwritten or deleted in step 5, you can still recover
the deleted buffer contents as explained in step 4.

> In any case, I suspect that todo-mode.el itself deleted it, so I suggest
> the following patch as a safety measure.

This seems reasonable as a safeguard, but I wonder whether there are
other circumstances where it is needed.  If the above recipe is the only
trigger, then I think it should be avoided by prohibiting entering
todo-mode in the minibuffer (patch below), which would make the
safeguard superfluous for the above recipe.  But if there are other
triggers, then your patch may be needed.  However, since I haven't found
any other way to make the problem you encountered occur, I'm inclined to
install just the patch below; that way, if there is some other way to
get an empty but modified todo-mode buffer, we may find it and be able
to fix the underlying cause.  What do you think?

Incidentally, if `q' (todo-quit) is typed between steps 2 and 3 above,
then the minibuffer becomes occupied by whatever other-buffer returns,
and then typing `ESC ESC ESC' will result in that buffer being erased.
So this is another reason to make sure todo-mode stays out of the
minibuffer.

The following patch makes todo-show display the todo file in the
previous window when invoked in the minibuffer.  An alternative would be
to simply error out when invoking todo-show in the minibuffer.  But I
think it could be useful to display the todo file while in the
minibuffer.  Do you see a problem with this?

Steve Berman

diff --git a/lisp/calendar/todo-mode.el b/lisp/calendar/todo-mode.el
index dcc960f..3b8d8ab 100644
--- a/lisp/calendar/todo-mode.el
+++ b/lisp/calendar/todo-mode.el
@@ -732,42 +732,44 @@ corresponding todo file, displaying the corresponding 
category."
        (when (or (member file todo-visited)
                  (eq todo-show-first 'first))
          (unless (todo-check-file file) (throw 'end nil))
-         (set-window-buffer (selected-window)
-                            (set-buffer (find-file-noselect file 'nowarn)))
-         (if (equal (file-name-extension (buffer-file-name)) "toda")
-             (unless (derived-mode-p 'todo-archive-mode) (todo-archive-mode))
-           (unless (derived-mode-p 'todo-mode) (todo-mode)))
-         ;; When quitting an archive file, show the corresponding
-         ;; category in the corresponding todo file, if it exists.
-         (when (assoc cat todo-categories)
-           (setq todo-category-number (todo-category-number cat)))
-         ;; If this is a new todo file, add its first category.
-         (when (zerop (buffer-size))
-           (let (cat-added)
-             (unwind-protect
-                 (setq todo-category-number
-                       (todo-add-category todo-current-todo-file "")
-                       add-item todo-add-item-if-new-category
-                       cat-added t)
-               (if cat-added
-                   ;; If the category was added, save the file now, so we
-                   ;; don't risk having an empty todo file, which would
-                   ;; signal an error if we tried to visit it later,
-                   ;; since doing that looks for category boundaries.
-                   (save-buffer 0)
-                 ;; If user cancels before adding the category, clean up
-                 ;; and exit, so we have a fresh slate the next time.
-                 (delete-file file)
-                 ;; (setq todo-files (funcall todo-files-function))
-                 (setq todo-files (delete file todo-files))
-                 (when first-file
-                   (setq todo-default-todo-file nil
-                         todo-current-todo-file nil)
-                   (todo-reevaluate-default-file-defcustom))
-                 (kill-buffer)
-                 (keyboard-quit)))))
-         (save-excursion (todo-category-select))
-         (when add-item (todo-insert-item--basic)))
+         (with-selected-window (if (minibufferp) (previous-window)
+                                 (selected-window))
+           (set-window-buffer (selected-window)
+                              (set-buffer (find-file-noselect file 'nowarn)))
+           (if (equal (file-name-extension (buffer-file-name)) "toda")
+               (unless (derived-mode-p 'todo-archive-mode) (todo-archive-mode))
+             (unless (derived-mode-p 'todo-mode) (todo-mode)))
+           ;; When quitting an archive file, show the corresponding
+           ;; category in the corresponding todo file, if it exists.
+           (when (assoc cat todo-categories)
+             (setq todo-category-number (todo-category-number cat)))
+           ;; If this is a new todo file, add its first category.
+           (when (zerop (buffer-size))
+             (let (cat-added)
+               (unwind-protect
+                   (setq todo-category-number
+                         (todo-add-category todo-current-todo-file "")
+                         add-item todo-add-item-if-new-category
+                         cat-added t)
+                 (if cat-added
+                     ;; If the category was added, save the file now, so we
+                     ;; don't risk having an empty todo file, which would
+                     ;; signal an error if we tried to visit it later,
+                     ;; since doing that looks for category boundaries.
+                     (save-buffer 0)
+                   ;; If user cancels before adding the category, clean up
+                   ;; and exit, so we have a fresh slate the next time.
+                   (delete-file file)
+                   ;; (setq todo-files (funcall todo-files-function))
+                   (setq todo-files (delete file todo-files))
+                   (when first-file
+                     (setq todo-default-todo-file nil
+                           todo-current-todo-file nil)
+                     (todo-reevaluate-default-file-defcustom))
+                   (kill-buffer)
+                   (keyboard-quit)))))
+           (save-excursion (todo-category-select))
+           (when add-item (todo-insert-item--basic))))
        (setq todo-show-first show-first)
        (add-to-list 'todo-visited file)))))