Subject: Re: bug#35737: xref--original-command

From: Dmitry Gutov <[email-masked]>
Date: Wed, 15 May 2019 04:07:53 +0300

On 14.05.2019 23:53, Juri Linkov wrote:
> Remembering the command that created the *xref* buffer
> will allow such customization to treat RET differently,
> i.e. it makes sense for RET to quit the transient xref buffer
> displayed momentarily while navigating code with xref-find-definitions,
> whereas leaving the xref buffer visible while navigating matches
> in the xref buffer created by e.g. project-find-regexp:
> 
> (define-key xref--button-map [(control ?m)]
>    (lambda ()
>      (interactive)
>      (if (memq xref--original-command '(xref-find-definitions))
>          (call-interactively 'xref-quit-and-goto-xref)
>        (call-interactively 'xref-goto-xref))))

I'm all for customizability, but as I said in a past discussion on the 
subject, I don't think this by itself would be a good default behavior.

Having two buffers that looks basically the same but react to RET 
differently is not great for usability.

> Better ideas?

Ideally, the "transient" buffer should look differently from the 
"persistent" one.

To take VS Code as an example (I just to see how it behaves exactly), a 
search for references opens search results in the sidebar.

Whereas for definitions they have both "Go To Definition" and "Peek 
Definition". The latter shows the definitions kind of inline, in a 
not-exactly-a-popup kind of way 
(https://stackoverflow.com/questions/55599177/go-to-definition-in-vscode-preview-window-ruins-the-edito), 
and there, if you type RET, that triggers navigation to the selected 
definition. "Go To Definition", when there are several definitions, also 
switches to "Peek" mode by default.

Not sure how best to implement this different kind of display in Emacs, 
but the idea makes a lot of sense to me.

In Sublime, IIRC, for this they used the dropdown from their 
top-of-the-window command line, the same one that becomes active once 
one presses Ctrl-P.

> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index bf999aeb0d..5c38cac164 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -477,6 +477,9 @@ xref--original-window-intent
>   (defvar-local xref--original-window nil
>     "The original window this xref buffer was created from.")
>   
> +(defvar-local xref--original-command nil
> +  "The original command that created this xref buffer.")
> +
>   (defun xref--show-pos-in-buf (pos buf)
>     "Goto and display position POS of buffer BUF in a window.
>   Honor `xref--original-window-intent', run `xref-after-jump-hook'
> @@ -777,7 +788,8 @@ xref--analyze
>           (pop-to-buffer (current-buffer))
>           (goto-char (point-min))
>           (setq xref--original-window (assoc-default 'window alist)
> -              xref--original-window-intent (assoc-default 'display-action alist))
> +              xref--original-window-intent (assoc-default 'display-action alist)
> +              xref--original-command this-command)
>           (current-buffer)))))

I'm good with this patch, though, if you find it helpful for personal 
customization.

When we implement two different display strategies, though, we might 
choose between them inside xref--show-xrefs straight away, instead of 
saving the original command for later.