Poll: Change xref-show-definitions-function's default?

Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

Hi all,

In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit of 
cleanup and documented the existing alternative options.

What do you say we make one of them the default?

Re: Poll: Change xref-show-definitions-function's default?

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit
> of cleanup and documented the existing alternative options.
>
> What do you say we make one of them the default?

I vote for

+*** New alternative value for 'xref-show-definitions-function':
+'xref-show-definitions-completing-read'.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Poll: Change xref-show-definitions-function's default?

[Daniel Martín]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hi all,
>
> In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit
> of cleanup and documented the existing alternative options.
>
> What do you say we make one of them the default?

I've given a spin to the new options and I think I'd prefer the
transient option first, followed by choosing the definition from a
buffer (like it is now).  The completing-read option feels a little bit
akward to me because you first have to press TAB to see your options,
and then have to TAB complete on the "artificial" syntax that xref uses
for definitions (eg. "compile.el:(defun compile)").  The "transient"
option feels more natural.

Re: Poll: Change xref-show-definitions-function's default?

[Zhiwei Chen]

> On Dec 31, 2020, at 9:06 PM, Daniel Martín <mardani29@yahoo.es> wrote:
> 
> The completing-read option feels a little bit
> akward to me because you first have to press TAB to see your options,
> and then have to TAB complete on the "artificial" syntax that xref uses
> for definitions (eg. "compile.el:(defun compile)").  The "transient"
> option feels more natural.
> 

The `xref-show-definitions-buffer-at-bottom’ works well with or without icomplete-mode enabled.
But if icomplete-mode is enabled, `xref-show-definitions-completing-read’ is more intuitional.

