Subject: Re: bug#33870: 27.0.50; xref-goto-xref not configurable

From: Dmitry Gutov <[email-masked]>
Date: Tue, 11 Jun 2019 03:00:12 +0300

Hi Juri,

On 27.01.2019 22:29, Juri Linkov wrote:
>>> If only that patch were able to keep the current behavior by default.
>> Yep.  If Juri provides a simpler patch that does this I'm all for it.
> Ok, here's 100% backward-compatible patch:

So apparently this patch was installed, but without a link to the 
relevant bug report, and it was not closed either.

Which is just as well, because my testing shows that it's really not 
100% backward compatible.

1. display-buffer-in-previous-window, as I mentioned in another email, 
does not reliably use the supplied `previous-window' value.

2. 'C-x 4 .' followed by TAB (or RET) is broken: instead of using the 
other window, it uses the original window, just like M-. does. It's a 
misunderstanding inside the code, see below.

> xref.simplify.patch
> 
> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index 87ce2299c5..9522d7e475 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -474,27 +474,17 @@ xref--show-pos-in-buf
>             (or (eq xref--original-window-intent 'frame)
>                 pop-up-frames))
>            (action
> -          (cond ((memq
> -                  xref--original-window-intent
> -                  '(window frame))
> +          (cond ((eq xref--original-window-intent 'frame)
>                    t)
> +                ((eq xref--original-window-intent 'window)
> +                 '(display-buffer-same-window))

That's not what the `window' value means. It should mean "other window".

I really don't want to revert this change after all this discussion, but 
implementing it in a different way is not straightforward. My very first 
idea turned out to be to write it more or less like it was before.

But here's an alternative patch. Juri, what do you think? Does it keep 
your customizations working?
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index e88f30ca35..8769641b08 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -492,13 +492,14 @@ xref--show-pos-in-buf
           (cond ((eq xref--original-window-intent 'frame)
                  t)
                 ((eq xref--original-window-intent 'window)
-                 '(display-buffer-same-window))
+                 `((xref--display-buffer-in-other-window)
+                   (window . ,xref--original-window)))
                 ((and
                   (window-live-p xref--original-window)
                   (or (not (window-dedicated-p xref--original-window))
                       (eq (window-buffer xref--original-window) buf)))
-                 `((display-buffer-in-previous-window)
-                   (previous-window . ,xref--original-window))))))
+                 `((xref--display-buffer-in-window)
+                   (window . ,xref--original-window))))))
     (with-selected-window (display-buffer buf action)
       (xref--goto-char pos)
       (run-hooks 'xref-after-jump-hook)
@@ -507,6 +508,19 @@ xref--show-pos-in-buf
           (setq-local other-window-scroll-buffer buf)))
       (selected-window))))
 
+(defun xref--display-buffer-in-other-window (buffer alist)
+  (let ((window (assoc-default 'window alist)))
+    (cl-assert window)
+    (xref--with-dedicated-window
+     (with-selected-window window
+       (display-buffer buffer t)))))
+
+(defun xref--display-buffer-in-window (buffer alist)
+  (let ((window (assoc-default 'window alist)))
+    (cl-assert window)
+    (with-selected-window window
+      (display-buffer buffer '(display-buffer-same-window)))))
+
 (defun xref--show-location (location &optional select)
   "Help `xref-show-xref' and `xref-goto-xref' do their job.
 Go to LOCATION and if SELECT is non-nil select its window.  If