Re: [PATCH] Fix display-buffer-use-some-window to honor reusable-frames

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sat, 28 Jan 2023 11:46:30 +0100
> Cc: emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Martin, any comments?
> 
> Sorry but I had no idea what 'display-buffer-use-least-recent-window' is
> and what it is supposed to accomplish.

See bug#45688.  You took part in its discussion.

I'm not sure what are your recommendations regarding the feature
installed as result of that discussion.  Would you mind spelling them
out?

> (1) 'display-buffer-use-least-recent-window' claims to be a buffer
> display action function but IIUC it does not return the window used.
> For example, with emacs -Q do C-x 2, C-x o and evaluate
> 
> (display-buffer
>   (get-buffer-create "*foo*")
>   '((display-buffer-use-least-recent-window display-buffer-at-bottom)))
> 
> You get two windows showing *foo*.  This one is easy to fix.

Yes, easy to fix.

> (2) The current version of 'window-bump-use-time' changes the semantics
> of "least recently used window" without even mentioning that anywhere.
> For example, this code in sql.el
> 
>      (let ((one-win (eq (selected-window)
>                         (get-lru-window))))
> 
> will conclude that there is only one window even if another window
> recently created by 'display-buffer-use-least-recent-window' exists.  I
> have no idea how 'get-mru-window' could be affected.
> 
> This is a grave bug.  'window-bump-use-time' could try
> 
>    struct window *sw = XWINDOW (selected_window);
> 
>    if (w != sw)
>      {
>        w->use_time = ++window_select_count;
>        sw->use_time = ++window_select_count;
>      }
> 
> to make sure that the selected window invariably stays the most recently
> used one but this would require some testing.

Would it work to just temporarily select the window inside
display-buffer-use-least-recent-window, so that its use time is bumped
without any sneaky primitives?  Then we could remove
window-bump-use-time.

> (3) The idea of having one action function bump use times works iff
> 'display-buffer-use-least-recent-window' is the _only_ action function
> called by both user and application.  It breaks as soon as another
> action function kicks in - either because the other action function does
> not call 'get-lru-window' or because that other action function does not
> bump the use time of the window used.  This is impossible to fix with
> the current approach.

I'm okay with not fixing this.  After all, this is a feature that only
those who are interested will be using.  We can document this gotcha
of using it in the doc string.

> A few words about Tom's patch: 'display-buffer-use-some-window' must not
> pop up a new window.  It's doc-string clearly says to "Display BUFFER in
> an existing window."  Please don't ever try to change that.  Also, it
> should not care about 'reusable-frames'.  The latter is about reusing a
> window that already shows BUFFER and other action functions might be
> affected badly if 'display-buffer-use-some-window' tried to handle this
> too.  Finally, 'display-buffer-no-window' has no place in another action
> function.  It is strictly reserved to callers and users.

Thanks.  I hope Tom will amend the patch accordingly.