From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: 58343@debbugs.gnu.org
Cc: Alan Mackenzie <acm@muc.de>
Subject: bug#58343: 29.0.50; ELisp code run in "inconsitent" selected-window state
Date: Thu, 06 Oct 2022 20:06:25 -0400 [thread overview]
Message-ID: <jwvfsg0cxvw.fsf-monnier+emacs@gnu.org> (raw)
Package: Emacs
Version: 29.0.50
Hi Alan and friends,
In commit dfa3e6f424b20fe27d9041b2ce7d69811df5d8cd, Alan added the
following code to do_switch_frame:
diff --git a/src/frame.c b/src/frame.c
index ccac18d23c2..dc8045f41e6 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1564,6 +1564,13 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
if (! FRAME_MINIBUF_ONLY_P (XFRAME (selected_frame)))
last_nonminibuf_frame = XFRAME (selected_frame);
+ /* If the selected window in the target frame is its mini-window, we move
+ to a different window, the most recently used one, unless there is a
+ valid active minibuffer in the mini-window. */
+ if (EQ (f->selected_window, f->minibuffer_window)
+ && NILP (Fminibufferp (XWINDOW (f->minibuffer_window)->contents, Qt)))
+ Fset_frame_selected_window (frame, call1 (Qget_mru_window, frame), Qnil);
+
Fselect_window (f->selected_window, norecord);
/* We want to make sure that the next event generates a frame-switch
the problem with this is that it calls `Qget_mru_window` which is ELisp
code, and that we're in the very middle of changing the selected frame,
so we have already changed the `selected-frame` variable a few lines
earlier, but the select-window has not yet been adjusted accordingly.
Running ELisp code in a state where (selected-window) and
(frame-selected-window) aren't equal is a recipe for problems. I have
already wasted many hours in the past tracking down bugs linked to this
kind of situation (back when the mode-line was processed in such an
inconsistent state, for example), and I really don't want to go there.
So we should arrange to run this `get-mru-window` function at some other
time, for example a few lines earlier before we set `selected-frame`.
I don't understand this code nearly enough to know how to move the code,
because it probably interacts with the other statements in non-trivial
ways, so the patch below is just a naive suggestion (it seems to work
here without triggering my many sprinkled assertions checking that
`EQ (XFRAME (selected_frame)->selected_window, selected_window)`,
but it's a far cry from a confirmation that it's right).
Hopefully someone here is aware of some of the potential pitfalls.
Stefan
diff --git a/src/frame.c b/src/frame.c
index 91b9bec82c3..58b6ee50d23 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1503,18 +1503,8 @@ do_switch_frame (Lisp_Object frame, int for_deletion, Lisp_Object norecord)
sf->select_mini_window_flag = MINI_WINDOW_P (XWINDOW (sf->selected_window));
- selected_frame = frame;
-
move_minibuffers_onto_frame (sf, for_deletion);
- if (f->select_mini_window_flag
- && !NILP (Fminibufferp (XWINDOW (f->minibuffer_window)->contents, Qt)))
- f->selected_window = f->minibuffer_window;
- f->select_mini_window_flag = false;
-
- if (! FRAME_MINIBUF_ONLY_P (XFRAME (selected_frame)))
- last_nonminibuf_frame = XFRAME (selected_frame);
-
/* If the selected window in the target frame is its mini-window, we move
to a different window, the most recently used one, unless there is a
valid active minibuffer in the mini-window. */
@@ -1528,6 +1518,16 @@ do_switch_frame (Lisp_Object frame, int for_deletion, Lisp_Object norecord)
Fset_frame_selected_window (frame, w, Qnil);
}
+ selected_frame = frame;
+
+ if (f->select_mini_window_flag
+ && !NILP (Fminibufferp (XWINDOW (f->minibuffer_window)->contents, Qt)))
+ f->selected_window = f->minibuffer_window;
+ f->select_mini_window_flag = false;
+
+ if (! FRAME_MINIBUF_ONLY_P (XFRAME (selected_frame)))
+ last_nonminibuf_frame = XFRAME (selected_frame);
+
Fselect_window (f->selected_window, norecord);
/* We want to make sure that the next event generates a frame-switch
next reply other threads:[~2022-10-07 0:06 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-07 0:06 Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2022-10-12 18:08 ` bug#58343: 29.0.50; ELisp code run in "inconsitent" selected-window state Alan Mackenzie
2022-10-12 18:35 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-10-12 19:13 ` Alan Mackenzie
2022-10-12 21:07 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvfsg0cxvw.fsf-monnier+emacs@gnu.org \
--to=bug-gnu-emacs@gnu.org \
--cc=58343@debbugs.gnu.org \
--cc=acm@muc.de \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).