GNU bug report logs - #29619
Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier`

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29619 in the body.
You can then email your comments to 29619 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ray <emacsray <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Fwd: [xref.el] Add `xref-find-references` to
 `xref-prompt-for-identifier`
Date: Fri, 8 Dec 2017 13:27:58 -0800

Currently, xref-find-references prompts for the identifier to look up,
because it is not listed in xref-prompt-for-identifier:

(defcustom xref-prompt-for-identifier '(not xref-find-definitions
                                            xref-find-definitions-other-window
                                            xref-find-definitions-other-frame)

It will be much core convenient to add xref-find-references in the list.

Many language servers now support finding references. lsp-mode is a
clinet implementation for Emacs
and there is some work to make it into Emacs.
https://github.com/emacs-lsp/lsp-mode/issues/83

The typical usage of lsp-mode is to put point at some identifier and hit a
shortcut (by default M-?) to trigger xref-find-references. The prompt
is in many cases undesired.

Message #8 received at 29619 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Ray <emacsray <at> gmail.com>, 29619 <at> debbugs.gnu.org
Subject: Re: bug#29619: Fwd: [xref.el] Add `xref-find-references` to
 `xref-prompt-for-identifier`
Date: Sat, 9 Dec 2017 17:25:23 +0200

On 12/8/17 11:27 PM, Ray wrote:
> Currently, xref-find-references prompts for the identifier to look up,
> because it is not listed in xref-prompt-for-identifier:
> 
> (defcustom xref-prompt-for-identifier '(not xref-find-definitions
>                                              xref-find-definitions-other-window
>                                              xref-find-definitions-other-frame)
> 
> It will be much core convenient to add xref-find-references in the list.

We've discussed it before, and for now have settled on this list of 
non-prompting functions.

But thank you for your report. We may change the default if more people 
feel the same.

Message #11 received at 29619 <at> debbugs.gnu.org (full text, mbox):

From: Ray <emacsray <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 29619 <at> debbugs.gnu.org
Subject: Re: bug#29619: Fwd: [xref.el] Add `xref-find-references` to
 `xref-prompt-for-identifier`
Date: Sat, 9 Dec 2017 09:52:00 -0800

Thanks for being open to change the default if more people feel the same.

With the xref system people use drifting from tag based
(ctags,etags,GNU GLOBAL,cscope,...) tools to Language Server Protocol
(cquery, rls, ...),
a single identifier without position has become insufficient to
describe the one the user wants to look up. For example, a local
variable/struct/lambda `foo` may exist
in different functions.

I'm using a C++ language server called cquery. This is what I get (for
the argument `identifier`) when I hit the key bound to
`xref-find-definitions`:

#("QueryDatabase" 0 13 (fontified t ref-params (:textDocument (:uri
"file:///home/maskray/Dev/Util/cquery/src/query_utils.h") :position
(:line 11 :character 54) :context (:includeDeclaration :json-false))
def-params (:textDocument (:uri
"file:///home/maskray/Dev/Util/cquery/src/query_utils.h") :position
(:line 11 :character 54))))

Here the text properties are more useful than the identifier itself,
because LSP uses position instead of identifier to sending requests to
the language server.
https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#textdocumentpositionparams


This is my 2 cents. I am totally fine to live with the current default
`xref-prompt-for-identifier` because I can customize it.

On Sat, Dec 9, 2017 at 7:25 AM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> On 12/8/17 11:27 PM, Ray wrote:
>>
>> Currently, xref-find-references prompts for the identifier to look up,
>> because it is not listed in xref-prompt-for-identifier:
>>
>> (defcustom xref-prompt-for-identifier '(not xref-find-definitions
>>
>> xref-find-definitions-other-window
>>
>> xref-find-definitions-other-frame)
>>
>> It will be much core convenient to add xref-find-references in the list.
>
>
> We've discussed it before, and for now have settled on this list of
> non-prompting functions.
>
> But thank you for your report. We may change the default if more people feel
> the same.

Message #18 received at 29619 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Ray <emacsray <at> gmail.com>
Cc: 29619 <at> debbugs.gnu.org
Subject: Re: bug#29619: Fwd: [xref.el] Add `xref-find-references` to
 `xref-prompt-for-identifier`
Date: Sun, 10 Dec 2017 18:57:56 +0200

On 12/9/17 7:52 PM, Ray wrote:
> Thanks for being open to change the default if more people feel the same.
> 
> With the xref system people use drifting from tag based
> (ctags,etags,GNU GLOBAL,cscope,...) tools to Language Server Protocol
> (cquery, rls, ...),
> a single identifier without position has become insufficient to
> describe the one the user wants to look up. For example, a local
> variable/struct/lambda `foo` may exist
> in different functions.

That's fine, actually, and as designed. As long as the different global 
identifiers can be represented uniquely as strings (but using text 
properties for e.g. a local variable at point is good too).

> I'm using a C++ language server called cquery. This is what I get (for
> the argument `identifier`) when I hit the key bound to
> `xref-find-definitions`:
> 
> #("QueryDatabase" 0 13 (fontified t ref-params (:textDocument (:uri
> "file:///home/maskray/Dev/Util/cquery/src/query_utils.h") :position
> (:line 11 :character 54) :context (:includeDeclaration :json-false))
> def-params (:textDocument (:uri
> "file:///home/maskray/Dev/Util/cquery/src/query_utils.h") :position
> (:line 11 :character 54))))
> 
> Here the text properties are more useful than the identifier itself,
> because LSP uses position instead of identifier to sending requests to
> the language server.
> https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#textdocumentpositionparams

So it doesn't actually let you choose? Only supports the identifier at 
point?

Message #21 received at 29619 <at> debbugs.gnu.org (full text, mbox):

From: Fangrui Song <emacsray <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 29619 <at> debbugs.gnu.org
Subject: Re: bug#29619: Fwd: [xref.el] Add `xref-find-references` to
 `xref-prompt-for-identifier`
Date: Sun, 10 Dec 2017 23:18:54 -0800

On 2017-12-10, Dmitry Gutov wrote:
>On 12/9/17 7:52 PM, Ray wrote:
>>Thanks for being open to change the default if more people feel the same.
>>
>>With the xref system people use drifting from tag based
>>(ctags,etags,GNU GLOBAL,cscope,...) tools to Language Server Protocol
>>(cquery, rls, ...),
>>a single identifier without position has become insufficient to
>>describe the one the user wants to look up. For example, a local
>>variable/struct/lambda `foo` may exist
>>in different functions.
>
>That's fine, actually, and as designed. As long as the different 
>global identifiers can be represented uniquely as strings (but using 
>text properties for e.g. a local variable at point is good too).

>>I'm using a C++ language server called cquery. This is what I get (for
>>the argument `identifier`) when I hit the key bound to
>>`xref-find-definitions`:
>>
>>#("QueryDatabase" 0 13 (fontified t ref-params (:textDocument (:uri
>>"file:///home/maskray/Dev/Util/cquery/src/query_utils.h") :position
>>(:line 11 :character 54) :context (:includeDeclaration :json-false))
>>def-params (:textDocument (:uri
>>"file:///home/maskray/Dev/Util/cquery/src/query_utils.h") :position
>>(:line 11 :character 54))))
>>
>>Here the text properties are more useful than the identifier itself,
>>because LSP uses position instead of identifier to sending requests to
>>the language server.
>>https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#textdocumentpositionparams
>
>So it doesn't actually let you choose? Only supports the identifier at 
>point?

The identifier (if chosen from xref prompt) is ignored by Language
Server Protocol and only the position information is what matters.

According to https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#textDocument_references
When the user wants to find a reference of an identifier,
information of the following interface is sent to language servers.

   interface ReferenceParams extends TextDocumentPositionParams {
   	context: ReferenceContext
   }

TextDocumentPositionParams is the interesting one:

   interface TextDocumentPositionParams {
   	textDocument: TextDocumentIdentifier;   /// wrapper of filename
   	position: Position;  /// line, column; see, no identifier is used
   }

lsp-mode provides a backend of xref generic functions.
Here is how xref-find-references is implemented in lsp-mode:
https://github.com/emacs-lsp/lsp-mode/blob/master/lsp-methods.el#L1425

   (cl-defmethod xref-backend-references ((_backend (eql xref-lsp)) identifier)
     (let* ((properties (text-properties-at 0 identifier))
            (params (plist-get properties 'ref-params))
            (refs (lsp--send-request (lsp--make-request
                                      "textDocument/references"
                                      (or params (lsp--make-reference-params))))))
       (lsp--locations-to-xref-items refs)))

The `identifier` text itself is ignored and only the text properties
(which encode position information) are used.

Message #24 received at 29619 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Fangrui Song <emacsray <at> gmail.com>
Cc: 29619 <at> debbugs.gnu.org
Subject: Re: bug#29619: Fwd: [xref.el] Add `xref-find-references` to
 `xref-prompt-for-identifier`
Date: Mon, 11 Dec 2017 12:38:31 +0200

On 12/11/17 9:18 AM, Fangrui Song wrote:

> The identifier (if chosen from xref prompt) is ignored by Language
> Server Protocol and only the position information is what matters.

The value is not ignored. Only its text properties are used, but that's 
fine.

Looking at xref-backend-identifier-completion-table, it allows 
navigation to all (?) symbols in the "document", so the xref API seems 
to be used as intended.

Message #27 received at 29619 <at> debbugs.gnu.org (full text, mbox):

From: guillaume papin <guillaume.papin <at> epitech.eu>
To: "29619 <at> debbugs.gnu.org" <29619 <at> debbugs.gnu.org>, "dgutov <at> yandex.ru"
 <dgutov <at> yandex.ru>
Cc: "emacsray <at> gmail.com" <emacsray <at> gmail.com>
Subject: Re: bug#29619: Fwd: [xref.el] Add `xref-find-references` to
 `xref-prompt-for-identifier`
Date: Sun, 21 Jan 2018 22:20:24 +0000

On 2017/12/09 5:25 PM, Dmitry Gutov wrote:

> On 12/8/17 11:27 PM, Ray wrote:
> > Currently, xref-find-references prompts for the identifier to look up,
> > because it is not listed in xref-prompt-for-identifier:
> > 
> > (defcustom xref-prompt-for-identifier '(not xref-find-definitions
> >                                              xref-find-definitions-other-window
> >                                              xref-find-definitions-other-frame)
> > 
> > It will be much core convenient to add xref-find-references in the list.
> 
> We've discussed it before, and for now have settled on this list of 
> non-prompting functions.
> 
> But thank you for your report. We may change the default if more people 
> feel the same.

Just wanted to chime in to say that I feel the same as Ray.

The commands already have related keybindings, M-. and M-?.
I would find it more intuitive, if they also had similar behaviors.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.