unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier`
       [not found] <CAN30aBF0FcQpH+DDxR5ggWgwYHPn41nzdgNdUXSt2j0mXY39-A@mail.gmail.com>
@ 2017-12-08 21:27 ` Ray
  2017-12-09 15:25   ` Dmitry Gutov
  2018-01-21 22:20   ` guillaume papin
  0 siblings, 2 replies; 7+ messages in thread
From: Ray @ 2017-12-08 21:27 UTC (permalink / raw)
  To: 29619

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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier`
  2017-12-08 21:27 ` bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier` Ray
@ 2017-12-09 15:25   ` Dmitry Gutov
  2017-12-09 17:52     ` Ray
  2018-01-21 22:20   ` guillaume papin
  1 sibling, 1 reply; 7+ messages in thread
From: Dmitry Gutov @ 2017-12-09 15:25 UTC (permalink / raw)
  To: Ray, 29619

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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier`
  2017-12-09 15:25   ` Dmitry Gutov
@ 2017-12-09 17:52     ` Ray
  2017-12-10 16:57       ` Dmitry Gutov
  0 siblings, 1 reply; 7+ messages in thread
From: Ray @ 2017-12-09 17:52 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 29619

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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier`
  2017-12-09 17:52     ` Ray
@ 2017-12-10 16:57       ` Dmitry Gutov
  2017-12-11  7:18         ` Fangrui Song
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Gutov @ 2017-12-10 16:57 UTC (permalink / raw)
  To: Ray; +Cc: 29619

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?





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier`
  2017-12-10 16:57       ` Dmitry Gutov
@ 2017-12-11  7:18         ` Fangrui Song
  2017-12-11 10:38           ` Dmitry Gutov
  0 siblings, 1 reply; 7+ messages in thread
From: Fangrui Song @ 2017-12-11  7:18 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 29619


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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier`
  2017-12-11  7:18         ` Fangrui Song
@ 2017-12-11 10:38           ` Dmitry Gutov
  0 siblings, 0 replies; 7+ messages in thread
From: Dmitry Gutov @ 2017-12-11 10:38 UTC (permalink / raw)
  To: Fangrui Song; +Cc: 29619

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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier`
  2017-12-08 21:27 ` bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier` Ray
  2017-12-09 15:25   ` Dmitry Gutov
@ 2018-01-21 22:20   ` guillaume papin
  1 sibling, 0 replies; 7+ messages in thread
From: guillaume papin @ 2018-01-21 22:20 UTC (permalink / raw)
  To: 29619@debbugs.gnu.org, dgutov@yandex.ru; +Cc: emacsray@gmail.com

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.




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-01-21 22:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CAN30aBF0FcQpH+DDxR5ggWgwYHPn41nzdgNdUXSt2j0mXY39-A@mail.gmail.com>
2017-12-08 21:27 ` bug#29619: Fwd: [xref.el] Add `xref-find-references` to `xref-prompt-for-identifier` Ray
2017-12-09 15:25   ` Dmitry Gutov
2017-12-09 17:52     ` Ray
2017-12-10 16:57       ` Dmitry Gutov
2017-12-11  7:18         ` Fangrui Song
2017-12-11 10:38           ` Dmitry Gutov
2018-01-21 22:20   ` guillaume papin

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).