Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lsp-csharp.el: add handler for the "omnisharp/client/findReferences" #1797

Merged

Conversation

razzmatazz
Copy link
Collaborator

@razzmatazz razzmatazz commented Jun 13, 2020

This will make it possible to use code lenses to find symbol references.

Previously only reference count was displayed but now it possible to invoke action on the references element too (via lsp-avy-lens)

The related PR on server side is:

lsp-csharp.el Outdated
(defun lsp-csharp--action-client-find-references (action)
"Reads first argument from ACTION as Location and displays xrefs for that location
using the `textDocument/references' request."
(if-let ((arguments (gethash "arguments" action))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you rewrite it using lsp-protocol.el bindings?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not entirely sure.. Can you point me to any example of the code that does similar thing?

I do need to define schema for this particular command using lsp-protocol.el, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFACS there are existing types - Command, Location should be sufficient. Check this:

https://github.com/emacs-lsp/lsp-mode/blob/plists/lsp-mode.el#L2316

Copy link
Collaborator Author

@razzmatazz razzmatazz Jun 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand your pointer to lsp--lens-create-interactive-command.. – that function returns a lambda that invokes a command with command and args..

But.. do you mean, I need to decorate (with a macro presumably?) lsp-csharp--action-client-find-references function so lsp-protocol.el could somehow parse the conents of action into subcomponents that i actually need (and that I currently extract within if-let expression?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be something like this

Suggested change
(if-let ((arguments (gethash "arguments" action))
(-if-let* (((&Command: arguments?) action))

Or you can even change the defun to lsp-defun and

(lsp-defun lsp-csharp--action-client-find-references ((&Command :arguments?))

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the code using -if-let* / destructuring. Sorry for the long delay

@razzmatazz razzmatazz force-pushed the lsp-csharp-find-refs-client-command branch from 1175240 to 165cd3f Compare June 24, 2020 15:47
…command

This will make it possible to use lenses to find symbol references (it
was display-only previously)
@razzmatazz razzmatazz force-pushed the lsp-csharp-find-refs-client-command branch from 165cd3f to e4d613b Compare June 25, 2020 20:16
@github-actions github-actions bot added the client One or more of lsp-mode language clients label Sep 19, 2020
@yyoncho
Copy link
Member

yyoncho commented Sep 19, 2020

clients/lsp-csharp.el:238:1:Error: Unused lexical variable ‘command’

There is one warning which is breaking the CI, can you fix it?

@razzmatazz
Copy link
Collaborator Author

clients/lsp-csharp.el:238:1:Error: Unused lexical variable ‘command’

There is one warning which is breaking the CI, can you fix it?

Done

@yyoncho yyoncho merged commit c304c9d into emacs-lsp:master Sep 19, 2020
@yyoncho
Copy link
Member

yyoncho commented Sep 19, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client One or more of lsp-mode language clients
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants