Re: Adding support for xref jumping to headers/interfaces

[Dmitry Gutov]

On 07/11/2023 11:36, João Távora wrote:
> [Adding back emacs-devel, which I didn't mean to remove from my previous
> email.  This is also why I quote your message fully.  ]

Perfect.

> > > It wasn't very difficult, but I did have to scratch my head a bit until
> > > I found what I think is the key.  So first I did a reasonably simple
> > > commit to xref.el, which gives xref-find-extra a KIND argument, making
> > > it more versatile since it can now be used non-interactively.  I think
> > > the interactive behavior is completely unchanged.  The docstring might
> > > need a tweak or two.
> >
> > There is an omission in being able to input the identifier (fixable, of
> > course).
>
> Can you explain exactly the problem?  I'm happy to fix it.

Just that it does not ask for the identifier (with C-u or without); that 
it cannot. But if that won't be useful anyway (as you state below), 
there's no point in trying.

> > Though I'm not sure if that capability will be helpful for
> > those searches with an LSP client (with language servers providing
> > relatively limited identifier completion).
>
> Indeed, it's not.  For every reference kind _except_ the "definition"
> kind, where it does come in handy, since I have a hack for that
> too.  Indeed very handy (and very hackish, but that's life).

So... there's no way to take a "definition" symbol name (and associated 
metadata) and jump to the "declaration", for example? Using that hack of 
yours, or with a slight tweak.

> > I still note, though, that you kept those commands around:
> > eglot-find-declaration/implementation/typeDefinition. Whereas previously
> > I seem to recall you opined that the commands themselves shouldn't be in
> > Eglot.
> >
> > Would you say it's a backward compatibility concern, or something else
> > as well?
>
> The former.  IOW if xref.el had this capability in the past, I would have
> used it.
>
> But I still nonetheless think functions should be as versatile as possible,
> so that even what we designed as an interactive entry-point can be used
> as a library entry point.

Well, making those commands easy to write is a small task. We already 
have 'xref-show-xrefs' which you could call, and if my thinking is right 
(the bottom of the previous email), a new function called 
'xref-show-defs' which internally uses xref-show-definitions-function, 
would be just what the doctor ordered. No new backend methods necessary.

> > > Anyway, this new xref-find-extra aided greatly in adding Eglot support.
> > > It removes the need for a very hackish implementation of the existing
> > > 'eglot-find-declaration', 'eglot-find-implementation' and
> > > 'eglot-find-typeDefinition' commands.  And of course 'xref-find-extra'
> > > as an interactive entry point also works for finding these things.
> >
> > Whatever solution we decide to go with in the end, it should be
> > straightforward enough to create a helper that would assist in defining
> > such commands.
>
> Yes.  I do think this one is pretty good at doing that.  But I guess
> even with this one, you could offer a 'xref-define-interactive-finder-for-kind'
> macro that would automate it even further and help xref.el retain finer
> control over how exactly that finder behaves, for example vis-a-vis
> prefix arguments.

Right. Though if the backend cannot work on arbitrary symbol names (only 
on symbol at point), that kind of negates most of the benefits.

> > > One thing that is missing IMO, but is reasonably easy to fix, is an
> > > indirection to allow non-string objects to be used as KIND, but still
> > > have these objects be displayed to the user with a pretty string.  Of
> > > course, this is not essential -- I could just use strings as KINDs in
> > > eglot.el and be done with it -- but I feel it'd be a little nicer.
> > > Right now I'm using the symbols 'eglot--xref-implementation',
> > > 'eglot--xref-declaration', etc, which are printed as is to the user in
> > > the completing-read of 'xref-find-extra'.  Works for now.
> >
> > Using strings as KINDs is what allows us to use completing-read on them.
> > Internally, there could be of course some alist mapping them to complex
> > objects. If you have a better idea, I'm open.
>
> Yes, that indirection is what is needed.  It could be done via symbol
> properties (in which case KIND would be a symbol or a string), via
> a specific prescription that KIND can be (OBJ . DISPLAY), a specific
> prescription like (OBJ . DISPLAY-FN-TO-BE-CALLED-ON-OBJ) or just a
> generic function whose default implementation is #'identity thus
> letting completing read do whatever it does for that OBJ, which in
> the case of symbols is convert them to string and return that string).
> Very many ways to skin this cat, so just pick one.

