Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add roslyn lsp #6330

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add roslyn lsp #6330

wants to merge 1 commit into from

Conversation

seblj
Copy link

@seblj seblj commented Jul 1, 2024

Describe your changes

This adds the roslyn language server from https://github.com/dotnet/roslyn/ which is a part of the C# dev kit in vscode.

Issue ticket number and link

Checklist before requesting a review

  • I have successfully tested installation of the package.
  • I have successfully tested the package after installation.

This language server doesn't start correctly by naively using it with nvim-lspconfig or vim.lsp.start. There needs to be a little bit of trickery to start it, since the server sends the pipe to communicate through, instead of allowing the client to name a pipe or communicate through stdio.

However, I have forked a plugin that started on this, and I am now maintaining it. I got this issue seblj/roslyn.nvim#11 which asked if I would consider adding support for easier installation. The author of the issue created this repo https://github.com/Crashdummyy/roslynLanguageServer which gets a new version once a day.

This prompted me to think about adding it to mason, as it would really help with installation for me and whoever else is using the plugin😄

I tested to start it with my plugin, and it started just like it does when installing it manually. I also tested installation of all the different targets I added support for, and all of them was able to download just fine😄

Screenshots

@Conarius
Copy link
Contributor

Conarius commented Jul 1, 2024

Nice work though I don't think having the source file repo being different from the homepage url is a good idea as this will inevitably cause the downstream/upstream problem like users reporting bugs to the upstream project when it's a downstream issue and vice versa.
Though I might be a bit too cautious here and william might see things differently and would approve of it as it still is his decision

@seblj
Copy link
Author

seblj commented Jul 2, 2024

I can easily change the source url, no problem :)
I just went with it for now since I think I saw some other examples where this was done. I don't remember which package(s) this was from the top of mye head though

@Kurren123
Copy link

Would be awesome to have this merged. Updating this manually is a pain.

@SimonSchwendele
Copy link

SimonSchwendele commented Aug 4, 2024

Would be awesome to have this merged. Updating this manually is a pain.

I wrote a bash script to update the lsp in the meantime.

You just have to replace targetDir and rid with your nvim datadir and the rid of your system

@616b2f
Copy link

616b2f commented Aug 7, 2024

purl knows the concept of "repository_url" maybe this could be implemented for nuget, so we can reference the packages directly from microsoft repository?

@seblj seblj force-pushed the roslyn branch 2 times, most recently from e6860cb to 068f2c4 Compare August 22, 2024 17:17
@616b2f
Copy link

616b2f commented Sep 3, 2024

FYI: I am currently working on providing a way to install "regular" (not dotnet tool) nuget packages, which would just download the plain nuget and extract it into a directory.

This would allow to also use "repository_url" to pull nuget images from custom registries, such as the one from microsoft.

The file for roslyn would then look like this:

name: roslyn
description: |
  The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
homepage: https://github.com/dotnet/roslyn
licenses:
  - MIT
languages:
  - C#
categories:
  - LSP

source:
  id: pkg:nuget/Microsoft.CodeAnalysis.LanguageServer.neutral@4.12.0-2.24419.4?repository_url=https://pkgs.dev.azure.com/azure-public/vside/_packaging/vs-impl/nuget/v3/index.json
  download:
    file: Microsoft.CodeAnalysis.LanguageServer.neutral-{{version}}.nupkg

bin:
  roslyn: nuget:content/LanguageServer/neutral/Microsoft.CodeAnalysis.LanguageServer.dll

I will reference the PR as soon as I am finishing some cleanup.

Update package.yaml

Co-authored-by: SimonSchwendele <86782769+SimonSchwendele@users.noreply.github.com>
@seblj
Copy link
Author

seblj commented Sep 5, 2024

Sorry to ping you @williamboman

But is there anything else that I need to do? It seems like there is a lot of excitement for this, and I would really appreciate it if someone could review it so that we could (hopefully) get it merged😅 It would really make my life (and I guess others) easier to update the language server

@williamboman
Copy link
Member

williamboman commented Oct 22, 2024

Hey sorry for super late review, I see there's a lot of interest in this. I've had some reservations with this one because of:

  • Microsoft's Terms of Use explicitly prohibits redistributing software, although after some more reading I believe the MIT license with which this software is originally distributed under overrides this.
  • Unofficial sources are a last resort.
  • Releases are systematically removed in the Crashdummyy/roslynLanguageServer. This is especially problematic because of the frequent release cadence. I've been working on creating something similar to r-languageserver and julia-lsp for this but I won't have the time to finish it soon and I'd like to avoid pulling in more redistributions under mason-org.

Also as a side note, you can pretty easily create and use other registry sources. I see someone already provided an alternative registry for this so hopefully some people found that useful, also see https://github.com/williamboman/mason-registry-playground for a template for registries.

@Crashdummyy
Copy link

Hey sorry for super late review, I see there's a lot of interest in this. I've had some reservations with this one because of:

  • Microsoft's Terms of Use explicitly prohibits redistributing software, although after some more reading I believe the MIT license with which this software is originally distributed under overrides this.
  • Unofficial sources are a last resort.
  • Releases are systematically removed in the Crashdummyy/roslynLanguageServer. This is especially problematic because of the frequent release cadence. I've been working on creating something similar to r-languageserver and julia-lsp for this but I won't have the time to finish it soon and I'd like to avoid pulling in more redistributions under mason-org.

Also as a side note, you can pretty easily create and use other registry sources. I see someone already provided an alternative registry for this so hopefully some people found that useful, also see https://github.com/williamboman/mason-registry-playground for a template for registries.

I delete them frequently because I dont think its worth keeping them.
I will always keep the releases which are used in mason thats no issue on my side.

@seblj
Copy link
Author

seblj commented Oct 22, 2024

Alright! So if I understand you correctly, you are a bit hesitant to do it like this?

Is there something I could help with to make it available in core without depending on other third party registries?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants