GNU bug report logs - #74361
[PATCH] New option xref-navigation-display-window-action

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>, martin rudalics <rudalics <at> gmx.at>
Cc: 74361 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: bug#74361: [PATCH] New option xref-navigation-display-window-action
Date: Fri, 15 Nov 2024 14:13:32 +0200

> Cc: juri linkov <juri <at> linkov.net>
> Date: Fri, 15 Nov 2024 00:29:14 +0200
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> This adds a capability to customize the destination window selection 
> logic for navigation (xref-find-definitions, xref-go-back, 
> xref-go-forward) by allowing a user-supplied display window function.
> 
> Inspired by the Merlin package and its user option 
> merlin-locate-in-new-window 
> (https://github.com/ocaml/merlin/blob/a36f42a5b181d0c9cc84174e8eb241b11eeabc0f/emacs/merlin.el#L177C12-L177C39) 
> - where the value 'diff' uses a different window if the destination is 
> in an file different from the current one.
> 
> With the attached patch the customization looks a bit noisier though:
> 
>    (setq xref-navigation-display-window-action
>          '(display-buffer-reuse-window))
> 
> ^ This makes it try to reuse an existing window and fall back to 
> pop-to-window, but the effect is similar to what's described above.
> 
> Comments welcome.

I added Martin to the discussion, and have a few minor comments below.

> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index cc06e06ef78..670e80ea40b 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -431,6 +431,21 @@ xref-auto-jump-to-first-xref
>    :version "28.1"
>    :package-version '(xref . "1.2.0"))
>  
> +(defcustom xref-navigation-display-window-action nil
> +  "When non-nil, the display action to use for navigation commands.

This is too general, when taken alone (as in the apropos commands).  I
suggest something like

  If non-nil, the `display-buffer' action for showing results of Xref commands.

(The "navigation" part seems misleading, since xref-find-definitions
is not a navigation command.)

> +This does not affect commands that specify the action explicitly,

I guess "...that specify the window to use explicitly" is more
accurate?

> +such as `xref-find-definitions-other-window'."
> +  :type '(choice (const :tag "Use selected window" nil)
> +                 (const :tag "Reuse window showing destination or use another"

I think "If possible, reuse window already showing destination" is
better?

> +(defun xref--switch-to-buffer (buf)
> +  (if xref-navigation-display-window-action
> +      (pop-to-buffer buf xref-navigation-display-window-action)

Should we have some sanity checks for the value of
xref-navigation-display-window-action?  It's a user option, so
theoretically the user could use setq to set it to any value.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.