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] -lsp option seems to be ignored #1269

Closed
svenefftinge opened this issue Aug 15, 2018 · 21 comments
Closed

[LSP] -lsp option seems to be ignored #1269

svenefftinge opened this issue Aug 15, 2018 · 21 comments

Comments

@svenefftinge
Copy link

I want to start omnisharp-roslyn in LSP mode using stdio.
I have installed : https://github.com/OmniSharp/omnisharp-roslyn/releases/download/v1.32.2/omnisharp-linux-x86.tar.gz
When I run, I see output messages not conforming to LSP.

$ ./omnisharp/run -stdio -lsp

{"Event":"log","Body":{"LogLevel":"INFORMATION","Name":"OmniSharp.Stdio.Host","Message":"Starting OmniSharp on ubuntu 18.4 (x64)"},"Seq":1,"Type":"event"}
{"Event":"log","Body":{"LogLevel":"INFORMATION","Name":"OmniSharp.Services.DotNetCliService","Message":"DotNetPath set to dotnet"},"Seq":2,"Type":"event"}
{"Event":"log","Body":{"LogLevel":"INFORMATION","Name":"OmniSharp.MSBuild.Discovery.MSBuildLocator","Message":"Located 1 MSBuild instance(s)\n        1: StandAlone 15.0 - \"/workspace/theia-csharp-extension/csharp/omnisharp/omnisharp/msbuild/15.0/Bin\""},"Seq":3,"Type":"event"}
{"Event":"log","Body":{"LogLevel":"INFORMATION","Name":"OmniSharp.MSBuild.Discovery.MSBuildLocator","Message":"MSBUILD_EXE_PATH environment variable set to '/workspace/theia-csharp-extension/csharp/omnisharp/omnisharp/msbuild/15.0/Bin/MSBuild.dll'"},"Seq":4,"Type":"event"}
...

Connecting it to my client (Theia) doesn't work either. The initialize request is sent but never answered. Seems like I am doing something wrong.

@svenefftinge
Copy link
Author

@mickaelistria I saw you are using the same mode in aCute, but an older version. Do you have an idea what's wrong with my approach?

@david-driscoll
Copy link
Member

@svenefftinge try without -stdio and just do -lsp

@svenefftinge
Copy link
Author

No difference:

$ ./omnisharp/run -lsp
{"Event":"log","Body":{"LogLevel":"INFORMATION","Name":"OmniSharp.Stdio.Host","Message":"Starting OmniSharp on ubuntu 18.4 (x64)"},"Seq":1,"Type":"event"}
{"Event":"log","Body":{"LogLevel":"INFORMATION","Name":"OmniSharp.Services.DotNetCliService","Message":"DotNetPath set to dotnet"},"Seq":2,"Type":"event"}
{"Event":"log","Body":{"LogLevel":"INFORMATION","Name":"OmniSharp.MSBuild.Discovery.MSBuildLocator","Message":"Located 1 MSBuild instance(s)\n        1: StandAlone 15.0 - \"/workspace/theia-csharp-extension/csharp/omnisharp/omnisharp/msbuild/15.0/Bin\""},"Seq":3,"Type":"event"}
{"Event":"log","Body":{"LogLevel":"INFORMATION","Name":"OmniSharp.MSBuild.Discovery.MSBuildLocator","Message":"MSBUILD_EXE_PATH environment variable set to '/workspace/theia-csharp-extension/csharp/omnisharp/omnisharp/msbuild/15.0/Bin/MSBuild.dll'"},"Seq":4,"Type":"event"}
...

@svenefftinge
Copy link
Author

Btw. I am trying on ubunutu with dotnet sdk. I now noticed that on your home page it says that mono is required. Could that be the reason? (Sorry, I have now knowledge about .net)

@svenefftinge
Copy link
Author

Tried now with mono. Same result.

@mickaelistria
Copy link

Do you have an idea what's wrong with my approach?

No clue, sorry. I also don't have much time to try newer Omnishapr in aCute and see the effect. If you wish to try it, from a working state with aCute, you can try the settings described in https://github.com/eclipse/aCute#alternative-configuration to use different versions and see whether this is a regression.

@DustinCampbell
Copy link
Contributor

@david-driscoll : Does this maybe have something to do with the new CommandLineUtils that OmniSharp was updated to?

@mickaelistria
Copy link

Was there any progress with this issue? I'm considering upgrading aCute to latest OmniSharp release, but this seems like a blocker.

@svenefftinge
Copy link
Author

Does it mean you see the same behavior in aCute?

@mickaelistria
Copy link

mickaelistria commented Aug 28, 2018 via email

@svenefftinge
Copy link
Author

Ah ok. It really might be that I am doing something wrong. So it would be really awesome if someone could verify it in a different context :)

@mickaelistria
Copy link

I just tried with Eclipse aCute, using 1.32.2 instead of 1.29.0-beta2 and face similar error as the one reported initially (While 1.29.0.beta2 still works).
LSP support is a big issue for more and more adopters. Isn't there some automated test verifying it's not broken?

@patilarpith
Copy link

I can confirm that I'm facing same issue with monaco editor as well. Until Omnisharp v1.31.1 it seem to be working correctly but the newer releases [1.31.2 & 1.31.3] does not seem to start in lsp mode. monaco-languageclient sends initialize request but there is no response from Omnisharp server

@tomblind
Copy link

tomblind commented Oct 4, 2018

Can also confirm this for windows. Running "OmniSharp.exe -lsp" or "OmniSharp.exe --languageserver" from command line in either the x86 or x64 builds shows the same output as running in non-lsp mode instead of the expected nothing (as the lsp should be waiting for initialization request). It does not respond to any requests sent from the client. The last working version is 1.31.1 - every version after seems to be broken.

@patilarpith
Copy link

cc @rchande @DustinCampbell

@rchande
Copy link

rchande commented Nov 20, 2018

Ping @david-driscoll ?

@LoneBoco
Copy link
Contributor

LoneBoco commented Nov 27, 2018

public bool Stdio => _stdio.GetValueOrDefault(true);
public bool Lsp => _lsp.GetValueOrDefault(false);

Change this to:

        public bool Stdio => true;

        public bool Lsp => _lsp.HasValue();

I don't know if something changed with whatever is being used to parse command line arguments, but the actual value is set the null when dealing with a command line flag. You have to use the HasValue function to determine if the flag was set.

Since Stdio was always defaulting to true no matter what, just set it to true. And make Lsp check HasValue(). I've tested this and it works.

@Daniel-V1
Copy link

Sorry if this is a dumb question, but shouldn't you @LoneBoco open this as a pull request since you've got the fix? It looks like you've got a few other fixes as well in your fork that they would probably appreciate here. I'm a bit excited to see this fix get in, as I think it will close several issues, like #1312 for @razzmatazz , and once we get lsp, we can start trying to get https://github.com/yyoncho/dap-mode integration and https://github.com/Samsung/netcoredbg to have interactive debugging in emacs!

@LoneBoco
Copy link
Contributor

@Daniel-V1 Yeah, I submitted pull requests for two of my issues today. Of the remaining things I fixed, the URI one is not really a bug. Microsoft decided to not fix the issue because too many things were relying on the broken URIs so anybody who writes an LSP client has to manually fix the URI themselves. And I never got any guidance for the signature helper thing so I'm just keeping that locally for myself.

@Daniel-V1
Copy link

Thank you so much! Hopefully these pull requests will get merged quickly and then we can have lsp and all the goodies that come along with it!!

As an aside, personally I think https://github.com/LoneBoco/omnisharp-roslyn/commit/18866948e140ddc7ba78aabb3c272c5369736db8 for #1119 looks good too, especially if its so little code, and I like signature completion. So maybe you could submit it as well and see if they take it? Maybe they look at pull requests more closely than issues? There are certainly fewer of them.

@mickaelistria
Copy link

Thanks!
Any idea when a 1.32.9 including this fixed gets released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants