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

From: Juri Linkov <[email-masked]>
Date: Sat, 30 Nov 2024 20:03:27 +0200

>> So in conclusion it seems better to reuse 'category' in
>> display-buffer-use-some-window.  But not to set the window parameter
>> 'category' in window--display-buffer unnecessarily.  Instead
>> this window parameter could be set only in display-buffer-use-some-window
>> when failing to find a window with a category.  I mean something like this
>> in display-buffer-use-some-window
>>
>>    (if (get-window-with-category category 'visible nil not-this-window)
>>        (use window with category)
>>      ;; otherwise
>>      (use lru window by default)
>>      (set-window-parameter window 'category (cons category parameter)))
>
> And who would set up the 'category' parameter for the first window used
> by 'image-dired'?  In my initial proposal, the presence of a 'category'
> parameter would mean to _always_ set the parameter for the window used.
> And I even would have the value of the parameter made a list like
> '(category . (tex-shell comint)) if that window would have been used for
> both.

I agree with the need to set the window parameter always.
For example, I often use commands that override the default action
with e.g. windmove-display-up/down, etc.  Therefore needed
to add an advice that remembers the last window in the
buffer-local variable:

(defvar-local display-buffer-last-window nil)

(define-advice display-buffer-record-window (:after (type window buffer) set-last-window)
  (with-current-buffer (window-buffer)
    ;; TODO: maybe later turn cons into alist ((COMMAND1 . WINDOW1) (COMMAND2 . WINDOW2))
    (setq-local display-buffer-last-window (cons this-command window))))

(setq display-buffer-base-action
      '(nil . ((some-window
                . (lambda (_buffer alist)
                    (let ((last-window (buffer-local-value
                                        'display-buffer-last-window
                                        (window-buffer))))
                      (or (and (eq this-command (car last-window))
                               (window-live-p (cdr last-window))
                               (cdr last-window))
                          (get-mru-window nil nil t))))))))

This uses the buffer-local variable in the original buffer
because IIUC setting a window parameter like in your patch
can create such a situation that more than one window
will have the same window parameter.  But need to keep
only one unique parameter in the last window that displayed
the buffer.

> Another question: Would it make sense to have 'image-dired' do
>
> (display-buffer buf '(nil ((some-window . mru))))
>
> in a first phase before embarking on a more complex solution?

This looks like the best immediate solution.

However, I still like your proposal to use a category instead of lru,
that could be used here later as well.

> Or better
>
> (display-buffer
>   buf '(nil ((some-window . mru) (inhibit-same-window . t))))
>
> to make sure the selected window doesn't get used?

But get-mru-window already prevents selecting the selected window
by the arg NOT-SELECTED of get-mru-window in display-buffer-use-some-window:

  ((eq some-window-method 'mru)
   (get-mru-window nil nil t))
                           =