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

[Eli Zaretskii <eliz@gnu.org>]

Ping! Morgan, could you please try Martin's suggestions and report
back?

> Date: Sat, 9 Nov 2024 18:36:12 +0100
> Cc: 74246@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> 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