-
Notifications
You must be signed in to change notification settings - Fork 727
Support using dual OmniSharp servers to improve handling of .csproj and project.json #585
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
Conversation
…OmniSharp 'flavor'
| "name": "csharp", | ||
| "publisher": "ms-vscode", | ||
| "version": "1.3.0", | ||
| "version": "1.3.0-beta1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-beta1 [](start = 19, length = 6)
remove "-beta1"? #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope! 😄 See my email on releases that I sent last week.
Once I merge this, I'll put up a pre-release out of master for folks to test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, I'll change this when we merge to the release branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
| const options = this._readOptions(); | ||
|
|
||
| let flavor: omnisharp.Flavor; | ||
| if (options.path !== undefined && options.usesMono === true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I am reading this correctly, one can use the 'usesMono' option to force the use of mono OmniSharp, but we don't have a corresponding option to force the use of CoreCLR omnisharp for folks who want to use CoreCLR omnisharp with a .csproj. Do I have that right?
If so, would it make sense to also have a way to force CoreCLR for folks dealing with PCL projects targeting CoreCLR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not the intention here. Essentially, there's already an option to specify a path to a specific OmniSharp -- csharp.omnisharp. I've added a csharp.omnisharpUsesMono option that controls whether the OmniSharp specified via csharp.omnisharp should be launched via Mono or not. For CoreCLR, the user just has to point csharp.omnisharp to the CoreCLR executable and it'll be launched.
|
Otherwise LGTM |
Fixes #529
This is a fairly large change that largely cleans up the OmniSharp acquisition and launching code, but also adds support for two OmniSharp servers:
Essentially, instead of assuming a specific OmniSharp server on a particular platform, this change includes the notion of an OmniSharp "flavor", which is either CoreCLR, Desktop, or Mono.
cc @gregg-miskelly, @chuckries, @caslan, @rajkumar42, @Pilchie