Proposed removal of struct frame->select_mini_window_flag.

Proposed removal of struct frame->select_mini_window_flag.

[Alan Mackenzie]

I came across struct frame->select_mini_window_flag while looking at bug
#55414.

More accurately, my attention was diverted for over an hour, trying to
work out how this boolean could be maintained in a consistent state,
given that it gets splatted to false every time Fselect_window gets
called.

Then it dawned on me that the variable has no function - if the
mini-window is the selected window, this is fully represented by struct
frame->selected_window's value.

So, I propose to remove this variable and all its "uses".  Does anybody
object?

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Proposed removal of struct frame->select_mini_window_flag.

[Eli Zaretskii]

> Date: Wed, 18 May 2022 15:56:28 +0000
> From: Alan Mackenzie <acm@muc.de>
> 
> I came across struct frame->select_mini_window_flag while looking at bug
> #55414.
> 
> More accurately, my attention was diverted for over an hour, trying to
> work out how this boolean could be maintained in a consistent state,
> given that it gets splatted to false every time Fselect_window gets
> called.
> 
> Then it dawned on me that the variable has no function - if the
> mini-window is the selected window, this is fully represented by struct
> frame->selected_window's value.
> 
> So, I propose to remove this variable and all its "uses".  Does anybody
> object?

I don't think I follow.  Did you see this fragment in frame.c:

  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;

How will this logic work without that flag?

Re: Proposed removal of struct frame->select_mini_window_flag.

[Alan Mackenzie]

Hello, Eli.

On Wed, May 18, 2022 at 19:19:49 +0300, Eli Zaretskii wrote:
> > Date: Wed, 18 May 2022 15:56:28 +0000
> > From: Alan Mackenzie <acm@muc.de>

> > I came across struct frame->select_mini_window_flag while looking at bug
> > #55414.

> > More accurately, my attention was diverted for over an hour, trying to
> > work out how this boolean could be maintained in a consistent state,
> > given that it gets splatted to false every time Fselect_window gets
> > called.

> > Then it dawned on me that the variable has no function - if the
> > mini-window is the selected window, this is fully represented by struct
> > frame->selected_window's value.

> > So, I propose to remove this variable and all its "uses".  Does anybody
> > object?

> I don't think I follow.  Did you see this fragment in frame.c:

>   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;

Yes, I saw it.

> How will this logic work without that flag?

There is no logic which uses it.  These lines in frame.c set the flag in
sf (the old frame), and test it in f (the new frame).  It is intended to
be a durable flag saying whether or not the mini-window in a non-selected
frame is to be selected at the next do_switch_frame () into it.  This
might make sense if there were something setting f->selected_window without
going through the proper functions.  But there isn't, apart from in an
initialisation of a new struct frame in xdisp.c.

What we do have is the setting of the flag unconditionally to false in
select_window (window.c), with no attempt being made to save and restore
the current value.  Considering that Fselect_window can be called from
gui_consider_frame_title (xdisp.c) during a redisplay (with the current
bug fix), this means the flag can be initialised to false asynchronously
at any time.  This is strong evidence that it is not really used.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Proposed removal of struct frame->select_mini_window_flag.

[Eli Zaretskii]

> Date: Wed, 18 May 2022 17:03:13 +0000
> Cc: emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> >   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;
> 
> Yes, I saw it.
> 
> > How will this logic work without that flag?
> 
> There is no logic which uses it.

How do you mean "no logic"?  I mean this logic, right there:

  if (f->select_mini_window_flag
      && !NILP (Fminibufferp (XWINDOW (f->minibuffer_window)->contents, Qt)))
    f->selected_window = f->minibuffer_window;


> These lines in frame.c set the flag in
> sf (the old frame), and test it in f (the new frame).  It is intended to
> be a durable flag saying whether or not the mini-window in a non-selected
> frame is to be selected at the next do_switch_frame () into it.  This
> might make sense if there were something setting f->selected_window without
> going through the proper functions.

The code above does just that, does it not?  It directly sets
f->selected_window.

> What we do have is the setting of the flag unconditionally to false in
> select_window (window.c), with no attempt being made to save and restore
> the current value.  Considering that Fselect_window can be called from
> gui_consider_frame_title (xdisp.c) during a redisplay (with the current
> bug fix), this means the flag can be initialised to false asynchronously
> at any time.

What do you mean by "asynchronously"?  gui_consider_frame_title is not
called asynchronously, at least under my interpretation of that term.