yegappan / lsp

Language Server Protocol (LSP) plugin for Vim9
MIT License
447 stars 43 forks source link

`<c-n>` on some LSP completion options triggers an "Item not found ... ParseMarkdown" error #508

Closed ghost closed 1 month ago

ghost commented 2 months ago

H, there. I'm not sure when this started happening, but I typed:

<style>
.title {
    border-

and then hit <c-n> to scroll through the list of completions. It seems to scroll correctly, but there is this error for every time I scroll:

Error detected while compiling CompleteChanged Autocommands for "<buffer=1>"..function <SNR>43_LspResolve[11]..<SNR>43_ShowCompletionDocumentatio
n[59]..FileType Autocommands for "*"..function <SNR>19_LoadFTPlugin[18]..script /nix/store/9ywm4gggqcwacn30qbjp5apqwk57zzkc-lsp/ftplugin/lspgfm.v
im[76]..function <SNR>123_RenderGitHubMarkdownText:
line   16:
E1048: Item not found in script: ParseMarkdown

ft is set to html in this file.

ghost commented 2 months ago

This only occurs on LSP items in the completion list.

Konfekt commented 2 months ago

Same issue here calling :LspHover with clangd.

Konfekt commented 2 months ago

I am not sure where the issue is as https://github.com/yegappan/lsp/blob/dfc001ab109cb16ef4379d3d74554158bbebdcc5/ftplugin/lspgfm.vim#L60 , https://github.com/yegappan/lsp/blob/dfc001ab109cb16ef4379d3d74554158bbebdcc5/ftplugin/lspgfm.vim#L3 and https://github.com/yegappan/lsp/blob/dfc001ab109cb16ef4379d3d74554158bbebdcc5/autoload/lsp/markdown.vim#L611 show that the function is defined

odormond commented 2 months ago

I've encountered the same issue and managed to track it down to the way vim handle autoloading and compilation.

The simplest way to reproduce the issue is to run vim -c 'setf lspgfm'.

This trigger the filetype plugin lspgfm that import as an autoloaded module lsp/markdown.vim aliased as md. Due to the autoloading, md turns out to be empty as the compilation of lsp/markdown.vim is delayed until it's used. When RenderGitHubMarkdownText() is called at the end of the filetype plugin, it tries to access the ParseMarkdown function that is supposed to be present in md. One would expect that to trigger the loading and compilation of the lsp/markdown.vim but that's not the case (and in a sense that would defeat the purpose of using an import autoload).

If something triggers the autoloading of lsp/markdown.vim before the filetype plugin is run, everything works fine as shown by running: vim -c 'echo lsp#markdown#list_pattern' -c 'setf lspgfm'.

Grepping for autoload in the code base return just a few hits:

ftplugin/lspgfm.vim:import autoload 'lsp/markdown.vim' as md
plugin/lsp.vim:import '../autoload/lsp/options.vim'
plugin/lsp.vim:import autoload '../autoload/lsp/lsp.vim'
test/markdown_tests.vim:import '../autoload/lsp/markdown.vim' as md

So it looks like autoloading is already bypassed in both the tests and the main plugin. Following these examples, I tried the following patch that fixes the issue for me:

diff --git a/ftplugin/lspgfm.vim b/ftplugin/lspgfm.vim
index 2e9bf76..8050184 100644
--- a/ftplugin/lspgfm.vim
+++ b/ftplugin/lspgfm.vim
@@ -1,6 +1,6 @@
 vim9script

-import autoload 'lsp/markdown.vim' as md
+import '../autoload/lsp/markdown.vim' as md

 # Update the preview window with the github flavored markdown text
 def UpdatePreviewWindowContents(bnr: number, contentList: list<dict<any>>)
ghost commented 2 months ago

@yegappan the patch above is working for me to fix this issue entirely

Konfekt commented 2 months ago

Regarding

One would expect that to trigger the loading and compilation of the lsp/markdown.vim but that's not the case (and in a sense that would defeat the purpose of using an import autoload).

and

plugin/lsp.vim:import '../autoload/lsp/options.vim' plugin/lsp.vim:import autoload '../autoload/lsp/lsp.vim' test/markdown_tests.vim:import '../autoload/lsp/markdown.vim' as md

I am wondering what the purpose of the autoload folder is. I thought it to be what you claim is not the case

odormond commented 2 months ago

Regarding

One would expect that to trigger the loading and compilation of the lsp/markdown.vim but that's not the case (and in a sense that would defeat the purpose of using an import autoload).

and

plugin/lsp.vim:import '../autoload/lsp/options.vim' plugin/lsp.vim:import autoload '../autoload/lsp/lsp.vim' test/markdown_tests.vim:import '../autoload/lsp/markdown.vim' as md

I am wondering what the purpose of the autoload folder is. I thought it to be what you claim is not the case

The autoload folder is used to look for files whose loading is delayed until they are needed and it works as intended when using mappings.

As I'm thinking this is a vim bug. It can be reproduced even without involving filetype plugins. I've opened an issue for it: https://github.com/vim/vim/issues/14775

Shane-XB-Qian commented 2 months ago

github.com/vim/vim/pull/14565#issuecomment-2062967639

jacobwhall commented 2 months ago

Not seeing this issue, I opened #511 describing the same bug. My setup is described there, as well as the same fix @odormond came up with above. I'll close that issue in favor of this one.

julien commented 1 month ago

@odormond that change fixes it for me, thanks.

ghost commented 1 month ago

Fixed by https://github.com/vim/vim/pull/14885