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

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

[Ray]

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.

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

[Dmitry Gutov]

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.

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

[Ray]

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

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

[Dmitry Gutov]

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?

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

[Fangrui Song]

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.

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

[Dmitry Gutov]

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.

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

[guillaume papin]

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.