Re: Proposed removal of struct frame->select_mini_window_flag.

[Alan Mackenzie <acm@muc.de>]

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).