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

[Tom Gillespie <tgbugs@gmail.com>]

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