Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gopls] delay diagnostics or not run them in insert mode #127

Closed
urandom opened this issue Feb 17, 2020 · 35 comments
Closed

[gopls] delay diagnostics or not run them in insert mode #127

urandom opened this issue Feb 17, 2020 · 35 comments
Labels
question Further information is requested

Comments

@urandom
Copy link
Contributor

urandom commented Feb 17, 2020

Is there any way to force a particular lsp server to delay its diagnostics, or not run them during insert mode?

gopls is tends to be quite chatty when i'm writing and i see a lot of temporary diagnostics virtual texts. Worse yet, at some point in time, the following messages (in a different buffer) start being printed out, causing pauses in the ui:

LSP[gopls] 2020/02/17 09:01:01 failed to fix AST: unable to parse defer or go from *ast.BadStmt: no defer or go statement found

@urandom urandom added the bug Something isn't working label Feb 17, 2020
@urandom
Copy link
Contributor Author

urandom commented Feb 17, 2020

sorry about the bug label, this is more of a question/documentation issue

@lithammer
Copy link
Collaborator

Ideally I would like an option to suspend all textDocument/publishDiagnostics communication in insert mode (i.e. both inbound and outbound). Or at the very least add some kind of debounce.

It's possible to mess around with a custom vim.lsp.callbacks['textDocument/publishDiagnostics'] callback to remove most of the noise. But that just means throwing away the response, the LSP will still perform a lot of (needless) processing on every keypress. Which can be a huge drain on CPU and battery.

@h-michael h-michael added documentation Improvements or additions to documentation question Further information is requested and removed bug Something isn't working labels Feb 18, 2020
@h-michael
Copy link
Contributor

@urandom Would you show me a sample codes or project?

@urandom
Copy link
Contributor Author

urandom commented Feb 18, 2020

@h-michael
It seems that the code really doesn't matter. Any partial statement will obviously trigger diagnostics, and some will cause that blocking error as well, though I'm not exactly sure what causes the later.

I've noticed it mostly happening when I start writing struct literals, like:
https://github.com/globusdigital/feature-toggles/blob/7ab6dd1a839c3a58354f95fb689d4cc558232de9/toggle/toggle_test.go#L37

the cursor position would be at:
{name:|CURSOR|}
and the error will popup, freezing nvim for a second.

though it also sometimes occurs when i write if statements.

@norcalli
Copy link

