Xref completion

Xref completion

[Pierre Neidhardt]

[-- Attachment #1.1: Type: text/plain, Size: 791 bytes --]

Hello!

Currently, when a xref query hits multiple candidates Emacs shows a Xref
buffer with the various candidates and their location.

While informative, it's a bit slow in terms of UX because the user has
to navigate an extra buffer just to confirm the candidate.

I've addressed this issue as part of my work on Helm-SLY, a Helm for the
fork of SLIME.

https://github.com/emacs-helm/helm-sly

In particular, I've overridden the behaviour with regards to Xrefs and
instead of showing the Xref buffer, I'm now offering completion: the
user can match against the symbol but also against the file and line
position.

See screenshot attached.

What do you think of adding this behaviour to upstream Emacs' xref?

Cheers!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

[-- Attachment #2: helm-sly-xref-demo.png --]
[-- Type: image/png, Size: 49209 bytes --]

Re: [SPAM UNSURE] Xref completion

[Stephen Leake]

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> Hello!
>
> Currently, when a xref query hits multiple candidates Emacs shows a Xref
> buffer with the various candidates and their location.
>
> While informative, it's a bit slow in terms of UX because the user has
> to navigate an extra buffer just to confirm the candidate.
>
> I've addressed this issue as part of my work on Helm-SLY, a Helm for the
> fork of SLIME.
>
> https://github.com/emacs-helm/helm-sly
>
> In particular, I've overridden the behaviour with regards to Xrefs and
> instead of showing the Xref buffer, I'm now offering completion: the
> user can match against the symbol but also against the file and line
> position.
>
> See screenshot attached.
>
> What do you think of adding this behaviour to upstream Emacs' xref?

Sometimes choosing a single definition is what you want, other times you
want to see the list. So this would have to use a different keybinding
or prefix.

-- 
-- Stephe

Written from unceded ancestral homelands of the Huichiun, an Ohlone
people,for which I pay Shuumi Land Tax
(https://sogoreate-landtrust.com/shuumi-land-tax) as an inadequate token
of reparation.

Re: [SPAM UNSURE] Xref completion

[Pierre Neidhardt]

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

Why not indeed!

Is someone interested in working on this?
I don't have the time to work on it myself, unfortunately.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

Re: [SPAM UNSURE] Xref completion

[Basil L. Contovounesios]

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> Sometimes choosing a single definition is what you want, other times you
> want to see the list. So this would have to use a different keybinding
> or prefix.

A bit off-topic, but I'd personally prefer more *xref* buffer navigation
bindings, such as next/previous file, over completion.

-- 
Basil

Re: Xref completion

[Eli Zaretskii]

> From: Pierre Neidhardt <mail@ambrevar.xyz>
> Date: Tue, 17 Nov 2020 19:13:18 +0100
> 
> Currently, when a xref query hits multiple candidates Emacs shows a Xref
> buffer with the various candidates and their location.
> 
> While informative, it's a bit slow in terms of UX because the user has
> to navigate an extra buffer just to confirm the candidate.
> 
> I've addressed this issue as part of my work on Helm-SLY, a Helm for the
> fork of SLIME.
> 
> https://github.com/emacs-helm/helm-sly
> 
> In particular, I've overridden the behaviour with regards to Xrefs and
> instead of showing the Xref buffer, I'm now offering completion: the
> user can match against the symbol but also against the file and line
> position.
> 
> See screenshot attached.

I'm not sure I understand the UI and the interaction.  When there are
several hits, they usually are spelled (almost) the same.  So how
would the user pick up what he/she wants using the completion UI?

And where are the candidates shown -- in the minibuffer? or in a
special buffer, like Xref does?

Re: Xref completion

[Pierre Neidhardt]

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

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not sure I understand the UI and the interaction.  When there are
> several hits, they usually are spelled (almost) the same.  So how
> would the user pick up what he/she wants using the completion UI?

The file usually differs, so that's one way to narrow down.
Another way is to simply navigate the candidates (arrow keys, tab, etc.).

> And where are the candidates shown -- in the minibuffer? or in a
> special buffer, like Xref does?

In the minibuffer.

What I'm suggesting is to use Emacs' default completion framework so
that Xref completion would work with anything: ido, fido, company, Helm,
etc.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]

Re: Xref completion

[Juri Linkov]

> In particular, I've overridden the behaviour with regards to Xrefs and
> instead of showing the Xref buffer, I'm now offering completion: the
> user can match against the symbol but also against the file and line
> position.

It seems this should be possible by customizing the value of
xref-show-definitions-function to a function that would redirect
a list of found results to completing-read.

Re: Xref completion

[Dmitry Gutov]

On 17.11.2020 21:27, Juri Linkov wrote:
>> In particular, I've overridden the behaviour with regards to Xrefs and
>> instead of showing the Xref buffer, I'm now offering completion: the
>> user can match against the symbol but also against the file and line
>> position.
> It seems this should be possible by customizing the value of
> xref-show-definitions-function to a function that would redirect
> a list of found results to completing-read.

Yup.

We'd be happy to add an alternative value for this variable that the 
users who prefer this behavior can set.

Contribute away!

Re: Xref completion

[William Xu]

Juri Linkov <juri@linkov.net> writes:

>> In particular, I've overridden the behaviour with regards to Xrefs and
>> instead of showing the Xref buffer, I'm now offering completion: the
>> user can match against the symbol but also against the file and line
>> position.
>
> It seems this should be possible by customizing the value of
> xref-show-definitions-function to a function that would redirect
> a list of found results to completing-read.

Wow, that is a life saver.

(defun my-xref--show-defs-minibuffer (fetcher alist)
  (let* ((xrefs (funcall fetcher))
         (xref-alist (xref--analyze xrefs))
         (xref (if (not (cdr xrefs))
                   (car xrefs)
                 (cadr (assoc (completing-read "Jump to definition: " xref-alist)
                              xref-alist)))))
    (xref-pop-to-location xref (assoc-default 'display-action alist))))

(setq xref-show-definitions-function 'my-xref--show-defs-minibuffer)

-- 
William

Re: Xref completion

[Dmitry Gutov]

On 17.11.2020 23:16, William Xu wrote:
> (defun my-xref--show-defs-minibuffer (fetcher alist)
>    (let* ((xrefs (funcall fetcher))
>           (xref-alist (xref--analyze xrefs))
>           (xref (if (not (cdr xrefs))
>                     (car xrefs)
>                   (cadr (assoc (completing-read "Jump to definition: " xref-alist)
>                                xref-alist)))))
>      (xref-pop-to-location xref (assoc-default 'display-action alist))))

A solid try, but note you might have a problem when there are several 
matches in the same file: you won't be able to navigate to any but the 
first one.

Of course, depending on your current programming language, this might be 
not important.

Re: Xref completion

[William Xu]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 17.11.2020 23:16, William Xu wrote:
>> (defun my-xref--show-defs-minibuffer (fetcher alist)
>>    (let* ((xrefs (funcall fetcher))
>>           (xref-alist (xref--analyze xrefs))
>>           (xref (if (not (cdr xrefs))
>>                     (car xrefs)
>>                   (cadr (assoc (completing-read "Jump to definition: " xref-alist)
>>                                xref-alist)))))
>>      (xref-pop-to-location xref (assoc-default 'display-action alist))))
>
> A solid try, but note you might have a problem when there are several
> matches in the same file: you won't be able to navigate to any but the 
> first one.
>
> Of course, depending on your current programming language, this might
> be not important.

In that case, we can just prepend the line and summary in front of the filename? 

(defun my-xref--show-defs-minibuffer (fetcher alist)
  (let* ((xrefs (funcall fetcher))
         (xref-alist (xref--analyze xrefs))
         xref-alist-with-line-info
         xref)

    (cl-loop for ((group . xrefs) . more1) on xref-alist
             do
             (cl-loop for (xref . more2) on xrefs do
                      (with-slots (summary location) xref
                        (let ((line (xref-location-line location)))
                          (push (cons (format "%d: %s %s" line summary group) xref)
                                xref-alist-with-line-info)))))

    (setq xref (if (not (cdr xrefs))
                   (car xrefs)
                 (cdr (assoc (completing-read "Jump to definition: " xref-alist-with-line-info)
                             xref-alist-with-line-info))))

    (xref-pop-to-location xref (assoc-default 'display-action alist))))

-- 
William

Re: Xref completion

[Dmitry Gutov]

On 18.11.2020 09:35, William Xu wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> On 17.11.2020 23:16, William Xu wrote:
>>> (defun my-xref--show-defs-minibuffer (fetcher alist)
>>>     (let* ((xrefs (funcall fetcher))
>>>            (xref-alist (xref--analyze xrefs))
>>>            (xref (if (not (cdr xrefs))
>>>                      (car xrefs)
>>>                    (cadr (assoc (completing-read "Jump to definition: " xref-alist)
>>>                                 xref-alist)))))
>>>       (xref-pop-to-location xref (assoc-default 'display-action alist))))
>>
>> A solid try, but note you might have a problem when there are several
>> matches in the same file: you won't be able to navigate to any but the
>> first one.
>>
>> Of course, depending on your current programming language, this might
>> be not important.
> 
> In that case, we can just prepend the line and summary in front of the filename?

Yup.

I'm not sure why prepend and not append, but I suppose it's a matter of 
preference.

Also, xref-location-line can return nil (then format will error out 
because of %d). So after a little experimenting, this is the form I 
ended up with:

   (format "%s:%s: %s" group line summary)

> (defun my-xref--show-defs-minibuffer (fetcher alist)
>    (let* ((xrefs (funcall fetcher))
>           (xref-alist (xref--analyze xrefs))
>           xref-alist-with-line-info
>           xref)
> 
>      (cl-loop for ((group . xrefs) . more1) on xref-alist
>               do
>               (cl-loop for (xref . more2) on xrefs do
>                        (with-slots (summary location) xref
>                          (let ((line (xref-location-line location)))
>                            (push (cons (format "%d: %s %s" line summary group) xref)
>                                  xref-alist-with-line-info)))))
> 
>      (setq xref (if (not (cdr xrefs))
>                     (car xrefs)
>                   (cdr (assoc (completing-read "Jump to definition: " xref-alist-with-line-info)
>                               xref-alist-with-line-info))))
> 
>      (xref-pop-to-location xref (assoc-default 'display-action alist))))
> 

Re: Xref completion

[William Xu]

Dmitry Gutov <dgutov@yandex.ru> writes:

> I'm not sure why prepend and not append, but I suppose it's a matter
> of preference.

In my case, some directory/file names are extremely long... But either
way is fine.

> Also, xref-location-line can return nil (then format will error out
> because of %d). So after a little experimenting, this is the form I 
> ended up with:
>
>   (format "%s:%s: %s" group line summary)

Thanks for catching that. I didn't know that "%s" prints not only
strings, but any types.. 

It would be nice if the change can be merged. Then I only need one line
setting.

-- 
William

Re: Xref completion

[Dmitry Gutov]

On 18.11.2020 20:53, William Xu wrote:
> It would be nice if the change can be merged.

As soon as we get some confirmation from other people that this is the 
Xref completion behavior that they like.

Re: Xref completion

[Juri Linkov]

>> It would be nice if the change can be merged.
>
> As soon as we get some confirmation from other people that this is the Xref
> completion behavior that they like.

+1.  Whereas (format "%s:%s:%s" group line summary) is like grep format,
when 'line' is 'nil', better to remove useless ':nil:' string from completions.

Re: Xref completion

[Dmitry Gutov]

On 18.11.2020 22:39, Juri Linkov wrote:
> +1.  Whereas (format "%s:%s:%s" group line summary) is like grep format,
> when 'line' is 'nil', better to remove useless ':nil:' string from completions.

Fair point.

Re: Xref completion

[João Távora]

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

On Wed, Nov 18, 2020 at 7:36 AM William Xu <william.xwl@gmail.com> wrote:

> Dmitry Gutov <dgutov@yandex.ru> writes:
>
> > On 17.11.2020 23:16, William Xu wrote:
> >> (defun my-xref--show-defs-minibuffer (fetcher alist)
> >>    (let* ((xrefs (funcall fetcher))
> >>           (xref-alist (xref--analyze xrefs))
> >>           (xref (if (not (cdr xrefs))
> >>                     (car xrefs)
> >>                   (cadr (assoc (completing-read "Jump to definition: "
> xref-alist)
> >>                                xref-alist)))))
> >>      (xref-pop-to-location xref (assoc-default 'display-action alist))))
> >
> > A solid try, but note you might have a problem when there are several
> > matches in the same file: you won't be able to navigate to any but the
> > first one.
> >
> > Of course, depending on your current programming language, this might
> > be not important.
>
> In that case, we can just prepend the line and summary in front of the
> filename?
>

I'd just like to note that in certain applications (like SLY/SLIME where
this request
hails from), sometimes xrefs are grouped not by file, but by type (a
symbols's
references includes "who sets", "who calls", "who reads", and so on) . So
if possible, and in general, this type of solution should be thought as
"prepend/append group name".  There can even be multiple grouping
strategies.

But one can also think about: select group first, then item within group.

Just my two cents.
João

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

Re: Xref completion

[Dmitry Gutov]

On 18.11.2020 20:47, João Távora wrote:

> I'd just like to note that in certain applications (like SLY/SLIME where 
> this request
> hails from), sometimes xrefs are grouped not by file, but by type (a 
> symbols's
> references includes "who sets", "who calls", "who reads", and so on) . So
> if possible, and in general, this type of solution should be thought as
> "prepend/append group name".  There can even be multiple grouping
> strategies.

We don't currently support group nesting, do we? I'm not sure how this 
can work without showing the file name or an equivalent substitute (like 
the package name) in the group. Does the file name go in each item's 
summaries?

In any case, we're currently discussing xref-show-definitions-function, 
and not xref-show-xrefs-function.

It's only used for the results of xref-find-definitions, so "who calls" 
and "who reads" won't be there. Only "who sets/defines".

> But one can also think about: select group first, then item within group.

I feel that might be under-utilizing the fuzzy matching capabilities of 
the existing completion frameworks: it should be usually quicker to 
match on both in one step.

OTOH, we should create a shortcut for the case when every group contains 
only one element: then completion would only show the group names 
(meaning file/package names), without summaries. That might work well in 
certain environments (like Java?).

Re: Xref completion

[William Xu]

Dmitry Gutov <dgutov@yandex.ru> writes:

> OTOH, we should create a shortcut for the case when every group
> contains only one element: then completion would only show the group
> names (meaning file/package names), without summaries. That might work
> well in certain environments (like Java?).

I like this idea. So we can display a mix of both.

One more point, I'm not sure if the order of the candidates are
important or not, but we can keep it the same order as in xref buffer.

(defun xref--show-defs-minibuffer (fetcher alist)
  (let* ((xrefs (funcall fetcher))
         (xref-alist (xref--analyze xrefs))
         xref-alist-with-line-info
         xref)

    (cl-loop for ((group . xrefs) . more1) on xref-alist
             do
             (let ((show-summary (> (length xrefs) 1)))
               (cl-loop for (xref . more2) on xrefs do
                        (with-slots (summary location) xref
                          (let* ((line (xref-location-line location))
                                 (line-fmt (if line (format "%s:" line) ""))
                                 (candidate
                                  (if show-summary
                                      (format "%s:%s%s" group line-fmt summary)
                                    (format "%s" group))))
                            (push (cons candidate xref) xref-alist-with-line-info))))))

    (setq xref (if (not (cdr xrefs))
                   (car xrefs)
                 (cdr (assoc (completing-read "Jump to definition: "
                                              (reverse xref-alist-with-line-info))
                             xref-alist-with-line-info))))

    (xref-pop-to-location xref (assoc-default 'display-action alist))))

-- 
William

Re: Xref completion

[Dmitry Gutov]

On 19.11.2020 10:19, William Xu wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> OTOH, we should create a shortcut for the case when every group
>> contains only one element: then completion would only show the group
>> names (meaning file/package names), without summaries. That might work
>> well in certain environments (like Java?).
> 
> I like this idea. So we can display a mix of both.

Looking good!

> One more point, I'm not sure if the order of the candidates are
> important or not, but we can keep it the same order as in xref buffer.

Not sure all completion UIs will keep that order, but sure.

Re: Xref completion

[Juri Linkov]

> One more point, I'm not sure if the order of the candidates are
> important or not, but we can keep it the same order as in xref buffer.

Yes, better to keep the same order.

One additional question: is it possible to use only relative file names,
relative to the project root dir?  Absolute file names are too long.

BTW, like you created xref--show-defs-minibuffer
as a possible option for xref-show-definitions-function,
wouldn't it be also nice to do the same and create
xref--show-xref-minibuffer as a possible option for
xref-show-xrefs-function?

Re: Xref completion

[Dmitry Gutov]

On 02.12.2020 23:44, Juri Linkov wrote:
>> One more point, I'm not sure if the order of the candidates are
>> important or not, but we can keep it the same order as in xref buffer.
> 
> Yes, better to keep the same order.
> 
> One additional question: is it possible to use only relative file names,
> relative to the project root dir?  Absolute file names are too long.

Not always possible to do it against the project root 
(xref-find-definitions does not depend on project.el), but you can still 
find the longest parent directory and cut it off, similarly to how it's 
done in project--read-file-cpd-relative.

> BTW, like you created xref--show-defs-minibuffer
> as a possible option for xref-show-definitions-function,
> wouldn't it be also nice to do the same and create
> xref--show-xref-minibuffer as a possible option for
> xref-show-xrefs-function?

Don't you usually want to iterate across multiple results, when the 
command is project-find-regexp or xref-find-references? Then the same 
logic will not work.

I mean, sometimes you probably want to find just one match, but changing 
xref-show-xrefs-function will affect all invocations.

Or maybe I should ask: how will xref--show-xref-minibuffer be different 
from xref--show-defs-minibuffer?

Re: Xref completion

[Juri Linkov]

>> BTW, like you created xref--show-defs-minibuffer
>> as a possible option for xref-show-definitions-function,
>> wouldn't it be also nice to do the same and create
>> xref--show-xref-minibuffer as a possible option for
>> xref-show-xrefs-function?
>
> Don't you usually want to iterate across multiple results, when the command
> is project-find-regexp or xref-find-references? Then the same logic will
> not work.

Right, xref--show-xref-minibuffer would be not as useful as
xref--show-defs-minibuffer is.  Hope to see xref--show-defs-minibuffer
in xref.el soon.

Re: Xref completion

[Dmitry Gutov]

On 03.12.2020 23:09, Juri Linkov wrote:
>>> BTW, like you created xref--show-defs-minibuffer
>>> as a possible option for xref-show-definitions-function,
>>> wouldn't it be also nice to do the same and create
>>> xref--show-xref-minibuffer as a possible option for
>>> xref-show-xrefs-function?
>>
>> Don't you usually want to iterate across multiple results, when the command
>> is project-find-regexp or xref-find-references? Then the same logic will
>> not work.
> 
> Right, xref--show-xref-minibuffer would be not as useful as
> xref--show-defs-minibuffer is.  Hope to see xref--show-defs-minibuffer
> in xref.el soon.

I've pushed it now with some changes, hope you all like the result.

Shortening the group part (in most common cases) is among them.

Also added some highlighting with corresponding xref faces. It now looks 
closer to Pierre's original screenshot.

Removed the bit of logic that hid the summaries when they are 
technically unnecessary because in my testing it made completion 
slightly less useful by hiding information (sorry). Anybody who really 
preferred the previous version, please try it again with c86dc8d488 
reverted locally, and maybe propose a user option for that behavior.

BTW, Juri, how do you think this new function compares against 
xref--show-defs-buffer-at-bottom?

On the one hand, it feels kinda faster, on the other, it lacks the 
ability to "look around" when you really need it. In an ideal world, I 
guess the UI would be somewhere in the middle.

Re: Xref completion

[William Xu]

Dmitry Gutov <dgutov@yandex.ru> writes:

> I've pushed it now with some changes, hope you all like the result.
>
> Shortening the group part (in most common cases) is among them.
>
> Also added some highlighting with corresponding xref faces. It now
> looks closer to Pierre's original screenshot.

The result looks nice.

> Removed the bit of logic that hid the summaries when they are
> technically unnecessary because in my testing it made completion 
> slightly less useful by hiding information (sorry).

If the summary contains different info, like the actual line from the
group (in case it is a file), that may be useful to show it, for
example:

/foo.cpp:40:struct Foo : Bar
/foo2.cpp:42:struct Foo : Foo2

However, at the moment it only shows the same summary info for each
line, which doesn't seem really useful:

/foo.cpp:40:Foo
/foo2.cpp:42:Foo

Or one can also just add this info into the reading prompt:
    (format "Jump to definition of `%s': " summary)

-- 
William

Re: Xref completion

[Dmitry Gutov]

On 05.12.2020 14:21, William Xu wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> I've pushed it now with some changes, hope you all like the result.
>>
>> Shortening the group part (in most common cases) is among them.
>>
>> Also added some highlighting with corresponding xref faces. It now
>> looks closer to Pierre's original screenshot.
> 
> The result looks nice.

Thanks.

>> Removed the bit of logic that hid the summaries when they are
>> technically unnecessary because in my testing it made completion
>> slightly less useful by hiding information (sorry).
> 
> If the summary contains different info, like the actual line from the
> group (in case it is a file), that may be useful to show it, for
> example:
> 
> /foo.cpp:40:struct Foo : Bar
> /foo2.cpp:42:struct Foo : Foo2

That's how it usually looks in my testing, with the etags backend. E.g.:

Possible completions are:
dispextern.h:2703:  int vpos;
indent.h:29:    EMACS_INT vpos;
window.h:97:  int hpos, vpos;

(After I try to navigate to the definitions 'vpos' from xdisp.c:1499).

Of course, in the end it depends on each backend to put useful into into 
summaries. And in the case of etags, that depends on how the definition 
is written (that the identifier is not alone on its line).

> However, at the moment it only shows the same summary info for each
> line, which doesn't seem really useful:
> 
> /foo.cpp:40:Foo
> /foo2.cpp:42:Foo

Which backend is this? Also etags?

> Or one can also just add this info into the reading prompt:
>      (format "Jump to definition of `%s': " summary)

I suppose it could be a special case, but we'd have to pipe through the 
initial input as well (it's not currently available). So hopefully there 
is some better solution.

Re: Xref completion

[William Xu]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Of course, in the end it depends on each backend to put useful into
> into summaries. And in the case of etags, that depends on how the
> definition is written (that the identifier is not alone on its line).

You are right. It is backend specific. So it makes more sense, showing
the summary by default as you are currently doing.

>> However, at the moment it only shows the same summary info for each
>> line, which doesn't seem really useful:
>> /foo.cpp:40:Foo
>> /foo2.cpp:42:Foo
>
> Which backend is this? Also etags?

Eglot with ccls. Sounds like something to improve in either eglot or ccls. 

-- 
William

Re: Xref completion

[Dmitry Gutov]

On 06.12.2020 10:30, William Xu wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> Of course, in the end it depends on each backend to put useful into
>> into summaries. And in the case of etags, that depends on how the
>> definition is written (that the identifier is not alone on its line).
> 
> You are right. It is backend specific. So it makes more sense, showing
> the summary by default as you are currently doing.
> 
>>> However, at the moment it only shows the same summary info for each
>>> line, which doesn't seem really useful:
>>> /foo.cpp:40:Foo
>>> /foo2.cpp:42:Foo
>>
>> Which backend is this? Also etags?
> 
> Eglot with ccls. Sounds like something to improve in either eglot or ccls.

Seems like.

It could also be a limitation of the LSP protocol (looking at 
https://microsoft.github.io/language-server-protocol/specifications/specification-3-15/#location, 
it doesn't have a field for "line text").

But if so, maybe Eglot could visit the files preemptively and extract 
the line contents. Unlike some other operations, there's unlikely to be 
many results in that list, so it shouldn't take long.

It seems to be trying to do something like this already: 
https://github.com/joaotavora/eglot/blob/master/eglot.el#L1927, maybe 
file a report for this particular case.

Re: Xref completion

[William Xu]

Dmitry Gutov <dgutov@yandex.ru> writes:

> It could also be a limitation of the LSP protocol (looking at
> https://microsoft.github.io/language-server-protocol/specifications/specification-3-15/#location,
> it doesn't have a field for "line text").

Would have been nicer if the protocol supports it, then one won't have
to look into the file manually. 

> But if so, maybe Eglot could visit the files preemptively and extract
> the line contents. Unlike some other operations, there's unlikely to
> be many results in that list, so it shouldn't take long.
>
> It seems to be trying to do something like this already:
> https://github.com/joaotavora/eglot/blob/master/eglot.el#L1927, maybe
> file a report for this particular case.

Thanks for the info. My usecase is running the ccls via docker, so the
file path reported by ccls is inside the docker. No wonder Eglot fails
to find the file, thus falling back to so called "dumb strategy".

I could work around it by changing the file path: 

---------------------------------8<------------------------------------- 
(defun xwl-convert-to-docker-path (file)
  (if (and (string-match "foo" (cdr (project-current)))
           (string-match "^/usr/include/" file))
      (concat "/docker:foo:" file)
    file))

(defun xwl-check-docker (orig-fun file &rest args)
  (or (ignore-errors (apply orig-fun (cons file args)))
      (apply orig-fun (cons (xwl-convert-to-docker-path file) args))))

(advice-add 'file-readable-p :around 'xwl-check-docker)
(advice-add 'insert-file-contents :around 'xwl-check-docker)
---------------------------------8<------------------------------------- 

-- 
William

Re: Xref completion

[Dmitry Gutov]

On 06.12.2020 16:00, William Xu wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> It could also be a limitation of the LSP protocol (looking at
>> https://microsoft.github.io/language-server-protocol/specifications/specification-3-15/#location,
>> it doesn't have a field for "line text").
> 
> Would have been nicer if the protocol supports it, then one won't have
> to look into the file manually.

Someone should probably look into bringing that up to LSP folks, but 
even if successful, that's going to be a long process.

>> But if so, maybe Eglot could visit the files preemptively and extract
>> the line contents. Unlike some other operations, there's unlikely to
>> be many results in that list, so it shouldn't take long.
>>
>> It seems to be trying to do something like this already:
>> https://github.com/joaotavora/eglot/blob/master/eglot.el#L1927, maybe
>> file a report for this particular case.
> 
> Thanks for the info. My usecase is running the ccls via docker, so the
> file path reported by ccls is inside the docker. No wonder Eglot fails
> to find the file, thus falling back to so called "dumb strategy".

IDK, if Eglot knows how to navigate to a file, it will know how to read 
a line from it. So it should be possible to add that behavior.

What the performance is going to be, however, is something we'll only 
know after we try.

> I could work around it by changing the file path:
> 
> ---------------------------------8<-------------------------------------
> (defun xwl-convert-to-docker-path (file)
>    (if (and (string-match "foo" (cdr (project-current)))
>             (string-match "^/usr/include/" file))
>        (concat "/docker:foo:" file)
>      file))
> 
> (defun xwl-check-docker (orig-fun file &rest args)
>    (or (ignore-errors (apply orig-fun (cons file args)))
>        (apply orig-fun (cons (xwl-convert-to-docker-path file) args))))
> 
> (advice-add 'file-readable-p :around 'xwl-check-docker)
> (advice-add 'insert-file-contents :around 'xwl-check-docker)
> ---------------------------------8<-------------------------------------

Advising these common functions looks suspicious to me. But hey, if it 
works, it works. You could also try editing the project via Tramp. Not 
sure if Eglot supports that (if it does not, adding support shouldn't be 
too hard), but apparently lsp-mode does.

Re: Xref completion

[Juri Linkov]

> I've pushed it now with some changes, hope you all like the result.
>
> Shortening the group part (in most common cases) is among them.
>
> Also added some highlighting with corresponding xref faces. It now looks
> closer to Pierre's original screenshot.

Thanks, everything works well.  Would be nicer to add all available
choices to xref-show-definitions-function (i.e. "at bottom", "minibuffer").

> BTW, Juri, how do you think this new function compares against
> xref--show-defs-buffer-at-bottom?
>
> On the one hand, it feels kinda faster, on the other, it lacks the ability
> to "look around" when you really need it. In an ideal world, I guess the UI
> would be somewhere in the middle.

There are still too many differences between them.  The most noteworthy
is that xref--show-defs-minibuffer doesn't show completions by default,
only after TAB TAB.

Re: Xref completion

[Dmitry Gutov]

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

On 05.12.2020 21:52, Juri Linkov wrote:
>> I've pushed it now with some changes, hope you all like the result.
>>
>> Shortening the group part (in most common cases) is among them.
>>
>> Also added some highlighting with corresponding xref faces. It now looks
>> closer to Pierre's original screenshot.
> 
> Thanks, everything works well.  Would be nicer to add all available
> choices to xref-show-definitions-function (i.e. "at bottom", "minibuffer").

Of course, a bit later.

BTW, when we do that, should we also rename all involved functions to 
"public" names?

Putting them into the :type of a defcustom probably involves some kind 
of social contract.

>> BTW, Juri, how do you think this new function compares against
>> xref--show-defs-buffer-at-bottom?
>>
>> On the one hand, it feels kinda faster, on the other, it lacks the ability
>> to "look around" when you really need it. In an ideal world, I guess the UI
>> would be somewhere in the middle.
> 
> There are still too many differences between them.  The most noteworthy
> is that xref--show-defs-minibuffer doesn't show completions by default,
> only after TAB TAB.

That depends on the completion UI in use.

To see them right away, just start with 'M-x fido-mode'.

Or here is the config I'm currently trying out. It needs both ivy and 
ivy-posframe installed, though:

(setq xref-show-definitions-function 'xref--show-defs-minibuffer)

(defun complete-with-ivy (fun &rest args)
   (let ((completing-read-function #'ivy-completing-read))
     (apply fun args)))

(advice-add #'xref--show-defs-minibuffer :around #'complete-with-ivy)

;; These last two forms are unrelated, here for completeness:

(advice-add #'project-find-file-in :around #'complete-with-ivy)

(advice-add #'project-prompt-project-dir :around #'complete-with-ivy)

;; See the result on the attached screenshot.

[-- Attachment #2: Screenshot from 2020-12-05 23-25-42.png --]
[-- Type: image/png, Size: 271527 bytes --]

Re: Xref completion

[Juri Linkov]

>>> Also added some highlighting with corresponding xref faces. It now looks
>>> closer to Pierre's original screenshot.
>> Thanks, everything works well.  Would be nicer to add all available
>> choices to xref-show-definitions-function (i.e. "at bottom", "minibuffer").
>
> Of course, a bit later.
>
> BTW, when we do that, should we also rename all involved functions to
> "public" names?

Indeed, these functions are not "internal".

>>> On the one hand, it feels kinda faster, on the other, it lacks the ability
>>> to "look around" when you really need it. In an ideal world, I guess the UI
>>> would be somewhere in the middle.
>> There are still too many differences between them.  The most noteworthy
>> is that xref--show-defs-minibuffer doesn't show completions by default,
>> only after TAB TAB.
>
> That depends on the completion UI in use.
>
> To see them right away, just start with 'M-x fido-mode'.
>
> Or here is the config I'm currently trying out. It needs both ivy and
> ivy-posframe installed, though:

> ;; See the result on the attached screenshot.

But matches are not highlighted on your screenshot, whereas on Pierre's screenshot
matches are highlighted.

Re: Xref completion

[Dmitry Gutov]

On 06.12.2020 22:46, Juri Linkov wrote:
>>>> Also added some highlighting with corresponding xref faces. It now looks
>>>> closer to Pierre's original screenshot.
>>> Thanks, everything works well.  Would be nicer to add all available
>>> choices to xref-show-definitions-function (i.e. "at bottom", "minibuffer").
>>
>> Of course, a bit later.
>>
>> BTW, when we do that, should we also rename all involved functions to
>> "public" names?
> 
> Indeed, these functions are not "internal".

All right.

>>>> On the one hand, it feels kinda faster, on the other, it lacks the ability
>>>> to "look around" when you really need it. In an ideal world, I guess the UI
>>>> would be somewhere in the middle.
>>> There are still too many differences between them.  The most noteworthy
>>> is that xref--show-defs-minibuffer doesn't show completions by default,
>>> only after TAB TAB.
>>
>> That depends on the completion UI in use.
>>
>> To see them right away, just start with 'M-x fido-mode'.
>>
>> Or here is the config I'm currently trying out. It needs both ivy and
>> ivy-posframe installed, though:
> 
>> ;; See the result on the attached screenshot.
> 
> But matches are not highlighted on your screenshot, whereas on Pierre's screenshot
> matches are highlighted.

That's only because there is no input on my screenshot (I aimed to show 
other things). And Pierre's features input "noti".

Ivy, of course, highlights the matches.

Re: Xref completion

[Juri Linkov]

>>>>> On the one hand, it feels kinda faster, on the other, it lacks the ability
>>>>> to "look around" when you really need it. In an ideal world, I guess the UI
>>>>> would be somewhere in the middle.
>>>> There are still too many differences between them.  The most noteworthy
>>>> is that xref--show-defs-minibuffer doesn't show completions by default,
>>>> only after TAB TAB.
>>>
>>> That depends on the completion UI in use.
>>>
>>> To see them right away, just start with 'M-x fido-mode'.
>>>
>>> Or here is the config I'm currently trying out. It needs both ivy and
>>> ivy-posframe installed, though:
>> 
>>> ;; See the result on the attached screenshot.
>> But matches are not highlighted on your screenshot, whereas on Pierre's
>> screenshot
>> matches are highlighted.
>
> That's only because there is no input on my screenshot (I aimed to show
> other things). And Pierre's features input "noti".
>
> Ivy, of course, highlights the matches.

But why 'M-.' doesn't highlight the identifier name in the *xref* buffer,
neither in the *Completions* buffer when xref--show-defs-minibuffer is used?

Re: Xref completion

[Dmitry Gutov]

On 06.12.2020 23:34, Juri Linkov wrote:
>> That's only because there is no input on my screenshot (I aimed to show
>> other things). And Pierre's features input "noti".
>>
>> Ivy, of course, highlights the matches.
> 
> But why 'M-.' doesn't highlight the identifier name in the *xref* buffer,
> neither in the *Completions* buffer when xref--show-defs-minibuffer is used?

I don't know, never saw the need? And it might be non-trivial to 
implement in many cases.

The identifier name that was searched for might not even appear in the 
list verbatim.

Like, the user could have searched for the qualified name, but the line 
only contains its "local" name. Or the user might have searched for an 
alias. Similarly, it won't be in the definition's file.

Re: Xref completion

[Daniel Martín]

Dmitry Gutov <dgutov@yandex.ru> writes:
>
> On the one hand, it feels kinda faster, on the other, it lacks the
> ability to "look around" when you really need it. In an ideal world, I 
> guess the UI would be somewhere in the middle.

One of the reasons I see people integrate Ivy/Helm with Xref is to have
fuzzy filtering of the results, but I don't see why we couldn't have
that in the *xref* buffer.

First I thought if we could implement this by extending an existing
Emacs command like flush-lines or occur to "teach" it about the
specificities of the *xref* buffer, but that doesn't seem very clean.

So perhaps we could add a new command that asks for a regexp in the
minibuffer and filters the results in the *xref* buffer to only contain
those items from a group that matches that regular expression.  With q
you will go back to show the complete list of xref items and groups.  Is
there any core Emacs package that performs a similar filtering?  We
could learn from its workflow, then.

Why matching the group exclusively? In my opinion, matching the group
only is better because the typical use case I have in mind is a big
codebase with thousands of xref results that are spread around hundreds
of files.  You usually want to filter by the string that varies the
most, which in most cases is the group (eg. if I only want to see
results from unit tests, then I'd filter the *xref* buffer by
"*Tests.cpp"; If I only want to see results from header files, I'd
filter by "*.h").

Thoughts?

Re: Xref completion

[jixiuf]

> 2020年12月7日 下午5:27，Daniel Martín <mardani29@yahoo.es> 写道：
> 
> Dmitry Gutov <dgutov@yandex.ru> writes:
>> 
>> On the one hand, it feels kinda faster, on the other, it lacks the
>> ability to "look around" when you really need it. In an ideal world, I 
>> guess the UI would be somewhere in the middle.
> 
> One of the reasons I see people integrate Ivy/Helm with Xref is to have
> fuzzy filtering of the results, but I don't see why we couldn't have
> that in the *xref* buffer.
> 
> First I thought if we could implement this by extending an existing
> Emacs command like flush-lines or occur to "teach" it about the
> specificities of the *xref* buffer, but that doesn't seem very clean.
I think it’s great to make *xref*  supporting wgrep or occur-edit-mode.

Re: Xref completion

[Dmitry Gutov]

On 07.12.2020 17:12, jixiuf wrote:

> I think it’s great to make *xref*  supporting wgrep or occur-edit-mode.

Given the abstraction level, it might be a fair amount of work. If 
someone wanted to work on that patch, though, I'd be happy to give some 
advice.

In the meantime, I think the xref-query-replace-in-results command 
covers 90% of use cases.

Re: Xref completion

[Dmitry Gutov]

On 07.12.2020 11:27, Daniel Martín wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
>>
>> On the one hand, it feels kinda faster, on the other, it lacks the
>> ability to "look around" when you really need it. In an ideal world, I
>> guess the UI would be somewhere in the middle.
> 
> One of the reasons I see people integrate Ivy/Helm with Xref is to have
> fuzzy filtering of the results, but I don't see why we couldn't have
> that in the *xref* buffer.

I think xref buffers have other ways of reaching the same goal. E.g., if 
we're talking about the results of project-find-regexp or 
xref-find-apropos, those commands themselves allow you to enter search 
queries where you can choose the appropriate amount of "fuzziness".

That said...

> First I thought if we could implement this by extending an existing
> Emacs command like flush-lines or occur to "teach" it about the
> specificities of the *xref* buffer, but that doesn't seem very clean.

...adding a command like xref-flush-groups shouldn't hurt. You can file 
a feature request with 'M-x report-emacs-bug' and perhaps even attach a 
patch?

> So perhaps we could add a new command that asks for a regexp in the
> minibuffer and filters the results in the *xref* buffer to only contain
> those items from a group that matches that regular expression.  With q
> you will go back to show the complete list of xref items and groups.

Hmm, if mean dynamically? That sounds a bit more complicated but hey, 
there can be some way to implement this without too much effort.

> Is
> there any core Emacs package that performs a similar filtering?  We
> could learn from its workflow, then.

swiper, perhaps?

Not a core package, but it is in GNU ELPA.

> Why matching the group exclusively? In my opinion, matching the group
> only is better because the typical use case I have in mind is a big
> codebase with thousands of xref results that are spread around hundreds
> of files.  You usually want to filter by the string that varies the
> most, which in most cases is the group (eg. if I only want to see
> results from unit tests, then I'd filter the *xref* buffer by
> "*Tests.cpp"; If I only want to see results from header files, I'd
> filter by "*.h").

Sounds reasonable. If you wanted to filter by summary contents anyway, 
you can just use swiper.