Re: Adding support for xref jumping to headers/interfaces

[Spencer Baugh <sbaugh@janestreet.com>]

Dmitry Gutov <dgutov@yandex.ru> writes:
> On 17/05/2023 00:10, Spencer Baugh wrote:
>> An option I didn't mention originally: Perhaps xref-find-definitions
>> could just show both implementation and interface.  That would remove
>> the need for any new keybindings.  The UI might be worse but perhaps
>> it could be improved.
>> This is inspired by this comment:
>> https://github.com/joaotavora/eglot/issues/302#issuecomment-534202561
>> The relevant excerpt:
>>> By the way, on a tangent, I think it's a mistake on LSP's part to
>>> import C++/Java's ecosystem of possible symbol loci. In Common Lisp,
>>> there are many different types of construct associated with a symbol
>>> and they are all "definitions". A good IDE like SLIME will use
>>> something akin to xref-find-definitions (indeed xref is modelled after
>>> SLIME) and categorize them properly in the result buffer if there is
>>> more than one. So the command should have been textDocument/definition
>>> with some kind of subtype parameter, or at least this is where xref
>>> should evolve to, i.e. xref should keep the single command
>>> xref-find-definitions and maybe augment it with some "definition-type"
>>> parameter.
>
> Perhaps it /should/ (xref-find-definitions to include both
> implementation and interface). And include the typeDefinition entries
> for langs that have those. Etc.
>
> Would the UI be worse? Let's try to find out.
>
> Attached is the patch along the lines that we discussed: a new command
> xref-find-extra currently bound to M-' just for the sake of this
> experiment (eliminating the need to set up define-key to try out the
> "fast" workflow). It uses the symbol at point or asks for it, then
> polls the backend for available kinds, does completing-read and asks
> the backend for definitions of that kind. And shows the list or jumps
> to the unique one.
>
> To have something to test, I implemented this for Elisp. However, all
> known kinds are already printed by the regular xref-find-definitions
> output in that backend. So the result turned out interesting, but a
> little impractical: with Elisp, we already make the choice between the
> kinds in the Xref output buffer, when that buffer is shown at all (as
> Eli pointed out, actually). So... we could proceed in that direction
> and recommend that all kinds of definitions would be added there.

This (and Joao's patch adding support for eglot) is very interesting,
thank you for implementing it!

We also discussed a UI which shows all kinds of definitions in a single
buffer.  Maybe the prompt for KIND should default to "all" which shows
all kinds of definitions?

I notice the API doesn't support "fetch all kinds of definitions";
extra-defs requires specifying a specific kind.  Perhaps just specifying
a nil KIND should request all kinds, and extra-defs should tag each
returned object with the kind?  We could also just make a separate call
to extra-defs for each kind returned by extra-kinds, but that would be
allow less space for the backend to batch its operations.

> But let's try to answer the question: which workflows would the
> attached approach work better for? I could give one example: in Java,
> you sometimes (often?) want to find the locations where the current
> definition near point is overridden by subclasses. We could call it
> "find overrides", for example. But it does not just filter by kind
> (e.g. method overrides), it also filters by class hierarchy,
> i.e. showing only definitions in the subclasses of the current class,
> or interface.
>
> Examples of such searches:
>
> https://stackoverflow.com/questions/11849631/how-to-find-the-overridden-method-in-eclipse
>   https://stackoverflow.com/questions/1143267/finding-overriding-methods
>
> Is this still something people care about? Does jdt-ls even support this?
>
> Or perhaps the main value would be in actually having separate
> commands which could be bound to a submap with faster key sequences
> and/or to the context menu (with context-menu-mode on). Then our
> solution should be more in line with either defining a number of
> additional named commands (for mode universal kinds) and/or making it
> easier to define new such commands for users/3rd-party packages/etc.

That's an interesting idea.  So maybe M-' (or whatever) could be a new
prefix for a prefix-map which contains both universal and mode-specific
kinds of lookups.

So maybe something which looks kinda like (not suggesting final
bindings, just trying to feel out what it would be like):

generic eglot-find-implementation:   M-' i
generic eglot-find-declaration:      M-' d
generic eglot-find-typeDefinition:   M-' t
xref-extra-kind, prompting for kind: M-' M-'
xref-extra-kind, showing all:        M-' a

And if there was something mode-specific, like the Java overriding
method thing, it could be e.g. M-' o

I like this sort of design a lot actually:

- it's faster to type than having to complete the kind name (IMO, having
  to complete the kind name makes the current patch quite clumsy)
- it allows a common set of keys between all modes
- it's extensible by individual modes without too much trouble
- it works naturally with context-menu-mode and other menus
  (although I suppose we could automatically populate the context-menu
  with the output from xref-backend-extra-kinds)
- users could still use completing-read to type the kind

Plus, if we do use M-' or any other short binding for this, we should
almost certainly make it the start of a new prefix-map rather than bind
M-' directly to the new command; doing otherwise would be wasteful of
valuable keybinding space.

> Please try out the attached (with implementation for Eglot hopefully
> coming soon) and let me know your thoughts (you all).