zed-industries / zed

Code at the speed of thought – Zed is a high-performance, multiplayer code editor from the creators of Atom and Tree-sitter.
https://zed.dev
Other
35.33k stars 1.79k forks source link

C# Support: Add treesitter and OmniSharp LSP support #6908

Closed fminkowski closed 4 months ago

fminkowski commented 4 months ago

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

Resolves issue #5299

Release Notes:

VSCode

vscode

Zed

zed
cla-bot[bot] commented 4 months ago

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'.

just-ero commented 4 months ago

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

fminkowski commented 4 months ago

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.

just-ero commented 4 months ago

This is the OmniSharp-Roslyn LSP.

Yes, OmniSharp is outdated.

fminkowski commented 4 months ago

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?

just-ero commented 4 months ago

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

fminkowski commented 4 months ago

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

No problem! Glad we got it figured out.

333fred commented 4 months ago

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.

solrevdev commented 4 months ago

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

fminkowski commented 4 months ago

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.

333fred commented 4 months ago

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.

fminkowski commented 4 months ago

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.

solrevdev commented 4 months ago

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

maxbrunsfeld commented 4 months ago

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

SomeoneToIgnore commented 4 months ago

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.

333fred commented 4 months ago

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.

CyrusNajmabadi commented 4 months ago

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.

fminkowski commented 4 months ago

@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!

CyrusNajmabadi commented 4 months ago

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?

dibarbet commented 4 months ago

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.

fminkowski commented 4 months ago

@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 commented 4 months ago

@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.

maxbrunsfeld commented 4 months ago

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.

fminkowski commented 4 months ago

@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.

maxbrunsfeld commented 4 months ago

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

davidnagli commented 1 month ago

@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

fminkowski commented 1 month ago

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