bug#4061: 23.1.50; C-x C-v and saveplace

[Karl Fogel <kfogel@red-bean.com>]

I know what causes this now.  saveplace.el works like this:

  (add-hook 'kill-buffer-hook 'save-place-to-alist)

Now, `save-place-to-alist' checks `buffer-file-name' and (properly) does
nothing if there is no associated file.  Since `find-alternate-file'
unsets `buffer-file-name' after renaming the old buffer but before
killing it, that effectively makes `save-place-to-alist' a no-op in the
old buffer.

It's not even clear what the most desirable behavior is.  For example,
in `find-alternate-file' (without my patch), if the old buffer is
modified, should we still save place before killing it?  I think so; or
rather, I think we should do whatever saveplace.el does if one kills a
modified buffer the normal way.

I'm still thinking.  My patch below isn't really the right thing (see
below for why), but I wanted to record this all here to remember it.

[[[
* emacs/emacs-cvs/lisp/files.el
  (find-alternate-file): Restore certain state in the old buffer
    before killing it, so that hooks behave as expected.  This addresses
    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4061.

NOTE: DRAFT PATCH ONLY, DO NOT COMMIT.  With this patch, doing C-x C-v
in a modified buffer visiting a file causes the user to be prompted to
save buffer " **lose**" (see files.el:find-alternate-file for why)
after they have successfully found their new file.  That is hardly a
desirable behavior.

I will post for others' thoughts on whether the original bug is a bug,
and if it is what is the best way to fix it.
]]]

[[[
* emacs/emacs-cvs/lisp/files.el
  (find-alternate-file): Restore certain state in the old buffer
    before killing it, so that hooks behave as expected.  This addresses
    http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4061.

NOTE: DRAFT PATCH ONLY, DO NOT COMMIT.  With this patch, doing C-x C-v
in a modified buffer visiting a file causes the user to be prompted to
save buffer " **lose**" (see files.el:find-alternate-file for why)
after they have successfully found their new file.  That is hardly a
desirable behavior.

I will post for others' thoughts on whether the original bug is a bug,
and if it is what is the best way to fix it.
]]]

Index: lisp/files.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/files.el,v
retrieving revision 1.1076
diff -u -r1.1076 files.el
--- lisp/files.el	4 Sep 2009 03:18:11 -0000	1.1076
+++ lisp/files.el	4 Sep 2009 21:30:00 -0000
@@ -1507,17 +1507,24 @@
 	  ;; Likewise for dired buffers.
 	  (setq dired-directory nil)
 	  (find-file filename wildcards))
-      (when (eq obuf (current-buffer))
-	;; This executes if find-file gets an error
-	;; and does not really find anything.
-	;; We put things back as they were.
-	;; If find-file actually finds something, we kill obuf below.
-	(setq buffer-file-name ofile)
-	(setq buffer-file-number onum)
-	(setq buffer-file-truename otrue)
-	(setq dired-directory odir)
-	(lock-buffer)
-	(rename-buffer oname)))
+      (progn
+        ;; There's some state that we want to restore in obuf before
+        ;; we kill obuf, whether find-file succeeded or not.  For
+        ;; example, we restore buffer-file-name so that certain hooks
+        ;; (e.g., 'save-place-to-alist in 'kill-buffer-hook) can
+        ;; behave as expected.
+        (save-excursion
+          (set-buffer obuf)
+          (setq buffer-file-name ofile)
+          (setq buffer-file-number onum)
+          (setq buffer-file-truename otrue)
+          (setq dired-directory odir))
+	;; On the other hand, if find-file got an error and did not
+        ;; really find anything, we want to put everything back as it
+        ;; was, including the lock and the buffer name.
+        (when (eq obuf (current-buffer))
+          (lock-buffer)
+          (rename-buffer oname))))
     (unless (eq (current-buffer) obuf)
       (with-current-buffer obuf
 	;; We already asked; don't ask again.