plugins/roslyn: init #3166
plugins/roslyn: init #3166
Conversation
KennethHoff
force-pushed
the
roslyn-nvim
branch
2 times, most recently
from
43139f2 to
f27337f
Compare
plugins/by-name/roslyn/default.nix
Outdated
| -- roslyn-ls adds this command | ||
| exe = 'Microsoft.CodeAnalysis.LanguageServer' |
There was a problem hiding this comment.
As @GaetanLepage said, we don't normally set defaults for setup() options.
It can be done with the plugins.roslyn.settings option if needed, but usually such auto-config would be gated behind a non-settings option (e.g. plugins.roslyn.configureLsp).
Again, for enablign the language server: this isn't normally done by nixvim plugins, however there are a handful of exceptions. Unless it was essentially a requirement and it never makes sense to have the plugin without the lsp, we would usually want this gated behind a non-settings option.
FYI: when working with language servers in nixvim, you typically use the plugins.lsp and plugins.lsp.servers.* options.
Having the non-settings toggle option allows users to enable/disable our custom stuff if they'd rather do it themselves a different way.
There was a problem hiding this comment.
It absolutely does not make sense for this plugin to exist without the LSP. The only thing the plugin does besides adding the LSP is adding a few commands to stop, restart, and configure the LSP server.
plugins/by-name/roslyn/default.nix
Outdated
| extraConfigLua = '' | ||
| require('roslyn').setup({ | ||
| -- roslyn-ls adds this command | ||
| exe = 'Microsoft.CodeAnalysis.LanguageServer' |
There was a problem hiding this comment.
As @GaetanLepage said, we don't normally set defaults for setup() options.
It can be done with the plugins.roslyn.settings option if needed, but usually such auto-config would be gated behind a non-settings option (e.g. plugins.roslyn.configureLsp).
Again, for enablign the language server: this isn't normally done by nixvim plugins, however there are a handful of exceptions. Unless it was essentially a requirement and it never makes sense to have the plugin without the lsp, we would usually want this gated behind a non-settings option.
FYI: when working with language servers in nixvim, you typically use the plugins.lsp and plugins.lsp.servers.* options.
Having the non-settings toggle option allows users to enable/disable our custom stuff if they'd rather do it themselves a different way.
Finally, this call to setup is automatically done by mkNeovimPlugin. It converts settings from nix to lua, and calls require('roslyn').setup(settings).
| extraConfigLua = '' | |
| require('roslyn').setup({ | |
| -- roslyn-ls adds this command | |
| exe = 'Microsoft.CodeAnalysis.LanguageServer' |
plugins/by-name/roslyn/default.nix
Outdated
| # TODO: Figure out how to add this package. | ||
| # home.packages = [ pkgs.roslyn-ls ]; |
There was a problem hiding this comment.
Assuming this is a language server, what we probably want is to enable plugins.lsp.servers.roslyn.enable. However, this option doesn't seem to exist.
Our lsp server options are generated from nvim-lspconfig, so perhaps the language server is not part of that project yet?
Once there is a plugins.lsp.servers.roslyn.enable option, we'd either want some kinda warning when it should be enabled but isn't, or we'd want some non-settings option that when enabled will enable the LSP automatically. Typically, we use warnings and assume the end-user is capable of configuring plugins.lsp themselves.
There was a problem hiding this comment.
roslyn-nvim is essentially a language server with a few extra options, because Roslyn is very complex. It effectively supercedes the csharp_ls LSP.
The following is the summary on the Roslyn.nvim repo
This is an actively maintained & upgraded fork that interacts with the improved & open-source C# Roslyn language server, meant to replace the old and discontinued OmniSharp. This language server is currently used in the Visual Studio Code C# Extension, which is shipped with the standard C# Dev Kit.
This standalone plugin was necessary because Roslyn uses a non-standard method of initializing communication with the client and requires additional custom integrations, unlike typical LSP setups in Neovim.
I believe that this plugin ideally would be lsp.servers.roslyn.enable. Would that be possible?
There was a problem hiding this comment.
Looks like nvim-lspconfig has https://github.com/razzmatazz/csharp-language-server and https://github.com/OmniSharp/omnisharp-roslyn, but not https://github.com/dotnet/vscode-csharp which the roslyn-ls package is based on?
There was a problem hiding this comment.
This standalone plugin was necessary because Roslyn uses a non-standard method of initializing communication with the client and requires additional custom integrations, unlike typical LSP setups in Neovim.
Ah interesting. So if I'm understanding correctly, the roslyn-nvim plugin exists as an alternative to nvim-lspconfig for setting up and configuring the language server?
I'm curious if the plugin does anything else and whether it is useful at all when the language server isn't installed?
There was a problem hiding this comment.
As I said here, I believe this plugin is essentially noop without the LSP
| packPathName = "roslyn.nvim"; | ||
| package = "roslyn-nvim"; | ||
|
|
||
| maintainers = [ lib.maintainers.GaetanLepage ]; |
There was a problem hiding this comment.
Why not adding yourself as the maintainer?
| # TODO: Figure out how to add this package, as it's the source of the `Microsoft.CodeAnalysis.LanguageServer` command, which is required for the LSP to function | ||
| # home.packages = [ pkgs.roslyn-ls ]; |
There was a problem hiding this comment.
I'm unfamiliar with this LS, so I don't know if it is better to contribute to nvim-lspconfig (so that our plugins.lsp.servers options can be used) or to assume all configuration will be done by this plugin separately from nvim-lspconfig.
If the LS can't really be configured by nvim-lspconfig, the next best thing would be to use our new dependencies options.
To do this, you would edit modules/dependencies.nix, and add "roslyn-ls" to the list. Then you would edit extraConfig here, and define dependencies.roslyn-ls.enable = lib.mkDefault true.
No description provided.