bug#74246: [PATCH] Reuse display windows in image-dired

[martin rudalics via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

 >> When using image-dired, one can use SPC (image-dired-display-next)
 >> repeatedly to display successive images in a dedicated buffer.  However,
 >> it seems to choose a different window each time I press SPC.
 >>
 >> I can reproduce this in "emacs -Q" when I have 4 windows.
 >>
 >> The following patch works on my system.
 >
 > Thanks.
 >
 > Martin and Stefan, any comments or suggestions?

It's a problem we've seen in other contexts before: For historical
reasons 'display-buffer' by default tries to use the least recently used
window which changes whenever we display an image in it.  Juri invented
the 'some-window' buffer display option and I quote the corresponding
entry from the Elisp manual here:

      The above describes the behavior when the ‘some-window’ ALIST entry
      is ‘lru’ or ‘nil’ which is the default.  Another possible value is
      ‘mru’.  If, for example, ‘display-buffer-base-action’ is customized
      to ‘(nil . ((some-window . mru)))’, then this function will prefer
      the most recently used window.  This will try to display several
      buffers from consecutive calls of ‘display-buffer’ in the same
      window.  Consider a configuration of three or more windows where a
      user wants to consult, in a non-selected window, one after the
      other, the results of a query spread among several buffers.  With
      the ‘lru’ strategy, Emacs may continuously choose another window
      because the least recently used window changes with every call of
      ‘display-buffer-use-some-window’.  With the ‘mru’ strategy, the
      window chosen would always remain the same, resulting in a
      predictable user experience.

So I think that Morgan should try to use this approach first.  But if
the image is always displayed in 'image-dired-image-display' mode then
the appropriate action function 'image-dired' should use is
'display-buffer-reuse-mode-window'.

+  (let* ((buf (get-buffer image-dired-display-image-buffer))
+         (windows (and buf
+                       (get-buffer-window-list buf t t)))
+         (cur-win (selected-window)))
      (when buf
        (kill-buffer buf))
      (when-let ((buf (find-file-noselect file nil t)))
+      (dolist (window windows)
+        (set-window-buffer window buf))

Is this supposed to show the same image in all windows that previously
displayed 'image-dired-display-image-buffer'?

FWIW, I would rewrite 'image-dired-display-image' as the (largely
untested)

(defun image-dired-display-image (file &optional _ignored)
   "Display image FILE in the image buffer window.
If FILE is an image, the window will use `image-dired-image-mode'
which is based on `image-mode'."
   (declare (advertised-calling-convention (file) "29.1"))
   (setq file (expand-file-name file))
   (when (not (file-exists-p file))
     (error "No such file: %s" file))
   (let* ((buffer (get-buffer-create image-dired-display-image-buffer))
	 (window (get-buffer-window buffer)))
     (find-file-noselect-1 buffer file nil t nil nil)
     (with-current-buffer buffer
       (if (string-match (image-file-name-regexp) file)
           (image-dired-image-mode)
         ;; Support visiting PDF files.
         (normal-mode)))

     (unless window
	(display-buffer buffer))))

instead of doing all that 'kill-buffer' (which could delete the selected
window as a side-effect so the final 'select-window' will throw an
error) 'rename-buffer' rigmarole.

martin