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

From: Juri Linkov <[email-masked]>
Date: Sat, 07 Dec 2024 19:13:14 +0200

>>> The second specification has the drawback that _any_ 'display-buffer'
>>> call that relies on 'display-buffer-use-some-window' may use that
>>> window.  Just think of an error occurring during that call: The
>>> *backtrace* buffer will pop up in the window specified by that variable
>>> although it is by no means related to it.
>>
>> Indeed, this is not good.  So only a category can ensure that
>> the same display-buffer call is used?
>
> You mean that is uses the same window?

I meant that only the same command (and therefore the same
display-buffer call) should reuse the same window.
This is why I checked for 'this-command' in the posted snippet.
But instead of associating with a command name an alternative approach
would be to use a category.

The *backtrace* buffer is not a problem because it uses own
display-buffer alist that overrides display-buffer-use-some-window.

> There cannot be such a guarantee.
> That window might have been (de-)selected, (temporarily)
> deleted, made dedicated and for any such reason may have become
> unsuitable for 'display-buffer'.  In the course of years people have
> added more and more conditions why 'display-buffer' should avoid certain
> windows.  Think of 'display-buffer-avoid-small-windows' - a global
> option that completely sidesteps the internal alist concept.

display-buffer-use-some-window could recheck if the window is still suitable
for every use of the same command and display-buffer call.

>> It's fine to set a buffer-local variable in the buffer where the user
>> types a key that displays the target buffer from another source buffer.
>> As long as the same buffer is used to get the value of this variable.
>
> I have no idea of a secure way to retrieve that buffer.

It's always the current buffer, even if another buffer is used
as the source buffer.

>> There are two goals:
>>
>> 1. replace the current lru with another default that reuses a previous window.
>>     But not complicating all exiting display-buffer calls by requiring
>>     each of them to set a buffer-local variable.  When a standard variable
>>     will be set, then it can be shared by different calls.
>
> Whatever we do: The buffer-local variable would have to be an option to
> fix the existing misbehavior of lru.  And it should be used only in
> cases where 'display-buffer-reuse-window' can't find a suitable windows.
>
> 'image-dired' could safely use 'display-buffer-reuse-window' because it
> eventually displays its images always in the same buffer.  It cannot do
> that right away in its present version because that one does ...

What if the user wants to display an image buffer in another window?
Then 'display-buffer-reuse-window' can't be used.

>   (let ((buf (get-buffer image-dired-display-image-buffer))
>         (cur-win (selected-window)))
>     (when buf
>       (kill-buffer buf))
>     (when-let ((buf (find-file-noselect file nil t)))
>       (pop-to-buffer buf)
>
> ... first pop up a buffer showing 'file' which means that
> 'display-buffer-reuse-window' usually won't find such a window ...
>
>       (rename-buffer image-dired-display-image-buffer)
>
> ... and then renames that buffer to 'image-dired-display-image-buffer' -
> a buffer that 'display-buffer-reuse-window' would have found if it has
> been displayed at least once.  Note that the present lru mischief
> doesn't happen for the first image displayed.  It happens for successive
> image displays only, where 'image-dired-display-image-buffer' has been
> already displayed at least once.
>
> And note the 'kill-buffer' killing 'image-dired-display-image-buffer'.
> That one defies any attempt to set a buffer-local variable in that
> buffer.

A buffer-local variable should be set in the source buffer only,
not in the image buffer.

>> 2. make user customization easy by using a special symbol like
>>     '(some-window . reuse).  But directly using the variable
>>     is also fine, e.g. `(some-window . ,last-window)
>
> I would have to understand the semantics of 'reuse' first.  What
> 'display-buffer' cannot handle currently is something like the
> 'image-dired' scenario above where the buffer would not be renamed.  In
> that case it would be nice if the caller could set a 'category' or a
> 'some-window' alist entry and 'display-buffer' could use that to find a
> window with the same 'category' or 'some-window' value and display the
> buffer there.

I have absolutely no problems with existing code
in 'image-dired-display-image' because of using:

(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))))))))