desktop.el patch for the trunk... and 22.2?

desktop.el patch for the trunk... and 22.2?

[Juanma Barranquero]

Assuming no opposition ;-) I'll install the following patch in the trunk ASAP.

This message, however, is arguing for it to be on the 22.2 release branch.

What it does:

 - Basically, it fixes a bug where users trying to use the
`desktop-not-loaded-hook' to load a desktop find themselves, when
saving the desktop, being asked for a directory to save the desktop
file that Emacs should already know. That is:
    - desktop.el finds a locked desktop, and calls the hook
    - in the hook, the user does something to release the lock and
reload the desktop
    - when exiting Emacs, or saving the desktop, Emacs asks the user
where to save the desktop file [this is the bug]
The reason is that the `desktop-dirname' variable is being set to nil
at the wrong place (too late).

 - The bug was introduced by me (see lisp/Changelog entry of
2007-06-20) while fixing another related bug which was part of the
original commit of the desktop locking code.

 - Additionally, the warning about the desktop not being loaded is
suppressed if the hook sets `desktop-dirname'. The reason is that, if
the variable is set, the user has done something to modify it, most
likely to load a desktop, and he'll want to see messages generated by
his hook code, not a warning likely to be inaccurate at that point.
[This part of the patch is not strictly a bug fix, so it could be
dropped of the 22.2 patch]

Why do I think it merits being in the release:

- It is a bug, if a rather minor one, and the desktop locking code was
not in 22.1, so this is the first public release of that code.

- No users of a released Emacs are using the feature right now, so, no
impact, compatibility-wise.

- Also, even for users of the pretests or the CVS, using the
desktop-not-loaded-hook (to load the desktop or any other thing) is
probably quite uncommon.

- The patch is tiny, so it's not hard to review.

Thoughts?

             Juanma


2008-02-29  Juanma Barranquero  <lekktu@gmail.com>

	* desktop.el (desktop-read): Set `desktop-dirname' to nil before
	calling `desktop-not-loaded-hook' to allow modifying it.
	Don't show warning message if `desktop-dirname' was modified.


Index: lisp/desktop.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/desktop.el,v
retrieving revision 1.124
diff -u -2 -r1.124 desktop.el
--- lisp/desktop.el	29 Feb 2008 03:02:25 -0000	1.124
+++ lisp/desktop.el	29 Feb 2008 16:33:32 -0000
@@ -970,9 +970,9 @@
 		       (not (y-or-n-p (format "Warning: desktop file appears to be
in use by PID %s.\n\
 Using it may cause conflicts.  Use it anyway? " owner)))))
-	      (progn
-		(let ((default-directory desktop-dirname))
-		  (run-hooks 'desktop-not-loaded-hook))
+	      (let ((default-directory desktop-dirname))
 		(setq desktop-dirname nil)
-		(message "Desktop file in use; not loaded."))
+		(run-hooks 'desktop-not-loaded-hook)
+		(unless desktop-dirname
+		  (message "Desktop file in use; not loaded.")))
 	    (desktop-lazy-abort)
 	    ;; Evaluate desktop buffer and remember when it was modified.

Re: desktop.el patch for the trunk... and 22.2?

[Chong Yidong]

"Juanma Barranquero" <lekktu@gmail.com> writes:

> Assuming no opposition ;-) I'll install the following patch in the
> trunk ASAP.
>
> This message, however, is arguing for it to be on the 22.2 release
> branch.
>
> 2008-02-29  Juanma Barranquero  <lekktu@gmail.com>
>
> 	* desktop.el (desktop-read): Set `desktop-dirname' to nil before
> 	calling `desktop-not-loaded-hook' to allow modifying it.
> 	Don't show warning message if `desktop-dirname' was modified.

The patch looks good, so go ahead and check it into the branch.
Thanks.