Adds support for different omnisharp "engines"

[david-driscoll]

Adds support for different OmniSharp "engines" and moves to using 0-based indices.

@JoeRobich I'm mainly looking to test the waters on your end if this is something you would be okay with. At the end of the day I think we only need one engine, but the idea here is that we would have a nice way to fallback to the old one in the event there were problems.

The goal here is to move the extension forward a little bit:

• Today the entire buffer is sent each request and updated on the server side
  • This is pretty inefficient and with big documents causes some problems.
• Today diagnostics / codecheck is done as a client pull instead of server push

This replaces the custom omnisharp protocol with the vscode language client. However all of the features of the language client are disabled, this means that requests are processed the same as they always have been (with full buffers).

Consider this phase 1, the goal of phase 2 could be to move away from sending the buffer every time and instead using the text document events built into lsp. Phase 3 might involve removing some handlers for operations that don't have special behaviors (metadata, tests, etc).

Things that need to be done:

• Test with mono / linux / mac
• Get the latest lsp shipped into omnisharp-roslyn

This depends on OmniSharp/omnisharp-roslyn#1815 which adds the support for calling exising omnisharp endpoints via LSP.

[JoeRobich]

I think this is a step in the right direction. Would definitely want to make sure changing the indexing doesn't break the Razor or Debugger extensions.

@NTaylorMullen @gregg-miskelly

[NTaylorMullen]

Really cool idea! Ya the indexing is definitely concerning. However; given we rely on LSP contracts I'd imagine wherever we depend on you would just work because you also need to respect those contracts in order for VSCode to "ok" your data.

[david-driscoll]

So the reason for the change of indexing is to follow the lsp spec (0 based vs 1 based). The only reason omnisharp had been ones based originally was due to the legacy how emacs/vim did line numbers internally.

NOTE: Omnisharp does 0-based indexes today, and the configuration setting just turns on a jsonconverter that transforms the 0-based indexes to 1-based. So today we're translating twice (0 -> 1, 1 -> 0) 😄

I'll take a look and see if there are things that need to be done in razor plugin, but

[codecov]

Codecov Report

Merging #3817 (0741c53) into master (2882168) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3817      +/-   ##
==========================================
+ Coverage   85.99%   86.01%   +0.01%     
==========================================
  Files          60       60              
  Lines        1878     1880       +2     
  Branches      218      218              
==========================================
+ Hits         1615     1617       +2     
  Misses        202      202              
  Partials       61       61              

Flag	Coverage Δ
integration	100.00% <ø> (ø)
unit	86.01% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/omnisharp/options.ts	94.79% <100.00%> (+0.11%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47d640b...f5e3025. Read the comment docs.

[JoeRobich]

Very excited for this work to get in.

[david-driscoll]

updated to actually run on Mac and hopefully linux @333fred let me know

[JoeRobich]

I have brought the changes from this PR up to date and pushed it as branch feature/lsp-engine. We can continue to evolve these changes from the feature branch.