-
-
Notifications
You must be signed in to change notification settings - Fork 895
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
Support C# via Omnisharp #815
Comments
Specifically, that NullReferenceException was while handing a textDocument/documentSymbol request. |
I am having some troubles installing the server, here it is my initial analysis:
|
Looks like the
|
Just saw your comment. The call stack is below, but tracking it back led me to the above. Yeah, comparing it to vscode is a good idea. I'll see if I can get an I/O trace from there to compare. Omnisharp seems to send a vast number of
|
If you're using the MELPA unstable version, setting |
@tom-bowles I still get
using the latest version from https://github.com/OmniSharp/omnisharp-roslyn/releases . |
Yeah, the fix commit hasn't made it into a release yet. I had to build csharp-language-server-protocol and omnisharp and then copy the binaries from the former (containing the fix) into the publish folder of the latter (containing the omnisharp exe). I think the existing omnisharp release will start working once a new csharp-language-server-protocol release is published to nuget.org. |
For some reason, I am unable to build the libraries on my machine so I will wait to be released before doing more investigations unless there is some other way to get the proper build. Btw it seems like vscode is not using lsp protocol. |
I've asked on Slack if there's any widely-used client that uses LSP, but no response. It's not clear how mature the LSP implementation is. |
Hi @tom-bowles, all I had to do to get this running in another LSP server was patch the case of null responses you mentioned in #815 (comment), using the latest release of Omnisharp. So if you could find a way to workaround that issue in If I get a chance I will test the configuration you listed above later today. The more support, the better! |
@tom-bowles if you compile the branch @yyoncho I tried using the following init code:
And it got the error below after hitting
|
@sebasmonia should be fixed with 31ebd7e |
Added the line manually to my lsp-mode. Not sure if that's on lsp-mode or Omnisharp Rosly... Guessing that once Omnisharp merges the branch lsp-mode can support C# out of the box? It would be awesome :) |
We have to check the server response and then eventually debug the C# completion handling. The issue might be caused by the project being not imported properly. It will be a bit harder to find the root cause because vscode does not use lsp protocol and we won't have reference lsp implementation.
We try to support as much stuff as possible out of the box. |
@sebasmonia if it works then we could capture and compare the wire data. I am still failing to build it locally on my ubuntu, I even tried in a docker container and it still fails with the following error:
My guess is that the issue is caused by the mono version. I hope that this version will be merged soon so we could do the corresponding fix in lsp-mode. |
I started a new session, killed the "Events" buffer after adding
Hope this helps! |
Thank you! Sorry for bothering you but in order to guess what is happening in the lsp-mode I will the corresponding lsp-mode logs. BTW do you know when the feature branch will be merged and an omnisharp Linux binary will be published? |
Not a bother at all, happy to help. Attached the two logs. This time with a fresh LSP install I got the methods for string! which is great. If I try with an "int" from the beginning, lsp-mode will repro the screenshot from before (instead of showing the methods for the instance it gives me all the keywords). |
Hi @sebasmonia and @yyoncho, thanks for for looking at this! I've built the latest
I haven't managed to get any completions yet, but maybe I haven't configured lsp-mode right. |
Actually, I am getting completions back but it's an empty list. The logging looks healthy as far as I can tell. I do get completions with omnisharp-emacs. |
Oh, if I close the file and reopen it I get completions. Edit - D'Oh, no I don't. I was getting default company completions. So the communication between lsp-mode and Omnisharp seems to be working, but everything returned by Omnisharp is nulls, empty lists, etc. Edit - OK, was being an idiot. The csproj file wasn't in the .sln. Getting completions of a sort through company-lsp now. Still getting various errors and haven't got anything besides completion yet, but definitely progress. |
I think the reason for the lack of context is that no information is being sent about edits. If I type a letter into my buffer, I see a bunch of Presumably for the same reason, if I move the cursor beyond what was the end of the buffer when omnisharp was started I get
|
@tom-bowles can you post the response of |
Got it working by forcing it to use
|
Setting |
Does everything work if you do that? We should report an omnisharp bug to populate |
Actually Anyway, with the I'm not getting any flycheck info yet, and the pop-up docs have
|
The first issue that we should fix is to set the version to LSP3
This should be done by changing lsp--client-capabilities to sent the expected version. I was unable to find that field in the lsp specification. |
|
I think I see why omnisharp's returning textDocumentSync 0. I've raised an issue at OmniSharp/csharp-language-server-protocol#162 with the details. |
If I understand correctly, I think the slashes that appear in the hover messages are because omnisharp is escaping everything that can be escaped (it uses |
|
First reason I get no diagnostics is that the the file I had open when I initialised lsp-mode doesn't seem to have been registered as open inside omnisharp. lsp-mode sent the didOpen event but I'm guessing that happened while the omnisharp server was still loading stuff and it didn't register it. If I close the document and open it again, reconnecting to the existing omnisharp server, it does register the didOpen and I start getting diagnostic messages. However, I still see no diagnostics in emacs. Not sure why that is yet. |
@tom-bowles can you try to debug lsp--on-diagnostics ? Your observation might be caused by a lsp-mode not being able to find the corresponding buffers. |
Turns out it was me not having configured lsp-ui correctly to get the diagnostics in flycheck. Getting diagnostics now (as long as the server is already running when I open the document) - nice! There's a fix being worked on for the server returning None for textDocumentSync (OmniSharp/csharp-language-server-protocol#162) but once fixed it will still return Full rather than Incremental as that's the lowest common denominator among the things they support and lsp-mode doesn't appear to support (based on its initialize request) dynamic registration to get this on a file-by-file basis. Presumably Full is supposed to work, so my problem then becomes why is lsp-mode only sending didChange notifications when the server asks for Incremental, but sends nothing when it asks for Full? |
Full sync is delayed but it should work. There are multiple servers that support only full synchronization. |
The branch hasn't been merged into the last release, but at least it was updated from master yesterday. @tom-bowles haven't been following all the details on this issue but letting you know in case other fixes from master help your testing in lsp-mode. |
Just in case nobody noticed: Omnisharp server has merged the LSP-mode branch now: OmniSharp/omnisharp-roslyn@dacc49a Does that mean we can move forward with lsp-mode support? 😃 |
Almost! They still don't have an actual release with this branch merged. |
Latest release in the Omnisharp repo includes the fix: https://github.com/OmniSharp/omnisharp-roslyn/releases/tag/v1.34.3 |
PR submitted for this issue. Feel free to provide review. |
Holy crap! Now we can see if dap mode will work with netcoredbg!!! Wooot! |
One thing to notice is that root detection wasn't working for me 🤔 I ended up adding a .projectile file to each folder. |
@sebasmonia I guess you have set lsp-auto-guess to t? When you do that it is expected that you have configured projectile/project.el to guess/find the correct root. In this case, projectile was not configured to guess CSharp files(e. g. the sln files are not considered as a project root). |
For the test project I was using I had only a csproj. Guessing that both sln and csproj should be considered roots (with sln taking precedence)? Realistically almost all projects are part of a solution so IDK if it matters too much. |
Realistically, there's almost always a
Usually these projects are contained in some other obvious structure, like a git-repo. For me at least, this has always been sufficient to work efficiently with |
* Add C#-support via Roslyn. This fixes #815. * Fix error in argument * Add C# to language-list in README * C#: Automatically resolve need for mono based on OS. - Simplify defcustoms. * Fix typo
Omnisharp has supported LSP for ages, and it would really lovely if lsp-mode were to work with it.
I managed to get it sort-of running with
I had to build a copy of csharp-language-server-protocol (a nuget
dependency of omnisharp-roslyn) with this commit:
OmniSharp/csharp-language-server-protocol@21907ad. That
solved this problem: joaotavora/eglot#241.
However, I still get various other problems, including
Error running timer: (wrong-type-argument hash-table-p nil)
errors and a null reference exception on startup:The text was updated successfully, but these errors were encountered: