GNU bug report logs - #12766
read-from-minibuffer does not preserve current-buffer

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 12766 in the body.
You can then email your comments to 12766 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: bug-gnu-emacs <at> gnu.org
Subject: read-from-minibuffer does not preserve current-buffer
Date: Mon, 29 Oct 2012 16:29:29 -0400

Package: Emacs
Version: 24.2.50


% src/emacs -Q --eval "(setq initial-frame-alist '((minibuffer . nil)))"
M-: (with-temp-buffer (list (read-string "toto: ") (current-buffer))) RET

In Emacs 24.2 this always shows the current-buffer to be the temp buffer
(which is a killed buffer by the time M-: prints its result).
But in Emacs trunk, the result depends on whether you issue the M-:
from the normal frame (where it works right) or from the special
minibuffer-only frame, where current-buffer after read-string is
*minibuf-0*!

This is a serious problem, which introduces subtle bugs that can be
pretty difficult to track down.


        Stefan

Message #8 received at 12766 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 12766 <at> debbugs.gnu.org
Subject: Re: bug#12766: read-from-minibuffer does not preserve current-buffer
Date: Tue, 30 Oct 2012 11:27:58 +0100

> % src/emacs -Q --eval "(setq initial-frame-alist '((minibuffer . nil)))"
> M-: (with-temp-buffer (list (read-string "toto: ") (current-buffer))) RET
>
> In Emacs 24.2 this always shows the current-buffer to be the temp buffer
> (which is a killed buffer by the time M-: prints its result).
> But in Emacs trunk, the result depends on whether you issue the M-:
> from the normal frame (where it works right) or from the special
> minibuffer-only frame, where current-buffer after read-string is
> *minibuf-0*!
>
> This is a serious problem, which introduces subtle bugs that can be
> pretty difficult to track down.

Two changes I made could be involved:

(1) `select-window' now always makes the window's buffer current.

(2) read_minibuf now calls set_window_buffer instead of
    Fset_window_buffer.

And there's also bug#10851 where `read-char-by-name' could change the
current buffer.

Also, is this related to the comment

       ;; FIXME: kill-buffer can change current-buffer in some odd cases.

in `with-temp-buffer'?

martin

Message #11 received at 12766 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 12766 <at> debbugs.gnu.org
Subject: Re: bug#12766: read-from-minibuffer does not preserve current-buffer
Date: Tue, 30 Oct 2012 09:51:18 -0400

>> % src/emacs -Q --eval "(setq initial-frame-alist '((minibuffer . nil)))"
>> M-: (with-temp-buffer (list (read-string "toto: ") (current-buffer))) RET
> Two changes I made could be involved:
> (1) `select-window' now always makes the window's buffer current.
> (2) read_minibuf now calls set_window_buffer instead of
>     Fset_window_buffer.

