fix: roslyn restart command not working

[GustavEikaas]

The Roslyn restart command does not work for me. When I execute it nothing happens and then I move my cursor and it logs out Roslyn server stopped but never any start event after that

This PR fixes the Roslyn restart command, atleast for me. It's kinda hacky but we can either forcestop the lsp server or do some yank with moving the cursor in the buffer. There might be other ways I experimented with :redraw and CursorHold exec. Those didnt work

[seblyng]

Interesting that it doesn't work🤔 Anyways, I like this idea since I don't like the restart code. I just copied something over from lspconfig if I remember correctly.

Just looked through this on mobile right now so haven't tested it yet, but I think the idea makes sense at least.

I don't remember why I didn't go with something like this or even if I testen something like this🤷‍♂️

[seblyng]

I think I would prefer if we didn't have this file. I think instead we can just use the interface directly which you created in event_emitter.lua

We should then add some emmylua hints to which events is available to on and emit.

I think for now it is okay to just have stopped as an event, and then we can just add started or other potential events when needed

[GustavEikaas]

Did some refactorings to make a concrete implementation with the things we need. Please re-review this file and see if it looks okay to you. Trying to keep the exposed signature small and succinct.

Also added emmylua hints and examples

[GustavEikaas]

Please also test out the :Roslyn restart command on the main branch just to make sure it also does not work for you.

We can probably continue this in another thread but the reason for this PR is because everytime I create a new file or install a nuget package the Roslyn server wont pick it up for some reason. This causes me to run :Roslyn restart pretty frequently.

[seblyng]

I think it mostly looks good now. Just a couple of questions really. The current restart actually works for me on main and I use it all the time, but we should of course fix it since it doesn't work for you👍

[seblyng]

Do we need to force it? It works for me without, and it prints out this message of the server quit with exit code x which would be nice to avoid if possible

[seblyng]

I was actually first thinking about making this and emit public and have the type of event be literal "stopped" to get some typing when using it. Then we could just add a new literal string as alternative to event to get hints if we add more events, instead of creating new methods.

However, I don't have that strong opinions on that after some thinking and am okay with this as well. What do you think?

[seblyng]

Can we not just return on here since that is also returning a a cleanup function?

[GustavEikaas]

Not at the computer rn but will fix the comments shortly, for me it was kinda wonky when i stopped without forcing. That required me to do weird stuff like vim.api.nvim_feedkeys("j", "n").

Very strange its working for you, are you on windows or linux. Which version of roslyn lsp are you using?

[GustavEikaas]

Okay I think I might've found it. I checked out latest main branch and add force stop to the client. Everything works perfectly. Wild guess, its a windows issue with delay during terminating processes?

local client = vim.lsp.get_clients({ name = "roslyn" })[1]
            if not client then
                return
            end

            local attached_buffers = vim.tbl_keys(client.attached_buffers)

            -- TODO: Change this to `client:request` when minimal version is `0.11`
            ---@diagnostic disable-next-line: missing-parameter
-           client.stop()
+           client.stop(true)

            local timer = vim.uv.new_timer()
            timer:start(
                500,
                100,
                vim.schedule_wrap(function()
                    -- TODO: Change this to `client:request` when minimal version is `0.11`
                    ---@diagnostic disable-next-line: missing-parameter
                    if client.is_stopped() then
                        for _, buffer in ipairs(attached_buffers) do
                            vim.api.nvim_exec_autocmds("FileType", { group = "Roslyn", buffer = buffer })
                        end
                    end

                    if not timer:is_closing() then
                        timer:close()
                    end
                end)
            )

[seblyng]

Okay yes I am using mac so it's probably some windows issue then. Keep the force then so that it works for everyone👍

[GustavEikaas]

Okay so just to clarify, do you still want the rest of this PR with event emit stuff or just add client.stop(true) and thats it? Up to you

[seblyng]

I think I want this PR since I don't really like the timer stuff

[GustavEikaas]

refactored the roslyn_emitter according to your wishes, I think it makes sense like this, using string literals.

If your plan was to expose these events as part of the public API we should update readme, otherwise please re-review and lmk if I need to do changes

[GustavEikaas]

Lastly I could make a is_windows() check to only force if on windows. Just to contain the tomfoolery. This would prevent Unix users from seeing the client quit with exit code 1 message

[seblyng]

If your plan was to expose these events as part of the public API we should update readme, otherwise please re-review and lmk if I need to do changes

I think we can wait with this👍 looks good now!

Lastly I could make a is_windows() check to only force if on windows. Just to contain the tomfoolery. This would prevent Unix users from seeing the client quit with exit code 1 message

Yes that sounds like a good idea! Otherwise looks good to me👍

[GustavEikaas]

Tested and verified on windows. Works like a charm🚀

[seblyng]

Thank you very much!😄

[GustavEikaas]

I would've cleaned up my commits if I knew you were going to merge and not squash merge 🙈. Will remember to next time

[seblyng]

Woops😅 I didn't think about that, but it's no problem