bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Al Petrofsky]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

While the minibuffer window is active, attempt to switch buffers in
an ordinary window like so:

emacs -Q
M-: (setq enable-recursive-minibuffers t) RET
M-x C-x o C-x b foo RET

In emacs 27 and earlier, that will switch the buffer in the main
window to the new buffer named "foo", but in emacs 28.2, it generates
a bogus "user-error: Cannot switch buffers in minibuffer window".

I get the same results with -nw.

[-- Attachment #2: Type: text/html, Size: 517 bytes --]

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> From: Al Petrofsky <al@petrofsky.org>
> Date: Thu, 8 Jun 2023 17:32:56 -0400
> 
> While the minibuffer window is active, attempt to switch buffers in
> an ordinary window like so:
> 
> emacs -Q
> M-: (setq enable-recursive-minibuffers t) RET
> M-x C-x o C-x b foo RET
> 
> In emacs 27 and earlier, that will switch the buffer in the main
> window to the new buffer named "foo", but in emacs 28.2, it generates
> a bogus "user-error: Cannot switch buffers in minibuffer window".

Seems like read-buffer-to-switch (called by "C-x b") changes the
selected-window: when it returns, the rest of the function runs with
the minibuffer window being the selected-window, which is wrong in
this case.

I couldn't find the offending change, but I doubt that bisection would
help us in this case, given how many water went under the bridge of
using the minibuffer and saving/restoring the window configuration.

The ugly kludge below seems to fix the problem.

Stefan and Martin, any better ideas or hints?

diff --git a/lisp/window.el b/lisp/window.el
index a11b1a5..6777944 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8941,7 +8941,9 @@ switch-to-buffer
                     "Cannot switch buffers in a dedicated window")))
                 ('pop nil)
                 (_ (set-window-dedicated-p nil nil) 'force-same-window)))))))
-     (list (read-buffer-to-switch "Switch to buffer: ") nil force-same-window)))
+     (save-selected-window
+       (list
+        (read-buffer-to-switch "Switch to buffer: ") nil force-same-window))))
   (let ((buffer (window-normalize-buffer-to-switch-to buffer-or-name))
         (set-window-start-and-point (not switch-to-buffer-obey-display-actions)))
     (cond

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Seems like read-buffer-to-switch (called by "C-x b") changes the
> selected-window: when it returns, the rest of the function runs with
> the minibuffer window being the selected-window, which is wrong in
> this case.

Are you 100% sure that's what happens?  AFAICT `read-buffer-to-switch`
uses the minibuffer in the normal way so when we exit the minibuffer the
selected window should definitely not still be the mini-window.
And if it happens with `read-buffer-to-switch` I can't see any reason
why it wouldn't happen for most/all other uses of the minibuffer.


        Stefan

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Al Petrofsky <al@petrofsky.org>,  martin rudalics <rudalics@gmx.at>,
>   63967@debbugs.gnu.org
> Date: Fri, 09 Jun 2023 11:08:43 -0400
> 
> > Seems like read-buffer-to-switch (called by "C-x b") changes the
> > selected-window: when it returns, the rest of the function runs with
> > the minibuffer window being the selected-window, which is wrong in
> > this case.
> 
> Are you 100% sure that's what happens?

Yes, I'm sure.  First, the window-minibuffer-p call in the interactive
form:

  (interactive
   (let ((force-same-window
          (unless switch-to-buffer-obey-display-actions
            (cond
             ((window-minibuffer-p) nil) <<<<<<<<<<<<<<<<<<<<<<<<<<<<
             ((not (eq (window-dedicated-p) t)) 'force-same-window)
             ((pcase switch-to-buffer-in-dedicated-window
                ('nil (user-error
                       "Cannot switch buffers in a dedicated window"))
                ('prompt
                 (if (y-or-n-p
                      (format "Window is dedicated to %s; undedicate it?"
                              (window-buffer)))
                     (progn
                       (set-window-dedicated-p nil nil)
                       'force-same-window)
                   (user-error
                    "Cannot switch buffers in a dedicated window")))
                ('pop nil)
                (_ (set-window-dedicated-p nil nil) 'force-same-window)))))))
     (list (read-buffer-to-switch "Switch to buffer: ") nil force-same-window)))

returns nil, as expected (since this runs in the window to which we
changed with "C-x p", before Emacs prompts for the buffer to switch
to).  But then the call to window-minibuffer-p in the body of the
function:

  (let ((buffer (window-normalize-buffer-to-switch-to buffer-or-name))
        (set-window-start-and-point (not switch-to-buffer-obey-display-actions)))
    (cond
     ;; Don't call set-window-buffer if it's not needed since it
     ;; might signal an error (e.g. if the window is dedicated).
     ((and (eq buffer (window-buffer))
           ;; pop-to-buffer-same-window might decide to display
           ;; the same buffer in another window
           (not switch-to-buffer-obey-display-actions)))
     ((and (window-minibuffer-p)  <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
           (not switch-to-buffer-obey-display-actions))
      (if force-same-window
          (user-error "Cannot switch buffers in minibuffer window")
        (pop-to-buffer buffer norecord)))

returns non-nil, although we were supposed to be out of the minibuffer
by that time.  I've verified that selected-window returns the original
non-minibuffer window in the first place and the minibuffer window in
the second.

I then ran the recipe under GDB, with a watchpoint on selected_window,
and clearly saw that it stays at its mini-window value by the time we
exit read-buffer-to-switch.

> And if it happens with `read-buffer-to-switch` I can't see any reason
> why it wouldn't happen for most/all other uses of the minibuffer.

It probably does, yes.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Gregory Heytings]

>
> I couldn't find the offending change
>

It's 7c2ebf6e23.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Fri, 09 Jun 2023 16:52:44 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Al Petrofsky <al@petrofsky.org>, martin rudalics <rudalics@gmx.at>, 
>     Stefan Monnier <monnier@iro.umontreal.ca>, 63967@debbugs.gnu.org
> 
> > I couldn't find the offending change
> 
> It's 7c2ebf6e23.

Thanks.  Yes, I imagined it would be it.  Which is why I said: it
won't help us to know which commit changed that.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Cc: al@petrofsky.org, rudalics@gmx.at, 63967@debbugs.gnu.org
> Date: Fri, 09 Jun 2023 19:09:19 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > And if it happens with `read-buffer-to-switch` I can't see any reason
> > why it wouldn't happen for most/all other uses of the minibuffer.
> 
> It probably does, yes.

AFAICT, the clobbering of the selected window happens in
minibuffer_unwind:

  static void
  minibuffer_unwind (void)
  {
    struct frame *f;
    struct window *w;
    Lisp_Object window;
    Lisp_Object entry;

    if (NILP (exp_MB_frame)) return; /* "Can't happen." */
    f = XFRAME (exp_MB_frame);
    window = f->minibuffer_window;
    w = XWINDOW (window);
    if (FRAME_LIVE_P (f))
      {
	/* minibuf_window = sf->minibuffer_window; */
	if (!NILP (w->prev_buffers))
	  {
	    entry = Fcar (w->prev_buffers);
	    w->prev_buffers = Fcdr (w->prev_buffers);
	    set_window_buffer (window, Fcar (entry), 0, 0);
	    Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
	    Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
	    /* set-window-configuration may/will have unselected the
	       mini-window as the selected window.  Restore it. */
	    Fset_frame_selected_window (exp_MB_frame, window, Qnil);  <<<<<<<<<
	  }
	else
	  set_window_buffer (window, nth_minibuffer (0), 0, 0);
      }
  }

And since minibuffer_unwind is called _after_
restore_window_configuration, it overwrites what the latter does.

Why minibuffer_unwind doesn't test some condition which would tell it
at all has to do something, I don't know.  It seems to think that the
frame exp_MB_frame is still live and that its "expired" minibuffer
needs to be replaced? why?  Is the logic inside read_minibuf_unwind
flawed in the case that enable_recursive_minibuffers is non-zero?

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Gregory Heytings]

>>> I couldn't find the offending change
>>
>> It's 7c2ebf6e23.
>
> Thanks.  Yes, I imagined it would be it.  Which is why I said: it won't 
> help us to know which commit changed that.
>

Given your last post, apparently it did help ;-)

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Fri, 09 Jun 2023 20:04:42 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: al@petrofsky.org, rudalics@gmx.at, monnier@iro.umontreal.ca, 
>     63967@debbugs.gnu.org
> 
> 
> >>> I couldn't find the offending change
> >>
> >> It's 7c2ebf6e23.
> >
> > Thanks.  Yes, I imagined it would be it.  Which is why I said: it won't 
> > help us to know which commit changed that.
> >
> 
> Given your last post, apparently it did help ;-)

Unfortunately, no, because the changeset is huge, and just knowing
what part of it could cause this is a non-trivial job.  Fortunately,
the code which clobbers the selected-window was clearly visible in
GDB, which is what led to my previous post.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Gregory Heytings]

>>>>> I couldn't find the offending change
>>>>
>>>> It's 7c2ebf6e23.
>>>
>>> Thanks.  Yes, I imagined it would be it.  Which is why I said: it 
>>> won't help us to know which commit changed that.
>>
>> Given your last post, apparently it did help ;-)
>
> Unfortunately, no, because the changeset is huge, and just knowing what 
> part of it could cause this is a non-trivial job.  Fortunately, the code 
> which clobbers the selected-window was clearly visible in GDB, which is 
> what led to my previous post.
>

I see.  Apparently we don't work the same way.  I didn't have time to 
investigate this issue further after bisecting, but the first thing I 
would have done is to determine which of the four calls to 
Fset_frame_selected_window in that changeset is responsible for that bug.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Sat, 10 Jun 2023 06:39:41 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: al@petrofsky.org, rudalics@gmx.at, monnier@iro.umontreal.ca, 
>     63967@debbugs.gnu.org
> 
> I see.  Apparently we don't work the same way.  I didn't have time to 
> investigate this issue further after bisecting, but the first thing I 
> would have done is to determine which of the four calls to 
> Fset_frame_selected_window in that changeset is responsible for that bug.

You assume that the only way to change the selected-window on the C
level is by calling Fset_frame_selected_window?  That's false; just
try grepping the C sources for "selected_window =".  And that's even
before you consider the possibilities of indirect setting, when the
actual setting is in Lisp via some proxy.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[martin rudalics]

 > diff --git a/lisp/window.el b/lisp/window.el
 > index a11b1a5..6777944 100644
 > --- a/lisp/window.el
 > +++ b/lisp/window.el
 > @@ -8941,7 +8941,9 @@ switch-to-buffer
 >                       "Cannot switch buffers in a dedicated window")))
 >                   ('pop nil)
 >                   (_ (set-window-dedicated-p nil nil) 'force-same-window)))))))
 > -     (list (read-buffer-to-switch "Switch to buffer: ") nil force-same-window)))
 > +     (save-selected-window
 > +       (list
 > +        (read-buffer-to-switch "Switch to buffer: ") nil force-same-window))))
 >     (let ((buffer (window-normalize-buffer-to-switch-to buffer-or-name))
 >           (set-window-start-and-point (not switch-to-buffer-obey-display-actions)))
 >       (cond

That wouldn't help in all other use cases of read_minibuf where the user
will be thrown back to the minibuffer window as well.  I'd rather try (the
still timid)

-static void minibuffer_unwind (void);
+static void minibuffer_unwind (Lisp_Object);
...
-  record_unwind_protect_void (minibuffer_unwind);
+  record_unwind_protect (minibuffer_unwind, selected_window);
...
-minibuffer_unwind (void)
+minibuffer_unwind (Lisp_Object old_selected_window)
...
+	  if (!EQ (old_selected_window, FRAME_SELECTED_WINDOW (f)))
+	    Fset_frame_selected_window (exp_MB_frame, window, Qnil);

since the last line seems to suggest that exp_MB_frame should not be the
selected frame.

martin

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Sat, 10 Jun 2023 08:52:57 +0200
> Cc: 63967@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > diff --git a/lisp/window.el b/lisp/window.el
>  > index a11b1a5..6777944 100644
>  > --- a/lisp/window.el
>  > +++ b/lisp/window.el
>  > @@ -8941,7 +8941,9 @@ switch-to-buffer
>  >                       "Cannot switch buffers in a dedicated window")))
>  >                   ('pop nil)
>  >                   (_ (set-window-dedicated-p nil nil) 'force-same-window)))))))
>  > -     (list (read-buffer-to-switch "Switch to buffer: ") nil force-same-window)))
>  > +     (save-selected-window
>  > +       (list
>  > +        (read-buffer-to-switch "Switch to buffer: ") nil force-same-window))))
>  >     (let ((buffer (window-normalize-buffer-to-switch-to buffer-or-name))
>  >           (set-window-start-and-point (not switch-to-buffer-obey-display-actions)))
>  >       (cond
> 
> That wouldn't help in all other use cases of read_minibuf where the user
> will be thrown back to the minibuffer window as well.  I'd rather try (the
> still timid)
> 
> -static void minibuffer_unwind (void);
> +static void minibuffer_unwind (Lisp_Object);
> ...
> -  record_unwind_protect_void (minibuffer_unwind);
> +  record_unwind_protect (minibuffer_unwind, selected_window);
> ...
> -minibuffer_unwind (void)
> +minibuffer_unwind (Lisp_Object old_selected_window)
> ...
> +	  if (!EQ (old_selected_window, FRAME_SELECTED_WINDOW (f)))
> +	    Fset_frame_selected_window (exp_MB_frame, window, Qnil);
> 
> since the last line seems to suggest that exp_MB_frame should not be the
> selected frame.

That works, but does it make sens to do the other stuff in that if
clause in that case?  We do:

      if (!NILP (w->prev_buffers))
	{
	  entry = Fcar (w->prev_buffers);
	  w->prev_buffers = Fcdr (w->prev_buffers);
	  set_window_buffer (window, Fcar (entry), 0, 0);
	  Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
	  Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
	  /* set-window-configuration may/will have unselected the
	     mini-window as the selected window.  Restore it. */
	  if (!EQ (orig_selected_window, FRAME_SELECTED_WINDOW (f)))
	    Fset_frame_selected_window (exp_MB_frame, window, Qnil);
	}

Does it make sense to manipulate the buffer, window-start, and
window-point of WINDOW if we are not going to make it the selected
window?

In general, I don't think I understand the logic of this function at
all.  Since this is about the "expired minibuffer", then the code
seems to assume that when a minibuffer is "expired", and there are
previous minibuffers for that mini-window, the mini-window stays the
selected-window?  But that is not true when recursive minibuffers are
enabled, right?

IOW, I wonder whether your suggested change just postpones the problem
by moving it to the rarer cases when more than one frame is involved
in this dance.  Why is it okay to set the selected-window of another
frame here?

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Gregory Heytings]

