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

feat: add support for Dagger CUE LSP #606

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dolanor
Copy link

@dolanor dolanor commented Aug 11, 2022

Hi,
we recently released a CUE LSP, and I followed vim-lsp documentation to configure it, and it works pretty well. But I wanted to make it even simpler to use it and started creating this PR.
But somehow, I can't make it work.
The configuration that works with bare vim-lsp is:

if executable('dagger')
    au User lsp_setup call lsp#register_server({
        \ 'name': 'dagger_cue_lsp',
        \ 'cmd': {server_info->['dagger', 'cuelsp']},
        \ 'allowlist': ['cue'],
        \ })
endif

function! s:on_lsp_buffer_enabled() abort
    setlocal omnifunc=lsp#complete
    setlocal signcolumn=yes
    if exists('+tagfunc') | setlocal tagfunc=lsp#tagfunc | endif
    nmap <buffer> gd <plug>(lsp-definition)
    nmap <buffer> gs <plug>(lsp-document-symbol-search)
    nmap <buffer> gS <plug>(lsp-workspace-symbol-search)
    nmap <buffer> gr <plug>(lsp-references)
    nmap <buffer> gi <plug>(lsp-implementation)
    "nmap <buffer> gt <plug>(lsp-type-definition)
    nmap <buffer> ,rn <plug>(lsp-rename)
    nmap <buffer> [g <plug>(lsp-previous-diagnostic)
    nmap <buffer> ]g <plug>(lsp-next-diagnostic)
    nmap <buffer> K <plug>(lsp-hover)

    let g:lsp_format_sync_timeout = 1000
    autocmd! BufWritePre *.cue call execute('LspDocumentFormatSync')

    " refer to doc to add more commands
endfunction

let g:lsp_log_file = expand('/tmp/vim-lsp.log')
let g:lsp_log_verbose = 1

let g:lsp_experimental_workspace_folders = get(g:, 'lsp_experimental_workspace_folders', 1)

augroup lsp_install
    au!
    " call s:on_lsp_buffer_enabled only for languages that has the server registered.
    autocmd User lsp_buffer_enabled call s:on_lsp_buffer_enabled()
augroup END

On a second improvement, I'd like to be able to use either the cuelsp server embedded in the dagger binary, but I want user who just want to use CUE not to have to download dagger. We have a standalone cuelsp binary as well. I think somehow haskell has 2 binaries as well, but I couldn't get the configuration right, so I reduced to just 1 case.

@dolanor dolanor marked this pull request as draft August 11, 2022 05:27
settings.json Outdated Show resolved Hide resolved
@mattn
Copy link
Owner

mattn commented Aug 11, 2022

Thank you. LGTM. Could you please follow the indentation to other's code?

@dolanor
Copy link
Author

dolanor commented Aug 16, 2022

I will check the indentation.
My biggest problem right now is that I can't make it work. With my code like that, I can't trigger an install with :LspInstallServer.
I get a: Server not found, and the log with debug displays:

mar. 16 août 2022 12:48:34:["s:on_text_document_did_change()",1]
mar. 16 août 2022 12:48:34:["s:send_didchange_queue() will be triggered"]
mar. 16 août 2022 12:48:34:["s:on_text_document_did_change()",1]
mar. 16 août 2022 12:48:34:["s:send_didchange_queue() will be triggered"]
mar. 16 août 2022 12:48:35:["s:send_event_queue()"]
mar. 16 août 2022 12:48:36:["s:on_text_document_did_save()",1]
mar. 16 août 2022 12:48:38:["s:on_text_document_did_close()",1]

I don't know what is wrong in my files for that to work.

@mikoto2000
Copy link
Contributor

@dolanor

  • settings.json's key is filetype
  • command name, file name(installer and settings) and function name need contain same string.
    • If the command name is dagger, the filename will be installer-dagger.sh, dagger.vim and server name is dagger.
    • if you want dagger_cue_lsp, you need shell script.(See:
      cat <<EOF > eclipse-jdt-ls
      #!/bin/sh
      DIR=\$(cd \$(dirname \$0); pwd)
      LAUNCHER=\$(ls \$DIR/plugins/org.eclipse.equinox.launcher_*.jar)
      java -Declipse.application=org.eclipse.jdt.ls.core.id1 -Dosgi.bundles.defaultStartLevel=4 -Declipse.product=org.eclipse.jdt.ls.core.product -Dlog.protocol=true -Dlog.level=ALL -noverify -Xmx1G -javaagent:\$DIR/lombok.jar -Xbootclasspath/a:\$DIR/lombok.jar -jar \$LAUNCHER -configuration \$DIR/$configDir -data \$DIR/data
      EOF
      )

try this diff.

diff --git a/installer/install-dagger-cue-lsp.sh b/installer/install-dagger.sh
similarity index 100%
rename from installer/install-dagger-cue-lsp.sh
rename to installer/install-dagger.sh
diff --git a/settings.json b/settings.json
index 98bfa41..1c9e3d3 100644
--- a/settings.json
+++ b/settings.json
@@ -340,7 +340,7 @@
       }
     }
   ],
-  "dagger_cue_lsp": [
+  "cue": [
     {
       "command": "dagger",
       "url": "https://github.com/dagger/dagger/",
diff --git a/settings/dagger-cue-lsp.vim b/settings/dagger.vim
similarity index 93%
rename from settings/dagger-cue-lsp.vim
rename to settings/dagger.vim
index 9443aa4..664097a 100644
--- a/settings/dagger-cue-lsp.vim
+++ b/settings/dagger.vim
@@ -1,10 +1,10 @@
 " NOTE: For compatibility, this looks up not only
 " dagger lsp's user config but also
 " cuelsp's one.
-augroup vim_lsp_settings_dagger_cue_lsp
+augroup vim_lsp_settings_dagger
   au!
   LspRegisterServer {
-      \ 'name': 'dagger_cue_lsp',
+      \ 'name': 'dagger',
       \ 'cmd': {server_info->
       \     lsp_settings#get('dagger_cue_lsp', 'cmd',
       \     [lsp_settings#exec_path('dagger')]+

NOTE: renamed two files.

  • install-dagger-cue-lsp.sh to install-dagger.sh
  • dagger-cue-lsp.vim to dagger.vim

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