Consistency for xref-show-{xrefs,definitions}-function

Consistency for xref-show-{xrefs,definitions}-function

[Gabriel do Nascimento Ribeiro]

I noticed that the options 'xref-show-xrefs-function' and
'xref-show-definitions-function' are not symmetrical.

The defcustom for 'xref-show-xrefs-function' does not have type choices
as 'xref-show-definitions-function' has. Furthermore, it's quite strange
to configure it as follow:

(setq xref-show-xrefs-function
      'xref-show-definitions-completing-read)

I suggest to create similar functions for 'xref-show-xrefs-function',
which at this moment can be just aliases but can be independent
functions in the future, if necessary:
  - xref-show-xrefs-buffer
  - xref-show-xrefs-buffer-at-bottom
  - xref-show-xrefs-completing-read

Also, the docstrings of both options could be improved a little bit to
give more information about the default choices.

I can submit a patch if these suggestions are accepted.

Regards,
Gabriel

Re: Consistency for xref-show-{xrefs,definitions}-function

[Daniel Martín]

Gabriel do Nascimento Ribeiro <gabriel376@hotmail.com> writes:

> I noticed that the options 'xref-show-xrefs-function' and
> 'xref-show-definitions-function' are not symmetrical.
>
> The defcustom for 'xref-show-xrefs-function' does not have type choices
> as 'xref-show-definitions-function' has. Furthermore, it's quite strange
> to configure it as follow:
>
> (setq xref-show-xrefs-function
>       'xref-show-definitions-completing-read)
>
> I suggest to create similar functions for 'xref-show-xrefs-function',
> which at this moment can be just aliases but can be independent
> functions in the future, if necessary:
>   - xref-show-xrefs-buffer
>   - xref-show-xrefs-buffer-at-bottom
>   - xref-show-xrefs-completing-read
>

I agree with your viewpoint, but do we need that much complexity? IIUC,
those functions mainly receive a list of xrefs and present them in a
certain way.  I'm not sure why we should have xref-show-xrefs-* and
xref-show-definitions-* (even if they are aliases).  I feel this
duplication won't scale well (imagine if we add support for any other
code relationship to xref).

Could we have a single implementation for each presentation style? That
would reduce confusion, and xref-show-xrefs-function and
xref-show-definitions-function may have different defaults to provide
the best possible UX for each operation.

Re: Consistency for xref-show-{xrefs,definitions}-function

[Dmitry Gutov]

Hi!

On 14.01.2021 22:27, Gabriel do Nascimento Ribeiro wrote:
> I noticed that the options 'xref-show-xrefs-function' and
> 'xref-show-definitions-function' are not symmetrical.
> 
> The defcustom for 'xref-show-xrefs-function' does not have type choices
> as 'xref-show-definitions-function' has. Furthermore, it's quite strange
> to configure it as follow:
> 
> (setq xref-show-xrefs-function
>        'xref-show-definitions-completing-read)
> 
> I suggest to create similar functions for 'xref-show-xrefs-function',
> which at this moment can be just aliases but can be independent
> functions in the future, if necessary:
>    - xref-show-xrefs-buffer
>    - xref-show-xrefs-buffer-at-bottom
>    - xref-show-xrefs-completing-read
> 
> Also, the docstrings of both options could be improved a little bit to
> give more information about the default choices.
> 
> I can submit a patch if these suggestions are accepted.

Why do you want this?

xref-find-definitions differs from the rest of Xref-using commands in 
that it usually only find one location, and when there are several, the 
user wants to quickly choose one of the alternatives, the faster the 
better. The different options for xref-show-definitions-function provide 
different solutions for that problem.

In all other cases returning multiple results is the common case, and 
you usually want to see all of them, not just one. For that, the 
completing-read UI, for example, is not very suitable.

Re: Consistency for xref-show-{xrefs,definitions}-function

[Gabriel do Nascimento Ribeiro]

>
> Why do you want this?
>
> xref-find-definitions differs from the rest of Xref-using commands in
> that it usually only find one location, and when there are several,
> the user wants to quickly choose one of the alternatives, the faster
> the better. The different options for xref-show-definitions-function
> provide different solutions for that problem.
>
> In all other cases returning multiple results is the common case, and
> you usually want to see all of them, not just one. For that, the
> completing-read UI, for example, is not very suitable.

Thanks for the inputs.

My use case is: when inside a project, the only option to jump to a
certain location is by using 'C-x p g' (which runs 'xref--show-xrefs')
and selecting the desired location from the list of matches in a
separated buffer. I would like to be able to select a single element
from this list of matches, preferably with 'completing-read'.

I took another look in the code, so I guess we can keep the xrefs
functions as it is and tweak 'project-find-regexp' to be more flexible
on this.

Re: Consistency for xref-show-{xrefs,definitions}-function

[Dmitry Gutov]

On 16.01.2021 20:35, Gabriel do Nascimento Ribeiro wrote:

> My use case is: when inside a project, the only option to jump to a
> certain location is by using 'C-x p g' (which runs 'xref--show-xrefs')
> and selecting the desired location from the list of matches in a
> separated buffer. I would like to be able to select a single element
> from this list of matches, preferably with 'completing-read'.

All right. Is that location a certain symbol's definition, or just some 
special location that can only be found by a grep search?

> I took another look in the code, so I guess we can keep the xrefs
> functions as it is and tweak 'project-find-regexp' to be more flexible
> on this.

But you still use project-find-regexp for its "normal" purpose, too, 
right? That is, finding all occurrences of a certain regexp. Maybe to 
rename all occurrences of a symbol, etc.

To only change its behavior in specific situations, you can add a new 
command to your init script and bind it to a key. The code can look like:

(defun project-find-regexp-and-jump ()
   (interactive)
   ;; Or use a particular function name here directly.
   (let ((xref-show-xrefs-function xref-show-definitions-function))
     (call-interactively #'project-find-regexp)))

Other suggestions welcome, of course.

Re: Consistency for xref-show-{xrefs,definitions}-function

[Gabriel do Nascimento Ribeiro]

>
> But you still use project-find-regexp for its "normal" purpose, too,
> right? That is, finding all occurrences of a certain regexp. Maybe to
> rename all occurrences of a symbol, etc.
>
> To only change its behavior in specific situations, you can add a new
> command to your init script and bind it to a key. The code can look
> like:
>
> (defun project-find-regexp-and-jump ()
>   (interactive)
>   ;; Or use a particular function name here directly.
>   (let ((xref-show-xrefs-function xref-show-definitions-function))
>     (call-interactively #'project-find-regexp)))
>
> Other suggestions welcome, of course.

Thanks for your suggestion! I have that function in my init.el called
'project-find-regexp-completing-read'. I started this thread just to
make sure this is not a dirty hack and the existing functions are
consistent. So I guess is everything okay =)

Re: Consistency for xref-show-{xrefs,definitions}-function

[Dmitry Gutov]

On 16.01.2021 22:31, Gabriel do Nascimento Ribeiro wrote:
>>
>> But you still use project-find-regexp for its "normal" purpose, too,
>> right? That is, finding all occurrences of a certain regexp. Maybe to
>> rename all occurrences of a symbol, etc.
>>
>> To only change its behavior in specific situations, you can add a new
>> command to your init script and bind it to a key. The code can look
>> like:
>>
>> (defun project-find-regexp-and-jump ()
>>    (interactive)
>>    ;; Or use a particular function name here directly.
>>    (let ((xref-show-xrefs-function xref-show-definitions-function))
>>      (call-interactively #'project-find-regexp)))
>>
>> Other suggestions welcome, of course.
> 
> Thanks for your suggestion! I have that function in my init.el called
> 'project-find-regexp-completing-read'. I started this thread just to
> make sure this is not a dirty hack and the existing functions are
> consistent. So I guess is everything okay =)

Glad that's working for you.

Regarding consistency, perhaps the names could be tweaked a bit, 
removing "definitions" from their names to avoid implying that the 
functions must only be used with that variable (even though using them 
as the "normal" value of xref-show-xrefs-function should be discouraged).

Perhaps like:

   xref-jump-or-show-locations-buffer
   xref-jump-or-show-transient-buffer-at-bottom
   xref-completing-read-and-jump

These a bit wordy, though, so ideas welcome.

This also ties into 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44611#93 (having a 
"public" show-xrefs function with a name that reflects its purpose of 
showing a list of search results).