bug#33992: 27.0.50; xref-find-definitions wastes too much space

[Dmitry Gutov]

Hi Juri,

On 28.05.2019 23:35, Juri Linkov wrote:

> FWIW, this is what I currently use to customize xref-find-definitions
> to act more like Completions:
>
> (custom-set-variables
>   '(display-buffer-alist
>     '((display-buffer-to-xref-p display-buffer-in-direction
>                                 (direction . below)
>                                 (window-height . fit-window-to-buffer)))))
>
> (defun display-buffer-to-xref-p (buffer-name _action)
>    (and (string-match-p "\\`\\*\\(xref\\)\\*\\(\\|<[0-9]+>\\)\\'"
>                         buffer-name)
>         (memq this-command '(xref-find-definitions))))
>
> (with-eval-after-load 'xref
>    (defvar xref--original-command nil)
>    (advice-add 'xref-find-definitions :after
>               (lambda (&rest _args)
>                  (with-current-buffer (window-buffer)
>                    (setq-local xref--original-command 'xref-find-definitions))))
>    (define-key xref--button-map [(control ?m)]
>      (lambda ()
>        (interactive)
>        (if (memq xref--original-command '(xref-find-definitions))
>            (call-interactively 'xref-quit-and-goto-xref)
>          (setq xref--original-window nil)
>          (call-interactively 'xref-goto-xref)))))

JFYI, one of the changes I've pushed yesterday was the patch I've shown 
before, and it should let you have the same behavior with one line:

(setq xref-show-definitions-function 'xref--show-defs-buffer-at-bottom)

It doesn't seem like xref-quit-and-goto-xref works well, though.

It doesn't always honor the intention to open the location in the window 
the command was called from. It can show the location in a different 
window, and then if I press M-, from there, Emacs does not return to the 
previous window configuration.

I've tracked this down to xref--show-pos-in-buf. Apparently, calling 
display-buffer with

  `((display-buffer-in-previous-window)
    (previous-window . ,xref--original-window))

as its second argument doesn't do the trick. Looking at it, it contains 
logic where when (eq window (selected-window)), it sets best-window to 
something else (in particular, it can favor a window where this buffer 
had been displayed previously). So, should some other action function be 
used?

I can trace this choice back to your commit 94b320849e9 where this bug 
was apparently introduced.