--
Zhiwei Chen

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 01.01.2021 14:25, Zhiwei Chen wrote:
> But if icomplete-mode is enabled, `xref-show-definitions-completing-read’ is more intuitional.

Even more so with (setq icomplete-show-matches-on-no-input t), I think.

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 31.12.2020 15:06, Daniel Martín wrote:

> I've given a spin to the new options and I think I'd prefer the
> transient option first, followed by choosing the definition from a
> buffer (like it is now).  The completing-read option feels a little bit
> akward to me because you first have to press TAB to see your options,
> and then have to TAB complete on the "artificial" syntax that xref uses
> for definitions (eg. "compile.el:(defun compile)").  The "transient"
> option feels more natural.

That's a fair criticism: the completing-read one works best when at 
least either icomplete-mode is on (preferably with 
icomplete-show-matches-on-no-input=t) so you see the options right away, 
or with Ivy completion (then you can easily choose among the options 
with C-n and C-p).

I'm fine with the "transient" version if we'll have some consensus on 
that, but I wonder what can be done to make 
xref-show-definitions-completing-read "friendlier" even with stock 
completing-read.

If we showed the default in the prompt, it would be easier to see what's 
going on. These can be pretty long lines, though, so the result might 
turn out to be clunkier, since the prompt will still be showing it even 
after you start typing, press TAB, etc.

Re: Poll: Change xref-show-definitions-function's default?

[João Távora]

Daniel Martín <mardani29@yahoo.es> writes:

> buffer (like it is now).  The completing-read option feels a little bit
> akward to me because you first have to press TAB to see your options,
> and then have to TAB complete on the "artificial" syntax that xref uses
> for definitions (eg. "compile.el:(defun compile)").

Yep, the syntax feels clunky to me. Some things don't lend themselves
well to representation in a flat list, and definitions from files --
which happen on a many-to-many relationship -- are one of them.

Though in fairness you can skip the TAB-pressing if you use another
completing-read interface, such as fido-mode.

João

Re: Poll: Change xref-show-definitions-function's default?

[Daniel Martín via "Emacs development discussions.]

João Távora <joaotavora@gmail.com> writes:

>
> Yep, the syntax feels clunky to me. Some things don't lend themselves
> well to representation in a flat list, and definitions from files --
> which happen on a many-to-many relationship -- are one of them.
>
> Though in fairness you can skip the TAB-pressing if you use another
> completing-read interface, such as fido-mode.
>

Yes, I suggested that assuming the default Emacs completion.  IMO, a new
buffer is what feels more "natural" in that case.  Emacs handles them
well and users can bury them if needed.

Even if not many people use the default Emacs completion, I think
xref-show-definitions-function should default to something that feels
right for them.  Alternative completion interfaces could easily default
to completing-read if they think the UX would be better.

Re: Poll: Change xref-show-definitions-function's default?

[João Távora]

Daniel Martín wrote:

> Emacs handles them  well and users can bury them if needed.

I agree with you.

In case it wasn't clear,  I think even with a non-default responsive
completion the new buffer is _still_ what feels more natural in this
case, because the information displayed in it is more complex than
just a mere flat list of things to choose from. So I think the current
default should stay.

João

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 04.01.2021 01:58, João Távora wrote:
> So I think the current
> default should stay.

Have you looked at both proposed alternatives?

Re: Poll: Change xref-show-definitions-function's default?

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 258 bytes --]

Yes. In terms of UI, not code.

On Mon, Jan 4, 2021, 00:07 Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 04.01.2021 01:58, João Távora wrote:
> > So I think the current
> > default should stay.
>
> Have you looked at both proposed alternatives?
>

[-- Attachment #2: Type: text/html, Size: 526 bytes --]

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 04.01.2021 10:48, João Távora wrote:
> Yes. In terms of UI, not code.

That's fine. It just seemed you criticized only one of the alternatives, 
and yet dismissed both.

Re: Poll: Change xref-show-definitions-function's default?

[João Távora]

I don't feel as strongly about the transient buffer as I do
about the completing-read option.  It doesn't have the same
clear disadvantages to me. But I'm more used to the current
default and I like it. I'll try the transient option for a while.

João

On Mon, Jan 4, 2021 at 11:16 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> On 04.01.2021 10:48, João Távora wrote:
> > Yes. In terms of UI, not code.
>
> That's fine. It just seemed you criticized only one of the alternatives,
> and yet dismissed both.



-- 
João Távora

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 04.01.2021 13:24, João Távora wrote:
> I don't feel as strongly about the transient buffer as I do
> about the completing-read option.  It doesn't have the same
> clear disadvantages to me. But I'm more used to the current
> default and I like it. I'll try the transient option for a while.

Thank you.

Re: Poll: Change xref-show-definitions-function's default?

[martin rudalics]

 > In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit of cleanup and documented the existing alternative options.
 >
 > What do you say we make one of them the default?

Sometimes I'd like to choose an alternative from a menu or some sort of
drop down list.  At least when I use the mouse.  Would that be feasible
to add, somehow?

martin

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 01.01.2021 09:59, martin rudalics wrote:
> Sometimes I'd like to choose an alternative from a menu or some sort of
> drop down list.  At least when I use the mouse.  Would that be feasible
> to add, somehow?

Can we generate a context menu with dynamic content at runtime? If so, 
that should be easy enough to do.

You can take xref-show-definitions-completing-read as an example, only 
in the end, instead of calling completing-read the code would create a 
menu, and item handlers should call xref-pop-to-location.

Re: Poll: Change xref-show-definitions-function's default?

[Philip K.]

martin rudalics <rudalics@gmx.at> writes:

>> In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit of cleanup and documented the existing alternative options.
>>
>> What do you say we make one of them the default?
>
> Sometimes I'd like to choose an alternative from a menu or some sort of
> drop down list.  At least when I use the mouse.  Would that be feasible
> to add, somehow?

Do you mean something like this:
https://github.com/fmdkdd/dotfiles/blob/master/emacs/.emacs.d/elisp/xref-posframe.el?

> martin
>
>

-- 
	Philip K.

Re: Poll: Change xref-show-definitions-function's default?

[martin rudalics]

 > Do you mean something like this:
 > https://github.com/fmdkdd/dotfiles/blob/master/emacs/.emacs.d/elisp/xref-posframe.el?

I don't know.  After evaluating with emacs -Q

(progn
   (load "~/temp/posframe.el")
   (load "~/temp/xref-posframe.el")
   (global-set-key (kbd "M-.") #'xref-posframe-dwim)
   (find-file "~/emacs-git/quick/src/xdisp.c")
   (re-search-forward "get_next_display_element"))

I typed M-. and after confirming the TAGS table prompt I got an *xref*
window below the xdisp.c window displaying

/home/martin/emacs-git/quick/src/xdisp.c
7244: #define GET_NEXT_DISPLAY_ELEMENT(
7396: get_next_display_element

What I expected was to show these two lines in a menu (or posframe)
right above or below the "get_next_display_element" string in xdisp.c,
allow navigation there via <up> and <down> and having <right> or <RET>
get me to the corresponding tag, simultaneously popping down the menu
(or posframe).  Have I done something wrong?

In cases where there is only one definition like `set_iterator_to_next'
below, M-. first gets me a four lines frame with a short description
and then gets me to the definition as expected.

