Re: Adding support for xref jumping to headers/interfaces

["João Távora" <joaotavora@gmail.com>]

[Adding back emacs-devel, which I didn't mean to remove from my previous
email.  This is also why I quote your message fully.  ]

On Tue, Nov 7, 2023 at 2:31 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
> Hi Joao,
>
> On 06/11/2023 02:22, João Távora wrote:
>
> > Alright, so I added Eglot support for the new xref-find-extra feature of
> > Xref.  I pushed two commits to that branch, feature/xref-find-extra.
>
> I'm happy to see the implementation was short enough. Hopefully that
> will make it easier to experiment, but also potentially pivot to
> similar/different solutions, abandoning most of the code, too.

Sure.

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

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

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

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

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

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.

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

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

> > Finally, I really like the M-' binding.  It'd be awesome if we could
> > somehow make it stick (who even uses 'abbrev-prefix-mark'), but I don't
> > have high hopes for it?
>
> Concur on all three points. I've never even heard of
> 'abbrev-prefix-mark' before, but it might be a difficult discussion anyway.
>
> > Or maybe xref-find-references could lead to xref-find-extra's menu when
> > a certain flag variable is set.  Then the M-? binding could stay.  Of
> > course this makes more sense if the current "references" kind is also
> > part of the choices offered in that menu.
>
> Actually, I imagined that a distinction between "find definition" and
> "find references" will remain. And so "xref-find-references" would not
> be one of the "extras" available, if only because it seems to make sense
> to use xref-show-xrefs-function for one and
> xref-show-definitions-function for the other (sometimes they are minorly
> different; sometimes there values do really different things).
>
> All this is still negotiable, of course.
>
> (Please feel free to reply Cc'ing emacs-devel, these are high-level
> details hopefully deserving public discussion.)

Yes, I forgot to add it in the first mail.

João