I think (1) is more likely to be the problem.  Could you check
read_minibuf to see which unwind is supposed to reset the
current buffer?
Maybe that code worked by accident (.e.g relying on the fact that
select-window didn't set current-buffer in that corner case) and the
right fix is simply to explicitly save&restore current buffer.

> Also, is this related to the comment
>        ;; FIXME: kill-buffer can change current-buffer in some odd cases.
> in `with-temp-buffer'?

No, (current-buffer) is evaluated before the buffer gets killed, and the
bug shows up with with-temp-buffer (the recipe is just shorter with
with-temp-buffer, but you can try it with (with-current-buffer
(get-buffer-create " *sm-test*") ...) to convince yourself).


        Stefan

Message #14 received at 12766 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 12766 <at> debbugs.gnu.org
Subject: Re: bug#12766: read-from-minibuffer does not preserve current-buffer
Date: Tue, 30 Oct 2012 19:49:23 +0100

>>> % src/emacs -Q --eval "(setq initial-frame-alist '((minibuffer . nil)))"
>>> M-: (with-temp-buffer (list (read-string "toto: ") (current-buffer))) RET
>> Two changes I made could be involved:
>> (1) `select-window' now always makes the window's buffer current.
>> (2) read_minibuf now calls set_window_buffer instead of
>>     Fset_window_buffer.
>
> I think (1) is more likely to be the problem.

Verified (sloppily).

> Could you check
> read_minibuf to see which unwind is supposed to reset the
> current buffer?

IIUC read_minibuf doesn't care about restoring the current buffer.

> Maybe that code worked by accident (.e.g relying on the fact that
> select-window didn't set current-buffer in that corner case) and the
> right fix is simply to explicitly save&restore current buffer.

FWIW the problem already happens when choose_minibuf_frame calls
Fset_frame_selected_window.  The patch below seems to fix it.

martin, completely lost in the labyrinth of read_minibuf


*** src/minibuf.c	2012-10-11 16:23:37 +0000
--- src/minibuf.c	2012-10-30 18:25:26 +0000
***************
*** 472,477 ****
--- 472,479 ----

    /* Choose the minibuffer window and frame, and take action on them.  */

+   record_unwind_protect (Fset_buffer, Fcurrent_buffer ());
+
    choose_minibuf_frame ();

    record_unwind_protect (choose_minibuf_frame_1, Qnil);

Message #17 received at 12766 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 12766 <at> debbugs.gnu.org
Subject: Re: bug#12766: read-from-minibuffer does not preserve current-buffer
Date: Tue, 30 Oct 2012 15:21:27 -0400

> FWIW the problem already happens when choose_minibuf_frame calls
> Fset_frame_selected_window.  The patch below seems to fix it.

Right, I would expect it to fix the problem, indeed.  IOW, please
install.  But I still wonder, how come this was not needed earlier.


        Stefan

Message #20 received at 12766 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 12766 <at> debbugs.gnu.org
Subject: Re: bug#12766: read-from-minibuffer does not preserve current-buffer
Date: Wed, 31 Oct 2012 11:27:15 +0100

> Right, I would expect it to fix the problem, indeed.  IOW, please
> install.

Installed in revision 110748 on trunk.

> But I still wonder, how come this was not needed earlier.

I didn't go through it but IIUC read_minibuf calls choose_minibuf_frame
which calls Fset_frame_selected_window for each frame which now, for the
selected window, unconditionally makes its buffer current.

For me the fact that unwinding a sequence of operations as

  choose_minibuf_frame ();

  record_unwind_protect (choose_minibuf_frame_1, Qnil);

  record_unwind_protect (Fset_window_configuration,
                         Fcurrent_window_configuration (Qnil));

  record_unwind_protect (Fset_window_configuration,
                         Fcurrent_window_configuration (mini_frame));

would work in practice constitutes a miracle already.

martin

Message #23 received at 12766 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 12766 <at> debbugs.gnu.org
Subject: Re: bug#12766: read-from-minibuffer does not preserve current-buffer
Date: Wed, 31 Oct 2012 10:07:08 -0400

> For me the fact that unwinding a sequence of operations as
>   choose_minibuf_frame ();
>   record_unwind_protect (choose_minibuf_frame_1, Qnil);
>   record_unwind_protect (Fset_window_configuration,
>                          Fcurrent_window_configuration (Qnil));
>   record_unwind_protect (Fset_window_configuration,
>                          Fcurrent_window_configuration (mini_frame));
> would work in practice constitutes a miracle already.

Yes, Emacs is amazing ;-)


        Stefan "who'd be happy to move read_minibuf to Lisp"

Message #26 received at 12766 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 12766 <at> debbugs.gnu.org
Subject: Re: bug#12766: read-from-minibuffer does not preserve current-buffer
Date: Fri, 09 Nov 2012 10:50:22 +0100

> Right, I would expect it to fix the problem, indeed.  IOW, please
> install.

Can we close this bug?

martin

Message #29 received at 12766 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 12766 <at> debbugs.gnu.org
Subject: Re: bug#12766: read-from-minibuffer does not preserve current-buffer
Date: Fri, 09 Nov 2012 09:20:21 -0500

>> Right, I would expect it to fix the problem, indeed.  IOW, please
>> install.
> Can we close this bug?

Yes,


        Stefan "still wondering how it worked in Emacs-23"

Message #34 received at 12766-done <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 12766-done <at> debbugs.gnu.org
Subject: Re: bug#12766: read-from-minibuffer does not preserve current-buffer
Date: Sat, 10 Nov 2012 12:07:04 +0100

>> Can we close this bug?
> 
> Yes.

Closed.

Thanks, martin

Previous Next

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