feat(roslyn_ls): add initial config as vim.lsp.config

[616b2f]

No description provided.

[seblyng]

When I forked the roslyn.nvim plugin, there was no easy way to start the server reliably because it didn't support --stdio or named pipes from the client. However, now it supports both.

While I will probably still use roslyn.nvim because it does a bit more clever root_dir detection (IMO), I think a minimal config could probably be good enough for a lot of people

[seblyng]

IMO, cmd should not be set by default, and should instead be set explicitly by the user. I think an example should be using --stdio flag for starting up the server

[616b2f]

I do think it would make sense to have a default cmd and using --stdio here make sense, as it's simpler to handle here.

Said that, it's tricky here to define a proper default cmd, because there is no common executable name that can be used here, I need to have another look on how the server is distributed, as there are platform specific nuget packages and some which are platform independent (which contain only .dll files that have to be called with dotnet command).

[seblyng]

Said that, it's tricky here to define a proper default cmd, because there is no common executable name that can be used here, I need to have another look on how the server is distributed, as there are platform specific nuget packages and some which are platform independent (which contain only .dll files that have to be called with dotnet command).

Exactly, that is why I don't think it makes sense to have a default cmd value, and instead have docs on an example

[616b2f]

I personally had issues to setup omnisharp back then because there was no default cmd and having something that works for some people and can be used as an example to write your own cmd is IMO better then having only something documented which should only theoretically work, but this is only my very personal opinion.

I just checked and in the platform specific nuget packages there are Microsoft.CodeAnalysis.LanguageServer.exe (for windows) and Microsoft.CodeAnalysis.LanguageServer (for linux and macos) executables in the /content/LanguageServer dir. So this could be used instead, so if the user put it in the path it can be picked up properly.

Try to test this also and adjust here after that.

[seblyng]

Yeah I sort of agree with it being cumbersome to setup without a default cmd, but when there is bad support in package managers for this server (I haven't seen a single one have it), it is also annoying having to add things to path

I don't have any big objections to this if others think it should be there. There at least needs to be documentation on how to install it and how to make the default cmd work

[616b2f]

This is where I hope we can merge williamboman/mason.nvim#1781 in future, I use this currently in combination with this package definition https://github.com/616b2f/mason-registry/blob/main/packages/roslyn/package.yaml.

Other alternative is to compile from source, or download and extract nuget files manually, none of which is convenient IMO.

I will try to write something up in the documentation part

[seblyng]

I am not sure about changing root_dir in on_attach. I don't know if it will have any effect

[justinmk]

Maybe try before_init instead of on_attach

[seblyng]

I think instead we can just use on_init, and just search in the current root_dir for a sln or a csproj file, because the root_markers should already have found a root_dir. before_init is also too early to send out the notify about opening a solution or project that is required. So something like this should work:

    on_init = {
        function(client)
            local root_dir = client.config.root_dir

            for entry, type in vim.fs.dir(root_dir) do
                if type == "file" and vim.endswith(entry, ".sln") then
                    on_init_sln(client, entry)
                end
            end

            for entry, type in vim.fs.dir(root_dir) do
                if type == "file" and vim.endswith(entry, ".csproj") then
                    on_init_project(client, { entry })
                end
            end
        end
    }

Looping twice to prefer a solution if we found both

[seblyng]

But we should maybe either wait for this: neovim/neovim#33485 or use root_dir with a function that searches for .sln first, and then search for .csproj, because the current way root_markers work will pretty much always find a .csproj file before finding a solution file, and that will not make it very useful, since you pretty much always want to attach to a solution if it exists

[616b2f]

I have to test this, as the current on_attach worked and I had an issue with jump to dependent nuget package sources where I added a workaround, not sure if this would still work with on_init, also the referenced issue could be a good addition, it may make sense to wait till it merged.

[justinmk]

If you are trying to conditionally enable the LS, root_dir function is the right approach: neovim/neovim#32037 (comment)

Simply don't call the callback if the LS should not be enabled for a buffer.

[seblyng]

Yeah I sort of agree with it being cumbersome to setup without a default cmd, but when there is bad support in package managers for this server (I haven't seen a single one have it), it is also annoying having to add things to path

I don't have any big objections to this if others think it should be there. There at least needs to be documentation on how to install it and how to make the default cmd work

[justinmk]

try rebasing. the lint CI is failing because this pr branch is out of date

[seblyng]

There are of course things to make solution resolution better, but this looks good to me as a minimal config that fits this repo

[616b2f]

There are of course things to make solution resolution better, but this looks good to me as a minimal config that fits this repo

Yes sure, we can also improve in future when we encounter actual issues or have some meaningful improvements regarding speed or reliability.