jose-elias-alvarez / nvim-lsp-ts-utils

Utilities to improve the TypeScript development experience for Neovim's built-in LSP client.
The Unlicense
438 stars 18 forks source link

make 'optimize imports' sync #3

Closed alex-popov-tech closed 3 years ago

alex-popov-tech commented 3 years ago

hey! thanks for pr! The purpose of that plugin is to make 'optimize imports' behave concise with all other commands ( to be sync by default ) I've tested this using example functions:

function _oi()
    local params = {
        command = "_typescript.organizeImports",
        arguments = {vim.api.nvim_buf_get_name(0)}
    }
    local responses = lsp.buf_request_sync(0, "workspace/executeCommand", params)
    vim.cmd(":noautocmd write")
end

function _oi2()
    require "nvim-lsp-ts-utils".organize_imports()
    vim.cmd(":noautocmd write")
end

function _oi3()
    vim.cmd(":LspOrganize")
    vim.cmd(":noautocmd write")
end

after _oi buffer is not in 'modified' state, when after _oi2, _oi3 it is, which makes me think that executeCommand you do is async...

jose-elias-alvarez commented 3 years ago

Thanks for this! It looks like you're right about the inconsistency. I'm actually looking into whether to make all LSP commands async to prevent blocking the editor as much as possible, but either way, I want to make things consistent, so I'll leave this open for now.

alex-popov-tech commented 3 years ago

im usually using sync version to hook up on bufwritepre for linting/formatting, so if possible leave sync version also ❤️

alex-popov-tech commented 3 years ago

as for async, there are two things to send requests:

                                                       *vim.lsp.buf_request()*
buf_request({bufnr}, {method}, {params}, {handler})
                Sends an async request for all active clients attached to the
                buffer.

                Parameters: ~
                    {bufnr}    (number) Buffer handle, or 0 for current.
                    {method}   (string) LSP method name
                    {params}   (optional, table) Parameters to send to the
                               server
                    {handler}  (optional, function) See |lsp-handler|

                Return: ~
                    2-tuple:
                    • Map of client-id:request-id pairs for all successful
                      requests.
                    • Function which can be used to cancel all the requests.
                      You could instead iterate all clients and call their
                      `cancel_request()` methods.

                                                  *vim.lsp.buf_request_sync()*
buf_request_sync({bufnr}, {method}, {params}, {timeout_ms})
                Sends a request to a server and waits for the response.

                Calls |vim.lsp.buf_request()| but blocks Nvim while awaiting
                the result. Parameters are the same as |vim.lsp.buf_request()|
                but the return result is different. Wait maximum of
                {timeout_ms} (default 100) ms.

                Parameters: ~
                    {bufnr}       (number) Buffer handle, or 0 for current.
                    {method}      (string) LSP method name
                    {params}      (optional, table) Parameters to send to the
                                  server
                    {timeout_ms}  (optional, number, default=100) Maximum time
                                  in milliseconds to wait for a result.

                Return: ~
                    Map of client_id:request_result. On timeout, cancel or
                    error, returns `(nil, err)` where `err` is a string
                    describing the failure reason.

so to make things async i believe we need to use async req with a callback?

alex-popov-tech commented 3 years ago

i can help with that if needed

jose-elias-alvarez commented 3 years ago

I think you're right about the callbacks. I believe plenary.nvim is also adding (or just added?) some sort of async / await functionality, which I want to look into, since it would be cleaner.

Thanks for pointing out the BufWritePre use case, too. Odds are I'll end up leaving in a synchronous version and an asynchronous version.

jose-elias-alvarez commented 3 years ago

I ended up leaving the original as async and adding a simple synchronous version in 03884455a9a41b860e1c5df772de5343aff2300b. Let me know if that works for you!