LSP: Return custom schema (osmd:/) URIs for go to definition in LSP

[razzmatazz]

This enables us to implement to parse the URI and to invoke o#/metadata
in LSP client to retrieve metadata source.

I think we might need to expose a capability for this via LSP mechanism in order
not to break anything for clients that know nothing about osmd:/ URIs but I am
not sure how to do that. Maybe @david-driscoll can chime in?

Also I am not 100% sure about osmd:/ URI schema, maybe the path should be formed differently?

Also Helpers.cs:FromUri(DocumentUri uri) has custom code that builds "metadata filename" as does SymbolExtensions.cs:GetFilePathForExternalSymbol but that code is internal.

https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Roslyn/Extensions/SymbolExtensions.cs#L293-L297

[razzmatazz]

You can take a look at corresponding emacs LSP client implementation for this feature here (should be fairly readable)

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

[filipw]

Thanks a lot, did you test this with decompilation enabled?
since decompilation is a feature toggle to replace metadata, it should work out of the box with this change too.

[filipw]

Also Helpers.cs:FromUri(DocumentUri uri) has custom code that builds "metadata filename" as does SymbolExtensions.cs:GetFilePathForExternalSymbol but that code is internal.

you can can InternalsVisibleTo for OmniSharp.LanguageServerProtocol to OmniSharp.Roslyn

[razzmatazz]

Thanks a lot, did you test this with decompilation enabled?
since decompilation is a feature toggle to replace metadata, it should work out of the box with this change too.

Decompilation works too! Actually I never tried it myself, cool. You can jump from metadata file to metadata file transitively as well:

[razzmatazz]

Also Helpers.cs:FromUri(DocumentUri uri) has custom code that builds "metadata filename" as does SymbolExtensions.cs:GetFilePathForExternalSymbol but that code is internal.

you can can InternalsVisibleTo for OmniSharp.LanguageServerProtocol to OmniSharp.Roslyn

That sounds a little bit dirty to me.. :) and I wonder if the nicer thing would be to lift the of "external symbol file path" abstraction into a separate class or just mark the method public? But yeah, I can add the InternalsVisibleTo attribute, -- that should get the job done.

[razzmatazz]

Also Helpers.cs:FromUri(DocumentUri uri) has custom code that builds "metadata filename" as does SymbolExtensions.cs:GetFilePathForExternalSymbol but that code is internal.

you can can InternalsVisibleTo for OmniSharp.LanguageServerProtocol to OmniSharp.Roslyn

That sounds a little bit dirty to me.. :) and I wonder if the nicer thing would be to lift the of "external symbol file path" abstraction into a separate class or just mark the method public? But yeah, I can add the InternalsVisibleTo attribute, -- that should get the job done.

I have updated the commit by adding a small note here as a comment w.r.t. $metadata$ filename schema. Somehow I didn't feel to be authoritative to do any of the larger scale changes.

[david-driscoll]

Little bit of feedback.

Also a thing of note: I'm looking at creating an extension for the lsp spec to allow for file metadata, to try and standardize the process. Hence the scheme name change to just omnisharp.

My thinking is that we'll be able to advertise a capability that this language server hosts a given "scheme" (ie omnisharp) and have that wired into editors like vscode automagically. And that provider would be called to get the content instead of the usual location like the disk, etc.

[razzmatazz]

Is there something I can do to advance this PR towards a merge? Or is the approach still in limbo for some reason, or maybe because VS Code guys might decide to implement metadata loading over LSP in a different way?

[razzmatazz]

I have added tests for the LSP.OmniSharpDefinitionHandler + go to definition in metadata: 6fa8458

[CsBigDataHub]

@razzmatazz ,

I have cloned your fork

• git clone https://github.com/razzmatazz/omnisharp-roslyn.git
• git checkout -b lsp-expose-metadata-uri-on-definition origin/lsp-expose-metadata-uri-on-definition
• ./build.sh --publish-all --archive
• unzipped omnisharp-osx.zip from artifacts/packages
• set lsp-csharp-omnisharp-roslyn-server-dir to newly unzipped path

yet I am still getting this error when I run lsp-goto-type-definition on Ilogger.LogInformation with cursor on LogInformation.

lsp-describe-session

 `-[-] csharp:59758
    |-[-] Buffers
    |  `-[+] ProxyFunction.cs
    `-[-] Capabilities
       |-[-] workspace:
       |  |-[X] fileOperations:
       |  `-[-] workspaceFolders:
       |     |-[X] changeNotifications: t
       |     `-[X] supported: t
       |-[X] implementationProvider:
       |-[X] experimental:
       |-[-] executeCommandProvider:
       |  `-[+] commands: [omnisharp/executeCodeAction omnisharp/executeCodeAction]
       |-[-] documentOnTypeFormattingProvider:
       |  |-[X] moreTriggerCharacter: [} )]
       |  `-[X] firstTriggerCharacter: ;
       |-[-] codeLensProvider:
       |  `-[X] resolveProvider: t
       |-[X] workspaceSymbolProvider:
       |-[X] documentSymbolProvider:
       |-[X] referencesProvider:
       |-[X] definitionProvider:
       |-[-] signatureHelpProvider:
       |  `-[X] triggerCharacters: [. ? []
       |-[-] completionProvider:
       |  |-[X] triggerCharacters: [.  ]
       |  `-[X] resolveProvider: t
       |-[X] hoverProvider:
       `-[-] textDocumentSync:
          |-[-] save:
          |  `-[X] includeText: t
          |-[X] change: 1
          `-[X] openClose: t

Let me know if you need something else or if I could have done anything different!

[razzmatazz]

@CsBigDataHub you also need to use updated lsp-mode that can read resulting refs to code in dll:

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

Not sure if you noticed that -- I did not merge lsp-mode PR since this PR on omnisharp-roslyn is still in limbo.

[CsBigDataHub]

@razzmatazz Is it possible for you to pull the latest changes from lsp-mode master branch into razzmatazz:lsp-expose-metadata-uri-on-definition. Looks like the code is little outdated and triggering errors.

[CsBigDataHub]

@filipw @david-driscoll

I have tested this PR and can confirm that it works well with emacs lsp-mode. please refer to my notes in #2079 (comment), emacs-lsp/lsp-mode#2573 (comment).

This is a handy feature and would like to see it merged. Thanks for maintaining omnisharp.

@razzmatazz Thanks for this feature.

[jcs090218]

Any progress on this? I was hoping this can be merged so we can start seeing metadata in Emacs!

Friendly ping, @david-driscoll @filipw

[ricardomga]

Can we have some update on this PR, please? It would be great to see this merged 🙂

[razzmatazz]

Hey. I am cancelling this, as lsp-csharp.el in emacs-lsp will introduce support for this using an alternative lsp server:

• lsp-csharp.el: introduce support for csharp-ls (alternative) language server emacs-lsp/lsp-mode#3096

Other people can resubmit the content of this PR and tune it to their needs but I don't want to wait another year...