emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Adding support for xref jumping to headers/interfaces


From: Dmitry Gutov
Subject: Re: Adding support for xref jumping to headers/interfaces
Date: Mon, 27 Nov 2023 17:17:51 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0

On 26/11/2023 22:30, João Távora wrote:
Some major modes will support things which other major modes do not
support.  Users will use the best one, whatever it is, and we'll change
the default to whatever it is, eventually.  I don't see an issue.

I don't see why we have to limit the users to specific major mode, when
one might have functionality others don't have, and vice versa.

It's unclear what you are understanding of Spencer's suggestion, which
is also mine.

I hope it's clear that in mine and Spencer's view if there is
support for a given finding a given thing of kind foo for a given
language, then there is an easy command for doing so.  And if there
isn't that foo, there isn't a command for doing so.  This is the way
it should be.

In my view, again, we should not limit ourselves to a triad
of kinds clumsily inherited from LSP.  We should limit that LSP
concept to Eglot.  It is not flexible enough, because it can't
be.  But Xref is for much more than LSP.

What it seems you are saying is that we never should try to extend the set of definition-finding commands above what's already in xref.el.

Because it seems clear that we're unlikely to find a more widely-supported set of additional kinds that the aforementioned triad, in any medium-term future.

Like now, for example, many will likely stay with CC Mode for a while
because of it more lax behavior in macro-heavy codebases, where
c-ts-mode chokes and shows syntax errors.

Whatever you are suggesting, I don't understand.  Whatever xref backend
is written for c++-mode with its special-purpose commands for finding
C++ things can be used for both c++-mode and c++-ts-mode.  Just put that
code in a special file.

What about default key bindings?

Anyway, I've pushed an update to the same branch
(feature/xref-find-extra). Again, it's something to try out, not a done
deal:

* The command is called xref-find-all-definitions, with appropriate
behavior when invoked without prefix argument (appends the results from
all kinds and shows them together, with duplicates removed).

I think this is a mistake and a regression from xref-find-extra.
The command should be called xref-find-all, xref-find, or simply
xref, since definition is a category that doesn't span a lot of
useful cross-referenceable constructs in many languages. In C++,
for one, a declaration is absolutely _not_ a definition.

It's a "definition" in at least one Xref-related sense: it should be dispatched through xref-show-definitions-function.

Further, I'm not sure that when a user looks up all definition-related things for a symbol, they wouldn't want to see the "declaration". In fact, if we classify 'defgeneric' as declarations and 'defmethod' as implementations, I'm pretty sure I would want to see the former in the list.

And you yourself mentioned that "type definitions" might be suitable for that list (which I found surprising at first). So it seems clear that there is no single red line.

* The Eglot implementation doesn't include 'references' in the list of
kinds anymore, just because those are not definitions.

A mistake, but at least I can roll it back easily.
I would like the new interactive "find all" command (whatever you decide)
to show me "references" as one of the kinds of thing to search for.

Then the list of results would drown in "references", wouldn't it?

And its output would (or should) be the same as 'xref-find-references'. That would make it fairly pointless, I believe.

Why are you rolling back this useful stuff? Are you even testing?  Because
the current branch doesn't even work in Eglot.

Sorry, I didn't test the Eglot-specific commands. But their definitions should be 100% the same as the new Xref ones.

* New commands for the 3 popular kinds, bound to "M-' e", "M-' i" and
"M-'t". The command which shows all moved to "M-' M-'".

A mistake, again.  I wonder why you are doing these opinionated
changes to the branch you yourself proposed and which I put work
into.

For experimentation.

If you are proposing this is brought to master for experimentation,
and if you are wary of this step, then may I suggest we push the older
more conservative version, perhaps with some naming changes?

I'm not yet seeing a common basic version for which there would be agreement. You just called a renaming a mistake and disagreed with the principle of what "definition kinds" should be.

This new version isn't something we can roll back easily, while the
more conservative approach could eventually be enhanced with the three
"popular kinds" later on.

I'm not proposing any merge to master yet.

What does everyone think?

xref-find-all-definitions is an interactive native-compiled Lisp
function in `xref.el'.

at point, prompt for the identifier.  Interactively, show matches
for all supported kinds.  When invoked with prefix argument,
prompt for KIND.

I can't get this to work btw.   If I add a prefx argument, it
prompts me for the identifier, which I don't want.

What's what the docstring says, and it's what Felician brought up as a problem. Do you want two separate commands? And one of them would always prompt for the kind to use?

So will you please roll back some of your changes so we can
at least get something working again?

Everything is "working" there, except for Eglot-specific commands, which are trivial to fix. I just did that, you can test.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]