Ensure that OmniSharp is launched on global mono even when a version or 'latest' specifed #2221
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2221 +/- ##
=========================================
+ Coverage 59.08% 59.6% +0.52%
=========================================
Files 77 77
Lines 3759 3748 -11
Branches 547 544 -3
=========================================
+ Hits 2221 2234 +13
+ Misses 1367 1343 -24
Partials 171 171
Continue to review full report at Codecov.
|
src/omnisharp/server.ts
Outdated
|
|
||
| return launchOmniSharp(cwd, args, launchPath).then(async value => { | ||
| this.eventStream.post(new ObservableEvents.OmnisharpLaunch(value.usingMono, value.command, value.process.pid)); | ||
| return launchOmniSharp(cwd, args, launchInfo).then(async launchResult => { |
There was a problem hiding this comment.
Can we use async await here as well ?
DustinCampbell
force-pushed
the
fix-omnisharp-launch
branch
from
f70dd65 to
e94e5ef
Compare
DustinCampbell
force-pushed
the
fix-omnisharp-launch
branch
from
e94e5ef to
22fa49f
Compare
|
Hmmm... I"m not sure what's up with codecov. It seems to show lots of uncovered blocks in OmniSharpManager.ts, but those blocks are clearly covered by feature tests. It's especially confusing that the report shows increased coverage or no change in all impacted files. Weird! @TheRealPiotrP, is this a known issue? Edit: Also, looking at the report, I see |
|
@DustinCampbell We do not have code coverage for feature tests right now. I have created an issue and will look into it - #2227 |
|
Thanks @akshita31! Are you all ok if I go ahead and merge this in that case? The OmniSharpManager.ts file is really quite well-covered if feature tests are included. |
|
@DustinCampbell Sure go ahead 😀 |
3 participants
While @rchande and I were investigating a different issue, we noticed that OmniSharp was always being launched with the run script when
"omnisharp.path"is set to"latest". Essentially, it would only try to launch OmniSharp on the globally-installed Mono if the user hasn't set"omnisharp.path"at all, which really isn't what we want.Here are the desired behaviors for Linux/OSX:
"omnisharp.path"at all, launch the default OmniSharp. If a valid version of Mono is installed, launch with Mono; otherwise, use the run script."omnisharp.path"to an absolute path, assume that path is OmniSharp and launch it. If the user also sets"omnisharp.useMono"totrue, launch with Mono. If a valid version of Mono can't be found, throw an error."omnisharp.path"to a version or"latest", first, ensure the appropriate version of OmniSharp is downloaded. Then, if a valid version of Mono is installed, launch with Mono; otherwise, use the run script.This change fixes
#3above.As part of this change, I've done a bit of refactoring to simplify things a bit. The OmniSharpManager has been changed to return two paths: the path to the run script, and the path to launch with Mono. In addition, the calculation of the path to the default OmniSharp has been pushed into the OmniSharpManager. The launcher code has also been simplified a bit. Finally, I changed the logging to only say "started with Mono" if OmniSharp was actually launched with Mono, and I added the version number of Mono.