>> I see.  Apparently we don't work the same way.  I didn't have time to 
>> investigate this issue further after bisecting, but the first thing I 
>> would have done is to determine which of the four calls to 
>> Fset_frame_selected_window in that changeset is responsible for that 
>> bug.
>
> You assume that the only way to change the selected-window on the C 
> level is by calling Fset_frame_selected_window?  That's false; just try 
> grepping the C sources for "selected_window =".  And that's even before 
> you consider the possibilities of indirect setting, when the actual 
> setting is in Lisp via some proxy.
>

I don't think it's useful to argue, I'm just observing that we don't work 
the same way.  But no, I don't assume anything, on the contrary.  Just 
commenting out each one of the Fset_frame_selected_window in that 
changeset in turn, and checking whether doing that fixes the bug, shows 
that the culprit is the call to Fset_frame_selected_window in 
minibuffer_unwind.  Doing that does not involve any guesswork, and the 
result is 100% accurate.  Of course, that's not the end of the story, and 
of course, it could have turned out that none of these four calls is the 
culprit (in which case I would probably have checked the five calls to 
set_window_buffer).

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[martin rudalics]

 > That works, but does it make sens to do the other stuff in that if
 > clause in that case?  We do:
 >
 >        if (!NILP (w->prev_buffers))
 > 	{
 > 	  entry = Fcar (w->prev_buffers);
 > 	  w->prev_buffers = Fcdr (w->prev_buffers);
 > 	  set_window_buffer (window, Fcar (entry), 0, 0);
 > 	  Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
 > 	  Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
 > 	  /* set-window-configuration may/will have unselected the
 > 	     mini-window as the selected window.  Restore it. */
 > 	  if (!EQ (orig_selected_window, FRAME_SELECTED_WINDOW (f)))
 > 	    Fset_frame_selected_window (exp_MB_frame, window, Qnil);
 > 	}
 >
 > Does it make sense to manipulate the buffer, window-start, and
 > window-point of WINDOW if we are not going to make it the selected
 > window?