I suppose xref-backend-definition-kinds could return any data structure 
that completing-read could use? A.g. an alist.

> But just to be clear: whatever the technique used, that "mapping" you
> speak of (which I call "indirection"), could _not_ be "internal".  It
> would have to be exposed so the backend can control how the OBJs it
> returns from 'xref-backend-extra-kinds' are converted to strings.

Hm, why not? You set up an internal var with the mapping, return a list 
of strings, get one of those stings, look up the "complex" var in the 
mapping, and use it. Text properties are a good option too, but IIRC 
there was a problem with completing-read stripping them.

> > > Another idea I had while doing this is that the name xref-find-extra
> > > could be actually something like xref-find-by-kind.  What I mean is that
> > > I found myself adding the existing kinds "definition" and "references"
> > > to that "extra" set.  I think this is more comfortable for the user that
> > > even though they are not "extras".
> >
> > This is one of the questions I'd like to see answered:
> >
> > - Should a command like this include existing kinds (that
> > xref-find-definitions already lists)?
>
> My answer is yes.  But we could leave that up to the backend.

We would ultimately leave it all to the backend, of course, but we 
should provide some guidelines. And these choices should inform the 
commands we add and how we organize them.

> I
> intend to remap M-?  to xref-find-extra myself, losing the speed of
> xref-find-references for "all references", but gaining in versatility.
> Of course a similar effect could be achieved via some kind of prefix
> argument or variable controlling the behaviour of xref-find-references.
>
> Another thing that I think should be changed is the name of the generic
> function xref-backend-extra-defs.  It should be IMO xref-backend-extra-refs
> because a "definition" is a type of "reference" to a symbol/name.  Likewise
> to mentions of "definition" in the new docstrings.

We use xref-show-xrefs-function for "references" and 
xref-show-definitions-function for "definitions". By default they differ 
in what happens if there is just 1 match (the latter just jumps to it). 
But one can also customize xref-show-definitions-function to 
xref-show-definitions-completing-read (which I did), and then you get a 
direct jump (with completion) for definitions, and a list of all matches 
for references. A definition could be called a subtype of a reference, 
but they often carry different information in practice as well.

Perhaps we could add two commands: xref-find-defs-by-kind and 
xref-find-xrefs-by-kind. I wonder which "kinds" would get sorted in the 
second category along with "references", though.

> > - Should there be other kinds in it, absent from xref-find-definitions?
> > If yes, of kinds of "kinds" (:-)) should be in one set but not the other?
>
> Up to the backend.
>
> > - Should xref-find-definitions include the results of
> > eglot-find-declaration and eglot-find-typeDefinition? Does it include
> > them already?
>
> That's up to the backend, Eglot in this case.  And the Eglot backend
> leaves that up to the LSP server, so it's a question that can't be answered.

Eglot could make several queries and append the results. It might be 
slower in certain cases, but I'm not sure it will be slow enough for the 
users to take notice. In most cases, anyway.

That might lead to a better UX as well. For example, what kind of 
symbols does eglot-find-typeDefinition work on? Type references? If you 
invoke xref-find-definitions instead, does it find any other kind of 
definitions for them?

Speaking of guidelines and semantics again:

1. If we were to expect that "xref-find-definitions" finds all kinds of 
definitions, we could instead just add a new method to xref-item to 
indicate kind. And then filter. Theoretically less efficient, but if 
xref-find-definitions will remain the main tool for people, saving on 
work in rare situations might not be optimal.

2. If we say that xref-find-extra includes kinds that are already in 
"definitions", then the name "extra" is invalid, and 
xref-find-...-by-kind seems more apt as the name of a command. If it 
only included "extra" kinds, OTOH, the names would hold.

3. If xref-find-...-by-kind allows you to find both kinds included in 
the set of "definitions" and not, that becomes the most awkward option 
to describe, in the docstrings and the manual. In any case, we'd have to 
use the most careful naming.