* bug#12766: read-from-minibuffer does not preserve current-buffer
@ 2012-10-29 20:29 Stefan Monnier
2012-10-30 10:27 ` martin rudalics
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2012-10-29 20:29 UTC (permalink / raw)
To: 12766
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
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#12766: read-from-minibuffer does not preserve current-buffer
2012-10-29 20:29 bug#12766: read-from-minibuffer does not preserve current-buffer Stefan Monnier
@ 2012-10-30 10:27 ` martin rudalics
2012-10-30 13:51 ` Stefan Monnier
0 siblings, 1 reply; 10+ messages in thread
From: martin rudalics @ 2012-10-30 10:27 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 12766
> % 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
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#12766: read-from-minibuffer does not preserve current-buffer
2012-10-30 10:27 ` martin rudalics
@ 2012-10-30 13:51 ` Stefan Monnier
2012-10-30 18:49 ` martin rudalics
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2012-10-30 13:51 UTC (permalink / raw)
To: martin rudalics; +Cc: 12766
>> % 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
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#12766: read-from-minibuffer does not preserve current-buffer
2012-10-30 13:51 ` Stefan Monnier
@ 2012-10-30 18:49 ` martin rudalics
2012-10-30 19:21 ` Stefan Monnier
0 siblings, 1 reply; 10+ messages in thread
From: martin rudalics @ 2012-10-30 18:49 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 12766
>>> % 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);
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#12766: read-from-minibuffer does not preserve current-buffer
2012-10-30 18:49 ` martin rudalics
@ 2012-10-30 19:21 ` Stefan Monnier
2012-10-31 10:27 ` martin rudalics
2012-11-09 9:50 ` martin rudalics
0 siblings, 2 replies; 10+ messages in thread
From: Stefan Monnier @ 2012-10-30 19:21 UTC (permalink / raw)
To: martin rudalics; +Cc: 12766
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#12766: read-from-minibuffer does not preserve current-buffer
2012-10-30 19:21 ` Stefan Monnier
@ 2012-10-31 10:27 ` martin rudalics
2012-10-31 14:07 ` Stefan Monnier
2012-11-09 9:50 ` martin rudalics
1 sibling, 1 reply; 10+ messages in thread
From: martin rudalics @ 2012-10-31 10:27 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 12766
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#12766: read-from-minibuffer does not preserve current-buffer
2012-10-31 10:27 ` martin rudalics
@ 2012-10-31 14:07 ` Stefan Monnier
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Monnier @ 2012-10-31 14:07 UTC (permalink / raw)
To: martin rudalics; +Cc: 12766
> 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"
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#12766: read-from-minibuffer does not preserve current-buffer
2012-10-30 19:21 ` Stefan Monnier
2012-10-31 10:27 ` martin rudalics
@ 2012-11-09 9:50 ` martin rudalics
2012-11-09 14:20 ` Stefan Monnier
1 sibling, 1 reply; 10+ messages in thread
From: martin rudalics @ 2012-11-09 9:50 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 12766
> Right, I would expect it to fix the problem, indeed. IOW, please
> install.
Can we close this bug?
martin
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-11-10 11:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-29 20:29 bug#12766: read-from-minibuffer does not preserve current-buffer Stefan Monnier
2012-10-30 10:27 ` martin rudalics
2012-10-30 13:51 ` Stefan Monnier
2012-10-30 18:49 ` martin rudalics
2012-10-30 19:21 ` Stefan Monnier
2012-10-31 10:27 ` martin rudalics
2012-10-31 14:07 ` Stefan Monnier
2012-11-09 9:50 ` martin rudalics
2012-11-09 14:20 ` Stefan Monnier
2012-11-10 11:07 ` martin rudalics
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).