Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix completion for clojure-lsp(check type of results) #16

Merged
merged 1 commit into from
Aug 18, 2022

Conversation

sakuraiyuta
Copy link
Contributor

Fix #15: autoload/ddc-vim-lsp.vim:12 to check type of results and set isIncomplete to v:false if it's list.

before

スクリーンショット 2022-08-11 21 45 19

after

スクリーンショット 2022-08-11 21 44 30

@shun
Copy link
Owner

shun commented Aug 12, 2022

@sakuraiyuta
Thanks for you PR !

I wonder why the type of this result is not object...
I check the spec for lsp then, the type of result should be string | number | boolean | object | null if my understanding is correct.
So, I hesitated to merge PR so far ...
Do you have any idea ?

image

  • The spec is here .

@sakuraiyuta
Copy link
Contributor Author

@shun Thanks for reply!

I check the spec for lsp then, the type of result should be string | number | boolean | object | null if my understanding is correct.

Thank you for refer the spec.
But I got them as list when vim-lsp send textDocument/completion...that's strange.

After that, I found that result is one item dictionary collectly if I select completion word(vim-lsp send completionItem/resolve).

Thought of something, reasons may make the problem are:

  • vim-lsp fails to convert results
  • clojure-lsp returns illigal responses
  • other plugin's affects LSP's responses(e.g. conjure)

I will test that on minimal environment and seek more.

@shun
Copy link
Owner

shun commented Aug 12, 2022

@sakuraiyuta

I'm not familiar with clojure, and I tried with vim-lsp and asyncomplete.vim as reference.
However, I can't get any candidates for clojure code.
Have you ever got any candidates for clojure code with vim-lsp without ddc ?

Below is my minimal env which I tried.

set encoding=utf-8
scriptencoding utf-8
set nocp

let g:loaded_getscript = 1
let g:loaded_getscriptPlugin = 1
let g:loaded_logiPat = 1
let g:loaded_spellfile_plugin = 1
let g:loaded_rrhelper = 1
let g:loaded_vimball = 1
let g:loaded_vimballPlugin = 1

let s:dein_repo_dir = expand('~/.cache/vim.min/repos/github.com/Shougo/dein.vim')

if !isdirectory(s:dein_repo_dir)
	execute '!git clone https://github.com/Shougo/dein.vim' s:dein_repo_dir
endif
execute 'set runtimepath^=' . s:dein_repo_dir

if dein#load_state('~/.cache/vim.min/')
    call dein#begin('~/.cache/vim.min/')
    call dein#add('prabirshrestha/vim-lsp')
    call dein#add('mattn/vim-lsp-settings')
    call dein#add('prabirshrestha/asyncomplete.vim')
    call dein#add('prabirshrestha/asyncomplete-lsp.vim')
    call dein#end()
    call dein#save_state()
endif

if dein#check_install()
    call dein#install()
endif
filetype plugin indent on
syntax enable

let g:lsp_log_file = expand('/tmp/vim-lsp.log')
inoremap <expr> <Tab>   pumvisible() ? "\<C-n>" : "\<Tab>"
inoremap <expr> <S-Tab> pumvisible() ? "\<C-p>" : "\<S-Tab>"
inoremap <expr> <cr>    pumvisible() ? asyncomplete#close_popup() : "\<cr>"

@sakuraiyuta
Copy link
Contributor Author

sakuraiyuta commented Aug 12, 2022

@shun

I tested on minimal docker image(ubuntu 20.04 focal), using same as .vimrc you refered, and confirm that asyncomplete can do completion.

I found that response and results(vimscript value) from clojure-lsp/vim-lsp are not changed(list, not dictionary), but asyncomplete succeed opening completion float-window.

yuta@yuta-win10-wsl__home_yuta.screen.2022-08-12.18-43-21.mp4

Notice: found another issue.

  • Default clojure-lsp (installed by :LspInstallServer) can't run.
    • execute :LspInstallServer once, and replace installed binary to latest standalone binary (clojure-lsp-native-linux-amd64.zip on my env) to work.
    • It's not reason on JRE(apt install openjdk-*-jre-headless makes no effects)
    • Maybe vim-lsp-settings issue.

Try solution above and you can avoid the issue.

@sakuraiyuta
Copy link
Contributor Author

@shun

Surprisingly, LSP's specification also says possible to return array.

WSA000128

result: CompletionItem[] | CompletionList | null. If a CompletionItem[] is provided it is interpreted to be complete. So it is the same as { isIncomplete: false, items }

You said that below, It seems correct, I think so:

I check the spec for lsp then, the type of result should be string | number | boolean | object | null if my understanding is correct.

but it seems to be in conflict with sentence I refered above.

That made us confused.

If it's correct that "results are possible to return array", my PR has no problems...maybe.

@shun
Copy link
Owner

shun commented Aug 16, 2022

@sakuraiyuta
Thanks for your feedback.

I will check this tommorow,
I’m sorry to give you the late feedback.

@shun shun merged commit 8035fe3 into shun:main Aug 18, 2022
@shun
Copy link
Owner

shun commented Aug 18, 2022

@sakuraiyuta

Potentially, result takes some types, so we should check result which type is dictionary.

Thank you !

@sakuraiyuta sakuraiyuta deleted the fix/clojure/completion branch August 18, 2022 05:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't use completion with clojure-lsp
2 participants