zed icon indicating copy to clipboard operation
zed copied to clipboard

C# Support: Add treesitter and OmniSharp LSP support

Open fminkowski opened this issue 1 year ago • 19 comments

This PR adds the C# tree-sitter grammar. It also adds OmniSharp-Roslyn for LSP support.

Resolves issue #5299

Release Notes:

  • Added C# support

VSCode

vscode

Zed

zed

fminkowski avatar Jan 28 '24 03:01 fminkowski

We require contributors to sign our Contributor License Agreement, and we don't have @fminkowski on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

cla-bot[bot] avatar Jan 28 '24 03:01 cla-bot[bot]

It is not recommended to use OmniSharp anymore now that the Roslyn LSP is out. OmniSharp is far inferior.

just-ero avatar Jan 28 '24 06:01 just-ero

It is not recommended to use OmniSharp anymore now that the Roslyn LSP is out. OmniSharp is far inferior.

This is the OmniSharp-Roslyn LSP.

fminkowski avatar Jan 28 '24 15:01 fminkowski

This is the OmniSharp-Roslyn LSP.

Yes, OmniSharp is outdated.

just-ero avatar Jan 28 '24 17:01 just-ero

This is the OmniSharp-Roslyn LSP.

Yes, OmniSharp is outdated.

This is using the same version that the C# dev kit in VS Code uses when the useModernNet is set to true. You can see this by the package that is downloaded - omnisharp-osx-{ARCH}-net6.0.tar.gz. I think having them the same is a good idea for now. Can you provide additional documentation for what you are suggesting?

fminkowski avatar Jan 28 '24 18:01 fminkowski

I apologize for derailing this topic, it appears I was misinformed.

just-ero avatar Jan 28 '24 19:01 just-ero

I apologize for derailing this topic, it appears I was misinformed.

No problem! Glad we got it figured out.

fminkowski avatar Jan 28 '24 19:01 fminkowski

This is using the same version that the C# dev kit in VS Code uses when the useModernNet is set to true.

Hi, O# maintainer and roslyn team member here. This is not correct. There are 2 extensions at play here: the C# extension, and DevKit. The C# extension is the entirely-open-source bit (except the debugger), and has 2 available servers. By default, it uses the Roslyn LSP server, but you can switch it back to use O# if you so choose (in either framework or modern variants, which is what the useModernNet switch governs). Note that when you do so, vscode doesn't actually use O# in LSP mode, it uses it in a custom protocol that predates and inspired LSP. Both of these servers can be freely used anywhere you choose, including Zed, as they are both MIT licensed.

On the other hand, DevKit is a closed-source extension that adds additional functionality to the C# experience. It forces the use of the Roslyn LSP and is not licensed for use outside of vscode.

As I said, you are free to use either the Roslyn LSP or O#. If I were making a new editor support, I would probably go with the pure Roslyn solution, but it's up to you, and I don't want to imply that O# is a bad choice, because it's not. But I do expect that the pure Roslyn LSP will be more actively maintained and improved at this point.

333fred avatar Jan 29 '24 07:01 333fred

Thanks for the clarification, @333fred . How much extra work, in your estimation, would be involved in converting or changing the current PR to use the pure Roslyn solution?

Just a ballpark figure, a rough guess. Would it range from trivial to a complete overhaul, essentially starting from scratch and being a completely different project/PR?

If I were making a new editor support, I would probably go with the pure Roslyn solution

solrevdev avatar Jan 29 '24 10:01 solrevdev

Note that when you do so, vsc

Thanks for further clarifying @333fred! The useModernNet flag is confusing me.

Do we have to create a project, pull in the Microsoft.CodeAnalysis package, and then build to get the executable? I'm not finding any prepackaged solutions. It looks like the vscode-csharp repo maintains signed builds according to the contributing docs. If that is the case, I would consider going with the OmniSharp LSP mode for now and then adding support for the Roslyn LSP later.

As @solrevdev said, a ballpark estimate of what this would take will be really helpful. Along with any documentation that you think will help.

fminkowski avatar Jan 29 '24 15:01 fminkowski

