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: omnicompl may no completeItems yet #438

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Shane-XB-Qian
Copy link
Contributor

lspserver may have no completeItems yet, then omnicompl will be 'err'.

@@ -506,7 +505,7 @@ def g:LspOmniFunc(findstart: number, base: string): any
count += 1
endwhile

if lspserver.omniCompletePending
if lspserver.omniCompletePending || !lspserver->has_key('completeItems')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lspserver.completeItems field is set to an empty List in line 488 above, when the omni-completion function
is called with findstart' set to true. Under what conditions, the completeItemskey will not be present inlspserver`?

@Shane-XB-Qian
Copy link
Contributor Author

Shane-XB-Qian commented Jan 9, 2024 via email

@yegappan
Copy link
Owner

yegappan commented Jan 9, 2024

  • if lspserver.omniCompletePending || !lspserver->has_key('completeItems') The lspserver.completeItems field is set to an empty List in line 488 above, when the omni-completion function is called with findstart' set to true. Under what conditions, the completeItemskey will not be present inlspserver`?

somehow if omniCompletePending is false, then lspserver maybe no completeItems yet.

Under what conditions did you see this problem?

@Shane-XB-Qian
Copy link
Contributor Author

Shane-XB-Qian commented Jan 9, 2024 via email

@yegappan
Copy link
Owner

yegappan commented Jan 9, 2024

  • if lspserver.omniCompletePending || !lspserver->has_key('completeItems') The lspserver.completeItems field is set to an empty List in line 488 above, when the omni-completion function is called with findstart' set to true. Under what conditions, the completeItemskey will not be present inlspserver`? > > somehow if omniCompletePending is false, then lspserver maybe no completeItems yet. Under what conditions did you see this problem?

Under when if omniCompletePending is false, there is no 'lspserver.completeItems = []'

In the two places where omniCompletePending is set to false, lspserver.completeItems is initialized.
So I don't see a condition where omniCompletePending is false but completeItems is missing
from lspserver.

Can you use describe the steps to reproduce this problem (e.g. LSP server, plugin configuration
and the keys pressed)?

@Shane-XB-Qian
Copy link
Contributor Author

Shane-XB-Qian commented Jan 9, 2024 via email

@techntools
Copy link

techntools commented Feb 21, 2024

Screenshot from 2024-02-22 03-07-02

My cursor is between { }

After this fix, error is gone. No completions though. Should I expect completions ?

@Shane-XB-Qian @yegappan

@yegappan
Copy link
Owner

Which language server are you using? Can you include the LSP plugin settings? It will also be useful to include a sample code that reproduces the problem.

@Shane-XB-Qian
Copy link
Contributor Author

After this fix, error is gone. No completions though. Should I expect completions ?

i donot know,
to me, it should be "searching",
// then later (somehow?) it showed the completions, or "failure",
maybe more make sense.

@Shane-XB-Qian
Copy link
Contributor Author

this PR shoud be helping such flow, but i donot know why it is showing "searching" forever... 🤷
// vs #462 which just to cancel the compl.

@Shane-XB-Qian
Copy link
Contributor Author

or it was quickly enough to "cancel"?
but later somehow it may get the completions (of/at that) anyway.
// so i donot know, it is up to yegappan, and #462

@yegappan
Copy link
Owner

This is a regression caused by the PR #418. I have committed 0b9bba0 to address this issue.

@Shane-XB-Qian
Copy link
Contributor Author

I have committed 0b9bba0 to address this issue.

i think it became worse, tho logically maybe you were right if -2, but it would be no message about the status?
'cancel' meant you passed [] then no result, or here this PR let it be showing "searching",
tho not sure why it was showing forever, vs [] made it be failed directly.

so i donot know your first commit at #462 was better or here mine this one was preferred.
// anyway, personally i donot like your second commit for #462, but up to you, i am not going to argue too much.

@techntools
Copy link

Which language server are you using? Can you include the LSP plugin settings? It will also be useful to include a sample code that reproduces the problem.

Apologies for the late reply

I am using https://github.com/typescript-language-server/typescript-language-server

My settings

hi Err ctermfg=Red      ctermbg=NONE cterm=bold
hi Wrn ctermfg=Yellow   ctermbg=NONE cterm=bold
hi Hnt ctermfg=Blue     ctermbg=NONE cterm=bold
hi Inf ctermfg=White    ctermbg=NONE cterm=bold

au BufWinEnter * sign define LspDiagError     text=E   texthl=Err numhl=Err linehl=
au BufWinEnter * sign define LspDiagWarning   text=W   texthl=Wrn numhl=Wrn linehl=
au BufWinEnter * sign define LspDiagHint      text=H   texthl=Hnt numhl=Hnt linehl=
au BufWinEnter * sign define LspDiagInfo      text=I   texthl=Inf numhl=Inf linehl=

call LspOptionsSet(
            \ {
            \    'showSignature': v:false,
            \    'autoComplete': v:false,
            \    'showDiagOnStatusLine': v:true,
            \    'noNewlineInCompletion': v:true,
            \    'completionMatcher': 'icase',
            \    'keepFocusInReferences': v:true,
            \    'highlightDiagInline': v:false,
            \ }
            \ )

let lspServers = [
            \ {
            \    'name': 'clangd',
            \    'filetype': ['c', 'cpp'],
            \    'path': '/usr/bin/clangd',
            \    'args': ['--background-index']
            \ },
            \ {
            \    'filetype': ['javascript', 'typescript'],
            \    'path': 'typescript-language-server',
            \    'args': ['--stdio']
            \ },
            \ {
            \    'filetype': 'python',
            \    'path': '/home/santosh/.local/bin/pylsp',
            \    'args': ['--check-parent-process', '-v']
            \ },
            \ {
            \	 'filetype': 'vim',
            \	 'path': 'vim-language-server',
            \	 'args': ['--stdio']
            \ },
            \ ]

call LspAddServer(lspServers)

au FileType c,python,javascript,typescript,vim,sh nnoremap <buffer> <silent> gd :LspGotoDefinition<CR>

au FileType javascriptreact,typescriptreact nnoremap <buffer> <silent> gd :LspGotoDefinition<CR>

au User LspAttached set keywordprg=:LspHover

@yegappan
Copy link
Owner

Which language server are you using? Can you include the LSP plugin settings? It will also be useful to include a sample code that reproduces the problem.

Apologies for the late reply

I am using https://github.com/typescript-language-server/typescript-language-server

Thanks for the details. This problem should be addressed by 0b9bba0. Can you try the latest version of the plugin?

@techntools
Copy link

No error anymore.

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.

3 participants