bug#22377: 25.0.50; Issues involving desktop.el

[Stephen Berman]

0. Make sure there is no .emacs.desktop file that will be found by
   desktop-read.
1. emacs -Q
2. M-x todo-show to visit an existing Todo file or create a new one.
3. M-x desktop-save, use defaults.
4. C-x C-c
5. emacs -Q
6. M-x desktop-read
=> The Todo file is displayed in Todo mode but is not narrowed to the
   current category as it should be, and the echo area displays this
   message: "Desktop: 1 frame, 0 buffers restored, 1 failed to restore."

tl;dr: The patch below fix this; the following paragraphs explain the
issues in more detail and justify the fix (also for doc-view).

In addition to the echo area message, the *Messages* buffer contains
this error message: "Desktop: Can’t load buffer Bla.todo: Wrong type
argument: number-or-marker-p, nil".  This comes from
desktop-create-buffer and stepping through it (or setting debug-on-error
between steps 5 and 6 above) shows that the error actually comes from
todo-current-category and is raised because todo-category-number, which
is set by the desktop buffer mode handler todo-restore-desktop-buffer,
is nil instead of a number.  The value of todo-category-number is
supposed to be written to the .emacs.desktop file by calling
todo-desktop-save-buffer, which is the buffer-local value of
desktop-save-buffer in Todo mode -- but only when desktop-save-mode is
non-nil.  However, nothing prohibits invoking desktop-save when
desktop-save-mode is nil, which in the above recipe results in the error
observed.

The simplest fix for this problem is to remove the conditioning in
todo-mode.el on setting desktop-save-buffer.  In fact, there are only a
handful of libraries in Emacs that set desktop-save-buffer (rmail, eww,
dired, info, vc-dir, mh-folder, doc-view, and todo-mode) and all but two
set it unconditionally -- the exceptions are todo-mode and doc-view (and
the former is because I took doc-view as a model when I added desktop
support to todo-mode in response to bug#15225).  And indeed, doc-view
has the same type of desktop restoration issue as todo-mode: in the
above recipe, replace step 2 by visiting e.g. some multipage PDF file,
scroll to e.g. page 5, then continue the recipe with step 3; after step
6, the PDF file is restored to page 1 instead of page 5, and the same
message as above is shown in the echo area.  Removing the conditioning
on setting desktop-save-buffer in doc-view.el solves the wrong page
display.

Both doc-view and, following it, todo-mode not only condition setting
desktop-save-buffer on non-nil desktop-save-mode but also on this
variable not being void.  But isn't that only needed if these modes were
invoked in an Emacs lacking desktop.el?  At least the current version of
todo-mode.el won't work in an Emacs old enough not to have desktop.el.
And since the other libraries mentioned above do not have this
condition, it seems unnecessary (dired does have `(eval-when-compile
(require 'desktop))' but I wonder if that is still needed; and
mh-folder.el also has similar checks but I think it's intended to work
in older Emacsen).  In any case, simply removing the entire condition on
desktop-save-buffer from both doc-view and todo-mode does not make the
byte compiler complain.

There still remains the issue of the message "Desktop: 1 frame, 0
buffers restored, 1 failed to restore."  It is wrong, because the buffer
visiting the Todo file (or the PDF file in the doc-view case) is
restored, though incorrectly displayed.  In fact, removing the condition
on desktop-save-buffer fixes the display but does not eliminate the
erroneous message.  This comes from desktop-read and is triggered by
incrementing desktop-buffer-fail-count in desktop-create-buffer.  This
happens because the todo-mode and doc-view desktop buffer mode handlers
do not return a buffer.  The doc string of desktop-buffer-mode-handlers
seems to allow this, but the result will invariably be a message about
failing to restore buffers, which is clearly wrong for todo-mode and
doc-view.  The fix in both case is simply to have the handlers in these
modes return the current buffer, which is what the desktop buffer mode
handlers in the other libraries with desktop support do.  I can't think
of a successful use of a desktop buffer mode handler that fails to
restore a buffer; if there isn't one, it would be better for the doc
string of desktop-buffer-mode-handlers to explicitly say that the
handler should return a buffer.

The patch below fixes the display issues and the erroneous message in
todo-mode and doc-view.  Although I maintain only the former code, I'm
pretty sure the fix is appropriate for the latter as well, so if there
are no objections to the patch within a few days or so, I'll commit it
to emacs-25.  (I'll leave the desktop-buffer-mode-handlers doc string
issue to someone more familiar with that library.)


In GNU Emacs 25.0.50.4 (x86_64-suse-linux-gnu, GTK+ Version 3.14.15)
 of 2016-01-15 built on rosalinde
Repository revision: 016b3d5894b8c424eab262aeefc646c6cd03a70a
Windowing system distributor 'The X.Org Foundation', version 11.0.11601000
System Description:     openSUSE 13.2 (Harlequin) (x86_64)


diff --git a/lisp/calendar/todo-mode.el b/lisp/calendar/todo-mode.el
index 353ca69..ebf0a82 100644
--- a/lisp/calendar/todo-mode.el
+++ b/lisp/calendar/todo-mode.el
@@ -5230,7 +5230,8 @@ todo-restore-desktop-buffer
   (with-current-buffer buffer
     (widen)
     (let ((todo-category-number (cdr (assq 'catnum misc))))
-      (todo-category-select))))
+      (todo-category-select)
+      (current-buffer))))
 
 (add-to-list 'desktop-buffer-mode-handlers
             '(todo-mode . todo-restore-desktop-buffer))
@@ -6579,8 +6580,7 @@ todo-modes-set-2
   "Make some settings that apply to multiple Todo modes."
   (add-to-invisibility-spec 'todo)
   (setq buffer-read-only t)
-  (when (and (boundp 'desktop-save-mode) desktop-save-mode)
-    (setq-local desktop-save-buffer 'todo-desktop-save-buffer))
+  (setq-local desktop-save-buffer 'todo-desktop-save-buffer)
   (when (boundp 'hl-line-range-function)
     (setq-local hl-line-range-function
                (lambda() (save-excursion
diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 2868113..06cf8dc 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -1714,7 +1714,8 @@ doc-view-restore-desktop-buffer
       ;; window-parameters in the window-state(s) and then restoring this
       ;; window-state should call us back (to interpret/use those parameters).
       (doc-view-goto-page page)
-      (when slice (apply 'doc-view-set-slice slice)))))
+      (when slice (apply 'doc-view-set-slice slice))
+      (current-buffer))))
 
 (add-to-list 'desktop-buffer-mode-handlers
             '(doc-view-mode . doc-view-restore-desktop-buffer))
@@ -1788,9 +1789,7 @@ doc-view-mode
              nil t)
     (add-hook 'clone-indirect-buffer-hook 'doc-view-clone-buffer-hook nil t)
     (add-hook 'kill-buffer-hook 'doc-view-kill-proc nil t)
-    (when (and (boundp 'desktop-save-mode)
-              desktop-save-mode)
-      (setq-local desktop-save-buffer 'doc-view-desktop-save-buffer))
+    (setq-local desktop-save-buffer 'doc-view-desktop-save-buffer)
 
     (remove-overlays (point-min) (point-max) 'doc-view t) ;Just in case.
     ;; Keep track of display info ([vh]scroll, page number, overlay,