unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Alan Mackenzie <acm@muc.de>
To: martin rudalics <rudalics@gmx.at>
Cc: 55412@debbugs.gnu.org, tanzer@swing.co.at, Eli Zaretskii <eliz@gnu.org>
Subject: bug#55412: 28.1; In Emacs 28.1, using ':eval' in 'frame-title-format' doesn't work properly
Date: Fri, 20 May 2022 11:33:45 +0000	[thread overview]
Message-ID: <Yod8mfQv2TGNmmJp@ACM> (raw)
In-Reply-To: <408f7b88-2000-bd10-bd9d-4e06018e8ff0@gmx.at>

Hello, Martin.

On Fri, May 20, 2022 at 10:23:17 +0200, martin rudalics wrote:
>  > It would be good to have some of these explanations in comments there.

> I understand that we want a quick solution for the present bug so it can
> be included in Emacs 28.2.  But I still don't understand why nobody even
> cared to try the patch I sent earlier.

My apologies for not being aware of it.  Your post from last Sunday with
the patch was before I was in the thread, and I missed, in your
references to the patch, that it was a recent post in this thread.

I've just spent about an hour perusing the patch, and have the following
observations and questions.

The patch is too large to go into the emacs-28 branch, so for a "quick
safe fix", we'd still need the smaller patch I posted a small number of
days ago, or something like it.

with_window_selected is not re-entrant, since it uses a static data
structure.  Is this deliberate, or just a case of not yet making it
re-entrant?

I get nervous when I see "selected_frame = ...;" or "selected_window =
....;" outside of do_switch_frame or the corresponding window function,
since it means selected_{frame,window} have been set without regard to
all the other things which must be kept in synch with them.  At the
moment, all these "wild" settings of selected_frame are in xdisp.c,
which I suppose has special reasons for them.

with_window_selected would add another "wild" setting of selected_frame,
this one outside of xdisp.c (in window.c) and I ask whether or not this
is a good thing.

I think the root of the problem your patch is trying to solve is that
the same C code is used for implementing C-x 5 o and lower level,
temporary, frame switches.  If this is the case, a good way of
proceeding would be to refactor do_switch_frame into a function
appropriate for lower level C calls, and the remainder for C-x 5 o.  For
example, the call to move_minibuffers_onto_frame is clearly needed for
C-x 5 o, but is a complicated nuisance for lower level C.  With such a
new extracted C function, we could replace the "selected_frame = ...;"
in xdisp.c by calls to this function.  Maybe.

> With the current code, whenever there are at least two frames present,
> 'gui_consider_frame_title' calls via Fselect_window among others

> redisplay_other_windows

> Fredirect_frame_focus

> resize_mini_window

> move_minibuffers_onto_frame

> and sets

> last_nonminibuf_frame

> internal_last_event_frame

Yes.  It also clears f->select_mini_window_flag, which I posted a
separate thread on emacs-devel about.  Here I think the problem is the
same as above: Fselect_window is a high level function for doing things
like C-x o, but we really need a low level function with which we could
do a controlled temporary switch to a different window, without the
unwanted complications of move_minibuffers_onto_frame etc..

> Has anyone ever tried to understand the implications of all these?  Why
> should redisplay indiscriminately set 'windows_or_buffers_changed' when
> recomputing the frame title?  Why should we try to redirect frame focus
> which is already sufficiently hairy by itself so hardly anyone really
> understands what it does.  Why should formatting the frame title try to
> resize a mini window or move minibuffers onto the selected frame?

I think my analysis above might point a way out of these problems.

> Why set last_nonminibuf_frame which might affect 'display-buffer' and
> apparently relies on some internal kludgery to set it precisely to the
> same value it had before title line formatting started.  And why reset
> internal_last_event_frame which also appears complicated enough to not
> touch it unless you know precisely why and when.

> Similar things happen with mode lines display - unwind_format_mode_line
> apparently can call Fselect_window up to three times in a row with the
> implications sketched above.

> When trying to fix Bug#32777 I spent some time investigating these
> issues but never found out why on earth we should call routines like
> 'select-frame' and 'select-window' from redisplay.  If there is any
> rationale for these, it should be explained in comments first before
> moving on to explain why moving minibuffers between frames can go awry.

I don't think redisplay should be calling Fselect_window or
do_switch_frame at all.  Instead we should call (not yet existing) lower
level functions.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).





  parent reply	other threads:[~2022-05-20 11:33 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-14 15:45 bug#55412: 28.1; In Emacs 28.1, using ':eval' in 'frame-title-format' doesn't work properly Christian Tanzer
2022-05-14 16:58 ` Eli Zaretskii
2022-05-14 17:07   ` Eli Zaretskii
2022-05-15  9:48   ` Alan Mackenzie
2022-05-15 10:16     ` Eli Zaretskii
2022-05-16 20:52   ` Alan Mackenzie
2022-05-17 13:25     ` Eli Zaretskii
2022-05-18  7:19     ` martin rudalics
2022-05-18 11:35       ` Eli Zaretskii
2022-05-18 15:01         ` martin rudalics
2022-05-18 15:12           ` Alan Mackenzie
2022-05-18 15:49           ` Alan Mackenzie
2022-05-18 16:03             ` Eli Zaretskii
2022-05-18 16:38               ` Alan Mackenzie
2022-05-18 16:51                 ` Eli Zaretskii
2022-05-20  8:23                   ` martin rudalics
2022-05-20 10:58                     ` Eli Zaretskii
2022-05-21  8:32                       ` martin rudalics
2022-05-21  8:35                         ` Eli Zaretskii
2022-05-20 11:33                     ` Alan Mackenzie [this message]
2022-05-20 12:10                       ` Eli Zaretskii
2022-05-21  8:32                       ` martin rudalics
2022-05-21  8:59                         ` Eli Zaretskii
2022-05-19  7:18             ` martin rudalics
2022-05-20 20:39   ` Alan Mackenzie
2022-05-21 11:25     ` tanzer--- via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-05-21 12:35       ` Eli Zaretskii
2022-05-22 17:34       ` Alan Mackenzie
2022-05-15  9:28 ` martin rudalics

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=Yod8mfQv2TGNmmJp@ACM \
    --to=acm@muc.de \
    --cc=55412@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=rudalics@gmx.at \
    --cc=tanzer@swing.co.at \
    /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).