@lithammer I would ignore the cost of the processing on the keypress if you're using an LSP at all on a battery and just disable it if that is a concern. There's no way to use an LSP without sending the textDocument updates, which, currently, send the whole text document due to a bug preventing incremental updates in the core LSP (but to be fair, a lot of LSP servers don't support incremental as well).

So I'd say the cost of parsing the json is pretty insignificant compared to the cost of displaying virtual text and updating it etc, so a custom callback which returns early if you're in insert mode for the LSP server of your choice (or all of them) is a solid idea.

If you want help with that, let me know and I can show an example, but it should be relatively easy (if you're not familiar with Lua, then it's still a pretty basic Lua task).

@h-michael
Copy link
Contributor

LSP[gopls] 2020/02/17 09:01:01 failed to fix AST: unable to parse defer or go from *ast.BadStmt: no defer or go statement found

This is not diagnostic information but gopls internal error message.
So there may be room for a little better nvim's lsp error handling. I'll fix that later.

@dbarrosop
Copy link

@norcalli other lsp plugins for nvim supports it and the experience overall is so much better. Specially if you prefer to run your own diagnostic tools with neomake

@h-michael
Copy link
Contributor

@dbarrosop
The LSP notifies the server of the text change each time the text changes. The server will notify you if there are any diagnostics at that time. Neomake runs at any time, so comparing is nonsense.
Anyway, as mentioned above, there is room for improvement, so we will fix that point.

@h-michael h-michael removed the documentation Improvements or additions to documentation label Feb 18, 2020
@dbarrosop
Copy link

dbarrosop commented Feb 18, 2020

You may disagree and it may or may not be feasible to disable diagnostics client side but it's certainly not nonsense. I'd rather run diagnostics/notify the server when I know I am done typing than having it tell me my syntax is incorrect as I type.

I also don't think disabling the LSP is a suitable workaround to avoid battery drain due to excessive diagnostics. The LSP does way more things than just provide diagnostics.

A better compromise might be to delay notifying the server as the original submitter suggested.

Anyway, I really appreciate the work being done here and hence why I migrated from my previous plugin, to help out testing and provide feedback.

@bfredl
Copy link
Member

bfredl commented Feb 18, 2020

I also don't think disabling the LSP is a suitable workaround to avoid battery drain due to excessive diagnostics. The LSP does way more things than just provide diagnostics.
A better compromise might be to delay notifying the server as the original submitter suggested.

Though this is contradictory. We don't notify the server to get diagnostics specifically, rather all functionality of the server requires frequent updates of buffer to function properly. Especially completion requires the latest text of the line being edited while insert mode is still in progress. Servers are suppose throttle reparsing etc themselves for a reasonable tradeoff between speedy diagnostics and CPU usage. Servers might have their specific options to configure, like clangd allows you to explicitly request or suppress reparsing (though the default works perfectly fine to me), perhaps gopls has something similar?

@lithammer
Copy link
Collaborator

lithammer commented Feb 18, 2020

If you want help with that, let me know and I can show an example, but it should be relatively easy (if you're not familiar with Lua, then it's still a pretty basic Lua task).

I managed to scramble something like this together. Not sure it's the most idiomatic approach though.

do
  local default_callback = vim.lsp.callbacks["textDocument/publishDiagnostics"]
  local err, method, params, client_id

  vim.lsp.callbacks["textDocument/publishDiagnostics"] = function(...)
    err, method, params, client_id = ...
    if vim.api.nvim_get_mode().mode ~= "i" then
      publish_diagnostics()
    end
  end

  function publish_diagnostics()
    default_callback(err, method, params, client_id)
  end
end

local on_attach = function(_, bufnr)
  vim.api.nvim_command [[autocmd InsertLeave <buffer> lua publish_diagnostics()]]
end

nvim_lsp.gopls.setup({on_attach=on_attach})

@urandom
Copy link
Contributor Author

urandom commented Feb 19, 2020

A response regarding this issue from the gopls maintainer:

golang/go#37259 (comment)

@Shatur
Copy link
Contributor

Shatur commented Feb 22, 2020

I have the same issue with GDScript and C++ (clangd):
asciicast

@lithammer, your solution not worked for me :(

@urandom
Copy link
Contributor Author

urandom commented Feb 23, 2020

The biggest problem with this issue is that nvim-lsp will display internal lsp server errors as blocking messages, thus interrupting typing. Having diagnostics on incomplete statements, while annoying, is not disruptive.

@Shatur
Copy link
Contributor

Shatur commented Feb 23, 2020

There is also a visual bug with a lot of diagnostics: the text blinks and overwrites even after I stop typing.

asciicast

@mfussenegger
Copy link
Member

@urandom Could you call :lua vim.lsp.set_log_level('trace') when the issue appears and then take a look at the logs in ~/.local/share/nvim/vim-lsp.log ? (or wherever lua vim.lsp.set_log_level('trace') points to).

I suspect the error message you get that interrupts typing isn't related to diagnostics at all, but rather is something sent to window/showMessage - which is then calling https://github.com/neovim/neovim/blob/c036e24f39481f2b7872659c076b53435192003d/runtime/lua/vim/lsp/callbacks.lua#L208

@urandom
Copy link
Contributor Author

urandom commented Feb 23, 2020

There's quite a log of output being generated:

https://gist.github.com/urandom/53bccd1a28b0ef865fd82cad244b90af

The output is for when i start typing to some point after the error is displayed. From what I can tell, the error is a result of a didChange message

@mfussenegger
Copy link
Member

mfussenegger commented Feb 23, 2020

Well, my suspicion was almost right, it's window/logMessage, not window/showMessage:

[ DEBUG ] 2020-02-23T20:18:35Z+0100 ] /usr/share/nvim/runtime/lua/vim/lsp.lua:367 ]	"notification"	"window/logMessage"	{  message = "2020/02/23 20:18:35 failed to fix AST: unable to parse defer or go from *ast.BadStmt: no defer or go statement found",  type = 1}

type = 1 is Error.

As a workaround you could set

vim.lsp.callbacks["window/logMessage"] = function log_message(_, _, result, client_id)
  local message_type = result.type
  local message = result.message
  local client = vim.lsp.get_client_by_id(client_id)
  local client_name = client and client.name or string.format("id=%d", client_id)
  if not client then
    err_message("LSP[", client_name, "] client has shut down after sending the message")
  end
  local message_type_name = vim.lsp.protocol.MessageType[message_type]
  vim.api.nvim_out_write(string.format("LSP[%s][%s] %s\n", client_name, message_type_name, message))
  return result
end

mfussenegger added a commit to mfussenegger/neovim that referenced this issue Feb 23, 2020
Fixes neovim/nvim-lspconfig#127

Same could also happen with haskell-ide-engine, where messages as
follows were sent:

```
"window/showMessage"
{
  client_id = 1,
  params = {
    message = '"cannot satisfy -package-id junittui-0.1.0.0-AmJpwWxXMEHEoylTJC167n\\n    (use -v for more informatio n)"',
    type = 1
  }
}
```
@urandom
Copy link
Contributor Author

urandom commented Feb 23, 2020

It doesn't seem like the workaround is working. It is still sending in a trailing newline, so it will be displayed immediately in a blocking manner, from what I understand.

EDIT: actually, the error is different. It's complaining that it doesn't know what this protocol is

@mfussenegger
Copy link
Member

mfussenegger commented Feb 23, 2020

It doesn't seem like the workaround is working. It is still sending in a trailing newline, so it will be displayed immediately in a blocking manner, from what I understand.

EDIT: actually, the error is different. It's complaining that it doesn't know what this protocol is

Can you change the 2 lines to:

local message_type_name = vim.lsp.protocol.MessageType[message_type]
vim.api.nvim_out_write(string.format("LSP[%s][%s] %s\n", client_name, message_type_name, message))