Thanks, martin

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 02.01.2021 17:43, martin rudalics wrote:
>  > Do you mean something like this:
>  > 
> https://github.com/fmdkdd/dotfiles/blob/master/emacs/.emacs.d/elisp/xref-posframe.el? 
> 
> 
> I don't know.  After evaluating with emacs -Q
> 
> (progn
>    (load "~/temp/posframe.el")
>    (load "~/temp/xref-posframe.el")
>    (global-set-key (kbd "M-.") #'xref-posframe-dwim)
>    (find-file "~/emacs-git/quick/src/xdisp.c")
>    (re-search-forward "get_next_display_element"))
> 
> I typed M-. and after confirming the TAGS table prompt I got an *xref*
> window below the xdisp.c window displaying
> 
> /home/martin/emacs-git/quick/src/xdisp.c
> 7244: #define GET_NEXT_DISPLAY_ELEMENT(
> 7396: get_next_display_element
> 
> What I expected was to show these two lines in a menu (or posframe)
> right above or below the "get_next_display_element" string in xdisp.c,
> allow navigation there via <up> and <down> and having <right> or <RET>
> get me to the corresponding tag, simultaneously popping down the menu
> (or posframe).  Have I done something wrong?

Indeed, xref-posframe-dwim doesn't handle the "difficult" case, and it's 
not a suitable xref-show-definitions-function value.

It's a neat experimental feature, though.

Re: Poll: Change xref-show-definitions-function's default?

[martin rudalics]

 > Indeed, xref-posframe-dwim doesn't handle the "difficult" case, and
 > it's not a suitable xref-show-definitions-function value.

Here I'm using my homespun set of functions because I hate that window
popped up by the default 'xref-find-definitions'.  I would have written
a popup menu version but I still can't grok 'xref--insert-xrefs' and
probably never will ...

martin

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 03.01.2021 10:39, martin rudalics wrote:
>  > Indeed, xref-posframe-dwim doesn't handle the "difficult" case, and
>  > it's not a suitable xref-show-definitions-function value.
> 
> Here I'm using my homespun set of functions because I hate that window
> popped up by the default 'xref-find-definitions'.  I would have written
> a popup menu version but I still can't grok 'xref--insert-xrefs' and
> probably never will ...

xref-show-definitions-completing-read doesn't use xref--insert-xrefs.

Can you use it as the example?

You can basically take the first 2/3rds of that function, and then build 
the menu based on the value of xref-alist-with-line-info.

Re: Poll: Change xref-show-definitions-function's default?

[Philip K.]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hi all,
>
> In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit
> of cleanup and documented the existing alternative options.

I wanted to test it with a few more options, so I tried
xref-find-apropos, but that doesn't seem to be affected by chaining the
option. Is this intentional?

> What do you say we make one of them the default?
>
>

-- 
	Philip K.

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 02.01.2021 12:24, Philip K. wrote:
>> In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit
>> of cleanup and documented the existing alternative options.
> I wanted to test it with a few more options, so I tried
> xref-find-apropos, but that doesn't seem to be affected by chaining the
> option. Is this intentional?

Yes, that one is affected by xref-show-xrefs-function. Along with all 
other users of the Xref UI.

xref-find-definitions is different from all of them in that, most of the 
time, the user wants to go to a single definition. Whereas in the other 
cases they would often want to do something with all found locations.

Re: Poll: Change xref-show-definitions-function's default?

[Philip K.]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hi all,
>
> In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit
> of cleanup and documented the existing alternative options.

Another idea, might be to have a "silent" option, that doesn't display
the buffer by default, but leaves a message that there are more results.

> What do you say we make one of them the default?

-- 
	Philip K.

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 04.01.2021 14:12, Philip K. wrote:
> Another idea, might be to have a "silent" option, that doesn't display
> the buffer by default, but leaves a message that there are more results.

What happens next, then? How will the user get to any of those results?

Re: Poll: Change xref-show-definitions-function's default?

[Philip K.]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 04.01.2021 14:12, Philip K. wrote:
>> Another idea, might be to have a "silent" option, that doesn't display
>> the buffer by default, but leaves a message that there are more results.
>
> What happens next, then? How will the user get to any of those results?

The idea would be that the interaction would look something like this:

1. The user invokes xref-find-definitions.
2. Instead of presenting the solutions, it jumps to the first one, and
   if there are more, this is indicated in the minibuffer.
3. Other matches can be displayed using next-error/previous-error
4. At this point, the buffer could be opened, or one could stick to the
   minibuffer and generate a message like "showing definition N out of
   M".

I haven't looked into next-error/previous-error, so it might be that
this isn't feasible, without rewriting a lot of code.

It goes without saying that this should not be the default option.

-- 
	Philip K.

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 04.01.2021 16:21, Philip K. wrote:

> The idea would be that the interaction would look something like this:
> 
> 1. The user invokes xref-find-definitions.
> 2. Instead of presenting the solutions, it jumps to the first one, and
>     if there are more, this is indicated in the minibuffer.
> 3. Other matches can be displayed using next-error/previous-error
> 4. At this point, the buffer could be opened, or one could stick to the
>     minibuffer and generate a message like "showing definition N out of
>     M".

It's a good plan, but the devil is in the details.

> I haven't looked into next-error/previous-error, so it might be that
> this isn't feasible, without rewriting a lot of code.

next-error/previous-error might be a bit unreliable when there are other 
next-error-capable buffers around. And especially when the user has 
customized next-error-find-buffer-function. Anyway, you can try and see.

Also re. 4, if the buffer is not needed, you could probably use the 
fileloop package. It might need to be updated to behave "lazily", however.

> It goes without saying that this should not be the default option.

If you want to work on such patch for the optional behavior, I'd be 
happy to review it.

Re: Poll: Change xref-show-definitions-function's default?

[Juri Linkov]

> In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit of
> cleanup and documented the existing alternative options.
>
> What do you say we make one of them the default?

As part of this poll we could also decide what to do with the TAB keybinding
because especially after changing the value of 'xref-show-definitions-function'
to 'xref-show-definitions-completing-read' and using the *xref* buffer
exclusively for the grep-like output, it makes more sense for
TAB to behave as in grep-mode.

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 04.01.2021 19:42, Juri Linkov wrote:
>> In commit 8c383456291185b029b469061338b5b797a49747 I have done a bit of
>> cleanup and documented the existing alternative options.
>>
>> What do you say we make one of them the default?
> 
> As part of this poll we could also decide what to do with the TAB keybinding
> because especially after changing the value of 'xref-show-definitions-function'
> to 'xref-show-definitions-completing-read' and using the *xref* buffer
> exclusively for the grep-like output, it makes more sense for
> TAB to behave as in grep-mode.

No need to poll about that, I think.

As long as we change the default to one of the current two alternatives, 
TAB won't play any role in either (the -at-bottom function sets up RET 
to call the same command).

