GNU bug report logs - #4061
23.1.50; C-x C-v and saveplace

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 4061 in the body.
You can then email your comments to 4061 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: Leo <sdl.web <at> gmail.com>
To: emacs-pretest-bug <at> gnu.org
Subject: 23.1.50; C-x C-v and saveplace
Date: Thu, 06 Aug 2009 04:29:21 +0100

Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:

With saveplace enabled, it will be nice if place (point) can survive C-x
C-v. At the moment, C-x C-v will move the point to a 'random' place.

Leo

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

From: Leo <sdl.web <at> gmail.com>
To: 4061 <at> debbugs.gnu.org
Subject: Re: bug#4061: 23.1.50; C-x C-v and saveplace
Date: Sat, 15 Aug 2009 21:18:10 +0100

On 2009-08-06 04:29 +0100, Leo wrote:
> Please describe exactly what actions triggered the bug
> and the precise symptoms of the bug:
>
> With saveplace enabled, it will be nice if place (point) can survive C-x
> C-v. At the moment, C-x C-v will move the point to a 'random' place.
>
> Leo

In `find-alternate-file', the old buffer is renamed, the new buffer is
created and then the renamed old buffer is killed. so the place is not
saved before re-opening the file. It seems at the moment for a user the
clean way to implement this feature is by using defadvice.

But I think this is a very useful feature to have.

-- 
Leo's Emacs uptime: 11 days, 19 hours, 50 minutes, 33 seconds

Message #15 received at 4061 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Chong Yidong <cyd <at> stupidchicken.com>
To: Karl Fogel  <kfogel <at> red-bean.com>
Cc: 4061 <at> debbugs.gnu.org, Leo <sdl.web <at> gmail.com>
Subject: Re: 23.1.50; C-x C-v and saveplace
Date: Sat, 15 Aug 2009 20:40:11 -0400

Hi Karl, are you still maintaining saveplace.el?  Could you take a look
at this bug report?


Leo <sdl.web <at> gmail.com> wrote:

> With saveplace enabled, it will be nice if place (point) can survive
> C-x C-v. At the moment, C-x C-v will move the point to a 'random'
> place.

Message #20 received at 4061 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Karl Fogel <kfogel <at> red-bean.com>
To: Chong Yidong <cyd <at> stupidchicken.com>
Cc: 4061 <at> debbugs.gnu.org, Leo <sdl.web <at> gmail.com>
Subject: Re: 23.1.50; C-x C-v and saveplace
Date: Sun, 16 Aug 2009 12:11:52 -0400

Chong Yidong <cyd <at> stupidchicken.com> writes:
> Hi Karl, are you still maintaining saveplace.el?  Could you take a look
> at this bug report?

Yes, I'll take a look.  Thank you for calling my attention to it.

-Karl

Message #25 received at 4061 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Leo <sdl.web <at> gmail.com>
To: Karl Fogel <kfogel <at> red-bean.com>
Cc: 4061 <at> debbugs.gnu.org, Chong Yidong <cyd <at> stupidchicken.com>
Subject: Re: bug#4061: 23.1.50; C-x C-v and saveplace
Date: Mon, 17 Aug 2009 15:01:11 +0100

On 2009-08-16 17:11 +0100, Karl Fogel wrote:
> Chong Yidong <cyd <at> stupidchicken.com> writes:
>> Hi Karl, are you still maintaining saveplace.el?  Could you take a look
>> at this bug report?
>
> Yes, I'll take a look.  Thank you for calling my attention to it.
>
> -Karl

I have been using the following simple defadvice.

(defadvice find-alternate-file (before save-place activate)
  "Save place before `find-alternate-file'."
  (save-place-to-alist))

But I think a solution involving no defadvice is better.

Hope this helps.

Leo

-- 
Leo's Emacs uptime: 13 days, 13 hours, 56 minutes, 25 seconds

Message #30 received at 4061 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Karl Fogel <kfogel <at> red-bean.com>
To: 4061 <at> debbugs.gnu.org
Cc: Leo <sdl.web <at> gmail.com>
Subject: Re: 23.1.50; C-x C-v and saveplace
Date: Fri, 04 Sep 2009 17:39:02 -0400

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://debbugs.gnu.org/cgi/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://debbugs.gnu.org/cgi/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.

Message #35 received at 4061 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Leo <sdl.web <at> gmail.com>
To: Karl Fogel <kfogel <at> red-bean.com>
Cc: 4061 <at> debbugs.gnu.org
Subject: Re: bug#4061: 23.1.50; C-x C-v and saveplace
Date: Sat, 05 Sep 2009 01:16:58 +0100

On 2009-09-04 22:39 +0100, Karl Fogel wrote:
> I know what causes this now.  saveplace.el works like this:
> 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.

Thank you for looking into it.

>
> 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 agree with this.

-- 
Leo's Emacs uptime: 2 days, 13 hours, 17 minutes, 9 seconds

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

From: Karl Fogel <kfogel <at> red-bean.com>
To: 4061-done <at> debbugs.gnu.org
Cc: Leo <sdl.web <at> gmail.com>
Subject: Re: bug#4061: 23.1.50; C-x C-v and saveplace
Date: Sat, 05 Sep 2009 12:39:02 -0400

Okay, this is fixed now:

  $ cat log-message.txt
  * lisp/files.el (find-alternate-file): Run `kill-buffer-hook' manually
    before killing the old buffer, since by the time `kill-buffer' is
    run so many buffer variables have been set to nil that it may not
    behave as expected.  (Bug#4061)
  $ cvs ci -F log-message.txt files.el ChangeLog
  /sources/emacs/emacs/lisp/files.el,v  <--  files.el
  new revision: 1.1078; previous revision: 1.1077
  /sources/emacs/emacs/lisp/ChangeLog,v  <--  ChangeLog
  new revision: 1.16100; previous revision: 1.16099
  Mailing notification to emacs-diffs <at> gnu.org... sent.
  $ 

Leo, please test if you get a chance and let us know if you run into any
problems.

Notes on closing:

I'm closing this report now by sending this mail to 4061-done@, as
documented on http://debbugs.gnu.org/Developer#closing.  I'm
not positive that's the Right Way to do it, but since much of the
documentation is still written in terms of Debian GNU/Linux instead of
Emacs, and http://debbugs.gnu.org/server-control#fixed
doesn't clarify either (for example, what should the "version" be?  The
version the bug was reported against, or the version the fix is likely
to be be released in?), this is as good a guess as any.  If someone
knows the Right Way, please tell me, or better yet, document it where
developers are likely to find it.

-Karl

Message #45 received at 4061 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Leo <sdl.web <at> gmail.com>
To: 4061 <at> debbugs.gnu.org
Cc: Karl Fogel <kfogel <at> red-bean.com>
Subject: Re: bug#4061: marked as done (23.1.50; C-x C-v and saveplace)
Date: Sat, 05 Sep 2009 17:54:43 +0100

On 2009-09-05 17:45 +0100, Emacs bug Tracking System wrote:
> Leo, please test if you get a chance and let us know if you run into
> any problems.

I downloaded the latest files.el and loaded with Emacs -q. After a brief
testing, it seems the bug has indeed been fixed. Thank you very much.

Best wishes,
Leo

-- 
Leo's Emacs uptime: 3 days, 5 hours, 56 minutes, 57 seconds

Previous Next

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