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

fix: don't blow up when nvim_buf_get_lines() returns Blobs #2050

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Sep 27, 2024

Some LSP servers may return binary garbage and nvim_buf_get_lines() will return a Blob instead of a String in those cases.

I added some print(vim.inspect()) debugging in
entry.get_documentation() to prove that by the time the text passes through there, it's already garbage.

Here's an excerpt from a sample line returned by nvim_buf_get_lines(), as rendered by vim.inspect():

default\0\0\0!      vim.opt.background = 'dark'\0\0\0000

(etc)

Now, this looks like an LSP bug to me, but I think we shouldn't allow buggy LSP output to crash nvim-cmp. "Be conservative in what you send, be liberal in what you accept" and all that.

So, degrade by coercing any Blob we see into a String before passing it to strdisplaywidth().

Closes: #820

Some LSP servers may return binary garbage and `nvim_buf_get_lines()`
will return a `Blob` instead of a `String` in those cases.

I added some `print(vim.inspect())` debugging in
`entry.get_documentation()` to prove that by the time the text passes
through there, it's already garbage.

Here's an excerpt from a sample line returned by `nvim_buf_get_lines()`,
as rendered by `vim.inspect()`:

    default\0\0\0!      vim.opt.background = 'dark'\0\0\0000

(etc)

Now, this looks like an LSP bug to me, but I think we shouldn't allow
buggy LSP output to crash nvim-cmp. "Be conservative in what you send,
be liberal in what you accept" and all that.

So, degrade by coercing any `Blob` we see into a `String` before passing
it to `strdisplaywidth()`.

Closes: hrsh7th#820
@hrsh7th
Copy link
Owner

hrsh7th commented Oct 21, 2024

Could you explain why and when nvim_buf_get_lines returns Blob instead of string...?

IMO, neovim's docs does not describe this behavior.

@wincent
Copy link
Contributor Author

wincent commented Oct 21, 2024

Could you explain why and when nvim_buf_get_lines returns Blob instead of string...?

IMO, neovim's docs does not describe this behavior.

Sadly, I have no idea why. I just know that it happens. I suspect that a buggy language server (in this case, lua-language-server) is somehow producing the right kind of binary garbage — likely an invalid string encoding — to cause Neovim to encode it as a Blob. But I believe it's not limited to lua-language-server, based on the comments on #820, which mention this happening in various scenarios.

Whatever the bug is, I'm seeing it when nvim-cmp shows the hover documentation for Enum types; note the artifacts in the text:

Enum type, showing the bug

For non-Enum types, it works fine:

non-Enum type, working fine

@hrsh7th
Copy link
Owner

hrsh7th commented Oct 21, 2024

thank you.

@hrsh7th hrsh7th merged commit 5d0651c into hrsh7th:main Oct 21, 2024
2 checks passed
@wincent wincent deleted the beware-blobs branch October 21, 2024 20:21
wincent added a commit to wincent/wincent that referenced this pull request Oct 21, 2024
Because I want this fix:

- hrsh7th/nvim-cmp#2050

* aspects/nvim/files/.config/nvim/pack/bundle/opt/nvim-cmp d818fd0...29fb485 (11):
  > feat: allow more completion highlight control (#1972)
  > Format with stylua
  > fix: use \<* notation for keymap.normalize (#2069)
  > fix: don't blow up when `nvim_buf_get_lines()` returns Blobs (#2050)
  > perf: use builtin for key normalization (#1935)
  > Avoid calling del_extmark on non existing buffer (#2053)
  > Format with stylua
  > add performance.filtering_context_budget fixes #2060
  > Format with stylua
  > perf: improve for source providing huge list of items (#1980)
  > Add newer fallback method for tree-sitter scopes (#2006)
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.

Error executing lua: Vim:E976: using Blob as a String, Puppeteer completion
2 participants