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

[Tom Gillespie <tgbugs@gmail.com>]

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