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

clients/lsp-csharp.el: Implement go-to definition in metadata. #2573

Closed

Conversation

razzmatazz
Copy link
Collaborator

This commit adds a handler for 'osmd://' uri when returned from
omnisharp-roslyn, parses the uri and issues corresponding 'o#/metadata'
request that is used to retrieve metadata source to display to the user.

The design is modelled after clients/lsp-clojure.el and lsp-java. I am not 100% about the directory: currently I save metadata .cs files under <project-dir>/.cache/lsp-csharp/metadata -- but maybe that needs to be a customizable variable or some other dir?

This PR is marked as draft as there is some changes pending to be merged into omnisharp-roslyn yet but open for review.

@github-actions github-actions bot added the client One or more of lsp-mode language clients label Jan 31, 2021
@razzmatazz
Copy link
Collaborator Author

This commit adds a handler for 'omnisharp:/' uri when returned from
omnisharp-roslyn, parses the uri and issues corresponding 'o#/metadata'
request that is used to retrieve metadata source to display to the user.
@razzmatazz
Copy link
Collaborator Author

FYI @CsBigDataHub I have rebased this PR on the latest lsp-mode/master commit

@CsBigDataHub
Copy link
Contributor

@razzmatazz Tested this PR and it works as intended.

I see few compilation warnings.

In lsp-csharp--omnisharp-metadata-uri-handler:
lsp-csharp.el:365:46: Warning: attempt to let-bind constant ‘:source-name’
lsp-csharp.el:365:59: Warning: attempt to let-bind constant ‘:source’
lsp-csharp.el:384:17: Warning: reference to free variable ‘source’

In end of data:
lsp-csharp.el:361:31: Warning: the function

again thanks for your work.

@jcs090218
Copy link
Member

@razzmatazz Any progress on this? Can't wait to have this feature! 😄

@jcs090218
Copy link
Member

jcs090218 commented Jul 3, 2021

The design is modelled after clients/lsp-clojure.el and lsp-java. I am not 100% about the directory: currently I save metadata .cs files under /.cache/lsp-csharp/metadata -- but maybe that needs to be a customizable variable or some other dir?

I think you might want to create a temporary buffer to display? Maybe

(with-current-buffer (get-buffer-create "*temp Metadata source name*")
  ;; Insert content here, etc.
  )

This way, it won't generate any file to user's disk.

@razzmatazz
Copy link
Collaborator Author

The design is modelled after clients/lsp-clojure.el and lsp-java. I am not 100% about the directory: currently I save metadata .cs files under /.cache/lsp-csharp/metadata -- but maybe that needs to be a customizable variable or some other dir?

I think you might want to create a temporary buffer to display? Maybe

(with-current-buffer (get-buffer-create "*temp Metadata source name*")
  ;; Insert content here, etc.
  )

This way, it won't generate any file to user's disk.

Hey @jcs090218. I remember temp buffers were not working for some reason, I think transitive ref find with xref was not working when buffer had no backing file.

@razzmatazz
Copy link
Collaborator Author

razzmatazz commented Jul 3, 2021

@razzmatazz Any progress on this? Can't wait to have this feature! 😄

It’s all on omnisharp-roslyn team. They don’t seem keen on merging my PR so this is in limbo :/

you may want to vouch on that PR by commenting, so maintainers would show it some attention:

@jcs090218
Copy link
Member

I remember temp buffers were not working for some reason, I think transitive ref find with xref was not working when buffer had no backing file.

Just to clarify, the temporary buffer refers to just a regular buffer but with invalid character (*) so it cannot be saved to disk. So you could create a buffer with name * [metadata] Source.cs * using the function get-buffer-create if it has not created.

you may want to vouch on that PR by commenting, so maintainers would show it some attention:

Definitely! Thanks for your hard work! 😄 👍

@razzmatazz
Copy link
Collaborator Author

cancelling in favor of #3096

sorry, omnisharp-roslyn is relucant to accept my patches and hacking my own LSP server is more fun :D

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.

None yet

3 participants