Skip to content

Conversation

@jmkelly
Copy link
Contributor

@jmkelly jmkelly commented Apr 26, 2025

I've had a few times where i wanted to stop and start the roslyn lsp and after doing a :Roslyn stop....there was no start. Maybe there is a good reason, but I was a bit confused so I've added it.

I took a look and saw that :Roslyn target does start, but it needed to select the sln file. I figure if there is only one sln file found, why ask? So I short cut it to start automatically if only one sln is found.

I'm using the conform plugin to force formatting.....but it appears it may be different from your defaults. If this is an issue, just let me know and I'll redo with your formatting.

Any comments, improvements, etc please let me know, as it my first lua PR. 🎉

@jmkelly
Copy link
Contributor Author

jmkelly commented Apr 26, 2025

one related question though, I figure there is a good reason, but any idea how we can hook into the native :LspStart, :LspStop and :LspRestart commands?

@seblyng
Copy link
Owner

seblyng commented Apr 26, 2025

Thanks! I will look at this later. I just didn't really have the need for Roslyn start when I initially implemented the commands, and I also think Roslyn restart might work as a start if I am not correct.

I would be open to adding a Roslyn start though.

one related question though, I figure there is a good reason, but any idea how we can hook into the native :LspStart, :LspStop and :LspRestart commands?

Yes. Previously it wasn't possible, because neovim didn't have a sense of a language server config. However, that is now fixed with vim.lsp.config which nvim-lspconfig have started to use.

There are two PR's that needs to be merged before they can be used:

After this, the only command really needed here is Roslyn target. However, I will not enforce a dependency on nvim-lspconfig here, so as long as the commands LspStart and etc is not upstreamed to neovim, I will keep the Roslyn commands here. If they however at one point do get upstreamed, I will probably deprecate the commands except for Roslyn target, but that doesn't seem to be in the very near future

@jmkelly jmkelly force-pushed the add-start-command branch 2 times, most recently from 164d15f to 01b8322 Compare April 27, 2025 00:11
@jmkelly jmkelly requested a review from seblyng April 27, 2025 00:17
@jmkelly jmkelly force-pushed the add-start-command branch from c1d7441 to 1dcc002 Compare April 27, 2025 10:17
@jmkelly
Copy link
Contributor Author

jmkelly commented Apr 27, 2025

I bailed on refactoring the target function and have just reverted it back to what it was before and only implemented the start functionality.

@jmkelly jmkelly requested a review from seblyng April 27, 2025 10:24
Copy link
Owner

@seblyng seblyng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me👍 thanks!

@seblyng seblyng merged commit 7f8c18c into seblyng:main Apr 27, 2025
3 checks passed
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.

2 participants