Subject: bug#74361: [PATCH] New option xref-navigation-display-window-action

From: martin rudalics <[email-masked]>
Date: Tue, 26 Nov 2024 10:15:11 +0100

> Better to use something else.  The 'category' alist entry
 > is intended to be used only in display-buffer calls like this:
 >
 >    (display-buffer (get-buffer-create "*info*")
 >                    '(nil (category . Info-mode)))
 >
 > to allow using 'category' in the condition part of display-buffer-alist
 > to match the display-buffer call:
 >
 >    (add-to-list 'display-buffer-alist
 >                 '((category . Info-mode)
 >                   (display-buffer-same-window)))

What I meant was to use precisely that as

     (add-to-list 'display-buffer-alist
                  '((category . foo)
                    (display-buffer-same-window)))

Info-mode OTOH is confusing here: It should IMO be matched by

          (buffer-mode (with-current-buffer buffer major-mode))

in 'display-buffer-reuse-mode-window' since Info-mode is the
buffer-local value of 'major-mode' in Info buffers.  Setting it up as a
category makes hardly sense to me.

'foo' would be much more generic (you don't say 'comint-mode' either in
your proposed change) and completely detached from the specific buffer
you intend to show (in general you can't guess 'comint' from the major
mode of the buffer to show).  Hence a category called 'foo' would in my
concept be matched by 'buffer-match-p' via 'category' passed as car of
CONDITION and a 'category' entry passed via the action alist by the
caller of 'display-buffer'.

 > What you proposed is a different usage of 'category'
 > that makes sense in the action part of display-buffer-alist:
 >
 >    (add-to-list 'display-buffer-alist
 >                 '("\*Help\*"
 >                   (display-buffer-same-window)
 >                   (category . Info-mode)))

No.  A user would have no idea of the name of the buffer.

 > It would be cleaner to use a different action for this feature, e.g.:
 >
 >    (add-to-list 'display-buffer-alist
 >                 '("\*Help\*"
 >                   (display-buffer-reuse-category-window)
 >                   (category . Info-mode)))
 >
 > or without an alist entry:
 >
 >    (add-to-list 'display-buffer-alist
 >                 '("\*Help\*"
 >                   (display-buffer-reuse-category-window)))

 > The only problem is that it will change the current default behavior,
 > so instead of using the lru window it will prefer to use the window
 > with the same category.

Right.  It would (1) address the problem raised in bugs like Bug#74246
and would (2) still allow users to handle 'display-comint-buffer-action'
and 'display-tex-shell-buffer-action' in your sense.  If and when for
the latter 'display-buffer-use-some-window' is called, the original
suggestion of the caller of 'display-buffer' has been levered out
already.

Obviously, the caller has to be aware of the fact that providing
(category . foo) without an action function will eventually give
'display-buffer-use-some-window' the opportunity to override the lru
with using a window that previously displayed a buffer that was supplied
with the same category argument.  Can you think of any harm this could
do?  I think the benefit of getting rid of the lru behavior in these
cases by far outweighs any such harms.

 > I customized 'display-buffer-base-action'
 > to use 'get-mru-window', so I won't be affected by this change,
 > but I care about other users.  Are you sure it would be expectable
 > for users to display buffers with the same category in one window?
 > How they could revert this behavior back to the previous default?

By adding a (some-window . nil) or (some-window . lru) entry which would
override the 'category' entry.

 > This is why I suggest to create a separate action.  If you are sure
 > it should be used by default, then it could be inserted
 > before display-buffer-use-some-window here:
 >
 >    (defconst display-buffer-fallback-action
 >      '((display-buffer--maybe-same-window
 >         display-buffer-reuse-window
 >         display-buffer--maybe-pop-up-frame-or-window
 >         display-buffer-in-previous-window
 >         display-buffer-reuse-category-window ;; <-- NEW!
 >         display-buffer-use-some-window
 >         display-buffer-pop-up-frame))
 >
 > Otherwise, users will be able to use the new action
 > display-buffer-reuse-category-window in their customizations
 > of display-buffer-alist.

What I want is to get rid of the lru behavior in the context of say
Bug#74246 for users who do _not want_ to tinker with
'display-buffer-alist'.  Most Emacs users know how to split windows.
Few of them may want to have ‘display-buffer’ use them up piecemeal by
eventually displaying images in all of them.

martin