This is the package source for the LSP: https://dev.azure.com/azure-public/vside/_artifacts/feed/msft_consumption/NuGet/Microsoft.CodeAnalysis.LanguageServer. I wouldn't expect that setup would be particularly more complex than O#, https://github.com/dotnet/vscode-csharp/blob/c5d0e9e98c0b925a4f74ae7c73a8d4b37f98005c/src/lsptoolshost/roslynLanguageServer.ts is how vscode does it. Feel free to reference with appropriate accredation. Note that we ship each platform with its own vsix, we acquire specific packages during build here: https://github.com/dotnet/vscode-csharp/blob/c5d0e9e98c0b925a4f74ae7c73a8d4b37f98005c/tasks/offlinePackagingTasks.ts#L169.

Definitely agree that this is a bit confusing. When we moved to the native Roslyn LSP, we wanted to make sure that people who were happy with O# didn't have the rug ripped out from under them, or that people who wanted to use their own other LSP could do so if they chose. Depending on which route you go, either @dibarbet (pure Roslyn LSP) or @JoeRobich (O# LSP) are great resources to reach out to if you run into problems.

333fred avatar Jan 29 '24 15:01 333fred

Thanks @333fred. It does not look as straightforward to integrate the Roslyn LSP as it is the OmniSharp LSP. I think using the Roslyn LSP will take some more research. I would like to move forward with OmniSharp at this time since it seems to fit with the current architecture - fetch an executable and spin it up. A lot of other editors are still using OmniSharp LSP too - nvim, helix, etc. Open to feedback on this, but I will not be pursuing Roslyn LSP any further with this PR.

fminkowski avatar Jan 29 '24 18:01 fminkowski

Open to feedback on this

Yeah, omnisharp gets a +1 from me for now. It does the job in sublime text for me. Thanks for adding C# @fminkowski

solrevdev avatar Jan 29 '24 18:01 solrevdev

Nice work @fminkowski, thanks for the PR. I left one comment.

maxbrunsfeld avatar Jan 29 '24 18:01 maxbrunsfeld

This looks close to be done, so it would be great to add a new docs entry to the https://github.com/zed-industries/zed/tree/main/docs/src/languages list before merging.

SomeoneToIgnore avatar Jan 29 '24 19:01 SomeoneToIgnore

I would like to move forward with OmniSharp at this time since it seems to fit with the current architecture - fetch an executable and spin it up

I don't want to push back against such a decision in general, but I will say that the roslyn LSP will be pretty much the same. Here's an example from nvim: https://github.com/jmederosalvarado/roslyn.nvim.

333fred avatar Jan 29 '24 19:01 333fred

To add on top of what Fred was saying, as one of the primary owners of the Roslyn LSP impl, i would strongly recommend going with our LSP and going the LSP route, vs the omnisharp route. A few of core reasons:

  1. LSP is an actual accepted protocol found almost everywhere now.
  2. We are actively maintaining and improving our LSP impl continuously.
  3. We'll def be fixing up issues in our LSP impl when we are non-conformant with the spec (for omnisharp, it's unclear to me if we even have a spec to be conformant with).
  4. We're investing our perf efforts in the LSP space.
  5. We're investing our feature improvement efforts in the LSP space. In other words, if a VS feature is currently limited in omnisharp/lsp, we'll be less likely to be updating the omnisharp side of things (esp. with the risk/concern about regressions since very few people are running that config), while we are very likely to update hte LSP side.

And so on and so forth.

CyrusNajmabadi avatar Jan 29 '24 22:01 CyrusNajmabadi

@CyrusNajmabadi I'm definitely up for researching using the Roslyn LSP. Where can I find documentation how how to use it, build it, where it lives, etc? I need something to point me in the right direction. Right now, all I have are links to how others are using it. With OmniSharp I was able to quickly and easily use it. But with the Roslyn LSP it has not been obvious. Can you point me in the right direction?

I am open to doing some follow up work to move over to the Roslyn LSP. But right now, I don't know what that requires. Whereas with OmniSharp, it was very straightforward.

I look forward to your help!

fminkowski avatar Jan 29 '24 22:01 fminkowski

Hi: You can see our language server here: https://github.com/dotnet/roslyn/tree/main/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer

It's just an exe that launches that then has a normal server that speaks the LSP protocol over a named pipe. The server launches, and sends the named pipe name back to the client to then connect to.

@davidbarbet could you link to the client side code that then interfaces with this?

CyrusNajmabadi avatar Jan 29 '24 22:01 CyrusNajmabadi

Where can I find documentation how how to use it, build it, where it lives, etc? I need something to point me in the right direction. could you link to the client side code that then interfaces with this?

Unfortunately we don't have great existing docs on how to use it - however, it shouldn't be a lot more complicated than O# to setup, so I'll try and outline the general steps.

  1. The executable binaries we create are published as nuget packages (1 for each architecture we ship plus a platform neutral version).
  2. The nuget packages are basically a glorified zip file. You'll just need to extract out the content folder somewhere. We do that here (shelling out to dotnet restore).
  3. Once the content is in a known location on the client side, you'll need to launch the Microsoft.CodeAnalysis.LanguageServer executable with the correct args. For VSCode, that happens here . You can ignore anything involving devkit. a. On MacOS, we do not ship an executable, so you would need to launch the server via dotnet Microsoft.CodeAnalysis.LanguageServer.dll
  4. After the server process starts, it will output the named pipe name via stdout that should be used for all LSP communication You'll need to hook up your LSP client to that named pipe. For vscode we do that here.
  5. So now the server should be running and connected to the client. However you'll need to tell the server what solution and/or projects should be loaded. This means sending a custom LSP request to the server with the solution or project paths, which we do here.
  6. After that there are other custom commands and LSP messages which you may want to implement (similar ones exist for O# as well). a. The server will send this command back to handle complex edits in completion (e.g. override completion). b. The server will send this command back to peek references in a code lens callback. c. And various others which require client interaction, on auto insert for documentation comments, fix all code action support, unit testing commands, etc. Everything in the lsptoolshost folder is everything we use on the client side for the Roslyn LSP.

There are likely to be a few rough edges here, but feel free to ping me on github (or email me directly) if you have questions or suggestions on how we can improve this.

dibarbet avatar Jan 30 '24 01:01 dibarbet

@dibarbet This is awesome! Thanks so much for sharing these details.

If @mikayla-maki and @maxbrunsfeld are ok with with the PR in its current state, we can move forward with this PR so Zed has C# syntax highlighting and at least OmniSharp support for now. And in the meantime, I can start working on getting the Roslyn LSP integrated. But with the details provided above, I think it will be possible, but may take a bit of fiddling.

fminkowski avatar Jan 30 '24 02:01 fminkowski

@dibarbet I am now able to run the Roslyn LSP thanks to your instructions. I now have a clearer understanding of how the Roslyn LSP works and what may need to be done to integrate it with Zed. Unfortunately, I don't think it is very straightforward to add at the moment, since it doesn't utilize stdin. Looking through the Zed source code, it looks like that is the only supported pipe right now. Maybe one of the maintainers can provide more insight on this?

So, I still think we should proceed with OmniSharp and I will continue researching what it will take to get Roslyn support added.

fminkowski avatar Jan 30 '24 04:01 fminkowski

I'm ok with merging the OmniSharp version. Then, if you're interested @fminkowski, you could open a PR that makes Zed compatible with Roslyn LSP, and switches to using that.

Do we need to talk to the LSP over a TCP socket instead of stdin?

@dibarbet Thanks so much for the rundown.

maxbrunsfeld avatar Jan 30 '24 05:01 maxbrunsfeld

@maxbrunsfeld Sounds good. Yes, it looks like it utilizes a domain socket for IPC. So it will require some additional changes to support. With the details that @dibarbet provided above I think I can get it to work, but it will take some time and it will require more feedback. So, let's move forward with this as is and I'll work on support for Roslyn LSP.

fminkowski avatar Jan 30 '24 14:01 fminkowski

Nice work @fminkowski. This will ship in Zed 0.121.

maxbrunsfeld avatar Jan 30 '24 17:01 maxbrunsfeld

@fminkowski @maxbrunsfeld Sorry for the noobie question... but how do I use this?

I'm new to Zed and don't see any docs on C# support or how to set up the Omnisharp LSP. I'm using Zed 0.132.2 (@maxbrunsfeld said this will ship in Zed 0.121) and it seems like out of the box it doesn't have C# support. Either I didn't configure something, or it's broken.

I don't see C# in the list of languages in the language selector, and it doesn't seem to get chosen when I open any .cs files. There's no syntax highlighting, etc.

Am I doing something wrong, or is this a bug? If it's the latter lmk and I'll open a separate issue for this.

Thanks for your hard work everyone! I'll be happy to contribute docs if needed once I understand how to fix this.

Screenshot 2024-04-25 at 11 21 51

davidnagli avatar Apr 25 '24 15:04 davidnagli

@davidnagli See if the extension is installed by running cmd+shift+p, search for extensions, and then see if C# is installed.

fminkowski avatar Apr 25 '24 16:04 fminkowski