I missed initially that protocol and api are probably not assigned to some local variable.

@urandom
Copy link
Contributor Author

urandom commented Feb 23, 2020

With that change I'm no longer seeing the error.

mfussenegger added a commit to mfussenegger/neovim that referenced this issue Feb 23, 2020
Fixes neovim/nvim-lspconfig#127

Same could also happen with haskell-ide-engine, where messages as
follows were sent:

```
"window/showMessage"
{
  client_id = 1,
  params = {
    message = '"cannot satisfy -package-id junittui-0.1.0.0-AmJpwWxXMEHEoylTJC167n\\n    (use -v for more informatio n)"',
    type = 1
  }
}
```

(cherry picked from commit ae4002a)
h-michael added a commit to h-michael/nvim-lspconfig that referenced this issue Feb 24, 2020
h-michael added a commit to h-michael/nvim-lspconfig that referenced this issue Feb 24, 2020
h-michael added a commit to h-michael/nvim-lspconfig that referenced this issue Feb 24, 2020
@Shatur
Copy link
Contributor

Shatur commented Feb 24, 2020

@lithammer, made your solution works for me by changing this line:

if vim.api.nvim_get_mode().mode ~= "i" then

into this:

if vim.api.nvim_get_mode().mode ~= "i" and vim.api.nvim_get_mode().mode ~= "ic" then

Thank you! Would be nice to see such option built-in.

@h-michael
Copy link
Contributor

@urandom Would you try this branch? neovim/neovim#11942

h-michael added a commit to h-michael/nvim-lspconfig that referenced this issue Feb 25, 2020
Add None because the user has no way to hide logMessage and showMessage.
ref: neovim#127
@urandom
Copy link
Contributor Author

urandom commented Feb 25, 2020

@urandom Would you try this branch? neovim/neovim#11942

Would that be with or without the other fix?

@h-michael
Copy link
Contributor

h-michael commented Feb 25, 2020

Would that be with or without the other fix?

Yes.

@urandom
Copy link
Contributor Author

urandom commented Feb 25, 2020

Would that be with or without the other fix?

Yes.

Yes is definitely not an appropriate answer for that question ;)

@urandom
Copy link
Contributor Author

urandom commented Feb 25, 2020

An initial test (overriding the previous fix) doesn't show any errors. I'll keep using it throughout the day to see if anything turns up

@h-michael
Copy link
Contributor

@urandom
Previously, both window/logMessage and window/showMessage were displayed on the screen.
The branch simply doesn't display window/logMessage on the screen.
The first problem presented was the above, regardless of textDocument/publishDiagnostics.

@ttttcrngyblflpp
Copy link

What should I add to my configuration if I want to hide all diagnostics? I'm having the same issue where clangd is warning me about syntax errors on every character typed.

@ttttcrngyblflpp
Copy link

ttttcrngyblflpp commented Mar 31, 2020

To answer my own question, setting textDocument/publishDiagnostics to a function that does nothing seems to do the trick.

@justinmk
Copy link
Member

justinmk commented Aug 30, 2020

setting textDocument/publishDiagnostics to a function that does nothing seems to do the trick.

That's actually not a bad way to do it. Guess we can mention it in the docs.

Alternatively use can set message_level = vim.lsp.protocol.MessageType.Error ( #145 ) and that is as far as we should go (i.e. we don't need #148 ).

If errors are being printed a lot that's a different problem...

@mjlbach
Copy link
Contributor

mjlbach commented Dec 18, 2020

@urandom Is this an issue anymore? see :help vim.lsp.diagnostic

You should be able to add

:lua << EOF
    vim.lsp.handlers["textDocument/publishDiagnostics"] = vim.lsp.with(
      vim.lsp.diagnostic.on_publish_diagnostics, {
        -- delay update diagnostics
        update_in_insert = false,
      }
    )
EOF

To get what you want.

@urandom
Copy link
Contributor Author

urandom commented Dec 24, 2020

@mjlbach
It looks like the issue is solved

@mjlbach mjlbach closed this as completed Jan 3, 2021
ammarnajjar added a commit to ammarnajjar/dotfiles that referenced this issue Feb 22, 2021
@alexzanderr
Copy link

@urandom Is this an issue anymore? see :help vim.lsp.diagnostic

You should be able to add

:lua << EOF
    vim.lsp.handlers["textDocument/publishDiagnostics"] = vim.lsp.with(
      vim.lsp.diagnostic.on_publish_diagnostics, {
        -- delay update diagnostics
        update_in_insert = false,
      }
    )
EOF

To get what you want.

it was so simple. wow

@yifan0414
Copy link

@urandom Is this an issue anymore? see :help vim.lsp.diagnostic

You should be able to add

:lua << EOF
    vim.lsp.handlers["textDocument/publishDiagnostics"] = vim.lsp.with(
      vim.lsp.diagnostic.on_publish_diagnostics, {
        -- delay update diagnostics
        update_in_insert = false,
      }
    )
EOF

To get what you want.

Thanks very much

haunt98 added a commit to haunt98/dotfiles that referenced this issue Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.