Then we can safely remove/change the TAB binding in 
xref--xref-buffer-mode, or alternatively, create a new major mode that 
is slightly more Grep-like, and use it in the default 
xref-show-xrefs-function.

Re: Poll: Change xref-show-definitions-function's default?

[Juri Linkov]

>> As part of this poll we could also decide what to do with the TAB keybinding
>> because especially after changing the value of 'xref-show-definitions-function'
>> to 'xref-show-definitions-completing-read' and using the *xref* buffer
>> exclusively for the grep-like output, it makes more sense for
>> TAB to behave as in grep-mode.
>
> No need to poll about that, I think.
>
> As long as we change the default to one of the current two alternatives,
> TAB won't play any role in either (the -at-bottom function sets up RET to
> call the same command).

I'm afraid the default of xref-show-definitions-function can't be changed
to xref-show-definitions-completing-read because as this poll indicates
it works satisfactorily only in combination with icomplete-mode and
also non-nil icomplete-show-matches-on-no-input.

> Then we can safely remove/change the TAB binding in xref--xref-buffer-mode,
> or alternatively, create a new major mode that is slightly more Grep-like,
> and use it in the default xref-show-xrefs-function.

Maybe a new submode could help to resolve this discrepancy.

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 06.01.2021 20:30, Juri Linkov wrote:

> I'm afraid the default of xref-show-definitions-function can't be changed
> to xref-show-definitions-completing-read because as this poll indicates
> it works satisfactorily only in combination with icomplete-mode and
> also non-nil icomplete-show-matches-on-no-input.

What about xref-show-definitions-buffer-at-bottom, then?

>> Then we can safely remove/change the TAB binding in xref--xref-buffer-mode,
>> or alternatively, create a new major mode that is slightly more Grep-like,
>> and use it in the default xref-show-xrefs-function.
> 
> Maybe a new submode could help to resolve this discrepancy.

Yes, that's the second option I meant.

Re: Poll: Change xref-show-definitions-function's default?

[Juri Linkov]

>> I'm afraid the default of xref-show-definitions-function can't be changed
>> to xref-show-definitions-completing-read because as this poll indicates
>> it works satisfactorily only in combination with icomplete-mode and
>> also non-nil icomplete-show-matches-on-no-input.
>
> What about xref-show-definitions-buffer-at-bottom, then?

Sorry, I have no opinion about xref-show-definitions-buffer-at-bottom,
I'm using xref-show-definitions-completing-read with icomplete.

>>> Then we can safely remove/change the TAB binding in xref--xref-buffer-mode,
>>> or alternatively, create a new major mode that is slightly more Grep-like,
>>> and use it in the default xref-show-xrefs-function.
>> Maybe a new submode could help to resolve this discrepancy.
>
> Yes, that's the second option I meant.

Maybe the TAB keybinding should depend on the value of
xref-show-definitions-function.  When it is customized to
xref-show-definitions-completing-read then don't bind TAB.

Re: Poll: Change xref-show-definitions-function's default?

[Dmitry Gutov]

On 07.01.2021 19:47, Juri Linkov wrote:
>>> I'm afraid the default of xref-show-definitions-function can't be changed
>>> to xref-show-definitions-completing-read because as this poll indicates
>>> it works satisfactorily only in combination with icomplete-mode and
>>> also non-nil icomplete-show-matches-on-no-input.
>>
>> What about xref-show-definitions-buffer-at-bottom, then?
> 
> Sorry, I have no opinion about xref-show-definitions-buffer-at-bottom,
> I'm using xref-show-definitions-completing-read with icomplete.

It's all right.

But at the moment, very few users have expressed their preference one 
way or another, which doesn't give me much to work with.

Guess we can try out the strategy discussed in another thread: announce 
the "temporary" switch in the default and collect the impressions a few 
weeks later.

>>>> Then we can safely remove/change the TAB binding in xref--xref-buffer-mode,
>>>> or alternatively, create a new major mode that is slightly more Grep-like,
>>>> and use it in the default xref-show-xrefs-function.
>>> Maybe a new submode could help to resolve this discrepancy.
>>
>> Yes, that's the second option I meant.
> 
> Maybe the TAB keybinding should depend on the value of
> xref-show-definitions-function.  When it is customized to
> xref-show-definitions-completing-read then don't bind TAB.

Aside from an effect like that being non-obvious, I'm afraid that can't 
help most of our users, who will likely stay on the default. Someone who 
changes their xref-show-definitions-function might as well change the 
bindings too, right? It's only a few lines in the init script.

It will also fail to help users who customize their 
xref-show-definitions-function to some other value (e.g. based on Ivy or 
Helm).

But if the change in the default fails, we'll go back and consider other 
options from bug#44611's discussion.