lsp-csharp.el: introduce a separate file for C#

[razzmatazz]

This adds ability to install the OmniSharp Roslyn server automatically.

The installation procedure works for me on macOS and Linux however I don't have a Windows machine around.

Server retrieval and extraction code was copied from:

• https://github.com/OmniSharp/omnisharp-emacs/blob/master/omnisharp-server-installation.el

and thus has been tested a bit "in the field" already.

@josteink could you take a look at this as well?

[yyoncho]

Looks good! I will wait for someone to confirm that it works fine on Windows. Other than that, it will be helpful for us if you can extract methods: download(url) and extract(file-name) which later will be helpful when we implement download support in core.

[josteink]

Hey guys.

This looks like a great PR! I've tested on Windows, and it's almost where it needs to be, but not quite:

Wrote c:/Users/josteink/.emacs.d/.cache/omnisharp-roslyn/v1.34.8/omnisharp-win-x86.zip
lsp-csharp: extracting "omnisharp-win-x86.zip" into "c:/Users/josteink/.emacs.d/.cache/omnisharp-roslyn/v1.34.8"
File mode specification error: (error Failed to auto-install the server; file "c:/Users/josteink/.emacs.d/.cache/omnisharp-roslyn/v1.34.8/run" was not found)

Basically on Windows, there is no run-file, so we'll need to detect Windows-specifically, and in those cases target OmniSharp.exe directly. I guess @razzmatazz can get that fixed in no time 😃

As for the actual Windows-bundle downloaded, I have a question: what's the rationale for choosing omnisharp-win-x86.zip over omnisharp-win-x64.zip?

Are there any functional differences? Shouldn't we prefer x64 if you are on a 64-bit system (or at least a 64-bit Emacs)?

If so, that is detectable by some simple code like this:

(defun is-64bit-os-p ()
  (not (null (string-match "^x86_64-.*" system-configuration))))

It may not be a big deal or very important, but I'd at least like to know that x86 wasn't chosen randomly 😄

[razzmatazz]

OK, I will try extracting download and extract fns from the code, it just would be nice if those could be reused from some generic emacs package instead, couldn't find one a couple years ago when this code was written for omnisharp-emacs.

As for x86/x64 selection, the reason was:

• https://lists.nongnu.org/archive/html/bug-gnu-emacs/2017-06/msg00893.html
• emacs-mirror/emacs@51b29de#diff-ef3502fa46b548a732d93b46355d6f80

The fix was already in for emacs26.4.. I believe. I will add emacs-version check to download x64 version instead on emacs26.4+.

[josteink]

That sounds perfectly reasonable, and answers my question 100%.

As such, I think adding 64-bit support would be nice, but not doing so either wouldn't be considered a deal-breaker on my end.

[razzmatazz]

@josteink I think I fixed the issue with Windows selection between x86/x64 bundles and starting "OmniSharp.exe" instead of "run". However, I cannot test it myself (yet) -- it would be nice if you could confirm.

@yyoncho I have extracted lsp-csharp--download and lsp-csharp--extract functions from the code, however I don't think lsp-csharp--extract is resuable at this form–it checks system-type to resolve type of the file FILENAME – which is not correct. Btw, there is also similar download/extract functionality in lsp-fsharp.el (https://github.com/emacs-lsp/lsp-mode/blob/master/lsp-fsharp.el#L191) – we should probably take the best bits from those two to in order to build a function you can add to the core.

[yyoncho]

@razzmatazz thank you. Yes, we have that(or similar) piece of code on several places, here it is the plan what we are going to try to achieve: #506 (comment)

[josteink]

I think I fixed the issue with Windows selection between x86/x64 bundles and starting "OmniSharp.exe" instead of "run". However, I cannot test it myself (yet) -- it would be nice if you could confirm.

@razzmatazz No problem. Will test tomorrow.

[josteink]

I've tested the latest changes today on my Windows-version of Emacs.

And it's definitely progress. lsp-mode now installs and runs Omnisharp-Roslyn! Whee! 🎉

What's not 100% right is the logic used to determine what version of Omnisharp-Roslyn to download for Windows.

The expression used is fairly involved, so I won't blame anyone for not getting it entirely when not having a system to test on:

(if (or (not (null (string-match "^i386-.*" system-configuration)))
        (version<= "26.4" emacs-version))
    "omnisharp-win-x86.zip"
  "omnisharp-win-x64.zip")

I did some slight testing and got the following results:

emacs-version
;; "27.0.50"

system-configuration
;; "x86_64-w64-mingw32"

(lsp-csharp--server-package-filename)
;; "omnisharp-win-x86.zip" <-- this is obviously wrong.

IMO there's too much null-checks and double-negatives in the current code to be able to think straight.

So instead of thinking about what we shouldn't have to obtain x64, I decided to think about what we should have for x64 instead. IMO that's much simpler, from what I can tell it works, and it looks like this:

(if (and (string-match "^x86_64-.*" system-configuration)
         (version<= "26.4" emacs-version))
    "omnisharp-win-x64.zip"
  "omnisharp-win-x86.zip")
;; "omnisharp-win-x64.zip"

So if you jam in that code instead of the existing one, I think this could should be good from a Windows point of view.

[razzmatazz]

@josteink thanks! – I think I have it fixed in f4e32a6 now..

[josteink]

LGTM. Thanks for adding this really neat functionality.

[yyoncho]

Thank you and congrats for your first lsp-mode PR! 🎉