-
Notifications
You must be signed in to change notification settings - Fork 59
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
Make omnifunc() complete keywords based on LSP triggerCharacters #418
Conversation
Fixed 2 bugs: - add triggerCharacters to lsp list for omniComplete option - process triggerCharacters based keywords in omnifunc() M autoload/lsp/completion.vim M autoload/lsp/lspserver.vim
Unit tests are failing in main branch also (without my changes). Can someone please look into this? |
- fixed omnifunc related tests - diag related tests are still failing M autoload/lsp/completion.vim M test/clangd_tests.vim
|
Some LSP servers complete after non-keyword characters like Diag related tests in clangd-tests are failing in main branch also. Not related to my changes. |
M autoload/lsp/completion.vim M test/clangd_tests.vim
M autoload/lsp/completion.vim
if opt.lspOptions.autoComplete | ||
if opt.lspOptions.autoComplete || opt.lspOptions.omniComplete | ||
lspserver.completionTriggerChars = | ||
caps.completionProvider->get('triggerCharacters', []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps just remove this 'if' which to do such assignment always if it had such cap,
besides 'omni complete' seems not only depended on 'omniComplete' this option only.
or actually i tried some, seems even no your changes still can get omni-compl items after e.g '.' |
@yegappan merged? do you really get it was an improvement? please check above my comment, it seems it worked even no this changes, though that assignment perhaps should do it which regardless which kind compl. |
This is an improvement as the code for determining the trigger kind and trigger character are now the same between auto completion and omni completion. Before this change, omni completion always passed an empty string as the trigger character. |
If you do not pass trigger char to LSP server you'll not get appropriate completion. You'll get generic completion that it would send even without any keyword before cursor. You can verify this with 'pylsp' server. 'clangd' behaves differently, as it sends appropriate completion even without trigger char. Since unit tests only use clangd this problem was not exposed. |
> @yegappan merged? do you really get it was an improvement? please check above my comment, it seems it worked even no this changes, though that assignment perhaps should do it which regardless which kind compl.
If you do not pass trigger char to LSP server you'll not get *appropriate* completion. You'll get generic completion that it would send even without any keyword before cursor. You can verify this with 'pylsp' server.
You can verify this with 'pylsp' server. 'clangd' behaves differently, as it sends appropriate completion even without trigger char.
ok, maybe that's the why, i actually donot use pylsp's completion, and not sure its impl was the right or usual way.
…--
shane.xb.qian
|
This is an improvement as the code for determining the trigger kind and trigger character are now the same between auto completion and omni completion. Before this change, omni completion always passed an empty string as the trigger character.
ok, to pure 'code' perspective, you are right, it looked more clear.
anyway, how about removing that `if opt.lspOptions.autoComplete || opt.lspOptions.omniComplete`?
// which do that assignment always, since now compl kind seems was regardless, and ...
…--
shane.xb.qian
|
Fixed 2 bugs:
M autoload/lsp/completion.vim
M autoload/lsp/lspserver.vim