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

From: Dmitry Gutov <[email-masked]>
Date: Mon, 18 Nov 2024 03:28:49 +0200

On 16/11/2024 21:12, Juri Linkov wrote:

> Instead of a new option and 'switch-to-buffer',
> a simpler way would be allow the users just to
> customize the category with 'display-buffer-same-window':
> 
>    (setq display-buffer-alist '(((category . xref)
>                                  (display-buffer-same-window))))
> 
> It works nicely when (category . xref) is added to the same place
> like in your patch in 'xref--show-pos-in-buf':
> 
> ```
> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index cf061a18ee0..0193c2e35e0 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -689,7 +689,7 @@ xref--show-pos-in-buf
>                         (eq (window-buffer xref--original-window) buf)))
>                    `((xref--display-buffer-in-window)
>                      (window . ,xref--original-window))))))
> -    (with-selected-window (display-buffer buf action)
> +    (with-selected-window (display-buffer buf (append action '((category . xref))))
>         (xref--goto-char pos)
>         (run-hooks 'xref-after-jump-hook)
>         (selected-window))))
> ```

Thanks! That covers the code path which uses 'display-buffer', and we 
also need to handle xref-pop-to-location, which calls switch-to-buffer 
now. Does

   (pop-to-buffer buf '((display-buffer-same-window) (category . xref)))

look compatible enough?

Also, when using the suggested change in xref--show-pos-in-buf, if we 
simply annotate it with 'category' in the display-buffer call, this also 
extends to the scenarios where xref--original-window-intent is non-nil.

See the attached patch where we make sure to exclude those cases. I 
suppose that loses the semantic consistency, though: 
xref-find-definitions-other-window and xref-find-definitions-other-frame 
are "xref" commands but they will call 'display-buffer' without 'category'.

The corresponding customization (described in the first message) can be:

   (setq display-buffer-alist '(((category . xref)
                                 (display-buffer-reuse-window
                                  display-buffer-use-some-window))))

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index cc06e06ef78..56e4b0c7518 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -513,6 +513,9 @@ xref-push-marker-stack
 ;;;###autoload
 (define-obsolete-function-alias 'xref-pop-marker-stack #'xref-go-back "29.1")
 
+(defun xref--switch-to-buffer (buf)
+  (pop-to-buffer buf '((display-buffer-same-window) (category . xref))))
+
 ;;;###autoload
 (defun xref-go-back ()
   "Go back to the previous position in xref history.
@@ -523,8 +526,8 @@ xref-go-back
         (user-error "At start of xref history")
       (let ((marker (pop (car history))))
         (xref--push-forward (point-marker))
-        (switch-to-buffer (or (marker-buffer marker)
-                              (user-error "The marked buffer has been deleted")))
+        (xref--switch-to-buffer (or (marker-buffer marker)
+                                    (user-error "The marked buffer has been deleted")))
         (goto-char (marker-position marker))
         (set-marker marker nil nil)
         (run-hooks 'xref-after-return-hook)))))
@@ -538,8 +541,8 @@ xref-go-forward
         (user-error "At end of xref history")
       (let ((marker (pop (cdr history))))
         (xref--push-backward (point-marker))
-        (switch-to-buffer (or (marker-buffer marker)
-                              (user-error "The marked buffer has been deleted")))
+        (xref--switch-to-buffer (or (marker-buffer marker)
+                                    (user-error "The marked buffer has been deleted")))
         (goto-char (marker-position marker))
         (set-marker marker nil nil)
         (run-hooks 'xref-after-return-hook)))))
@@ -612,7 +615,7 @@ xref-pop-to-location
                    (xref-location-marker (xref-item-location item))))
          (buf (marker-buffer marker)))
     (cl-ecase action
-      ((nil)  (switch-to-buffer buf))
+      ((nil)  (xref--switch-to-buffer buf))
       (window (pop-to-buffer buf t))
       (frame  (let ((pop-up-frames t)) (pop-to-buffer buf t))))
     (xref--goto-char marker))
@@ -688,7 +691,10 @@ xref--show-pos-in-buf
                   (or (not (window-dedicated-p xref--original-window))
                       (eq (window-buffer xref--original-window) buf)))
                  `((xref--display-buffer-in-window)
-                   (window . ,xref--original-window))))))
+                   (category . xref)
+                   (window . ,xref--original-window)))
+                (t
+                 '((category . xref))))))
     (with-selected-window (display-buffer buf action)
       (xref--goto-char pos)
       (run-hooks 'xref-after-jump-hook)