From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Spencer Baugh Newsgroups: gmane.emacs.devel Subject: Re: Adding support for xref jumping to headers/interfaces Date: Tue, 07 Nov 2023 11:51:49 -0500 Message-ID: References: <83pm9sfxfa.fsf@gnu.org> <861qm4tkih.fsf@stephe-leake.org> <71ea5e83-183f-2ae3-8146-6a31045a0309@yandex.ru> <834jqzafse.fsf@gnu.org> <83h6uv47e8.fsf@gnu.org> <4639d7ca-2109-864c-33c0-38e65f26f262@yandex.ru> <835ybb3txt.fsf@gnu.org> <83wn3q311i.fsf@gnu.org> <412afa2d-5dbc-52da-39c4-99be3873929c@yandex.ru> <83o7p20wdi.fsf@gnu.org> <72b09256-5a1b-8962-9e3c-7d2ffd0dc0d7@yandex.ru> <83ilf925n8.fsf@gnu.org> <95afa441-18ae-e62a-be16-be73a545bbba@yandex.ru> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="23488"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) To: emacs-devel@gnu.org Cancel-Lock: sha1:5r7KfHD62EnVE3p6sQ2v8Gsp72U= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Nov 07 20:26:34 2023 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1r0Rib-0005n9-Lh for ged-emacs-devel@m.gmane-mx.org; Tue, 07 Nov 2023 20:26:33 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1r0Rhp-0003nI-96; Tue, 07 Nov 2023 14:25:45 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r0PJ4-0003HL-Hj for emacs-devel@gnu.org; Tue, 07 Nov 2023 11:52:03 -0500 Original-Received: from ciao.gmane.io ([116.202.254.214]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r0PJ2-0004PC-H7 for emacs-devel@gnu.org; Tue, 07 Nov 2023 11:52:02 -0500 Original-Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1r0PJ0-0003d5-0o for emacs-devel@gnu.org; Tue, 07 Nov 2023 17:51:58 +0100 X-Injected-Via-Gmane: http://gmane.org/ Received-SPF: pass client-ip=116.202.254.214; envelope-from=ged-emacs-devel@m.gmane-mx.org; helo=ciao.gmane.io X-Spam_score_int: -16 X-Spam_score: -1.7 X-Spam_bar: - X-Spam_report: (-1.7 / 5.0 requ) BAYES_00=-1.9, HEADER_FROM_DIFFERENT_DOMAINS=0.25, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=no autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Tue, 07 Nov 2023 14:25:44 -0500 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:312320 Archived-At: Dmitry Gutov 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).