unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Consistency for xref-show-{xrefs,definitions}-function
@ 2021-01-14 20:27 Gabriel do Nascimento Ribeiro
  2021-01-14 21:04 ` Daniel Martín
  2021-01-15 17:20 ` Dmitry Gutov
  0 siblings, 2 replies; 7+ messages in thread
From: Gabriel do Nascimento Ribeiro @ 2021-01-14 20:27 UTC (permalink / raw)
  To: emacs-devel

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Consistency for xref-show-{xrefs,definitions}-function
  2021-01-14 20:27 Consistency for xref-show-{xrefs,definitions}-function Gabriel do Nascimento Ribeiro
@ 2021-01-14 21:04 ` Daniel Martín
  2021-01-15 17:20 ` Dmitry Gutov
  1 sibling, 0 replies; 7+ messages in thread
From: Daniel Martín @ 2021-01-14 21:04 UTC (permalink / raw)
  To: Gabriel do Nascimento Ribeiro; +Cc: emacs-devel

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.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Consistency for xref-show-{xrefs,definitions}-function
  2021-01-14 20:27 Consistency for xref-show-{xrefs,definitions}-function Gabriel do Nascimento Ribeiro
  2021-01-14 21:04 ` Daniel Martín
@ 2021-01-15 17:20 ` Dmitry Gutov
  2021-01-16 18:35   ` Gabriel do Nascimento Ribeiro
  1 sibling, 1 reply; 7+ messages in thread
From: Dmitry Gutov @ 2021-01-15 17:20 UTC (permalink / raw)
  To: Gabriel do Nascimento Ribeiro, emacs-devel

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.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Consistency for xref-show-{xrefs,definitions}-function
  2021-01-15 17:20 ` Dmitry Gutov
@ 2021-01-16 18:35   ` Gabriel do Nascimento Ribeiro
  2021-01-16 19:24     ` Dmitry Gutov
  0 siblings, 1 reply; 7+ messages in thread
From: Gabriel do Nascimento Ribeiro @ 2021-01-16 18:35 UTC (permalink / raw)
  To: emacs-devel

>
> 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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Consistency for xref-show-{xrefs,definitions}-function
  2021-01-16 18:35   ` Gabriel do Nascimento Ribeiro
@ 2021-01-16 19:24     ` Dmitry Gutov
  2021-01-16 20:31       ` Gabriel do Nascimento Ribeiro
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Gutov @ 2021-01-16 19:24 UTC (permalink / raw)
  To: Gabriel do Nascimento Ribeiro, emacs-devel

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.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Consistency for xref-show-{xrefs,definitions}-function
  2021-01-16 19:24     ` Dmitry Gutov
@ 2021-01-16 20:31       ` Gabriel do Nascimento Ribeiro
  2021-01-18  1:28         ` Dmitry Gutov
  0 siblings, 1 reply; 7+ messages in thread
From: Gabriel do Nascimento Ribeiro @ 2021-01-16 20:31 UTC (permalink / raw)
  To: emacs-devel

>
> 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 =)



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Consistency for xref-show-{xrefs,definitions}-function
  2021-01-16 20:31       ` Gabriel do Nascimento Ribeiro
@ 2021-01-18  1:28         ` Dmitry Gutov
  0 siblings, 0 replies; 7+ messages in thread
From: Dmitry Gutov @ 2021-01-18  1:28 UTC (permalink / raw)
  To: Gabriel do Nascimento Ribeiro, emacs-devel; +Cc: Juri Linkov

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).



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-01-18  1:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-14 20:27 Consistency for xref-show-{xrefs,definitions}-function Gabriel do Nascimento Ribeiro
2021-01-14 21:04 ` Daniel Martín
2021-01-15 17:20 ` Dmitry Gutov
2021-01-16 18:35   ` Gabriel do Nascimento Ribeiro
2021-01-16 19:24     ` Dmitry Gutov
2021-01-16 20:31       ` Gabriel do Nascimento Ribeiro
2021-01-18  1:28         ` Dmitry Gutov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).