Make omnifunc() complete keywords based on LSP triggerCharacters

[girishji]

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

[girishji]

Unit tests are failing in main branch also (without my changes). Can someone please look into this? I ran a few tests by hand and output is as expected. Looks like test framework has problem.

[Shane-XB-Qian]

2. you may need to explain more? what your change was trying to fix, and omni-compl was truly different like autocompl, but i donot know if it was really wrong? or your change was correct?
3. there were still some test failed.

[girishji]

2. you may need to explain more? what your change was trying to fix, and omni-compl was truly different like autocompl, but i donot know if it was really wrong? or your change was correct?
3. there were still some test failed.

Some LSP servers complete after non-keyword characters like ., >, [, etc. This was already handled in 24/7 completion but ignored in omni-completion. My changes bring omnifunc() to par.

Diag related tests in clangd-tests are failing in main branch also. Not related to my changes.

[Shane-XB-Qian]

Some LSP servers complete after non-keyword characters like ., >, [, etc. This was already handled in 24/7 completion but ignored in omni-completion. My changes bring omnifunc() to par.

or actually i tried some, seems even no your changes still can get omni-compl items after e.g '.'

[Shane-XB-Qian]

@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.

[yegappan]

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.

[girishji]

@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. 'clangd' behaves differently, as it sends appropriate completion even without trigger char. Since unit tests only use clangd this problem was not exposed.

[Shane-XB-Qian]

@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

[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