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

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

[Tom Gillespie]

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

Hi,
   Here is a patch to fix strange and disruptive behavior
caused by a bug in display-buffer-use-least-recent-window
and display-buffer-use-some-window. I have made the patch
against the emacs-29 branch. A detailed explanation is in
the commit message. Best!
Tom

[-- Attachment #2: 0001-Fix-display-buffer-use-some-window-to-honor-reusable.patch --]
[-- Type: text/x-patch, Size: 3706 bytes --]

From 60cc79088e440e66d24d4c13d91c4d3c8e44bda3 Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
Date: Thu, 26 Jan 2023 23:47:22 -0500
Subject: [PATCH] Fix display-buffer-use-some-window to honor reusable-frames
 setting.

* lisp/window.el (display-buffer-use-some-window): Honor user supplied
reusable-frames setting if it is present in the alist.
* lisp/window.el (display-buffer-use-least-recent-window): Set
reusable-frames nil unless it is already present in the alist.

The docs for 'display-buffer-use-least-recent-window' state that it
will pick the least recently used window or and if there is only a
single window it will split the window.

Prior to this commit that behavior was impossible to achieve because
'display-buffer-use-some-window' would attempt to find a valid window
in other frames. The old behavior is retained when reusable-frames is
not explicitly included in the alist. When reusable-frames is provided
in the alist then 'display-buffer-use-some-window' honors that and
will split the window if the value is (reusable-frames . nil) since
the user's intention is clear, they do not want to reuse some other
frame.

This makes it possible to obtain the documented behavior for
'display-buffer-use-least-recent-window' by adding reusable-frames nil
to the alist if it is absent.

I'm not sure that this behavior is exactly correct, but it prevents
the insanity of having multiple random windows in random frames
changed to the buffer to be displayed, destroying whatever state the
user was maintaining in those windows.
---
 lisp/window.el | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/lisp/window.el b/lisp/window.el
index 0cd30822ff6..9bdcf7b8b36 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8509,7 +8509,11 @@ display-buffer-use-least-recent-window
 when displaying buffers repeatedly, and if there's only a single
 window, it will split the window."
   (when-let ((window (display-buffer-use-some-window
-                      buffer (cons (cons 'inhibit-same-window t) alist))))
+                      buffer
+                      (let ((alist (cons (cons 'inhibit-same-window t) alist)))
+                        (if (assq 'reusable-frames alist)
+                            alist
+                          (cons (cons 'reusable-frames nil) alist))))))
     (window-bump-use-time window)))
 
 (defun display-buffer-use-some-window (buffer alist)
@@ -8530,11 +8534,27 @@ display-buffer-use-some-window
 called only by `display-buffer' or a function directly or
 indirectly called by the latter."
   (let* ((not-this-window (cdr (assq 'inhibit-same-window alist)))
+         (explicit-reusable-frames (assq 'reusable-frames alist))
+         (reusable-frames (cdr explicit-reusable-frames))
 	 (frame (or (window--frame-usable-p (selected-frame))
 		    (window--frame-usable-p (last-nonminibuffer-frame))))
 	 (window
 	  ;; Reuse an existing window.
 	  (or (get-lru-window frame nil not-this-window)
+          (and
+           explicit-reusable-frames
+           (or
+            (and
+             (not reusable-frames)
+             (let ((window (window--try-to-split-window (selected-window) alist)))
+               (unless (and not-this-window
+			                (eq window (selected-window)))
+		         window)))
+            (let ((window (get-buffer-window buffer reusable-frames)))
+		      (unless (and not-this-window
+			               (eq window (selected-window)))
+		        window))
+            (display-buffer-no-window buffer alist)))
 	      (let ((window (get-buffer-window buffer 'visible)))
 		(unless (and not-this-window
 			     (eq window (selected-window)))
-- 
2.39.1

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

[Tom Gillespie]

The behavior for this patch is not quite right.
The split window should be temporary if it is
created and the previous layout restored.
Need to check how to do that and will send
an updated patch.
Tom

On Fri, Jan 27, 2023 at 12:17 AM Tom Gillespie <tgbugs@gmail.com> wrote:
>
> Hi,
>    Here is a patch to fix strange and disruptive behavior
> caused by a bug in display-buffer-use-least-recent-window
> and display-buffer-use-some-window. I have made the patch
> against the emacs-29 branch. A detailed explanation is in
> the commit message. Best!
> Tom

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

[Tom Gillespie]

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

Here is the corrected patch which uses display-buffer-pop-up-window
instead of window--try-to-split-window. I tested the behavior of xemacs
and as far as I can tell display-buffer-use-least-recent-window now
matches. Best,
Tom

[-- Attachment #2: 0001-Fix-display-buffer-use-some-window-to-honor-reusable.patch --]
[-- Type: text/x-patch, Size: 3832 bytes --]

From e11331ed716c97d7cb1306037a60c31bc189af6d Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
Date: Thu, 26 Jan 2023 23:47:22 -0500
Subject: [PATCH] Fix display-buffer-use-some-window to honor reusable-frames
 setting.

* lisp/window.el (display-buffer-use-some-window): Honor user supplied
reusable-frames setting if it is present in the alist.
* lisp/window.el (display-buffer-use-least-recent-window): Set
reusable-frames nil unless it is already present in the alist.

The docs for 'display-buffer-use-least-recent-window' state that it
will pick the least recently used window or and if there is only a
single window it will split the window.

Prior to this commit that behavior was impossible to achieve because
'display-buffer-use-some-window' would attempt to find a valid window
in other frames.  The old behavior is retained when reusable-frames is
not explicitly included in the alist.  When reusable-frames is
provided in the alist then 'display-buffer-use-some-window' honors
that and will split the window if the value is (reusable-frames . nil)
since the user's intention is clear, they do not want to reuse some
other frame.  'display-buffer-pop-up-window' is used to split the
window in this case so that permenent changes are not made to the
window layout.

This makes it possible to obtain the documented behavior for
'display-buffer-use-least-recent-window' by adding reusable-frames nil
to the alist if it is absent.

I'm not sure that this behavior is exactly correct, but it prevents
the insanity of having multiple random windows in random frames
changed to the buffer to be displayed, destroying whatever state the
user was maintaining in those windows.
---
 lisp/window.el | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/lisp/window.el b/lisp/window.el
index 0cd30822ff6..86c26d3a087 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8509,7 +8509,11 @@ display-buffer-use-least-recent-window
 when displaying buffers repeatedly, and if there's only a single
 window, it will split the window."
   (when-let ((window (display-buffer-use-some-window
-                      buffer (cons (cons 'inhibit-same-window t) alist))))
+                      buffer
+                      (let ((alist (cons (cons 'inhibit-same-window t) alist)))
+                        (if (assq 'reusable-frames alist)
+                            alist
+                          (cons (cons 'reusable-frames nil) alist))))))
     (window-bump-use-time window)))
 
 (defun display-buffer-use-some-window (buffer alist)
@@ -8530,11 +8534,27 @@ display-buffer-use-some-window
 called only by `display-buffer' or a function directly or
 indirectly called by the latter."
   (let* ((not-this-window (cdr (assq 'inhibit-same-window alist)))
+         (explicit-reusable-frames (assq 'reusable-frames alist))
+         (reusable-frames (cdr explicit-reusable-frames))
 	 (frame (or (window--frame-usable-p (selected-frame))
 		    (window--frame-usable-p (last-nonminibuffer-frame))))
 	 (window
 	  ;; Reuse an existing window.
 	  (or (get-lru-window frame nil not-this-window)
+          (and
+           explicit-reusable-frames
+           (or
+            (and
+             (not reusable-frames)
+             (let ((window (display-buffer-pop-up-window buffer alist)))
+               (unless (and not-this-window
+			                (eq window (selected-window)))
+		         window)))
+            (let ((window (get-buffer-window buffer reusable-frames)))
+		      (unless (and not-this-window
+			               (eq window (selected-window)))
+		        window))
+            (display-buffer-no-window buffer alist)))
 	      (let ((window (get-buffer-window buffer 'visible)))
 		(unless (and not-this-window
 			     (eq window (selected-window)))
-- 
2.39.1

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

[Eli Zaretskii]

> From: Tom Gillespie <tgbugs@gmail.com>
> Date: Fri, 27 Jan 2023 01:19:46 -0500
> Cc: Lars Ingebrigtsen <larsi@gnus.org>
> 
> Here is the corrected patch which uses display-buffer-pop-up-window
> instead of window--try-to-split-window. I tested the behavior of xemacs
> and as far as I can tell display-buffer-use-least-recent-window now
> matches. Best,

Thanks.

Martin, any comments?

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

[martin rudalics]

 > Martin, any comments?

Sorry but I had no idea what 'display-buffer-use-least-recent-window' is
and what it is supposed to accomplish.  This part of its doc-string "but
will cycle through windows when displaying buffers repeatedly" seems to
describe what 'display-buffer-use-some-window' does.  The "if there's
only a single window, it will split the window" does not fit at all - it
would rely on some extraneous mechanism like a base action to do that
(and IIUC Tom's patch tries to address that issue).  OTOH, the fact that
'display-buffer-use-least-recent-window' bumps a window's use time is
nowhere mentioned AFAICT.

IIUC the idea of bumping the use time is to make sure that a subsequent
'display-buffer' does not reuse the window that's used here because we
pretend that that window was selected recently and so 'get-lru-window'
will avoid it.  The doc-string should have said that.

IMO the present approach has three defects which break things and
ultimately make it useless for 'display-buffer'.


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


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


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

One thing we could do is to add a new action alist entry say
'bump-use-time'.  People could use this to indicate that they want to
display several buffers in a row.  'window--display-buffer' would then
bump the use time of the window it uses when ALIST contains that entry.
I have no idea how this would work in practice.


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.

martin

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

[Eli Zaretskii]

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

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

[martin rudalics]

 >> 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 remember.  But I never read the code Lars installed including that of
'window-bump-use-time'.  I would not have left alone a doc-string saying

   Display BUFFER in an existing window, but that hasn't been used lately.

for example.

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

Most of it has been said back then in this dialogue:

   >> This
   >>
   >> +  (when-let ((window (display-buffer-use-some-window
   >> +                      buffer (cons (cons 'inhibit-same-window t) alist))))
   >> +    (window-bump-use-time window)))
   >>
   >> alone will bump the use time for a _reused_ window only.  A freshly
   >> popped up window will continue to be the first candidate for reuse and
   >> only then enter the cycle of windows to reuse.
   >
   > How do we bump the use to for a new window, then?

   By bumping the time stamp of _any_ window 'display-buffer' uses for
   displaying a buffer (just like XEmacs does).

   >> XEmacs treats _all_ windows it creates or uses equal in this regard
   >> including those on other frames.
   >
   > Oh, other frames, too...  I think we'll leave that as an exercise for
   > the reader.

   Then when you switch from one frame to another, any problems you've
   found on the previous frame will immediately reemerge on the new frame.

We need a way to bump the time stamp of _any_ window used.  Otherwise,
the various action functions will continue to fight each other.  Lars
wanted to go the way XEmacs did but stopped in the middle.  And XEmacs
uses

               (if (window-buffer window)
                   (save-excursion
                     (save-selected-window
                       (select-window window)
                       (record-buffer (window-buffer window)))))

to bump the use time of every window used by 'display-buffer'.

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

As Lars conceived it, independent 'display-buffer' calls should be able
to build on previous ones.  Otherwise, we could write a function like
'display-many-buffers-at-once', within that function mark all windows
used as temporarily dedicated to their buffers, and at the end restore
the previous dedicated states of these windows.  Obviously, a function
like 'display-many-buffers-at-once' would not qualify as buffer display
action function.

martin

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

[Eli Zaretskii]

> Date: Sat, 28 Jan 2023 16:35:52 +0100
> Cc: tgbugs@gmail.com, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > I'm not sure what are your recommendations regarding the feature
>  > installed as result of that discussion.  Would you mind spelling them
>  > out?
> 
> Most of it has been said back then in this dialogue:

It's a very long discussion.  Just reading all of it takes time, let
alone understanding all that you two have been considering.  I hoped
you could help me do this more efficiently.

>    >> This
>    >>
>    >> +  (when-let ((window (display-buffer-use-some-window
>    >> +                      buffer (cons (cons 'inhibit-same-window t) alist))))
>    >> +    (window-bump-use-time window)))
>    >>
>    >> alone will bump the use time for a _reused_ window only.  A freshly
>    >> popped up window will continue to be the first candidate for reuse and
>    >> only then enter the cycle of windows to reuse.
>    >
>    > How do we bump the use to for a new window, then?
> 
>    By bumping the time stamp of _any_ window 'display-buffer' uses for
>    displaying a buffer (just like XEmacs does).
> 
>    >> XEmacs treats _all_ windows it creates or uses equal in this regard
>    >> including those on other frames.
>    >
>    > Oh, other frames, too...  I think we'll leave that as an exercise for
>    > the reader.
> 
>    Then when you switch from one frame to another, any problems you've
>    found on the previous frame will immediately reemerge on the new frame.
> 
> We need a way to bump the time stamp of _any_ window used.

Sorry, I don't understand what that means in practice.  Specifically,
what do you mean by "any window used"? used by whom and under what
circumstances? or do you mean any window that is currently displayed
on all the frames?

> Otherwise,
> the various action functions will continue to fight each other.  Lars
> wanted to go the way XEmacs did but stopped in the middle.  And XEmacs
> uses
> 
>                (if (window-buffer window)
>                    (save-excursion
>                      (save-selected-window
>                        (select-window window)
>                        (record-buffer (window-buffer window)))))
> 
> to bump the use time of every window used by 'display-buffer'.

You mean, do this every time display-buffer is called and selects some
window?  But that would change our behavior for all the callers of
display-buffer, whose names is a legion.  Whereas the intent was to
provide an optional feature that hopefully doesn't affect any existing
behaviors.

>  > 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.
> 
> As Lars conceived it, independent 'display-buffer' calls should be able
> to build on previous ones.  Otherwise, we could write a function like
> 'display-many-buffers-at-once', within that function mark all windows
> used as temporarily dedicated to their buffers, and at the end restore
> the previous dedicated states of these windows.  Obviously, a function
> like 'display-many-buffers-at-once' would not qualify as buffer display
> action function.

What do you mean by "independent calls"?  Or what are "dependent
calls" for this purpose?  Without understanding that, I cannot see how
what you wrote here answers my question, sorry.

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

[martin rudalics]

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

 >> We need a way to bump the time stamp of _any_ window used.
 >
 > Sorry, I don't understand what that means in practice.  Specifically,
 > what do you mean by "any window used"? used by whom and under what
 > circumstances? or do you mean any window that is currently displayed
 > on all the frames?

I mean "any window used" by 'display-buffer' to display a buffer.

 >> Otherwise,
 >> the various action functions will continue to fight each other.  Lars
 >> wanted to go the way XEmacs did but stopped in the middle.  And XEmacs
 >> uses
 >>
 >>                 (if (window-buffer window)
 >>                     (save-excursion
 >>                       (save-selected-window
 >>                         (select-window window)
 >>                         (record-buffer (window-buffer window)))))
 >>
 >> to bump the use time of every window used by 'display-buffer'.
 >
 > You mean, do this every time display-buffer is called and selects some
 > window?  But that would change our behavior for all the callers of
 > display-buffer, whose names is a legion.  Whereas the intent was to
 > provide an optional feature that hopefully doesn't affect any existing
 > behaviors.

It would change the behavior iff a user or caller had added a non-nil
'bump-time-stamp' entry to the alist.  Existing behaviors would not be
affected.

I'll give you an example.  Apply the attached bump-use-time.diff.  Now
with emacs -Q evaluate:

(let* ((window-combination-resize t))
   (split-window)
   (split-window)
   (display-buffer (get-buffer-create "*foo*"))
   (display-buffer (get-buffer-create "*bar*")))

*foo* is nowhere displayed because 'display-buffer' displayed *bar* in
the least recently used window instead.  This is the behavior Lars
wanted to avoid.

Now instead do

(let* ((window-combination-resize t))
   (split-window)
   (split-window)
   (display-buffer
    (get-buffer-create "*foo*") '(nil (bump-use-time . t)))
   (display-buffer
    (get-buffer-create "*bar*") '(nil (bump-use-time . t))))

This is the behavior Lars wanted.  Look, no separate action function
needed at all.  If anyone has a solution to that problem that's simpler
than this two-liner, I'll be all ears.

 >>   > 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.
 >>
 >> As Lars conceived it, independent 'display-buffer' calls should be able
 >> to build on previous ones.  Otherwise, we could write a function like
 >> 'display-many-buffers-at-once', within that function mark all windows
 >> used as temporarily dedicated to their buffers, and at the end restore
 >> the previous dedicated states of these windows.  Obviously, a function
 >> like 'display-many-buffers-at-once' would not qualify as buffer display
 >> action function.
 >
 > What do you mean by "independent calls"?  Or what are "dependent
 > calls" for this purpose?  Without understanding that, I cannot see how
 > what you wrote here answers my question, sorry.

A dependent call would be one inside a function like
'display-many-buffers-at-once' where you want to avoid that the same
window is used for displaying first one and then another buffer.  Any
other calls would be independent.

But to return to your question "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?".  The XEmacs solution
cited above does precisely that and that's why I posted it here.

Why you would call a primitive like 'window-bump-use-time' "sneaky" is
beyond my comprehension.  Is it because I originally proposed it?

martin

[-- Attachment #2: bump-use-time.diff --]
[-- Type: text/x-patch, Size: 1434 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index 7d8ee48635..ef45f629c5 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -7276,6 +7276,10 @@ window--display-buffer
       ;; Unless WINDOW already shows BUFFER reset its dedicated flag.
       (set-window-dedicated-p window nil)
       (set-window-buffer window buffer))
+    (when (assq 'bump-use-time alist)
+      ;; Bump WINDOW's use time so 'get-lru-window' will try to avoid
+      ;; it.
+      (window-bump-use-time window))
     (let ((alist-dedicated (assq 'dedicated alist)))
       ;; Maybe dedicate WINDOW to BUFFER if asked for.
       (cond
diff --git a/src/window.c b/src/window.c
index 90fa6ac2df..a20ac70cbf 100644
--- a/src/window.c
+++ b/src/window.c
@@ -773,13 +773,18 @@ DEFUN ("window-use-time", Fwindow_use_time, Swindow_use_time, 0, 1, 0,
 
 DEFUN ("window-bump-use-time", Fwindow_bump_use_time,
        Swindow_bump_use_time, 0, 1, 0,
-       doc: /* Mark WINDOW as having been most recently used.
+       doc: /* Mark WINDOW as most recently used after the selected window.
 WINDOW must be a live window and defaults to the selected one.  */)
   (Lisp_Object window)
 {
   struct window *w = decode_live_window (window);
+  struct window *sw = XWINDOW (selected_window);
 
-  w->use_time = ++window_select_count;
+  if (w != sw)
+    {
+      w->use_time = ++window_select_count;
+      sw->use_time = ++window_select_count;
+    }
 
   return Qnil;
 }

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

[Eli Zaretskii]

> Date: Sun, 29 Jan 2023 18:39:36 +0100
> Cc: tgbugs@gmail.com, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> We need a way to bump the time stamp of _any_ window used.
>  >
>  > Sorry, I don't understand what that means in practice.  Specifically,
>  > what do you mean by "any window used"? used by whom and under what
>  > circumstances? or do you mean any window that is currently displayed
>  > on all the frames?
> 
> I mean "any window used" by 'display-buffer' to display a buffer.

Do you mean "every window used by display-buffer"?

>  >>                 (if (window-buffer window)
>  >>                     (save-excursion
>  >>                       (save-selected-window
>  >>                         (select-window window)
>  >>                         (record-buffer (window-buffer window)))))
>  >>
>  >> to bump the use time of every window used by 'display-buffer'.
>  >
>  > You mean, do this every time display-buffer is called and selects some
>  > window?  But that would change our behavior for all the callers of
>  > display-buffer, whose names is a legion.  Whereas the intent was to
>  > provide an optional feature that hopefully doesn't affect any existing
>  > behaviors.
> 
> It would change the behavior iff a user or caller had added a non-nil
> 'bump-time-stamp' entry to the alist.

And who will add that entry?  IOW, how would this feature work, with
the patch you proposed?

> But to return to your question "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?".  The XEmacs solution
> cited above does precisely that and that's why I posted it here.

So we could make a change in display-buffer-use-least-recent-window to
temporarily select the window, and then there would be no reason for
us to artificially bump the window's use-time via window-bump-use-time?

> Why you would call a primitive like 'window-bump-use-time' "sneaky" is
> beyond my comprehension.

Because it pretends a window was selected whereas it wasn't, and
because it causes issues that you yourself pointed out (and which now
require changes to bump the use-time of the selected window as well)?

> diff --git a/lisp/window.el b/lisp/window.el
> index 7d8ee48635..ef45f629c5 100644
> --- a/lisp/window.el
> +++ b/lisp/window.el
> @@ -7276,6 +7276,10 @@ window--display-buffer
>        ;; Unless WINDOW already shows BUFFER reset its dedicated flag.
>        (set-window-dedicated-p window nil)
>        (set-window-buffer window buffer))
> +    (when (assq 'bump-use-time alist)
> +      ;; Bump WINDOW's use time so 'get-lru-window' will try to avoid
> +      ;; it.
> +      (window-bump-use-time window))
>      (let ((alist-dedicated (assq 'dedicated alist)))
>        ;; Maybe dedicate WINDOW to BUFFER if asked for.
>        (cond
> diff --git a/src/window.c b/src/window.c
> index 90fa6ac2df..a20ac70cbf 100644
> --- a/src/window.c
> +++ b/src/window.c
> @@ -773,13 +773,18 @@ DEFUN ("window-use-time", Fwindow_use_time, Swindow_use_time, 0, 1, 0,
>  
>  DEFUN ("window-bump-use-time", Fwindow_bump_use_time,
>         Swindow_bump_use_time, 0, 1, 0,
> -       doc: /* Mark WINDOW as having been most recently used.
> +       doc: /* Mark WINDOW as most recently used after the selected window.
>  WINDOW must be a live window and defaults to the selected one.  */)
>    (Lisp_Object window)
>  {
>    struct window *w = decode_live_window (window);
> +  struct window *sw = XWINDOW (selected_window);
>  
> -  w->use_time = ++window_select_count;
> +  if (w != sw)
> +    {
> +      w->use_time = ++window_select_count;
> +      sw->use_time = ++window_select_count;
> +    }
>  
>    return Qnil;
>  }

I'm not sure I understand: is this instead of
display-buffer-use-least-recent-window, or in addition to it?  IOW,
how do you recommend we fix the issues that
display-buffer-use-least-recent-window introduced, which you mentioned
in your previous messages?

Also, with the fixes you propose, would there still be need in the
changes proposed by Tom, and if so, why?  It sounds like he is trying
to fix problems which your patches fix in a different way?

Bottom line: I'm struggling to understand what is TRT to be done about
these issues, first and foremost for Emacs 29 (since
display-buffer-use-least-recent-window is new in Emacs 29).  Would you
please help me figure that out?

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

[Tom Gillespie]

A quick note:

> first and foremost for Emacs 29 (since
> display-buffer-use-least-recent-window is new in Emacs 29)

Unfortunately it is not new in Emacs 29. It was already in Emacs 28.
So let's get it fixed (I'm about to send a revised patch).

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

[martin rudalics]

 >> I mean "any window used" by 'display-buffer' to display a buffer.
 >
 > Do you mean "every window used by display-buffer"?

Yes.

 >> It would change the behavior iff a user or caller had added a non-nil
 >> 'bump-time-stamp' entry to the alist.
 >
 > And who will add that entry?  IOW, how would this feature work, with
 > the patch you proposed?

Just as with any other action alist entry as listed in section 29.15.3
of the Elisp manual.  The example I gave shows what an application can
do to get the behavior wanted in Bug#45688.  If someone comes up with
another example, I can try to tell how to solve that.

 >> But to return to your question "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?".  The XEmacs solution
 >> cited above does precisely that and that's why I posted it here.
 >
 > So we could make a change in display-buffer-use-least-recent-window to
 > temporarily select the window, and then there would be no reason for
 > us to artificially bump the window's use-time via window-bump-use-time?

You would still "artificially bump the window's use time".  Just that
you would also make its buffer current, swap in its local variables,
mark windows for redisplay, maybe select another frame, move minibuffers
onto that frame, resize the minibuffer window ...  And then repeat all
that in order to reselect the previously selected window.  And at the
very end blame the WM when it raised a frame where all we wanted to do
was to change the use time of one of our windows.

Emacs with a separate minibuffer frame is broken here for almost a year
because someone does "temporarily select the window" and because
'select-window' now behaves in a way it never did.  I know that I'm
fighting against windmills here.  'select-window' is too heavyweight.
It should be reserved for the user to choose another window to work in.

 >> Why you would call a primitive like 'window-bump-use-time' "sneaky" is
 >> beyond my comprehension.
 >
 > Because it pretends a window was selected whereas it wasn't, and
 > because it causes issues that you yourself pointed out

And having 'display-buffer' select a window for the sole purpose to bump
its use time would not pretend anything?

 > (and which now
 > require changes to bump the use-time of the selected window as well)?

You would have to do that also when you explicitly select the window.

 > I'm not sure I understand: is this instead of
 > display-buffer-use-least-recent-window, or in addition to it?

It would be a lightweight alternative.

 > IOW,
 > how do you recommend we fix the issues that
 > display-buffer-use-least-recent-window introduced, which you mentioned
 > in your previous messages?

AFAICT Tom is trying to fix them.

 > Also, with the fixes you propose, would there still be need in the
 > changes proposed by Tom, and if so, why?  It sounds like he is trying
 > to fix problems which your patches fix in a different way?

'display-buffer-use-least-recent-window' is here since Emacs 28 so I
think that Tom's changes are needed.  What Tom is proposing is to have
'display-buffer-use-least-recent-window' encompass the behaviors of
'display-buffer-reuse-window' and 'display-buffer-pop-up-window' in
order to bump use times in these cases too.  My patch bumps use times
for every buffer display action once a 'bump-use-time' entry has been
added.

 > Bottom line: I'm struggling to understand what is TRT to be done about
 > these issues, first and foremost for Emacs 29 (since
 > display-buffer-use-least-recent-window is new in Emacs 29).  Would you
 > please help me figure that out?

'display-buffer-use-least-recent-window' is not new in Emacs 29.  Let's
wait for Tom to work out a fix for that.

martin

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

[Eli Zaretskii]

> Date: Mon, 30 Jan 2023 17:43:35 +0100
> Cc: tgbugs@gmail.com, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Bottom line: I'm struggling to understand what is TRT to be done about
>  > these issues, first and foremost for Emacs 29 (since
>  > display-buffer-use-least-recent-window is new in Emacs 29).  Would you
>  > please help me figure that out?
> 
> 'display-buffer-use-least-recent-window' is not new in Emacs 29.  Let's
> wait for Tom to work out a fix for that.

So when Tom's patch reaches its final form, there will be no other
significant issues with display-buffer-use-least-recent-window and its
collateral effects?  What about the change to window-bump-use-time --
will it still be needed then?

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

[martin rudalics]

 > So when Tom's patch reaches its final form, there will be no other
 > significant issues with display-buffer-use-least-recent-window and its
 > collateral effects?

What are its "collateral effects"?

 > What about the change to window-bump-use-time --
 > will it still be needed then?

Yes.

martin

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

[Eli Zaretskii]

> Date: Mon, 30 Jan 2023 18:57:58 +0100
> Cc: tgbugs@gmail.com, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > So when Tom's patch reaches its final form, there will be no other
>  > significant issues with display-buffer-use-least-recent-window and its
>  > collateral effects?
> 
> What are its "collateral effects"?

window-bump-use-time, for one.

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

[martin rudalics]

 >> What are its "collateral effects"?
 >
 > window-bump-use-time, for one.

Then I'm relieved.  I already thought of something real nasty like
'with-selected-window'.

martin

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

[Eli Zaretskii]

> Date: Tue, 31 Jan 2023 09:45:53 +0100
> Cc: tgbugs@gmail.com, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> What are its "collateral effects"?
>  >
>  > window-bump-use-time, for one.
> 
> Then I'm relieved.  I already thought of something real nasty like
> 'with-selected-window'.

In general, I had in mind all the issues you listed in

  https://lists.gnu.org/archive/html/emacs-devel/2023-01/msg00542.html

including the one with window-bump-use-time.

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

[martin rudalics]

 > In general, I had in mind all the issues you listed in
 >
 >    https://lists.gnu.org/archive/html/emacs-devel/2023-01/msg00542.html
 >
 > including the one with window-bump-use-time.

There's worse as I just noticed.  'display-buffer-avoid-small-windows':

- Invariably affects, once set, 'get-lru-window' which can be used
   anywhere outside the scope of 'display-buffer'.  So it's not only a
   misnomer but a typical example of a collateral effect.

- Is described in section 29.15.4 of the Elisp manual which talks about
   "older options" some of them candidates for obsoletion.

- Ignores that an action alist entry of that type already existed.  It's
   called 'window-min-height' and could have been easily used here.

Just that any such option should never be used in 'get-lru-window' but
exclusively in functions that pertain to 'display-buffer'.

martin

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

[Eli Zaretskii]

> Date: Wed, 1 Feb 2023 11:45:06 +0100
> Cc: tgbugs@gmail.com, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
> There's worse as I just noticed.  'display-buffer-avoid-small-windows':
> 
> - Invariably affects, once set, 'get-lru-window' which can be used
>    anywhere outside the scope of 'display-buffer'.  So it's not only a
>    misnomer but a typical example of a collateral effect.
> 
> - Is described in section 29.15.4 of the Elisp manual which talks about
>    "older options" some of them candidates for obsoletion.
> 
> - Ignores that an action alist entry of that type already existed.  It's
>    called 'window-min-height' and could have been easily used here.

Is this related to display-buffer-use-least-recent-window and/or
window-bump-use-time?  Or is this another set of issues?

Would it be possible to modify the effect of
display-buffer-avoid-small-windows so that it works via the
window-min-height alist entry?

> Just that any such option should never be used in 'get-lru-window' but
> exclusively in functions that pertain to 'display-buffer'.

I see.

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

[martin rudalics]

 > Is this related to display-buffer-use-least-recent-window and/or
 > window-bump-use-time?  Or is this another set of issues?

It's another set of issues loosely related to
'display-buffer-use-least-recent-window'.  I found it because I tried to
understand why that function did not always return the LRU window.  See
my other mail to Tom.

 > Would it be possible to modify the effect of
 > display-buffer-avoid-small-windows so that it works via the
 > window-min-height alist entry?

No.  'window-min-height' so far is honored only in functions of the
'display-buffer-in-direction' type because there it's easy to tell which
window should have that property.  It's not used in 'get-lru-window'
because that function is not used for buffer display alone.  As a rule,
buffer display options should never show up in functions that can be
used in another context.

We could give 'get-lru-window' an additional ALIST argument which would
indicate that it was indirectly called by 'display-buffer'.  I wouldn't
like it and I don't think we would be able to figure out all the
consequences in due time.  So we need a 'get-lru-window' clone that can
be used by buffer display and takes only an ALIST as argument and
nothing else.

'display-buffer-avoid-small-windows' is an attempt to reintroduce a
concept we tried to abandon many years ago.  It's inherently like
'pop-up-windows' where applications and users fought each other by
setting or binding that variable to their like with undefined outcome.

We tried to stop such fighting by giving users options like
'display-buffer-alist' and 'display-buffer-base-action' and giving
applications the ACTION argument with clear precedence rules.  There's
no precedence rule for 'display-buffer-avoid-small-windows'.

martin

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

[Tom Gillespie]

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

Thank you for the clarification. I tried to figure out what the right thing
to do was based on the docstrings, but missed a few critical points.

I think the right solution in this context is to not call
'display-buffer-use-some-window' in
'display-buffer-use-least-recent-window' and instead to implement
the behavior inside dbulrw itself.

With regard to the other issues about multiple frames, I was able
to get the behavior I wanted, or rather prevent the behavior I didn't
want by making dbulrw honor a reusable-frames setting so that it
doesn't leak out to other windows. I'm not going to wade into the
question of whether the resolution and tracking of least recently
used is happening correctly, I just need it to stop reusing windows
in other frames.

Will send another patch along.

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

[Tom Gillespie]

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

Here is an updated patch which does not touch
'display-buffer-use-some-window' at all, and that
corrects splitting behavior and return value for
'display-buffer-use-least-recent-window'.

If a patch to 'display-buffer-use-some-window'
to honor reusable-frames is desired, let's deal with
it separately. Best!
Tom

[-- Attachment #2: 0001-Fix-display-buffer-use-least-recent-window-to-split-.patch --]
[-- Type: text/x-patch, Size: 3930 bytes --]

From 274493ccb0e15aeec88b08fcf79180616deacf6f Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
Date: Thu, 26 Jan 2023 23:47:22 -0500
Subject: [PATCH] Fix display-buffer-use-least-recent-window to split, return
 window.

* lisp/window.el (display-buffer-use-least-recent-window): Return
window instead of nil, and actually split in single window case.

'display-buffer-use-least-recent-window' now returns window if one is
selected which prevents the rather nasty behavior of selecting a
window in the current frame and then also selecting a window in
another random frame as well (quite maddening).

The docs for 'display-buffer-use-least-recent-window' state that it
will pick the least recently used window or and if there is only a
single window it will split the window.

Prior to this commit that behavior was impossible to achieve because
'display-buffer-use-least-recent-window' reused the internals of
'display-buffer-use-some-window' which would attempt to find a valid
window in other frames and never split the current window.

'display-buffer-use-least-recent-window' no longer calls
'display-buffer-use-some-window' and instead directly calls
'get-lru-window' followed by 'display-buffer-pop-p-window' when
reusable-frames is not provided or nil, or 'get-buffer-window' when
reusable-frames is non-nil.  Once the window is selected it runs code
that has been duplicated from 'display-buffer-use-some-window'.
---
 lisp/window.el | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index 0cd30822ff6..6dd4fed1dd3 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8507,10 +8507,41 @@ display-buffer-use-least-recent-window
 This `display-buffer' action function is like
 `display-buffer-use-some-window', but will cycle through windows
 when displaying buffers repeatedly, and if there's only a single
-window, it will split the window."
-  (when-let ((window (display-buffer-use-some-window
-                      buffer (cons (cons 'inhibit-same-window t) alist))))
-    (window-bump-use-time window)))
+window, it will split the window.
+
+If `reusable-frames' is provided in the alist then
+`display-buffer-use-least-recent-window' will attempt to find a
+window in other frames accordingly. By default it only searches
+the current frame."
+  (let* ((not-this-window t)
+         (reusable-frames (assq 'reusable-frames alist))
+         (frame (or (window--frame-usable-p (selected-frame))
+                    (window--frame-usable-p (last-nonminibuffer-frame))))
+         (window (or (get-lru-window frame nil not-this-window)
+                     (if (not reusable-frames)
+                         (display-buffer-pop-up-window buffer alist)
+                       (window (get-buffer-window buffer reusable-frames)))))
+         (quit-restore (and (window-live-p window)
+			    (window-parameter window 'quit-restore)))
+         (quad (nth 1 quit-restore)))
+    (unless (eq window (selected-window))
+      (when (window-live-p window)
+        ;; If the window was used by `display-buffer' before, try to
+        ;; resize it to its old height but don't signal an error.
+        (when (and (listp quad)
+                   (integerp (nth 3 quad))
+                   (> (nth 3 quad) (window-total-height window)))
+          (condition-case nil
+              (window-resize window (- (nth 3 quad) (window-total-height window)))
+            (error nil)))
+        (prog1
+            (window--display-buffer buffer window 'reuse alist)
+          (window--even-window-sizes window)
+          (unless (cdr (assq 'inhibit-switch-frame alist))
+            (window--maybe-raise-frame (window-frame window))))))
+    (when window
+      (window-bump-use-time window)
+      window)))
 
 (defun display-buffer-use-some-window (buffer alist)
   "Display BUFFER in an existing window.
-- 
2.39.1

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

[martin rudalics]

 > Here is an updated patch which does not touch
 > 'display-buffer-use-some-window' at all, and that
 > corrects splitting behavior and return value for
 > 'display-buffer-use-least-recent-window'.

Thanks.  I see the following problems:

+         (window (or (get-lru-window frame nil not-this-window)
+                     (if (not reusable-frames)
+                         (display-buffer-pop-up-window buffer alist)
+                       (window (get-buffer-window buffer reusable-frames)))))

Here the last line gets me

In end of data:
window.el:8522:25: Warning: the function ‘window’ is not known to be defined.

Now if this frame contains more than one window, 'get-lru-window' will
usually (barring not fullwidth, dedicated ... windows) always succeed in
finding a window.  This is probably not the intended behavior if this
frame contains a reusable window - one that shows BUFFER already.  Maybe
you think that some other action function would handle that.  But then
why would you want to handle 'reusable-frames' here at all?

Also 'display-buffer-pop-up-window' is an action function which returns
a window _and_ displays the buffer in it.  So if it succeeds here, you
will end up displaying the buffer twice, once within that function and
once in

+            (window--display-buffer buffer window 'reuse alist)

below.  I can't tell you what consequences that might have but I think
it's a bad idea.  BTW, you don't have to bind 'not-this-window' - just
pass t as last argument to 'get-lru-window'.

+            (window--display-buffer buffer window 'reuse alist)

This would have to become

   (window--display-buffer buffer window 'window alist)

when 'display-buffer-pop-up-window' succeeds.

This

+        (prog1

has been cargo-culted from 'display-buffer-use-some-window'.  It's of no
use here because you return the window used below.

Finally, please fix the doc-string so it tells what this function really
does (and please leave two spaces between sentences).

Thanks again, martin

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

[Tom Gillespie]

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

Here is an updated patch to address Martin's comments.
More responses in line. Best!
Tom

> +                       (window (get-buffer-window buffer reusable-frames)))))
>
> Here the last line gets me

Argh. Yep, a copy/paste error out of a let form. Given your question
below I've rethought the issue, and now pass reusable-frames to
get-lru-window instead, which produces the expected lru behavior
when searching for a window over multiple frames.

> Now if this frame contains more than one window, 'get-lru-window' will
> usually (barring not fullwidth, dedicated ... windows) always succeed in
> finding a window.  This is probably not the intended behavior if this
> frame contains a reusable window - one that shows BUFFER already.  Maybe
> you think that some other action function would handle that.  But then
> why would you want to handle 'reusable-frames' here at all?

The reason I handle reusable-frames here is so that if a user wants to search
other frames for the least recently used window (e.g. they have 3
visible frames,
one for each monitor, and they want the lru window to be selected from any of
those visible frames). The implementation of this in that old patch
was completely
incorrect. In the new version reusable-frames is passed to get-lru-window, which
produces the expected results.

> Also 'display-buffer-pop-up-window' is an action function which returns
> a window _and_ displays the buffer in it.

Resolved so that we don't try to display the buffer twice.

> This
>
> +        (prog1
>
> has been cargo-culted from 'display-buffer-use-some-window'.  It's of no
> use here because you return the window used below.

Now gone.

> Finally, please fix the doc-string so it tells what this function really
> does (and please leave two spaces between sentences).

Done

[-- Attachment #2: 0001-Fix-display-buffer-use-least-recent-window-to-split-.patch --]
[-- Type: text/x-patch, Size: 3972 bytes --]

From 26fbad2bcf7cabf98678dda0095334fab7722480 Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
Date: Thu, 26 Jan 2023 23:47:22 -0500
Subject: [PATCH] Fix display-buffer-use-least-recent-window to split, return
 window.

* lisp/window.el (display-buffer-use-least-recent-window): Return
window instead of nil, and actually split in single window case.

'display-buffer-use-least-recent-window' now returns window if one is
selected which prevents the rather nasty behavior of selecting a
window in the current frame and then also selecting a window in
another random frame as well (quite maddening).

The docs for 'display-buffer-use-least-recent-window' state that it
will pick the least recently used window and if there is only a single
window it will split the window.

Prior to this commit that behavior was impossible to achieve because
'display-buffer-use-least-recent-window' reused the internals of
'display-buffer-use-some-window' which would attempt to find a valid
window in other frames and never split the current window.

'display-buffer-use-least-recent-window' no longer calls
'display-buffer-use-some-window' and instead directly calls
'get-lru-window' which is passed reusable-frames if it is provided.
If 'get-lru-window' returns nil then 'display-buffer-pop-p-window' is
used to split the current window. If an existing window is selected
then it runs code adapted from 'display-buffer-use-some-window'.
---
 lisp/window.el | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index 0cd30822ff6..c507f4c6c1f 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8507,10 +8507,38 @@ display-buffer-use-least-recent-window
 This `display-buffer' action function is like
 `display-buffer-use-some-window', but will cycle through windows
 when displaying buffers repeatedly, and if there's only a single
-window, it will split the window."
-  (when-let ((window (display-buffer-use-some-window
-                      buffer (cons (cons 'inhibit-same-window t) alist))))
-    (window-bump-use-time window)))
+window, it will split the window.
+
+If `reusable-frames' is provided in the alist then
+`display-buffer-use-least-recent-window' will attempt to find the
+least recent window in all matching frames.  By default it only
+searches the current frame."
+  (let* ((reusable-frames (cdr (assq 'reusable-frames alist)))
+         (frame (or (window--frame-usable-p (selected-frame))
+                    (window--frame-usable-p (last-nonminibuffer-frame))))
+         (window (or (get-lru-window (or reusable-frames frame) nil t))))
+    (let ((window
+           (if window
+               (let* ((quit-restore (and (window-live-p window)
+                                         (window-parameter window 'quit-restore)))
+                      (quad (nth 1 quit-restore)))
+                 (when (window-live-p window)
+                   ;; If the window was used by `display-buffer' before, try to
+                   ;; resize it to its old height but don't signal an error.
+                   (when (and (listp quad)
+                              (integerp (nth 3 quad))
+                              (> (nth 3 quad) (window-total-height window)))
+                     (condition-case nil
+                         (window-resize window (- (nth 3 quad) (window-total-height window)))
+                       (error nil)))
+                   (window--display-buffer buffer window 'reuse alist)
+                   (window--even-window-sizes window)
+                   (unless (cdr (assq 'inhibit-switch-frame alist))
+                     (window--maybe-raise-frame (window-frame window)))))
+             (display-buffer-pop-up-window buffer alist))))
+      (when window
+        (window-bump-use-time window)
+        window))))
 
 (defun display-buffer-use-some-window (buffer alist)
   "Display BUFFER in an existing window.
-- 
2.39.1

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

[martin rudalics]

This

+  (let* ((reusable-frames (cdr (assq 'reusable-frames alist)))
+         (frame (or (window--frame-usable-p (selected-frame))
+                    (window--frame-usable-p (last-nonminibuffer-frame))))
+         (window (or (get-lru-window (or reusable-frames frame) nil t))))

is probably not what you really want.  AFAICT you want to reuse a window
on a frame specified by 'reusable-frames' - that is a window that
already shows BUFFER.  But 'get-lru-window' only gives you the least
recently used window of such a frame.

What speaks against

(defun display-buffer-use-least-recent-window (buffer alist)
   "..."
   (let* ((alist (cons (cons 'inhibit-same-window t) alist))
          (window
           (or (display-buffer-reuse-window buffer alist)
               (display-buffer-use-some-window buffer alist)
               (display-buffer-pop-up-window buffer alist))))
     (when window
       (window-bump-use-time window)
       window)))

or something the like?

martin

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

[Tom Gillespie]

> is probably not what you really want.

You're right. I missed the case where we first want
to reuse an existing window if the buffer is already
displayed in it. Working on a patch to add that back.

> What speaks against
> ...
> (display-buffer-use-some-window buffer alist)
> ...
> or something the like?

The main reason not to take that approach is that it
requires changes to display-buffer-use-some-window
like the ones I made in my first patch so that it will
honor reusable-frames (though without the incorrect
call to display-buffer-pop-up-window). It seems
like it will be easier to reason about the impact of
the changes for the patch if we only need to assess
changes to display-buffer-use-least-recent-window.

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

[martin rudalics]

 > The main reason not to take that approach is that it
 > requires changes to display-buffer-use-some-window
 > like the ones I made in my first patch so that it will
 > honor reusable-frames (though without the incorrect
 > call to display-buffer-pop-up-window).

Why?  Any reusable frame would have been already honored by
'display-buffer-reuse-window'.

 > It seems
 > like it will be easier to reason about the impact of
 > the changes for the patch if we only need to assess
 > changes to display-buffer-use-least-recent-window.

Why would you want to change 'display-buffer-use-some-window'?

martin

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

[Tom Gillespie]

> Why?  Any reusable frame would have been already honored by
'display-buffer-reuse-window'.

If 'display-buffer-reuse-window' fails to find a valid window then
the next function to run will be 'display-buffer-use-some-window'.

In the current implementation of 'display-buffer-use-some-window'
get-lru-window only searches the existing frame and not-this-window
will be true, therefore whenever there is a single window in the
current frame 'display-buffer-use-some-window' will proceed on to
pick a window in another frame starting with visible then 0 for all-frames
when it calls
(get-largest-window 'visible nil not-this-window) and then
(get-largest-window 0 nil not-this-window)

This is not the behavior that we want because a single window in the
current frame will never be able to reach display-buffer-pop-up-window
since 'display-buffer-use-some-window' will succeed on a random window
in a random frame which will almost certainly not be the least recently used.

> Why would you want to change 'display-buffer-use-some-window'?

I don't, but in your example where we call it in
display-buffer-use-least-recent-window then it would have to be
modified since as is it is impossible to get the behavior we need
due to the calls to get-largest-window.

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

[Tom Gillespie]

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

> > is probably not what you really want.

> You're right. I missed the case where we first want
> to reuse an existing window if the buffer is already
> displayed in it. Working on a patch to add that back.

Here is the updated patch, but it comes with a question.
On review, the reason why I did not include a call to
display-buffer-reuse-window (or get-buffer-window)
internally is because in my config I have the following

(setq display-buffer-base-action
      '((display-buffer-reuse-window display-buffer-use-least-recent-window)))

I think it is probably correct to have display-buffer-use-least-recent-window
try to call display-buffer-reuse-window itself (effectively), however it does
seem like it makes display-buffer-use-least-recent-window less composable
because it can no longer be used to compose a base action that did not
reuse existing windows.

For example I can imagine that someone might want to display a buffer
twice and have the 2nd instance of the buffer pick the least recent window
by default. If we use this version of the patch that is no longer possible.

Therefore I think the prior version of the patch from
Sun, 29 Jan 2023 14:02:42 -0500 is probably the best.

Tom

[-- Attachment #2: 0001-Fix-display-buffer-use-least-recent-window-to-split-.patch --]
[-- Type: text/x-patch, Size: 4326 bytes --]

From 1ba0aa789ecdc48612e078893bdccd85112174db Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
Date: Thu, 26 Jan 2023 23:47:22 -0500
Subject: [PATCH] Fix display-buffer-use-least-recent-window to split, return
 window.

* lisp/window.el (display-buffer-use-least-recent-window): Return
window instead of nil, and actually split in single window case.

'display-buffer-use-least-recent-window' now returns window if one is
selected which prevents the rather nasty behavior of selecting a
window in the current frame and then also selecting a window in
another random frame as well (quite maddening).

The docs for 'display-buffer-use-least-recent-window' state that it
will pick the least recently used window and if there is only a single
window it will split the window.

Prior to this commit that behavior was impossible to achieve because
'display-buffer-use-least-recent-window' reused the internals of
'display-buffer-use-some-window' which would attempt to find a valid
window in other frames and never split the current window.

'display-buffer-use-least-recent-window' no longer calls
'display-buffer-use-some-window' and instead directly calls
'get-lru-window' which is passed reusable-frames if it is provided.
If 'get-lru-window' returns nil then 'display-buffer-pop-p-window' is
used to split the current window. If an existing window is selected
then it runs code adapted from 'display-buffer-use-some-window'.

This version of the patch calls get-buffer-window internally, which
reduces the usefulness of 'display-buffer-use-least-recent-window', on
the one hand, but on the other hand makes it possible to get generally
sane behavior using it by itself in 'display-buffer-base-action'.
---
 lisp/window.el | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index 0cd30822ff6..080d768d2c6 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8507,10 +8507,39 @@ display-buffer-use-least-recent-window
 This `display-buffer' action function is like
 `display-buffer-use-some-window', but will cycle through windows
 when displaying buffers repeatedly, and if there's only a single
-window, it will split the window."
-  (when-let ((window (display-buffer-use-some-window
-                      buffer (cons (cons 'inhibit-same-window t) alist))))
-    (window-bump-use-time window)))
+window, it will split the window.
+
+If `reusable-frames' is provided in the alist then
+`display-buffer-use-least-recent-window' will attempt to find the
+least recent window in all matching frames.  By default it only
+searches the current frame."
+  (let* ((reusable-frames (cdr (assq 'reusable-frames alist)))
+         (frame (or (window--frame-usable-p (selected-frame))
+                    (window--frame-usable-p (last-nonminibuffer-frame))))
+         (window (or (get-buffer-window buffer (or reusable-frames frame))
+                     (get-lru-window (or reusable-frames frame) nil t))))
+    (let ((window
+           (if window
+               (let* ((quit-restore (and (window-live-p window)
+                                         (window-parameter window 'quit-restore)))
+                      (quad (nth 1 quit-restore)))
+                 (when (window-live-p window)
+                   ;; If the window was used by `display-buffer' before, try to
+                   ;; resize it to its old height but don't signal an error.
+                   (when (and (listp quad)
+                              (integerp (nth 3 quad))
+                              (> (nth 3 quad) (window-total-height window)))
+                     (condition-case nil
+                         (window-resize window (- (nth 3 quad) (window-total-height window)))
+                       (error nil)))
+                   (window--display-buffer buffer window 'reuse alist)
+                   (window--even-window-sizes window)
+                   (unless (cdr (assq 'inhibit-switch-frame alist))
+                     (window--maybe-raise-frame (window-frame window)))))
+             (display-buffer-pop-up-window buffer alist))))
+      (when window
+        (window-bump-use-time window)
+        window))))
 
 (defun display-buffer-use-some-window (buffer alist)
   "Display BUFFER in an existing window.
-- 
2.39.1

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

[martin rudalics]

 > Here is the updated patch, but it comes with a question.
 > On review, the reason why I did not include a call to
 > display-buffer-reuse-window (or get-buffer-window)
 > internally is because in my config I have the following
 >
 > (setq display-buffer-base-action
 >        '((display-buffer-reuse-window display-buffer-use-least-recent-window)))

That's why 'display-buffer-use-least-recent-window' will never succeed
to become a good citizen in the department of buffer display functions.
The former will not bump the use time and if the window used by
'display-buffer-reuse-window' happens to be the LRU one,
'display-buffer-use-least-recent-window' may use it for displaying the
next buffer.  I tried to convince Lars that this is the basic problem of
his approach but he didn't listen.  If you try with a 'bump-use-time'
action alist entry you will see that it works.

 > I think it is probably correct to have display-buffer-use-least-recent-window
 > try to call display-buffer-reuse-window itself (effectively), however it does
 > seem like it makes display-buffer-use-least-recent-window less composable
 > because it can no longer be used to compose a base action that did not
 > reuse existing windows.

Or, for example, a window that previously displayed the buffer.

 > For example I can imagine that someone might want to display a buffer
 > twice and have the 2nd instance of the buffer pick the least recent window
 > by default. If we use this version of the patch that is no longer possible.

How would the other version handle it?  BTW in the version you attached
I see

+                     (get-lru-window (or reusable-frames frame) nil t))))

What's the purpose of 'reusable-frames' here?

martin

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

[Tom Gillespie]

> That's why 'display-buffer-use-least-recent-window' will never succeed
> to become a good citizen in the department of buffer display functions.
> The former will not bump the use time and if the window used by
> 'display-buffer-reuse-window' happens to be the LRU one,
> 'display-buffer-use-least-recent-window' may use it for displaying the
> next buffer.  I tried to convince Lars that this is the basic problem of
> his approach but he didn't listen.  If you try with a 'bump-use-time'
> action alist entry you will see that it works.

In testing I do indeed see the issue where display-buffer-reuse-window
does not bump the use time and thus the reused window which should
have been bumped remains the least recently used and is thus select
for the next buffer when it should not. I can also see, but have not tested
that bump-use-time in the action list would solve it.

> Or, for example, a window that previously displayed the buffer.

Yep, that one would be quite desirable if it could be achieved.

> How would the other version handle it?

The version that calls get-buffer-window internally reuses the window that
currently contains the buffer making no apparent change to the user.

The version that does NOT call get-buffer-widnow internally displays the
buffer in the least recently used window (which in principle might be the
buffer that it is already displayed in). Having played with it a bit, the
behavior is rather confusing and probably undesirable.

> BTW in the version you attached I see
> +                     (get-lru-window (or reusable-frames frame) nil t))))
>
> What's the purpose of 'reusable-frames' here?

From a previous message here is the scenario that I envisioned
and have tested:

> they have 3 visible frames, one for each monitor, and they want
> the lru window to be selected from any of those visible frames

They achieve this by including '(reusable-frames . visible) in the
alist. That way they can get the same behavior for all visible windows
as they would if they had a single monitor and a single frame.

In light of this discussion which patch do we want to go with? The one
that calls get-buffer-window internally or the one that does not? Once
we have the answer I will summarize what we have discussed here
in the comments and send a final patch.

Best,
Tom

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

[martin rudalics]

 >> BTW in the version you attached I see
 >> +                     (get-lru-window (or reusable-frames frame) nil t))))
 >>
 >> What's the purpose of 'reusable-frames' here?
 >
 >>From a previous message here is the scenario that I envisioned
 > and have tested:
 >
 >> they have 3 visible frames, one for each monitor, and they want
 >> the lru window to be selected from any of those visible frames
 >
 > They achieve this by including '(reusable-frames . visible) in the
 > alist. That way they can get the same behavior for all visible windows
 > as they would if they had a single monitor and a single frame.

Suppose a user provided a 'reusable-frames' entry and an application now
asks for 'display-buffer-use-least-recent-window'.  The customization
which earlier affected 'display-buffer-reuse-window' only would now
affect the search of a window that does not show BUFFER too.  This would
constitute an incompatible change we simply cannot explain.

We could add a new action alist entry like 'lru-frames'.  Then we could
use that in 'display-buffer-use-some-window' too.  Not really suitable
for Emacs 29 though.

Can't those 3 monitors people use 'display-buffer-use-some-frame' with a
suitable 'frame-predicate' instead?

 > In light of this discussion which patch do we want to go with? The one
 > that calls get-buffer-window internally or the one that does not? Once
 > we have the answer I will summarize what we have discussed here
 > in the comments and send a final patch.

Let's not rush things again.  We deal here with half-baked changes and
should try to put them straight to eliminate collateral effects instead
of adding new ones.

martin

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

[Tom Gillespie]

> Can't those 3 monitors people use 'display-buffer-use-some-frame' with a
> suitable 'frame-predicate' instead?

Yes, they can. Which means that we don't need such functionality
in display-buffer-use-least-recent-window. The only reason it was
needed previously was for the single window case, which is cleary
orthogonal to to the multi-monitor multi-frame case. The call to
get-lru-window inside display-buffer-use-some-window will produce
the desired behavior in that case.

> Let's not rush things again.  We deal here with half-baked changes and
> should try to put them straight to eliminate collateral effects instead
> of adding new ones.

Heh, fair enough.

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

[martin rudalics]

 > Heh, fair enough.

And note one thing: When you call 'get-lru-window' twice in a row, it
will usually _not_ return a window it has internally considered as
"second best" provided it finds a best one.

Consider a frame with three windows, one of them full width and the
other two, including the selected one, not full width.  In this case
'get-lru-window' will always return the full width window even if it has
a higher use time than the not selected, not full width one.

martin

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

[martin rudalics]

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

 > And note one thing: When you call 'get-lru-window' twice in a row, it
 > will usually _not_ return a window it has internally considered as
 > "second best" provided it finds a best one.
 >
 > Consider a frame with three windows, one of them full width and the
 > other two, including the selected one, not full width.  In this case
 > 'get-lru-window' will always return the full width window even if it has
 > a higher use time than the not selected, not full width one.

A tentative solution for this and the other problems mentioned in this
thread is in the attached diff.  It should be able to digest the
following form.

(let* ((window (selected-window))
        use-time)
   ;; Make window at bottom mru.
   (select-window (split-window))
   ;; Make window on right mru.
   (select-window (split-window window nil t))
   ;; Make initial window mru so the lru window is the one at bottom.
   (select-window window)
   (setq use-time (window-use-time))
   (display-buffer
    (get-buffer-create "*foo*")
    `((display-buffer-use-least-recent-window)
      (window-min-width . full-width)
      (lru-time . ,use-time)
      ))
   (display-buffer
    (get-buffer-create "*bar*")
    `((display-buffer-use-least-recent-window)
      (window-min-width . full-width)
      (lru-time . ,use-time)
      )))

Note how it uses a non-full-width window for *bar* despite the fact that
we would have preferred a full-width one.  And note the settings of
lru-time to the use time of the selected window before the first
'display-buffer' call.  Commenting out these lru-time settings will make
it use as window for *bar* the window it previously used for *foo*.

martin

[-- Attachment #2: Gillespie.diff --]
[-- Type: text/x-patch, Size: 5529 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index a11293d372..a0e09ab492 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8497,16 +8497,6 @@ display-buffer-in-previous-window
     (when (setq window (or best-window second-best-window))
       (window--display-buffer buffer window 'reuse alist))))
 
-(defun display-buffer-use-least-recent-window (buffer alist)
-  "Display BUFFER in an existing window, but that hasn't been used lately.
-This `display-buffer' action function is like
-`display-buffer-use-some-window', but will cycle through windows
-when displaying buffers repeatedly, and if there's only a single
-window, it will split the window."
-  (when-let ((window (display-buffer-use-some-window
-                      buffer (cons (cons 'inhibit-same-window t) alist))))
-    (window-bump-use-time window)))
-
 (defun display-buffer-use-some-window (buffer alist)
   "Display BUFFER in an existing window.
 Search for a usable window, set that window to the buffer, and
@@ -8559,6 +8549,92 @@ display-buffer-use-some-window
 	(unless (cdr (assq 'inhibit-switch-frame alist))
 	  (window--maybe-raise-frame (window-frame window)))))))
 
+(defun display-buffer--lru-window (alist)
+  "Return the least recently used window according to ALIST.
+Do not return a minibuffer window or a window dedicated to its
+buffer.  The following ALIST entries have a special meaning:
+
+- `lru-frames' specifies the frames to investigate and has the
+  same meaning as the ALL-FRAMES argument of `get-lru-window'.
+
+- `lru-time' specifies a use time.  Do not return a window whose
+  use time is higher than this.
+
+- `window-min-width' specifies a preferred minimum width in
+  canonical frame columns.  If it is the constant `full-width',
+  ask for a full-width window.
+
+- `window-min-height' specifies a preferred minimum height in
+  canonical frame lines.  If it is the constant `full-height',
+  ask for a full-height window.
+
+- `not-this-window' non-nil means to never return the selected
+  window.  Even if this entry is not present, try to avoid
+  returning the selected window."
+  (let ((windows
+         (window-list-1 nil 'nomini (cdr (assq 'lru-frames alist))))
+        (lru-time (cdr (assq 'lru-time alist)))
+        (min-width (cdr (assq 'window-min-width alist)))
+        (min-height (cdr (assq 'window-min-height alist)))
+        (not-selected (cdr (assq 'not-this-window alist)))
+        best-window best-time second-best-window second-best-time time)
+    (dolist (window windows)
+      (when (and (not (window-dedicated-p window))
+		 (or (not not-selected) (not (eq window (selected-window)))))
+	(setq time (window-use-time window))
+        (unless (and (numberp lru-time) (> time lru-time))
+	  (if (or (eq window (selected-window))
+                  (and min-width
+                       (or (and (numberp min-width)
+                                (< (window-width window) min-width))
+                           (and (eq min-width 'full-width)
+                                (not (window-full-width-p window)))))
+                  (and min-height
+                       (or (and (numberp min-height)
+                                (< (window-height window) min-height))
+                           (and (eq min-height 'full-height)
+                                (not (window-full-height-p window))))))
+              ;; This window is either selected or does not meet the size
+              ;; restrictions - so it's only a second best choice.  Try to
+              ;; find a more recently used one that fits.
+	      (when (or (not second-best-time) (< time second-best-time))
+	        (setq second-best-time time)
+	        (setq second-best-window window))
+            ;; This window is not selected and does meet the size
+            ;; restrictions.  It's the best choice so far.
+	    (when (or (not best-time) (< time best-time))
+	      (setq best-time time)
+	      (setq best-window window))))))
+    (or best-window second-best-window)))
+
+(defun display-buffer-use-least-recent-window (buffer alist)
+  "..."
+  (let* ((alist (cons (cons 'inhibit-same-window t) alist))
+         (window
+          (or (display-buffer-reuse-window buffer alist)
+              (let ((window (display-buffer--lru-window alist)))
+                (when (window-live-p window)
+                  (let* ((quit-restore (window-parameter window 'quit-restore))
+	                 (quad (nth 1 quit-restore)))
+                    ;; If the window was used by `display-buffer' before, try to
+                    ;; resize it to its old height but don't signal an error.
+                    (when (and (listp quad)
+		               (integerp (nth 3 quad))
+		               (> (nth 3 quad) (window-total-height window)))
+	              (condition-case nil
+	                  (window-resize
+                           window (- (nth 3 quad) (window-total-height window)))
+	                (error nil)))
+                    (prog1
+	                (window--display-buffer buffer window 'reuse alist)
+	              (window--even-window-sizes window)
+	              (unless (cdr (assq 'inhibit-switch-frame alist))
+	                (window--maybe-raise-frame (window-frame window)))))))
+              (display-buffer-pop-up-window buffer alist))))
+    (when window
+      (window-bump-use-time window)
+      window)))
+
 (defun display-buffer-no-window (_buffer alist)
   "Display BUFFER in no window.
 ALIST is an association list of action symbols and values.  See

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

[Tom Gillespie]

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

This patch produces the desired behavior for me and matches
the xemacs behavior when I use the following.

(setq display-buffer-base-action '((display-buffer-use-least-recent-window)
(not-this-window . t)))

The explicit not-this-window is required to get display-buffer-pop-up-window
to trigger in a single window single frame case. It might be worth adding a
note
to that effect to the docstring?

> Commenting out these lru-time settings will make
> it use as window for *bar* the window it previously used for *foo*.

This results in the behavior of the example matching that of xemacs.
With the lru-time option enabled the display multiple buffers case seems
to be served (I assume there is some other mechanism that can be used
to restore the previous window configuration in such cases that is not
included in this example).

The addition of lru-frames to decouple from reusable-frames is also good.
It produces the desired behavior in the context of multiple frames.

Many thanks!
Tom

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

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

[martin rudalics]

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

 > This patch produces the desired behavior for me and matches
 > the xemacs behavior when I use the following.
 >
 > (setq display-buffer-base-action '((display-buffer-use-least-recent-window)
 > (not-this-window . t)))
 >
 > The explicit not-this-window is required to get display-buffer-pop-up-window
 > to trigger in a single window single frame case. It might be worth adding a
 > note
 > to that effect to the docstring?

Silly me.  Please try again with the attached.  Then

(setq display-buffer-base-action '((display-buffer-use-least-recent-window)))
(display-buffer "*Messages*")

should work as expected.

 > This results in the behavior of the example matching that of xemacs.
 > With the lru-time option enabled the display multiple buffers case seems
 > to be served (I assume there is some other mechanism that can be used
 > to restore the previous window configuration in such cases that is not
 > included in this example).

Sorry.  I don't grok what you said in parentheses here.

martin

[-- Attachment #2: Gillespie.diff --]
[-- Type: text/x-patch, Size: 5469 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index a11293d372..dffcc14ac3 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8497,16 +8497,6 @@ display-buffer-in-previous-window
     (when (setq window (or best-window second-best-window))
       (window--display-buffer buffer window 'reuse alist))))
 
-(defun display-buffer-use-least-recent-window (buffer alist)
-  "Display BUFFER in an existing window, but that hasn't been used lately.
-This `display-buffer' action function is like
-`display-buffer-use-some-window', but will cycle through windows
-when displaying buffers repeatedly, and if there's only a single
-window, it will split the window."
-  (when-let ((window (display-buffer-use-some-window
-                      buffer (cons (cons 'inhibit-same-window t) alist))))
-    (window-bump-use-time window)))
-
 (defun display-buffer-use-some-window (buffer alist)
   "Display BUFFER in an existing window.
 Search for a usable window, set that window to the buffer, and
@@ -8559,6 +8549,91 @@ display-buffer-use-some-window
 	(unless (cdr (assq 'inhibit-switch-frame alist))
 	  (window--maybe-raise-frame (window-frame window)))))))
 
+(defun display-buffer--lru-window (alist)
+  "Return the least recently used window according to ALIST.
+Do not return a minibuffer window or a window dedicated to its
+buffer.  The following ALIST entries have a special meaning:
+
+- `lru-frames' specifies the frames to investigate and has the
+  same meaning as the ALL-FRAMES argument of `get-lru-window'.
+
+- `lru-time' specifies a use time.  Do not return a window whose
+  use time is higher than this.
+
+- `window-min-width' specifies a preferred minimum width in
+  canonical frame columns.  If it is the constant `full-width',
+  ask for a full-width window.
+
+- `window-min-height' specifies a preferred minimum height in
+  canonical frame lines.  If it is the constant `full-height',
+  ask for a full-height window.
+
+If ALIST contains a non-nil `inhibit-same--window' entry, never
+return the selected window."
+  (let ((windows
+         (window-list-1 nil 'nomini (cdr (assq 'lru-frames alist))))
+        (lru-time (cdr (assq 'lru-time alist)))
+        (min-width (cdr (assq 'window-min-width alist)))
+        (min-height (cdr (assq 'window-min-height alist)))
+        (not-selected (cdr (assq 'inhibit-same-window alist)))
+        best-window best-time second-best-window second-best-time time)
+    (dolist (window windows)
+      (when (and (not (window-dedicated-p window))
+		 (or (not not-selected) (not (eq window (selected-window)))))
+	(setq time (window-use-time window))
+        (unless (and (numberp lru-time) (> time lru-time))
+	  (if (or (eq window (selected-window))
+                  (and min-width
+                       (or (and (numberp min-width)
+                                (< (window-width window) min-width))
+                           (and (eq min-width 'full-width)
+                                (not (window-full-width-p window)))))
+                  (and min-height
+                       (or (and (numberp min-height)
+                                (< (window-height window) min-height))
+                           (and (eq min-height 'full-height)
+                                (not (window-full-height-p window))))))
+              ;; This window is either selected or does not meet the size
+              ;; restrictions - so it's only a second best choice.  Try to
+              ;; find a more recently used one that fits.
+	      (when (or (not second-best-time) (< time second-best-time))
+	        (setq second-best-time time)
+	        (setq second-best-window window))
+            ;; This window is not selected and does meet the size
+            ;; restrictions.  It's the best choice so far.
+	    (when (or (not best-time) (< time best-time))
+	      (setq best-time time)
+	      (setq best-window window))))))
+    (or best-window second-best-window)))
+
+(defun display-buffer-use-least-recent-window (buffer alist)
+  "..."
+  (let* ((alist (cons (cons 'inhibit-same-window t) alist))
+         (window
+          (or (display-buffer-reuse-window buffer alist)
+              (let ((window (display-buffer--lru-window alist)))
+                (when (window-live-p window)
+                  (let* ((quit-restore (window-parameter window 'quit-restore))
+	                 (quad (nth 1 quit-restore)))
+                    ;; If the window was used by `display-buffer' before, try to
+                    ;; resize it to its old height but don't signal an error.
+                    (when (and (listp quad)
+		               (integerp (nth 3 quad))
+		               (> (nth 3 quad) (window-total-height window)))
+	              (condition-case nil
+	                  (window-resize
+                           window (- (nth 3 quad) (window-total-height window)))
+	                (error nil)))
+                    (prog1
+	                (window--display-buffer buffer window 'reuse alist)
+	              (window--even-window-sizes window)
+	              (unless (cdr (assq 'inhibit-switch-frame alist))
+	                (window--maybe-raise-frame (window-frame window)))))))
+              (display-buffer-pop-up-window buffer alist))))
+    (when window
+      (window-bump-use-time window)
+      window)))
+
 (defun display-buffer-no-window (_buffer alist)
   "Display BUFFER in no window.
 ALIST is an association list of action symbols and values.  See

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

[Eli Zaretskii]

> Date: Fri, 3 Feb 2023 10:09:02 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > This patch produces the desired behavior for me and matches
>  > the xemacs behavior when I use the following.
>  >
>  > (setq display-buffer-base-action '((display-buffer-use-least-recent-window)
>  > (not-this-window . t)))
>  >
>  > The explicit not-this-window is required to get display-buffer-pop-up-window
>  > to trigger in a single window single frame case. It might be worth adding a
>  > note
>  > to that effect to the docstring?
> 
> Silly me.  Please try again with the attached.  Then
> 
> (setq display-buffer-base-action '((display-buffer-use-least-recent-window)))
> (display-buffer "*Messages*")
> 
> should work as expected.
> 
>  > This results in the behavior of the example matching that of xemacs.
>  > With the lru-time option enabled the display multiple buffers case seems
>  > to be served (I assume there is some other mechanism that can be used
>  > to restore the previous window configuration in such cases that is not
>  > included in this example).
> 
> Sorry.  I don't grok what you said in parentheses here.
> 
> martin
> 
> diff --git a/lisp/window.el b/lisp/window.el
> index a11293d372..dffcc14ac3 100644
> --- a/lisp/window.el
> +++ b/lisp/window.el

Tom, would you please respond to Martin's latest message (quoted
above)?  I'd like to wrap up the investigation of this issue and
install whatever changes you two agree upon, so that we could proceed
with producing the first pretest of Emacs 29.1.

TIA

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

[Tom Gillespie]

Apologies for the delay. The updated patch works as expected.
There is still some strangeness if '((display-buffer-use-least-recent-window))
is used by itself as compared to using
'((display-buffer-reuse-window display-buffer-use-least-recent-window))

Specifically, when exiting an org-src-edit buffer, display-buffer-reuse-window
must be present to prevent display-buffer-use-least-recent-window from
persisting
the pop-up window and/or changing a reused window to the buffer for
the org file.

I don't think this is a blocker however, because it is not entirely
clear where the
issue lies, whether it is with how org-edit-special sets up for
restoring buffers,
or whether there is something that is off in the most recent implementation of
display-buffer-use-least-recent-window.

I think we are ok to proceed with the latest patch.

> Sorry.  I don't grok what you said in parentheses here.

It was and aside about using the new code to enable display
of multiple buffers at the same time, it is not relevant here.

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

[martin rudalics]

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

 > Apologies for the delay. The updated patch works as expected.
 > There is still some strangeness if '((display-buffer-use-least-recent-window))
 > is used by itself as compared to using
 > '((display-buffer-reuse-window display-buffer-use-least-recent-window))
 >
 > Specifically, when exiting an org-src-edit buffer, display-buffer-reuse-window
 > must be present to prevent display-buffer-use-least-recent-window from
 > persisting
 > the pop-up window and/or changing a reused window to the buffer for
 > the org file.
 >
 > I don't think this is a blocker however, because it is not entirely
 > clear where the
 > issue lies, whether it is with how org-edit-special sets up for
 > restoring buffers,
 > or whether there is something that is off in the most recent implementation of
 > display-buffer-use-least-recent-window.
 >
 > I think we are ok to proceed with the latest patch.

I now tried to document everything I wrote so far.  The attached patch
also differs from the preceding ones in the following regards:

- 'display-buffer-reuse-window' calls 'display-buffer--lru-window' with
   suitable arguments.  This has the benefit that it honors the new
   minimum size entries as well which makes the documentation simpler.
   It has the disadvantage that if I didn't guess those arguments
   correctly, the default behavior of 'display-buffer-use-some-window'
   might change wrt previous versions.

- 'window-bump-use-time' was rewritten taking into account some of the
   issues I mentioned in my last posting.

- A 'bump-use-time' action alist entry has been added.

I'm not sure whether these changes should be made on the release branch.
IMO a suitable minimal change for the release branch would be to remove
all references to 'display-buffer-avoid-small-windows' and leave
'display-buffer-use-least-recent-window' and 'window-bump-use-time'
untouched.  The latter are here since Emacs 28, so there's a good excuse
for leaving them alone.

In either case note that I don't synch with Savannah so someone else
would have to apply these changes.

martin

[-- Attachment #2: Gillespie.diff --]
[-- Type: text/x-patch, Size: 27045 bytes --]

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 5b50d5f296..d177820bbc 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -629,6 +629,12 @@ Selecting Windows
 @code{nil}.  Hence, this macro is the preferred way to temporarily work
 with @var{window} as the selected window without needlessly running
 @code{buffer-list-update-hook}.
+
+Note that this macro temporarily puts the window management code in a
+unstable state.  In particular, the most recently window (see below)
+will not necessarily match the selected one.  Hence, functions like
+@code{get-lru-window} and @code{get-mru-window} may return unexpected
+results when called from within the scope of this macro.
 @end defmac
 
 @defmac with-selected-frame frame forms@dots{}
@@ -650,15 +656,16 @@ Selecting Windows
 integer that does increase monotonically with each call of
 @code{select-window} with a @code{nil} @var{norecord} argument.  The
 window with the lowest use time is usually called the least recently
-used window while the window with the highest use time is called the
-most recently used one (@pxref{Cyclic Window Ordering}).
+used window.  The window with the highest use time is called the most
+recently used one (@pxref{Cyclic Window Ordering}) and is usually the
+selected window unless @code{with-selected-window} has been used.
 @end defun
 
 @defun window-bump-use-time &optional window
-This function marks @var{window} as being the most recently used
-one.  This can be useful when writing certain @code{pop-to-buffer}
-scenarios (@pxref{Switching Buffers}).  @var{window} must be a live
-window and defaults to the selected one.
+This function marks @var{window} as being the second most recently used
+one.  It does nothing if @var{window} is the selected window or the
+selected window does not have the highest use time among all windows
+which may happen within the scope of @code{with-selected-window}.
 @end defun
 
 @anchor{Window Group}Sometimes several windows collectively and
@@ -2755,14 +2762,40 @@ Buffer Display Action Functions
 
 @defun display-buffer-use-some-window buffer alist
 This function tries to display @var{buffer} by choosing an existing
-window and displaying the buffer in that window.  It can fail if all
-windows are dedicated to other buffers (@pxref{Dedicated Windows}).
+window and displaying the buffer in that window.  It first tries to find
+a window that has not been used recently (@pxref{Cyclic Window
+Ordering}) on any frame specified by a @code{lru-frames} @var{alist}
+entry, falling back to the selected frame if no such entry exists.  It
+also prefers windows that satisfy the constraints specified by
+@code{window-min-width} and @code{window-min-height} @var{alist}
+entries; preferring full-width windows if no @code{window-min-width}
+entry is found.  Finally, it will not return a window whose use time is
+higher than that specified by any @code{lru-time} entry provided by
+@var{alist}.
+
+If no less recently used window is found, this function will try to use
+some other window, preferably a large window on some visible frame.  It
+can fail if all windows are dedicated to other buffers (@pxref{Dedicated
+Windows}).
 @end defun
 
 @defun display-buffer-use-least-recent-window buffer alist
-This function is like @code{display-buffer-use-some-window}, but will
-not reuse the current window, and will use the least recently
-switched-to window.
+This function is similar to @code{display-buffer-use-some-window}, but
+will try harder to not use the a recently used window.  In particular,
+it does not use the selected window.  In addition, it will first try to
+reuse a window that shows @var{buffer} already, base the decision
+whether it should use a window showing another buffer on that window's
+use time alone and pop up a new window if no usable window is found.
+
+Finally, this function will bump the use time (@pxref{Selecting
+Windows}) of any window it returns in order to avoid that further
+invocations will use that window for showing another buffer.  An
+application that wants to display several buffers in a row can help this
+function by providing a @code{lru-time} @var{alist} entry it has
+initially set to the value of the selected window's use time.  Each
+invocation of this function will then bump the use time of the window
+returned to a value higher than that and a subsequent invocation will
+inhibit this function to use a window it returned earlier.
 @end defun
 
 @defun display-buffer-in-direction buffer alist
@@ -3032,12 +3065,39 @@ Buffer Display Action Alists
 window.  All action functions that choose a window should process this
 entry.
 
+@vindex window-min-width@r{, a buffer display action alist entry}
+@item window-min-width
+The value specifies a minimum width of the window used, in canonical
+frame columns.  The special value @code{full-width} means the window
+chosen should be one has no other windows on the left or right it in its
+frame.
+
+This entry is currently honored by @code{display-buffer-use-some-window}
+and @code{display-buffer-use-least-recent-window} who try hard to avoid
+returning a less recently used window that does not satisfy it.
+
+Note that providing such an entry alone does not necessarily make the
+window as wide as specified by its value.  To actually resize an
+existing window or make a new window as wide as specified by that value,
+a @code{window-width} entry specifying that value should be provided as
+well.  Such a @code{window-width} entry can, however, specify a
+completely different value or ask the window width to be fit to that of
+its buffer in which case the @code{window-min-width} entry provides the
+guaranteed minimum width of the window used.
+
 @vindex window-min-height@r{, a buffer display action alist entry}
 @item window-min-height
-The value specifies a minimum height of the window used, in lines.  If
-a window is not or cannot be made as high as specified by this entry,
-the window is not considered for use.  The only client of this entry
-is presently @code{display-buffer-below-selected}.
+The value specifies a minimum height of the window used, in canonical
+frame lines.  The special value @code{full-height} means the window
+chosen should be a full-height window, one has no other windows above or
+below it in its frame.
+
+This entry is currently honored by @code{display-buffer-below-selected}
+which does not use a window that is not as high as specified by this
+entry.  It's also honored by @code{display-buffer-use-some-window} and
+@code{display-buffer-use-least-recent-window} which try hard to avoid
+returning a less recently used window if it does not satisfy this
+constraint.
 
 Note that providing such an entry alone does not necessarily make the
 window as tall as specified by its value.  To actually resize an
@@ -3166,6 +3226,40 @@ Buffer Display Action Alists
 processed only under certain conditions which are specified right
 after this list.
 
+@vindex lru-frames@r{, a buffer display action alist entry}
+@item lru-frames
+The value specifies the set of frames to search for window that can be
+used to display the buffer.  It is honored by
+@code{display-buffer-use-some-window} and
+@code{display-buffer-use-least-recent-window} when trying to find a less
+recently used window showing some other buffer.  Its values are the same
+as for the @code{reusable-frames} entry described above.
+
+@vindex lru-time@r{, a buffer display action alist entry}
+@item lru-time
+The value is supposed to specify a use time (@pxref{Selecting Windows}).
+This entry is honored by @code{display-buffer-use-some-window} and
+@code{display-buffer-use-least-recent-window} when trying to find a less
+recently used window showing some other buffer.  If a window's use time
+is higher than the value specified by this option, these action
+functions will not consider such a window for displaying the buffer.
+
+@vindex bump-use-time@r{, a buffer display action alist entry}
+@item bump-use-time
+If non-@code{nil}, such an entry will cause @code{display-buffer} to
+bump the use time (@pxref{Selecting Windows}) of the window it uses.
+This should avoid that this window is later used by action functions
+like @code{display-buffer-use-some-window} and
+@code{display-buffer-use-least-recent-window} for showing another
+buffer.
+
+There is a fine difference between using this entry and using the action
+function @code{display-buffer-use-least-recent-window}.  Calling the
+latter means to only bump the use times of windows that function uses
+for displaying the buffer.  The entry described here will cause
+@code{display-buffer} to bump the use time of @emph{any} window used for
+displaying a buffer.
+
 @vindex pop-up-frame-parameters@r{, a buffer display action alist entry}
 @item pop-up-frame-parameters
 The value specifies an alist of frame parameters to give a new frame,
@@ -3321,13 +3415,6 @@ Choosing Window Options
 that means not to split this way.
 @end defopt
 
-@defopt display-buffer-avoid-small-windows
-If non-@code{nil}, this should be a number.  Windows that have fewer
-lines than that will be avoided when choosing an existing window.  The
-value is interpreted in units of the frame's canonical line height,
-like @code{window-total-height} does (@pxref{Window Sizes}).
-@end defopt
-
 @defopt even-window-sizes
 This variable, if non-@code{nil}, causes @code{display-buffer} to even
 window sizes whenever it reuses an existing window, and that window is
@@ -3992,53 +4079,75 @@ The Zen of Buffer Display
 @code{display-buffer-below-selected} might be preferable because the
 selected window usually already has the user's attention.
 
-@item Handle subsequent invocations of @code{display-buffer}
-@code{display-buffer} is not overly well suited for displaying several
-buffers in sequence and making sure that all these buffers are shown
-orderly in the resulting window configuration.  Again, the standard
-action functions @code{display-buffer-pop-up-window} and
-@code{display-buffer-use-some-window} are not very suited for this
-purpose due to their somewhat chaotic nature in more complex
-configurations.
+@item Take care about which window is selected
+Many applications call @code{display-buffer} from within window
+excursions produced by @code{with-selected-window} or
+@code{select-window} calls with a non-@code{nil} @var{norecord}
+argument.  This is almost always a bad idea because the window selected
+within such an excursion is usually not the window selected in the
+configuration presented to the user.
+
+If, for example, a user had added an @code{inhibit-same-window} alist
+entry, that entry would have avoided the window selected within the
+scope of the excursion and not the window selected in the resulting
+configuration.  Even if no such entry has been added, the resulting
+behavior might be strange.  While in a frame containing one live
+window, evaluating the following form
 
-   To produce a window configuration displaying multiple buffers (or
-different views of one and the same buffer) in one and the same
-display cycle, Lisp programmers will unavoidably have to write
-their own action functions.  A few tricks listed below might help in
-this regard.
+@example
+@group
+(progn
+  (split-window)
+  (display-buffer "*Messages*"))
+@end group
+@end example
 
-@itemize @bullet
-@item
-Making windows atomic (@pxref{Atomic Windows}) avoids breaking an
-existing window composition when popping up a new window.
-The new window will pop up outside the composition instead.
+will display a window showing the @file{*Messages*} buffer at the bottom
+and leave the other window selected.  Evaluating the next form
 
-@item
-Temporarily dedicating windows to their buffers (@pxref{Dedicated
-Windows}) avoids using a window for displaying a different
-buffer.  A non-dedicated window will be used instead.
+@example
+@group
+(with-selected-window (split-window)
+  (display-buffer "*Messages*"))
+@end group
+@end example
 
-@item
-Calling @code{window-preserve-size} (@pxref{Preserving Window Sizes})
-will try to keep the size of the argument window unchanged when
-popping up a new window.  You have to make sure that another window in
-the same combination can be shrunk instead, though.
+will display @file{*Messages*} in a window on the top and select it
+which is usually not what @code{display-buffer} is supposed to do.
 
-@item
-Side windows (@pxref{Side Windows}) can be used for displaying
-specific buffers always in a window at the same position of a frame.
-This permits grouping buffers that do not compete for being shown at
-the same time on a frame and showing any such buffer in the same window
-without disrupting the display of other buffers.
+On the other hand, while evaluating the following form
 
-@item
-Child frames (@pxref{Child Frames}) can be used to display a buffer
-within the screen estate of the selected frame without disrupting that
-frame's window configuration and without the overhead associated with
-full-fledged frames as inflicted by @code{display-buffer-pop-up-frame}.
-@end itemize
-@end table
+@example
+@group
+(progn
+  (split-window)
+  (pop-to-buffer "*Messages*"))
+@end group
+@end example
+
+will correctly select the @file{*Messages*} buffer, the next form
+
+@example
+@group
+(progn
+  (split-window)
+  (with-selected-window (selected-window)
+    (pop-to-buffer "*Messages*")))
+@end group
+@end example
+
+will not.
 
+Also, invocations of action functions like
+@code{display-buffer-use-some-window} and
+@code{display-buffer-use-least-recent-window} that expect the selected
+window to have the highest use time among all windows, may fail to
+produce a window according to their specifications.
+
+Hence, an application that relies on using a window excursion should try
+to postpone the @code{display-buffer} call until after the excursion has
+terminated.
+@end table
 
 @node Window History
 @section Window History
diff --git a/lisp/window.el b/lisp/window.el
index 7d8ee48635..f149d176a9 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -2487,14 +2487,6 @@ get-window-with-predicate
 
 (defalias 'some-window 'get-window-with-predicate)
 
-(defcustom display-buffer-avoid-small-windows nil
-  "If non-nil, windows that have fewer lines than this are avoided.
-This is used by `get-lru-window'.  The value is interpreted in units
-of the frame's canonical line height, like `window-total-height' does."
-  :type '(choice (const nil) number)
-  :version "29.1"
-  :group 'windows)
-
 (defun get-lru-window (&optional all-frames dedicated not-selected no-other)
   "Return the least recently used window on frames specified by ALL-FRAMES.
 Return a full-width window if possible.  A minibuffer window is
@@ -2520,11 +2512,7 @@ get-lru-window
 - A frame means consider all windows on that frame only.
 
 Any other value of ALL-FRAMES means consider all windows on the
-selected frame and no others.
-
-`display-buffer-avoid-small-windows', if non-nil, is also taken into
-consideration.  Windows whose height is smaller that the value of that
-variable will be avoided if larger windows are available."
+selected frame and no others."
   (let ((windows (window-list-1 nil 'nomini all-frames))
         best-window best-time second-best-window second-best-time time)
     (dolist (window windows)
@@ -2534,9 +2522,6 @@ get-lru-window
                      (not (window-parameter window 'no-other-window))))
 	(setq time (window-use-time window))
 	(if (or (eq window (selected-window))
-                (and display-buffer-avoid-small-windows
-                     (< (window-height window)
-                        display-buffer-avoid-small-windows))
 		(not (window-full-width-p window)))
 	    (when (or (not second-best-time) (< time second-best-time))
 	      (setq second-best-time time)
@@ -7269,6 +7254,11 @@ window--display-buffer
 dedicated flag to that value.  In any other case, reset WINDOW's
 dedicated flag to nil.
 
+If ALIST contains a non-nil `bump-use-time' entry, bump use time
+of WINDOW so further calls of `display-buffer-use-some-window'
+and `display-buffer-use-least-recent-window' will try to avoid
+it.
+
 Return WINDOW if BUFFER and WINDOW are live."
   (when (and (buffer-live-p buffer) (window-live-p window))
     (display-buffer-record-window type window buffer)
@@ -7276,6 +7266,10 @@ window--display-buffer
       ;; Unless WINDOW already shows BUFFER reset its dedicated flag.
       (set-window-dedicated-p window nil)
       (set-window-buffer window buffer))
+    (when (cdr (assq 'bump-use-time alist))
+      ;; Bump WINDOW's use time so 'display-buffer--lru-window' will try
+      ;; to avoid it.
+      (window-bump-use-time window))
     (let ((alist-dedicated (assq 'dedicated alist)))
       ;; Maybe dedicate WINDOW to BUFFER if asked for.
       (cond
@@ -8497,15 +8491,64 @@ display-buffer-in-previous-window
     (when (setq window (or best-window second-best-window))
       (window--display-buffer buffer window 'reuse alist))))
 
-(defun display-buffer-use-least-recent-window (buffer alist)
-  "Display BUFFER in an existing window, but that hasn't been used lately.
-This `display-buffer' action function is like
-`display-buffer-use-some-window', but will cycle through windows
-when displaying buffers repeatedly, and if there's only a single
-window, it will split the window."
-  (when-let ((window (display-buffer-use-some-window
-                      buffer (cons (cons 'inhibit-same-window t) alist))))
-    (window-bump-use-time window)))
+(defun display-buffer--lru-window (alist)
+  "Return the least recently used window according to ALIST.
+Do not return a minibuffer window or a window dedicated to its
+buffer.  ALIST is a buffer display action alist as compiled by
+`display-buffer'.  The following ALIST entries are honored:
+
+- `lru-frames' specifies the frames to investigate and has the
+  same meaning as the ALL-FRAMES argument of `get-lru-window'.
+
+- `lru-time' specifies a use time.  Do not return a window whose
+  use time is higher than this.
+
+- `window-min-width' specifies a preferred minimum width in
+  canonical frame columns.  If it is the constant `full-width',
+  prefer a full-width window.
+
+- `window-min-height' specifies a preferred minimum height in
+  canonical frame lines.  If it is the constant `full-height',
+  prefer a full-height window.
+
+If ALIST contains a non-nil `inhibit-same--window' entry, do not
+return the selected window."
+  (let ((windows
+         (window-list-1 nil 'nomini (cdr (assq 'lru-frames alist))))
+        (lru-time (cdr (assq 'lru-time alist)))
+        (min-width (cdr (assq 'window-min-width alist)))
+        (min-height (cdr (assq 'window-min-height alist)))
+        (not-this-window (cdr (assq 'inhibit-same-window alist)))
+        best-window best-time second-best-window second-best-time time)
+    (dolist (window windows)
+      (when (and (not (window-dedicated-p window))
+		 (or (not not-this-window)
+                     (not (eq window (selected-window)))))
+	(setq time (window-use-time window))
+        (unless (and (numberp lru-time) (> time lru-time))
+	  (if (or (eq window (selected-window))
+                  (and min-width
+                       (or (and (numberp min-width)
+                                (< (window-width window) min-width))
+                           (and (eq min-width 'full-width)
+                                (not (window-full-width-p window)))))
+                  (and min-height
+                       (or (and (numberp min-height)
+                                (< (window-height window) min-height))
+                           (and (eq min-height 'full-height)
+                                (not (window-full-height-p window))))))
+              ;; This window is either selected or does not meet the size
+              ;; restrictions - so it's only a second best choice.  Try to
+              ;; find a more recently used one that fits.
+	      (when (or (not second-best-time) (< time second-best-time))
+	        (setq second-best-time time)
+	        (setq second-best-window window))
+            ;; This window is not selected and does meet the size
+            ;; restrictions.  It's the best choice so far.
+	    (when (or (not best-time) (< time best-time))
+	      (setq best-time time)
+	      (setq best-window window))))))
+    (or best-window second-best-window)))
 
 (defun display-buffer-use-some-window (buffer alist)
   "Display BUFFER in an existing window.
@@ -8529,7 +8572,11 @@ display-buffer-use-some-window
 		    (window--frame-usable-p (last-nonminibuffer-frame))))
 	 (window
 	  ;; Reuse an existing window.
-	  (or (get-lru-window frame nil not-this-window)
+	  (or (display-buffer--lru-window
+               ;; If ALIST specifies 'lru-frames' or 'window-min-width'
+               ;; let them prevail.
+               (append alist `((lru-frames . ,frame)
+                               (window-min-width . full-width))))
 	      (let ((window (get-buffer-window buffer 'visible)))
 		(unless (and not-this-window
 			     (eq window (selected-window)))
@@ -8559,6 +8606,76 @@ display-buffer-use-some-window
 	(unless (cdr (assq 'inhibit-switch-frame alist))
 	  (window--maybe-raise-frame (window-frame window)))))))
 
+(defun display-buffer-use-least-recent-window (buffer alist)
+  "Display BUFFER trying to avoid windows used recently.
+This is similar to `display-buffer-use-some-window' but tries
+hard to avoid using a window recently used by `display-buffer'.
+
+Distinctive features are:
+
+- Do not use the selected window.
+
+- Try first to reuse a window that shows BUFFER already on a
+  frame specified by a `reusable-frames' ALIST entry, using the
+  selected frame if no such entry has been specified.
+
+- Next try to show BUFFER in the least recently used window.  The
+  frames to search for such a window can be specified via a
+  `lru-frames' ALIST entry; if no such entry exists, search the
+  selected frame only.  In addition, try to satisfy constraints
+  specified by the following ALIST entries, if present:
+
+  `lru-time' specifies a use time.  Do not return a window whose
+    use time is higher than this.  When calling this action
+    function repeatedly (presumably to display several buffers in
+    a row), an application should first save the use time of the
+    selected window and pass that same value via such an entry in
+    each call of `display-buffer'.  This reduces the probability
+    that `display-buffer' uses the same window as a previous
+    call.
+
+  `window-min-width' specifies a preferred minimum width in
+    canonical frame columns.  If it is the constant `full-width',
+    prefer a full-width window.
+
+  `window-min-height' specifies a preferred minimum height in
+    canonical frame lines.  If it is the constant `full-height',
+    prefer a full-height window.
+
+- If the preceding steps fail, try to pop up a new window on the
+  selected frame.
+
+If a window is found, bump the use time of that window to the
+highest use time after the selected window.  This makes it less
+probable that a future invocation of this function uses that
+window for another buffer."
+  (let* ((alist (cons (cons 'inhibit-same-window t) alist))
+         (window
+          (or (display-buffer-reuse-window buffer alist)
+              (let ((window (display-buffer--lru-window alist)))
+                (when (window-live-p window)
+                  (let* ((quit-restore (window-parameter window 'quit-restore))
+	                 (quad (nth 1 quit-restore)))
+                    ;; If the window was used by `display-buffer' before, try to
+                    ;; resize it to its old height but don't signal an error.
+                    (when (and (listp quad)
+		               (integerp (nth 3 quad))
+		               (> (nth 3 quad) (window-total-height window)))
+	              (condition-case nil
+	                  (window-resize
+                           window (- (nth 3 quad) (window-total-height window)))
+	                (error nil)))
+                    (prog1
+	                (window--display-buffer buffer window 'reuse alist)
+	              (window--even-window-sizes window)
+	              (unless (cdr (assq 'inhibit-switch-frame alist))
+	                (window--maybe-raise-frame (window-frame window)))))))
+              (display-buffer-pop-up-window buffer alist))))
+    ;; Don't bump use time twice.
+    (when (and window (not (cdr (assq 'bump-use-time alist))))
+      (window-bump-use-time window))
+    window))
+
 (defun display-buffer-no-window (_buffer alist)
   "Display BUFFER in no window.
 ALIST is an association list of action symbols and values.  See
diff --git a/src/window.c b/src/window.c
index 90fa6ac2df..85f740d551 100644
--- a/src/window.c
+++ b/src/window.c
@@ -762,10 +762,15 @@ DEFUN ("set-window-combination-limit", Fset_window_combination_limit, Sset_windo
 
 DEFUN ("window-use-time", Fwindow_use_time, Swindow_use_time, 0, 1, 0,
        doc: /* Return the use time of window WINDOW.
-WINDOW must be a live window and defaults to the selected one.
-The window with the highest use time is the most recently selected
-one.  The window with the lowest use time is the least recently
-selected one.  */)
+WINDOW must specify a live window and defaults to the selected one.
+
+The window with the highest use time is usually the one most recently
+selected by calling `select-window' with NORECORD nil.  The window with
+the lowest use time is usually the least recently selected one chosen in
+such a way.
+
+Note that the use time of a window can be also changed by calling
+`window-bump-use-time' for that window.  */)
   (Lisp_Object window)
 {
   return make_fixnum (decode_live_window (window)->use_time);
@@ -773,15 +778,27 @@ DEFUN ("window-use-time", Fwindow_use_time, Swindow_use_time, 0, 1, 0,
 
 DEFUN ("window-bump-use-time", Fwindow_bump_use_time,
        Swindow_bump_use_time, 0, 1, 0,
-       doc: /* Mark WINDOW as having been most recently used.
-WINDOW must be a live window and defaults to the selected one.  */)
+       doc: /* Mark WINDOW as second most recently used.
+WINDOW must specify a live window.
+
+If WINDOW is not selected and the selected window has the highest use
+time of all windows, set the use time of WINDOW to that of the selected
+window, increase the use time of the selected window by one and return
+the new use time of WINDOW.  Otherwise, do nothing and return nil.  */)
   (Lisp_Object window)
 {
   struct window *w = decode_live_window (window);
+  struct window *sw = XWINDOW (selected_window);
 
-  w->use_time = ++window_select_count;
+  if (w != sw && sw->use_time == window_select_count)
+    {
+      w->use_time = window_select_count;
+      sw->use_time = ++window_select_count;
 
-  return Qnil;
+      return make_fixnum (w->use_time);
+    }
+  else
+    return Qnil;
 }
 \f
 DEFUN ("window-pixel-width", Fwindow_pixel_width, Swindow_pixel_width, 0, 1, 0,

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

[Eli Zaretskii]

> Date: Sun, 12 Feb 2023 10:33:11 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
> I now tried to document everything I wrote so far.  The attached patch
> also differs from the preceding ones in the following regards:
> 
> - 'display-buffer-reuse-window' calls 'display-buffer--lru-window' with
>    suitable arguments.  This has the benefit that it honors the new
>    minimum size entries as well which makes the documentation simpler.
>    It has the disadvantage that if I didn't guess those arguments
>    correctly, the default behavior of 'display-buffer-use-some-window'
>    might change wrt previous versions.
> 
> - 'window-bump-use-time' was rewritten taking into account some of the
>    issues I mentioned in my last posting.
> 
> - A 'bump-use-time' action alist entry has been added.
> 
> I'm not sure whether these changes should be made on the release branch.
> IMO a suitable minimal change for the release branch would be to remove
> all references to 'display-buffer-avoid-small-windows' and leave
> 'display-buffer-use-least-recent-window' and 'window-bump-use-time'
> untouched.  The latter are here since Emacs 28, so there's a good excuse
> for leaving them alone.
> 
> In either case note that I don't synch with Savannah so someone else
> would have to apply these changes.

Thanks, I installed all of it on the release branch, after adding the
log message.

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

[martin rudalics]

 > Thanks, I installed all of it on the release branch, after adding the
 > log message.

Thanks, in particular for fixing the orthography.  I'm trying to get a
workable build of the release branch by the end of this month and do
some further testing then.  Maybe we should also add some NEWS entries.

martin

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

[Eli Zaretskii]

> Date: Mon, 20 Feb 2023 10:03:20 +0100
> Cc: tgbugs@gmail.com, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Thanks, I installed all of it on the release branch, after adding the
>  > log message.
> 
> Thanks, in particular for fixing the orthography.  I'm trying to get a
> workable build of the release branch by the end of this month and do
> some further testing then.  Maybe we should also add some NEWS entries.

Thanks in advance.

Which functions/variables/symbols that are not already in NEWS need to
be called out there, in your opinion?

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

[martin rudalics]

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

 > Which functions/variables/symbols that are not already in NEWS need to
 > be called out there, in your opinion?

I'd suggest something like the attached.

martin

[-- Attachment #2: News.diff --]
[-- Type: text/x-patch, Size: 1513 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index 0106953c1e0..f975af98e2b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1175,6 +1175,33 @@ For example, a 'display-buffer-alist' entry of
 will make the body of the chosen window 40 columns wide.  For the
 height use 'window-height' and 'body-lines', respectively.
 
++++
+*** 'display-buffer' provides more options for using an existing window.
+The display buffer action functions 'display-buffer-use-window' and
+'display-buffer-use-least-recent-window' now honor the action alist
+entry 'window-min-height' as well as the entries listed below to make
+the display of several buffers in a row more amenable.
+
++++
+*** New buffer display action alist entry 'lru-frames'.
+This allows to specify which frames 'display-buffer' should consider
+when using an window that shows another buffer.
+
++++
+*** New buffer display action alist entry 'lru-time'.
+'display-buffer' will ignore windows with a use time higher than that
+when using an window that shows another buffer.
+
++++
+*** New buffer display action alist entry 'bump-use-time'.
+This has 'display-buffer' bump the use time of any window it returns,
+making it a less likely candidate for displaying another buffer.
+
++++
+*** New buffer display action alist entry 'window-min-width'.
+This allows to specify a minimum width of the window used to display a
+buffer.
+
 ---
 *** You can customize on which window 'scroll-other-window' operates.
 This is controlled by the new 'other-window-scroll-default' variable.

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

[Eli Zaretskii]

> Date: Mon, 20 Feb 2023 19:14:38 +0100
> Cc: tgbugs@gmail.com, emacs-devel@gnu.org, larsi@gnus.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Which functions/variables/symbols that are not already in NEWS need to
>  > be called out there, in your opinion?
> 
> I'd suggest something like the attached.

LGTM, thanks.

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

[martin rudalics]

 >> Why?  Any reusable frame would have been already honored by
 > 'display-buffer-reuse-window'.
 >
 > If 'display-buffer-reuse-window' fails to find a valid window then
 > the next function to run will be 'display-buffer-use-some-window'.
 >
 > In the current implementation of 'display-buffer-use-some-window'
 > get-lru-window only searches the existing frame and not-this-window
 > will be true, therefore whenever there is a single window in the
 > current frame 'display-buffer-use-some-window' will proceed on to
 > pick a window in another frame starting with visible then 0 for all-frames
 > when it calls
 > (get-largest-window 'visible nil not-this-window) and then
 > (get-largest-window 0 nil not-this-window)
 >
 > This is not the behavior that we want because a single window in the
 > current frame will never be able to reach display-buffer-pop-up-window
 > since 'display-buffer-use-some-window' will succeed on a random window
 > in a random frame which will almost certainly not be the least recently used.
 >
 >> Why would you want to change 'display-buffer-use-some-window'?
 >
 > I don't, but in your example where we call it in
 > display-buffer-use-least-recent-window then it would have to be
 > modified since as is it is impossible to get the behavior we need
 > due to the calls to get-largest-window.

I think you are right.  No more objections from my side but please leave
a comment in the code summarizing what you said here.

martin

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

[Juri Linkov]

> One thing we could do is to add a new action alist entry say
> 'bump-use-time'.  People could use this to indicate that they want to
> display several buffers in a row.  'window--display-buffer' would then
> bump the use time of the window it uses when ALIST contains that entry.
> I have no idea how this would work in practice.

I think this is a nice idea.  Another action alist entry could
also define whether to use 'lru' or 'mru', thus obsoleting
display-buffer-use-least-recent-window and avoiding
other code duplication.

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

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Tom Gillespie <tgbugs@gmail.com>,
>   emacs-devel@gnu.org,  larsi@gnus.org
> Date: Sun, 29 Jan 2023 19:48:35 +0200
> 
> > One thing we could do is to add a new action alist entry say
> > 'bump-use-time'.  People could use this to indicate that they want to
> > display several buffers in a row.  'window--display-buffer' would then
> > bump the use time of the window it uses when ALIST contains that entry.
> > I have no idea how this would work in practice.
> 
> I think this is a nice idea.  Another action alist entry could
> also define whether to use 'lru' or 'mru', thus obsoleting
> display-buffer-use-least-recent-window and avoiding
> other code duplication.

Rather than obsoleting it, I prefer to remove it.  The problems
pointed out by Martin quite clearly mean that no one uses it
seriously.

But I still hope we could change the implementation to avoid the
problems and keep the name, so solutions which do that are better IMO.

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

[martin rudalics]

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

This statement is much too harsh.  The "use time" concept is inherently
flawed ever since it was conceived and it's unlikely that we will ever
be able to fix that.

For example, the following form

(with-selected-window (split-window nil nil t)
   (eq (get-lru-window) (selected-window)))

will consider the selected window the least recently used one which
doesn't make sense.  One now might think that the "least recently used
window" is well-defined outside the scope of 'with-selected-window' but
even that is false.  Consider

(let (window)
   (with-selected-window (setq window (split-window nil nil t))
     (select-window (split-window window nil t)))
   (eq (selected-window) (get-mru-window)))

Here the window selected outside the scope of any window excursion is no
more the most recently used one.

This means that any code like the above cited

 >      (let ((one-win (eq (selected-window)
 >                         (get-lru-window))))

is based on the false assumption that if the selected window is the
least recently used one, this is tantamount to saying that the selected
window is alone on its frame.

Given the number of 'display-buffer' calls that occur within window
excursions like 'save-selected-window', 'with-selected-window' and the
like, there's no wonder that many of them have produced unexpected
results in the past.  To cite but a few, all of ‘next-error-no-select’,
‘xref--show-location’, ‘widget-button--check-and-call-button’,
‘table-generate-source’, ‘mail-recover’, ‘occur-mode-display-occurrence’
cannot hold apart the selected window from the most recently used one
when running 'display-buffer-use-some-window'.

What looks even more disturbing is that these functions are often
supposed to use a window that is not the selected one.  But for
'display-buffer' the selected window is the one temporarily selected by
these functions and not the one that will be selected after the temporal
selection has been left and the final result in form of a displayed
buffer is presented.  Which obviously means that all action functions
called by 'display-buffer' that deal with the selected window and its
avoidance via an 'inhibit-same-window' action alist entry are affected.

Consider

(with-selected-window (split-window)
   (display-buffer "*Messages*"))

*Messages* will be displayed in the upper window and that window will be
the selected one.  Hardly what 'display-buffer' is advertised to do.

But this also means that code bumping a window's use time can do that
any which way it likes.  There is no invariant that code should try to
preserve.  Although it might be a good idea to not bump any window's use
time in a state where the selected window's use time does not equal the
value of window_select_count as in the version below.

DEFUN ("window-bump-use-time", Fwindow_bump_use_time,
        Swindow_bump_use_time, 0, 1, 0,
        doc: /* Mark WINDOW as most recently used after the selected window.
WINDOW must specify a live window.

If WINDOW is not selected and the selected window has the highest use
time of all windows, set the use time of WINDOW to that of the selected
window, increase the use time of the selected window by one and return
the new use time of WINDOW.  Otherwise, do nothing and return nil.  */)
   (Lisp_Object window)
{
   struct window *w = decode_live_window (window);
   struct window *sw = XWINDOW (selected_window);

   if (w != sw && sw->use_time == window_select_count)
     {
       w->use_time = window_select_count;
       sw->use_time = ++window_select_count;

       return make_fixnum (w->use_time);
     }
   else
     return Qnil;
}

martin