If w->prev_buffers gets us the right minibuffer to display here (any
former dw->prev_buffers = sw->prev_buffers should guarantee that), then
for that buffer we have to establish the corresponding start and point
positions.  This makes sense regardless of whether the minibuffer window
will be selected or not.

IIUC the present code either assumes that the user was in the minibuffer
window before starting the last read_minibuf interaction or that
Fset_frame_selected_window does not select the window when the frame is
already selected.

 > In general, I don't think I understand the logic of this function at
 > all.  Since this is about the "expired minibuffer", then the code
 > seems to assume that when a minibuffer is "expired", and there are
 > previous minibuffers for that mini-window, the mini-window stays the
 > selected-window?  But that is not true when recursive minibuffers are
 > enabled, right?

More precisely, it should not be true when a nested read_minibuf was
invoked from a normal window as with C-x b, something that can happen
only when `enable-recursive-minibuffers' is non-nil.  This includes the
simpler C-h f C-x o C-x b which means that a user does not have to
enable them explicitly.

 > IOW, I wonder whether your suggested change just postpones the problem
 > by moving it to the rarer cases when more than one frame is involved
 > in this dance.  Why is it okay to set the selected-window of another
 > frame here?

I wouldn't know.  That's why I called my proposal "timid".

martin

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Alan,

Could you explain why/when we need to call `Fset_frame_selected_window`
in the code below:

>   static void
>   minibuffer_unwind (void)
>   {
>     struct frame *f;
>     struct window *w;
>     Lisp_Object window;
>     Lisp_Object entry;
>
>     if (NILP (exp_MB_frame)) return; /* "Can't happen." */
>     f = XFRAME (exp_MB_frame);
>     window = f->minibuffer_window;
>     w = XWINDOW (window);
>     if (FRAME_LIVE_P (f))
>       {
> 	/* minibuf_window = sf->minibuffer_window; */
> 	if (!NILP (w->prev_buffers))
> 	  {
> 	    entry = Fcar (w->prev_buffers);
> 	    w->prev_buffers = Fcdr (w->prev_buffers);
> 	    set_window_buffer (window, Fcar (entry), 0, 0);
> 	    Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
> 	    Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
> 	    /* set-window-configuration may/will have unselected the
> 	       mini-window as the selected window.  Restore it. */
> 	    Fset_frame_selected_window (exp_MB_frame, window, Qnil);  <<<<<<<<<
> 	  }
> 	else
> 	  set_window_buffer (window, nth_minibuffer (0), 0, 0);
>       }
>   }

I understand why we do the `set_window_buffer` and set its start and
point, but I can't see why we'd need to select the mini-window:
presumably if it needed to be (re)selected that should have been handled
by the window-config save&restore, no?


        Stefan

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Sat, 10 Jun 2023 16:51:47 +0200
> Cc: al@petrofsky.org, monnier@iro.umontreal.ca, 63967@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > That works, but does it make sens to do the other stuff in that if
>  > clause in that case?  We do:
>  >
>  >        if (!NILP (w->prev_buffers))
>  > 	{
>  > 	  entry = Fcar (w->prev_buffers);
>  > 	  w->prev_buffers = Fcdr (w->prev_buffers);
>  > 	  set_window_buffer (window, Fcar (entry), 0, 0);
>  > 	  Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
>  > 	  Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
>  > 	  /* set-window-configuration may/will have unselected the
>  > 	     mini-window as the selected window.  Restore it. */
>  > 	  if (!EQ (orig_selected_window, FRAME_SELECTED_WINDOW (f)))
>  > 	    Fset_frame_selected_window (exp_MB_frame, window, Qnil);
>  > 	}
>  >
>  > Does it make sense to manipulate the buffer, window-start, and
>  > window-point of WINDOW if we are not going to make it the selected
>  > window?
> 
> If w->prev_buffers gets us the right minibuffer to display here (any
> former dw->prev_buffers = sw->prev_buffers should guarantee that), then
> for that buffer we have to establish the corresponding start and point
> positions.  This makes sense regardless of whether the minibuffer window
> will be selected or not.

But read_minibuf_unwind already does all that when it restores the
window configuration, doesn't it?  Or is there something about the
mini-window that restoring window configuration doesn't handle
correctly?

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Alan Mackenzie]

Hello, Stefan.

On Sat, Jun 10, 2023 at 11:49:43 -0400, Stefan Monnier wrote:
> Hi Alan,

> Could you explain why/when we need to call `Fset_frame_selected_window`
> in the code below:

> >   static void
> >   minibuffer_unwind (void)
> >   {
> >     struct frame *f;
> >     struct window *w;
> >     Lisp_Object window;
> >     Lisp_Object entry;
> >
> >     if (NILP (exp_MB_frame)) return; /* "Can't happen." */
> >     f = XFRAME (exp_MB_frame);
> >     window = f->minibuffer_window;
> >     w = XWINDOW (window);
> >     if (FRAME_LIVE_P (f))
> >       {
> > 	/* minibuf_window = sf->minibuffer_window; */
> > 	if (!NILP (w->prev_buffers))
> > 	  {
> > 	    entry = Fcar (w->prev_buffers);
> > 	    w->prev_buffers = Fcdr (w->prev_buffers);
> > 	    set_window_buffer (window, Fcar (entry), 0, 0);
> > 	    Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
> > 	    Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
> > 	    /* set-window-configuration may/will have unselected the
> > 	       mini-window as the selected window.  Restore it. */
> > 	    Fset_frame_selected_window (exp_MB_frame, window, Qnil);  <<<<<<<<<
> > 	  }
> > 	else
> > 	  set_window_buffer (window, nth_minibuffer (0), 0, 0);
> >       }
> >   }

> I understand why we do the `set_window_buffer` and set its start and
> point, but I can't see why we'd need to select the mini-window:
> presumably if it needed to be (re)selected that should have been handled
> by the window-config save&restore, no?

There was a problem with the restoring of the window configuration
selecting a window other than the minibuffer; that is in circumstances
where the minibuffer window should have ended up being selected.

Fset_window_configuration seems to be the troublesome function in this
scenario.  As well as setting the window configuration, it also selects
a window.

Perhaps we should modify the minibuffer code to note which window should
be current after the completion or abortion of the minibuffer read
action.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Sat, 10 Jun 2023 19:42:04 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, al@petrofsky.org, rudalics@gmx.at,
>   63967@debbugs.gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > I understand why we do the `set_window_buffer` and set its start and
> > point, but I can't see why we'd need to select the mini-window:
> > presumably if it needed to be (re)selected that should have been handled
> > by the window-config save&restore, no?
> 
> There was a problem with the restoring of the window configuration
> selecting a window other than the minibuffer; that is in circumstances
> where the minibuffer window should have ended up being selected.
> 
> Fset_window_configuration seems to be the troublesome function in this
> scenario.  As well as setting the window configuration, it also selects
> a window.

Where is that problem and its circumstances described?  Is there some
bug number or a discussion on emacs-devel that you could point to?

> Perhaps we should modify the minibuffer code to note which window should
> be current after the completion or abortion of the minibuffer read
> action.

Isn't that simply "the window which was selected before entering the
minibuffer"?

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[martin rudalics]

 >> If w->prev_buffers gets us the right minibuffer to display here (any
 >> former dw->prev_buffers = sw->prev_buffers should guarantee that), then
 >> for that buffer we have to establish the corresponding start and point
 >> positions.  This makes sense regardless of whether the minibuffer window
 >> will be selected or not.
 >
 > But read_minibuf_unwind already does all that when it restores the
 > window configuration, doesn't it?  Or is there something about the
 > mini-window that restoring window configuration doesn't handle
 > correctly?

I can imagine the following scenario: read_minibuf saves and restores up
to two window configurations - the one of the frame from where it was
invoked and the one of the frame that owned the corresponding minibuffer
window at that time.  Now with 'minibuffer-follows-selected-frame'
non-nil there might be another frame the user may want to continue the
interaction with and the configuration of that frame will not be
automatically restored upon exiting read_minibuf.  Yet, the minibuffer
window on that frame should be restored properly to show its previous
buffer, point etc.

martin

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Alan Mackenzie]

Hello, Eli.

On Sun, Jun 11, 2023 at 08:03:23 +0300, Eli Zaretskii wrote:
> > Date: Sat, 10 Jun 2023 19:42:04 +0000
> > Cc: Eli Zaretskii <eliz@gnu.org>, al@petrofsky.org, rudalics@gmx.at,
> >   63967@debbugs.gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > I understand why we do the `set_window_buffer` and set its start and
> > > point, but I can't see why we'd need to select the mini-window:
> > > presumably if it needed to be (re)selected that should have been handled
> > > by the window-config save&restore, no?

> > There was a problem with the restoring of the window configuration
> > selecting a window other than the minibuffer; that is in circumstances
> > where the minibuffer window should have ended up being selected.

> > Fset_window_configuration seems to be the troublesome function in this
> > scenario.  As well as setting the window configuration, it also selects
> > a window.

> Where is that problem and its circumstances described?  Is there some
> bug number or a discussion on emacs-devel that you could point to?

I remember such a discussion vaguely, but after much searching, haven't
been able to find it again.  Sorry.

> > Perhaps we should modify the minibuffer code to note which window should
> > be current after the completion or abortion of the minibuffer read
> > action.

> Isn't that simply "the window which was selected before entering the
> minibuffer"?

Most of the time, yes.  If that window no longer exists on termination of
the minibuffer, or we've moved to a different frame, things aren't so
simple.

In read_minibuf (the meat of the minibuffer processing), there is a
variable calling_window which records that window.  It gets pushed onto
the binding stack (along with a lot of other variables) around the
recursive edit, and then possibly restored in the unwind_protect function
read_minibuf_unwind.  However, restore_window_configuration overrides it,
because of the order these things are pushed onto the binding stack by
read_minibuf.

Do you want me to fix this bug?  It seems there are quite a lot of people
who've made observations about it in the last couple of days.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Sun, 11 Jun 2023 13:40:52 +0000
> Cc: monnier@iro.umontreal.ca, al@petrofsky.org, rudalics@gmx.at,
>   63967@debbugs.gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > Fset_window_configuration seems to be the troublesome function in this
> > > scenario.  As well as setting the window configuration, it also selects
> > > a window.
> 
> > Where is that problem and its circumstances described?  Is there some
> > bug number or a discussion on emacs-devel that you could point to?
> 
> I remember such a discussion vaguely, but after much searching, haven't
> been able to find it again.  Sorry.

Too bad.  If someone can find it, please speak up.  It is important to
know what those situations are to make sure the solution we install
for this bug doesn't break those situations.

> > > Perhaps we should modify the minibuffer code to note which window should
> > > be current after the completion or abortion of the minibuffer read
> > > action.
> 
> > Isn't that simply "the window which was selected before entering the
> > minibuffer"?
> 
> Most of the time, yes.  If that window no longer exists on termination of
> the minibuffer, or we've moved to a different frame, things aren't so
> simple.

So you are saying that minibuffer_unwind exists only for the case when
that window no longer exists?  But if so, where's that condition being
tested, in a way that minibuffer_unwind is a no-op if that window
still exists?

> In read_minibuf (the meat of the minibuffer processing), there is a
> variable calling_window which records that window.  It gets pushed onto
> the binding stack (along with a lot of other variables) around the
> recursive edit, and then possibly restored in the unwind_protect function
> read_minibuf_unwind.  However, restore_window_configuration overrides it,
> because of the order these things are pushed onto the binding stack by
> read_minibuf.

I don't understand the need for recording that window separately.  The
window configuration restored by restore_window_configuration is
supposed to record it.

> Do you want me to fix this bug?  It seems there are quite a lot of people
> who've made observations about it in the last couple of days.

I want to fix the bug whose recipe is in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63967#5 and other
similar ones, yes.  Basically, if Emacs reads from a recursive
minibuffer when the selected window before that was not a mini-window,
we now signal a user-error, which is a regression since Emacs 28, and
I'd like that to be solved.  But please begin by looking at the
solution proposed by Martin in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63967#38 and tell if it
looks good to you, and if not, why not.

I'd also appreciate more commentary explaining the rationale for
minibuffer_unwind and the situations where it is needed.  But that is
less urgent.

Thanks.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Drew Adams]

> > > Perhaps we should modify the minibuffer code
> > > to note which window should be current after
> > > the completion or abortion of the minibuffer
> > > read action.
> 
> > Isn't that simply "the window which was selected
> > before entering the minibuffer"?
> 
> Most of the time, yes.  If that window no longer
> exists on termination of the minibuffer, or we've
> moved to a different frame, things aren't so simple.

You can and will ignore this, but IMO _all_ of
the above is misguided and short-sighted.  "Isn't
that simply...?" is just plain wrong - both the
question and any single answer.

It isn't "simply" _anything_ you can preconceive.
It's _whichever window ended up_ selected after
using the minibuffer.  Nothing more, nothing less.

Let's-hard-code-which-window-should-be-selected
is maybe related to the fact that I can no longer
use Emacs > 26.3.

What's wrong with attempts to predetermine which
window should be selected after a minibuffer
read?

It's the presumption that a minibuffer's only
purpose is to return something read, and that the
state of Emacs, including which windows exist and
which should be selected, should be the same as
before the minibuffer was entered - or should be
any other predefined state.  The selected window
here shouldn't be determined formulaically.

This completely prevents or interferes with code
that _does things_ while the minibuffer is active
with the intent of changing such state, e.g., the
intent to change the selected window, and not
just till minibuffer reading is finished.

There.  I've said it again.  And clearly Emacs
won't be going back to its former freedom in
this regard - 1000 ships have sailed.  But IMO
this is a great loss.  And it comes, I think,
from assuming that others use existing behavior
only the way you do.

My use of Emacs relies on doing lots of things
while a minibuffer is active - including things
that you might do only when it's inactive.  And
the changes I make while its active shouldn't
be overridden when a minibuffer is exited.

And yes, this can include changing the selected
window and focused frame.  I want to be able to
do that myself, interactively or by definition
from the function that invoked the minibuffer.

To my mind you've hobbled Emacs - specifically
its minibuffer, just as much as if you'd poked
out its eyes or cut off its legs.

If you'd really wanted to provide only some
_default_ behavior wrt choosing the window to
select here, then you'd have done that.  You'd
have provided some way (e.g., a variable) for
code to _prevent_ force-selecting the window
you've predetermined to be the "chosen one".

The Emacs minibuffer was "simply" better
before you simply "fixed" it.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Alan Mackenzie]

Hello, Drew.

On Sun, Jun 11, 2023 at 14:35:59 +0000, Drew Adams wrote:
> > > > Perhaps we should modify the minibuffer code
> > > > to note which window should be current after
> > > > the completion or abortion of the minibuffer
> > > > read action.

> > > Isn't that simply "the window which was selected
> > > before entering the minibuffer"?

> > Most of the time, yes.  If that window no longer
> > exists on termination of the minibuffer, or we've
> > moved to a different frame, things aren't so simple.

> You can and will ignore this, ....

I'm answering your post.  ;-)

> .... but IMO _all_ of the above is misguided and short-sighted.  "Isn't
> that simply...?" is just plain wrong - both the question and any single
> answer.

> It isn't "simply" _anything_ you can preconceive.
> It's _whichever window ended up_ selected after
> using the minibuffer.  Nothing more, nothing less.

That window is the miniwindow.  Unless we're talking about a recursive
minibuffer use from the miniwindow, we need to select some other window
after using the minibuffer, otherwise the user would need explicitly to
select a window herself.

> Let's-hard-code-which-window-should-be-selected
> is maybe related to the fact that I can no longer
> use Emacs > 26.3.

> What's wrong with attempts to predetermine which
> window should be selected after a minibuffer
> read?

> It's the presumption that a minibuffer's only
> purpose is to return something read, and that the
> state of Emacs, including which windows exist and
> which should be selected, should be the same as
> before the minibuffer was entered - or should be
> any other predefined state.  The selected window
> here shouldn't be determined formulaically.

It needs to be determined somehow.  That "somehow" would probably count
as a formula in some sense.

> This completely prevents or interferes with code
> that _does things_ while the minibuffer is active
> with the intent of changing such state, e.g., the
> intent to change the selected window, and not
> just till minibuffer reading is finished.

If you do C-x C-f, do wierd and wonderful things whilst the minibuffer is
active, then select a file to vist, you seem to be saying you want that
file to be visited on what became the selected window just before
terminating the minibuffer.  Am I right, here?

If so, that would involve some new options to enable this behaviour, and
some new complications in the code to support it.

> There.  I've said it again.  And clearly Emacs
> won't be going back to its former freedom in
> this regard - 1000 ships have sailed.  But IMO
> this is a great loss.  And it comes, I think,
> from assuming that others use existing behavior
> only the way you do.

As one of the people who implemented a recent change in the minibuffer,
I'm not sure this is entirely fair.  Efforts were made to avoid
restricting the ways the minibuffer could be used.  The code is anything
but simple and straightforward.

It might be useful to compare with the way most non-Emacs systems cope
with these problems: they use what are known as "modal" dialogue boxes.
Having initiated one of these, there is no way of doing anything else in
the system, including looking at a buried frame, until the "modal"
dialogue has been terminated or aborted.  I think Emacs's way is better,
even if not perfect.

> My use of Emacs relies on doing lots of things
> while a minibuffer is active - including things
> that you might do only when it's inactive.  And
> the changes I make while its active shouldn't
> be overridden when a minibuffer is exited.

That is complicated to specify and implement.

> And yes, this can include changing the selected
> window and focused frame.  I want to be able to
> do that myself, interactively or by definition
> from the function that invoked the minibuffer.

The command using the minibuffer often acts on a buffer, window, and/or
frame.  In such cases, chaos could result from having a different current
buffer, selected window, or selected frame from what the command thinks
it has.  It is possible that such chaos was what prompted the restoration
of the window configuration, etc., after using the minibuffer.

I've had a look into git blame minibuf.c, and it would appear that the
use of set-window-configuration is very old indeed - there is a comment
by Richard Stallman from 1996-04-07 saying:

    /* We have to do this after saving the window configuration
       since that is what restores the current buffer.  */

..

> To my mind you've hobbled Emacs - specifically
> its minibuffer, just as much as if you'd poked
> out its eyes or cut off its legs.

I assure you there was no malice intended.  ;-)

> If you'd really wanted to provide only some
> _default_ behavior wrt choosing the window to
> select here, then you'd have done that.  You'd
> have provided some way (e.g., a variable) for
> code to _prevent_ force-selecting the window
> you've predetermined to be the "chosen one".

As remarked above, some window has to be selected (? "force-selected"),
otherwise point would remain in the miniwindow.  That window needs to be
determined somehow.

> The Emacs minibuffer was "simply" better
> before you simply "fixed" it.

There were things wrong with it, which have been fixed.  You have
mentioned version 26.3 as the latest one you can use.  Are you saying
that the minibuffer was fine for you at that release, or was it better
even earlier?

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> From: Drew Adams <drew.adams@oracle.com>
> CC: "al@petrofsky.org" <al@petrofsky.org>,
>         "rudalics@gmx.at"
> 	<rudalics@gmx.at>,
>         "monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>,
>         "63967@debbugs.gnu.org" <63967@debbugs.gnu.org>
> Date: Sun, 11 Jun 2023 14:35:59 +0000
> 
> > > > Perhaps we should modify the minibuffer code
> > > > to note which window should be current after
> > > > the completion or abortion of the minibuffer
> > > > read action.
> > 
> > > Isn't that simply "the window which was selected
> > > before entering the minibuffer"?
> > 
> > Most of the time, yes.  If that window no longer
> > exists on termination of the minibuffer, or we've
> > moved to a different frame, things aren't so simple.
> 
> You can and will ignore this, but IMO _all_ of
> the above is misguided and short-sighted.  "Isn't
> that simply...?" is just plain wrong - both the
> question and any single answer.

I will indeed ignore most of what you say -- because you seem to
completely misunderstand what this discussion is about.  See below.

> It isn't "simply" _anything_ you can preconceive.
> It's _whichever window ended up_ selected after
> using the minibuffer.  Nothing more, nothing less.

Using the minibuffer doesn't by itself cause any window to be
selected.  Using the minibuffer just supplies to whatever command that
prompted the user with the response to the prompt.  Then the
minibuffer is exited, and only then the command continues its
execution, and could very well choose another window to be the
selected one.

This discussion is about what happens _immediately_ after
read-from-minibuffer exits, which just provides the response to the
prompt.  It has nothing to do with what you are talking about, and
doesn't affect the window that will be the selected one after the
command exits.

read-from-minibuffer invokes recursive-editing cycle.  When that cycle
ends, it should leave the window/frame configuration intact, or as
intact as possible.  Why? because most commands that invoke
read-from-minibuffer in its interactive form do not expect the window
configuration to change after the interactive form finishes its job.

And if you still don't agree, then let's agree to disagree.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[martin rudalics]

 > It might be useful to compare with the way most non-Emacs systems cope
 > with these problems: they use what are known as "modal" dialogue boxes.
 > Having initiated one of these, there is no way of doing anything else in
 > the system, including looking at a buried frame, until the "modal"
 > dialogue has been terminated or aborted.  I think Emacs's way is better,
 > even if not perfect.

Alas, this is no longer true.  Start Emacs 29 via

emacs -Q --eval "(setq default-frame-alist '((minibuffer . nil)))"

Now type C-x 5 o to switch to the normal, minibuffer-less frame and then
type C-x f.  Next try to access the normal frame via C-x o, C-x 5 o, the
window manager's Alt-TAB or any other key combination.  This used to
work ever since.  Now you are in a modal dialogue exactly as described
above.

martin

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Mon, 12 Jun 2023 09:17:29 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, "al@petrofsky.org" <al@petrofsky.org>,
>  "monnier@iro.umontreal.ca" <monnier@iro.umontreal.ca>,
>  "63967@debbugs.gnu.org" <63967@debbugs.gnu.org>
> From: martin rudalics <rudalics@gmx.at>
> 
> Alas, this is no longer true.  Start Emacs 29 via
> 
> emacs -Q --eval "(setq default-frame-alist '((minibuffer . nil)))"
> 
> Now type C-x 5 o to switch to the normal, minibuffer-less frame and then
> type C-x f.  Next try to access the normal frame via C-x o, C-x 5 o, the
> window manager's Alt-TAB or any other key combination.  This used to
> work ever since.  Now you are in a modal dialogue exactly as described
> above.

Probably a bug that needs to be reported and fixed.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Cc: al@petrofsky.org, rudalics@gmx.at, monnier@iro.umontreal.ca,
>  63967@debbugs.gnu.org
> Date: Sun, 11 Jun 2023 16:53:48 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Do you want me to fix this bug?  It seems there are quite a lot of people
> > who've made observations about it in the last couple of days.
> 
> I want to fix the bug whose recipe is in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63967#5 and other
> similar ones, yes.  Basically, if Emacs reads from a recursive
> minibuffer when the selected window before that was not a mini-window,
> we now signal a user-error, which is a regression since Emacs 28, and
> I'd like that to be solved.  But please begin by looking at the
> solution proposed by Martin in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63967#38 and tell if it
> looks good to you, and if not, why not.

Ping!  Any progress there?

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Alan Mackenzie]

Hello, Eli.

On Tue, Jun 13, 2023 at 21:43:13 +0300, Eli Zaretskii wrote:
> > Cc: al@petrofsky.org, rudalics@gmx.at, monnier@iro.umontreal.ca,
> >  63967@debbugs.gnu.org
> > Date: Sun, 11 Jun 2023 16:53:48 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > 
> > > Do you want me to fix this bug?  It seems there are quite a lot of people
> > > who've made observations about it in the last couple of days.
> > 
> > I want to fix the bug whose recipe is in
> > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63967#5 and other
> > similar ones, yes.  Basically, if Emacs reads from a recursive
> > minibuffer when the selected window before that was not a mini-window,
> > we now signal a user-error, which is a regression since Emacs 28, and
> > I'd like that to be solved.  But please begin by looking at the
> > solution proposed by Martin in
> > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63967#38 and tell if it
> > looks good to you, and if not, why not.

> Ping!  Any progress there?

Sorry.  I've had a few very busy days in real life.

Firstly, I'm reasonably sure that the Fset_frame_selected_window I put
into minibuf_unwind was a mistake based on the misconception that after
reading the minibuffer recursively, we always wanted to go back to the
next outer minibuffer.

I think Martin's patch (from 2023-06-10 08:52 +0200) is along the right
lines, i.e. go back to the invoking window rather than the minibuffer
window.  But I'm not sure whether it might be better, from the point of
view of preserving maintainability of read_minibuf, to try to get the
restore_window_buffer_configuration call to go back to the calling
window, rather than adding the extra parameter to minibuffer_unwind.

It would be somewhat complicated by the need to go to some other window
if that calling window no longer exists.  We need to decide which
window.  This dilemma might have been what moved me to put that
Fset_frame_selected_window in in the first place.  :-(

So I see the fix somewhat along the lines of removing the offending
Fset_frame_selected_window from minibuffer_unwind, and maybe rearranging
the calls to record_unwind_protect* in read_minibuf to arrange for the
window configuration call to do the job of restoring the "current"
window.

It seems to me you might want to put the fix into Emacs 29.  Am I right?

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Tue, 13 Jun 2023 21:36:37 +0000
> Cc: al@petrofsky.org, rudalics@gmx.at, monnier@iro.umontreal.ca,
>   63967@debbugs.gnu.org, acm@muc.de
> From: Alan Mackenzie <acm@muc.de>
> 
> So I see the fix somewhat along the lines of removing the offending
> Fset_frame_selected_window from minibuffer_unwind, and maybe rearranging
> the calls to record_unwind_protect* in read_minibuf to arrange for the
> window configuration call to do the job of restoring the "current"
> window.
> 
> It seems to me you might want to put the fix into Emacs 29.  Am I right?

Yes.  I wouldn't be pinging you if it weren't so.

So I hope you will present a suggested patch soon, and that it will be
simple and safe enough to install on the release branch.  TIA.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Alan Mackenzie]

Hello, Eli.

On Wed, Jun 14, 2023 at 15:15:14 +0300, Eli Zaretskii wrote:
> > Date: Tue, 13 Jun 2023 21:36:37 +0000
> > Cc: al@petrofsky.org, rudalics@gmx.at, monnier@iro.umontreal.ca,
> >   63967@debbugs.gnu.org, acm@muc.de
> > From: Alan Mackenzie <acm@muc.de>
> > 
> > So I see the fix somewhat along the lines of removing the offending
> > Fset_frame_selected_window from minibuffer_unwind, and maybe rearranging
> > the calls to record_unwind_protect* in read_minibuf to arrange for the
> > window configuration call to do the job of restoring the "current"
> > window.
> > 
> > It seems to me you might want to put the fix into Emacs 29.  Am I right?

> Yes.  I wouldn't be pinging you if it weren't so.

:-)

> So I hope you will present a suggested patch soon, and that it will be
> simple and safe enough to install on the release branch.  TIA.

OK, thanks.  I should have enough time to look at it properly from this
evening (European time).  I hope to have a patch ready on Friday, or
possibly Saturday.

As for "simple and safe enough", we'll see what we can manage.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Alan Mackenzie]

Hello, Eli.

On Wed, Jun 14, 2023 at 15:15:14 +0300, Eli Zaretskii wrote:
> > Date: Tue, 13 Jun 2023 21:36:37 +0000
> > Cc: al@petrofsky.org, rudalics@gmx.at, monnier@iro.umontreal.ca,
> >   63967@debbugs.gnu.org, acm@muc.de
> > From: Alan Mackenzie <acm@muc.de>
> > 
> > So I see the fix somewhat along the lines of removing the offending
> > Fset_frame_selected_window from minibuffer_unwind, and maybe rearranging
> > the calls to record_unwind_protect* in read_minibuf to arrange for the
> > window configuration call to do the job of restoring the "current"
> > window.
> > 
> > It seems to me you might want to put the fix into Emacs 29.  Am I right?

> Yes.  I wouldn't be pinging you if it weren't so.

> So I hope you will present a suggested patch soon, and that it will be
> simple and safe enough to install on the release branch.  TIA.

After spending quite some time looking at why that call to
Fset_frame_selected_window went in in the first place, and checking
quite a few of the bug scenarios from 2021, I'm now convinced that the
sole fix we need is to remove that call from minibuffer_unwind.

It's certainly a simple patch, and I think it's safe enough for Emacs
29.  Maybe other people (e.g. Martin) would be good enough to give it a
quick going over before we commit it.



diff --git a/src/minibuf.c b/src/minibuf.c
index d5f95968ae1..bcb7eb9375d 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1266,9 +1266,6 @@ minibuffer_unwind (void)
 	  set_window_buffer (window, Fcar (entry), 0, 0);
 	  Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
 	  Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
-	  /* set-window-configuration may/will have unselected the
-	     mini-window as the selected window.  Restore it. */
-	  Fset_frame_selected_window (exp_MB_frame, window, Qnil);
 	}
       else
 	set_window_buffer (window, nth_minibuffer (0), 0, 0);



-- 
Alan Mackenzie (Nuremberg, Germany).

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Eli Zaretskii]

> Date: Sat, 17 Jun 2023 11:31:54 +0000
> Cc: al@petrofsky.org, rudalics@gmx.at, monnier@iro.umontreal.ca,
>   63967@debbugs.gnu.org, acm@muc.de
> From: Alan Mackenzie <acm@muc.de>
> 
> > So I hope you will present a suggested patch soon, and that it will be
> > simple and safe enough to install on the release branch.  TIA.
> 
> After spending quite some time looking at why that call to
> Fset_frame_selected_window went in in the first place, and checking
> quite a few of the bug scenarios from 2021, I'm now convinced that the
> sole fix we need is to remove that call from minibuffer_unwind.
> 
> It's certainly a simple patch, and I think it's safe enough for Emacs
> 29.  Maybe other people (e.g. Martin) would be good enough to give it a
> quick going over before we commit it.

Thanks.

Martin, please comment.  Your own proposal was similar, so I hope you
will agree to this.

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[martin rudalics]

 > It's certainly a simple patch, and I think it's safe enough for Emacs
 > 29.  Maybe other people (e.g. Martin) would be good enough to give it a
 > quick going over before we commit it.

I cannot see any problems with it.

martin

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Alan Mackenzie]

Hello, Martin and Eli.

On Sat, Jun 17, 2023 at 15:52:22 +0200, martin rudalics wrote:
>  > It's certainly a simple patch, and I think it's safe enough for Emacs
>  > 29.  Maybe other people (e.g. Martin) would be good enough to give it a
>  > quick going over before we commit it.

> I cannot see any problems with it.

Thanks, Martin.

I've now committed the patch to the emacs-29 branch, and I'm closing the
bug with this post.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#63967: 28.2; switch-to-buffer in normal window fails if minibuffer window is active

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> After spending quite some time looking at why that call to
> Fset_frame_selected_window went in in the first place, and checking
> quite a few of the bug scenarios from 2021, I'm now convinced that the
> sole fix we need is to remove that call from minibuffer_unwind.

That was my intuition, but I wasn't confident enough in my understanding
of the code to suggest it :-)


        Stefan