jackguo380 / vim-lsp-cxx-highlight

Vim plugin for C/C++/ObjC semantic highlighting using cquery, ccls, or clangd
MIT License
337 stars 26 forks source link

Neovim support for text properties #12

Closed Kyran-C closed 4 years ago

Kyran-C commented 5 years ago

Enabling text properties on neovim seems to work, but then spits out errors after a couple minutes.

Error detected while processing function <SNR>82_hl_symbols_wrap[5]..<SNR>82_hl_symbols:
line   25:
E117: Unknown function: prop_type_get
Error detected while processing function <SNR>82_hl_symbols_wrap[5]..<SNR>82_hl_symbols:
line   25:
E15: Invalid expression: prop_type_get(l:hl_group)

Looks like it's running into trouble at autoload/lsp_cxx_hl/textprop/symbols.vim

I open a cpp file, make no edits, just wait a minute or two, and the error pops up. Looks like neovim doesn't have prop_type_get in its api. It's strange because everything else works fine. If I make some edits to the file, all the highlighting sticks to the text like you'd expect, rather than taking several seconds to refresh the highlighting.

I'm using: neovim nightly 0.4.0-1860-g0ec80d5f6 coc 0.0.74 ccls 20190826

jackguo380 commented 5 years ago

Hi Kryan-C,

If I am not mistaken, neovim does not support text properties at all, not just prop_type_get, it does have equivalent functionality provided by nvim_buf_add_highlight. vim-lsp-cxx-highlight doesn't support that currently but I plan on adding it soon.

It is strange that you said seems to be working, are you manually enabling the feature while editing? I tried neovim nightly v0.4.0-1928-gd3c17d50d and found it just fails and never highlights anything.

Kyran-C commented 5 years ago

Hello, Neovim added text properties (aka extended marks) in 0.4. I'm not sure how the API/implementation differs from Vim's.

I just tried testing again after updating Coc and Neovim and it's no longer working. I have no clue why or how it worked before. I would set let g:lsp_cxx_hl_use_text_props = 1 in my init.vim and on reloading vim it would enable semantic highlighting after a short delay. My color theme was just grayscale so it was obvious when it kicked in. Really strange...

On large files it's way too slow to wait for the highlighting to refresh, so I'll just stick with my grayscale theme until Neovim's text properties mature a bit and you get a chance to add support.

jackguo380 commented 5 years ago

So I looked into the neovim text properties and it appears to refer to an entirely different functionality, mainly attaching indicators/marks to text and such. This is quite different compared to +textprop from vim which currently can only be used to add sticky syntax highlighting. Their APIs are completely different as well.

I implemented support for neovim's equivalent to +textprop using nvim_buf_add_highlight this feature is automatically activated for nvim-0.3.0 or newer. So you should see the highlight stick to text as more lines are added, newly added text may take some time before highlight appears on it.

Let me know if you encounter any issues.

Kyran-C commented 5 years ago

This works great jack, thanks!

d-karl commented 4 years ago

I am having trouble with this. Recent ccls, vim-lsp, Nvim 0.4.2. Error message reads:

Error detected while processing function <SNR>216_hl_symbols_wrap[5]..<SNR>216_hl_symbols[58]..lsp_cxx_hl#textprop_nvim#buf_add_hl_offset[6]..<SNR>218_buf_add_hl:
line   20:
E5555: API call: Column value outside range
Error detected while processing function <SNR>216_hl_symbols_wrap[5]..<SNR>216_hl_symbols[58]..lsp_cxx_hl#textprop_nvim#buf_add_hl_offset[6]..<SNR>218_buf_add_hl:
line   20:
E5555: API call: Column value outside range
Error detected while processing function <SNR>216_hl_symbols_wrap[5]..<SNR>216_hl_symbols[58]..lsp_cxx_hl#textprop_nvim#buf_add_hl_offset[6]..<SNR>218_buf_add_hl:
line   20:
E5555: API call: Column value outside range

Highlighting stops working afterwards, the current file has highlighting that is off, only highlighting the start of some words, the ends of others. What can I do to help you track this down? I have currently gone down before the commit "Add support for neovim text properties(#12)" and everything is working again.

jackguo380 commented 4 years ago

Hi d-karl,

I just tried out my personal setup which uses LanguageClient-neovim + ccls (Most recent version compiled with LLVM 9) and I can edit the ccls code base just fine using a compile_commands.json output by cmake. I don't think the LSP client should affect the correctness since they all share the same parsing and calls to the neovim API.

Are you able to provide some code examples along with their compile_commands.json so I can reproduce it?

If not could you follow the steps shown in :help vim-lsp-cxx-highlight-troubleshooting and provide me with the log file of when you reproduced the issue? vim-lsp should also have its own logging (With all of the LSP messages in it), if you could turn that on and post that log here as well.

Thanks

jackguo380 commented 4 years ago

only highlighting the start of some words

Ah sorry @d-karl I should have realized this sooner given the number of issues that were filed from it. You are missing the initializationOption for using LSP Ranges instead of byte offsets:

{ "highlight": { "lsRanges" : true } }

Not sure which LSP client you are using but see these sample configurations and make sure your initializationOptions matches: vim-lsp LanguageClient-neovim coc.nvim

d-karl commented 4 years ago

That option helped, but didn't fix all of my problems. Highlighting now works once for every file. It does not update after I change the file's content. I only get new highlights after calling LspCxxHighlightDisable, LspCxxHighlightEnable.

Please find the logs attached. vim-lsp.log vim-lsp-cxx-hl.log

Edit: don't deep-dive on this yet, I'll take some time this weekend to investigate further, see if I can provide a minimal vimrc and check whether some other plugin interaction is messing with things.

jackguo380 commented 4 years ago

Is it related to #4? ccls only updates highlighting when you write the file to disk, this is to reduce the overhead of parsing and applying the highlighting.

Looking at the logs vim-lsp received 3 $ccls/publishSemanticHighlight messages and vim-lsp-cxx-highlight received all of them. I am concerned that toggling highlighting on/off is the fix, as you said it definitely points towards some plugin issue. Are you able to try a different LSP client like LanguageClient-neovim? Here's a sample init.vim:

call plug#begin('~/.local/share/nvim/plug')
Plug 'autozimu/LanguageClient-neovim', { 'branch': 'next', 'do': 'bash install.sh' }
Plug 'jackguo380/vim-lsp-cxx-highlight'
call plug#end()

let g:LanguageClient_loggingFile = expand('/tmp/LanguageClient.log')
let g:LanguageClient_loggingLevel = 'DEBUG'

let g:lsp_cxx_hl_log_file = '/tmp/lsp_cxx_hl.log'

let g:lsp_cxx_hl_verbose_log = 1

let s:ccls_settings = {
         \ 'highlight': { 'lsRanges' : v:true },
         \ }

let s:ccls_command = ['ccls', '-init=' . json_encode(s:ccls_settings), 
                  \ '-log-file=/tmp/ccls.log']

let g:LanguageClient_serverCommands = {
      \ 'c': s:ccls_command,
      \ 'cpp': s:ccls_command,
      \ 'objc': s:ccls_command,
      \ }
d-karl commented 4 years ago

Hey, I have your plugin running and using the new text properties when using an older version of vim-lsp. At this point, I am satisfied the problem lies with some recent vim-lsp commits, not with this plugin.

Sorry for the false alarm everything kind of imploded on me when I updated my Plugins..

jackguo380 commented 4 years ago

Its good to hear you got it working again.

Since I am a bit curious, did you bisect which commit in vim-lsp broke this plugin? Maybe there's something I can improve to prevent these kinds of things from happening in the future.

Thanks

d-karl commented 4 years ago

I had little time, but I am more than happy to gift it to plugins I use daily. I did the bisect, identified https://github.com/prabirshrestha/vim-lsp/commit/e6d4367db07ecfdbb40dee8da56e11e117a5c160 as the first bad commit.

jackguo380 commented 4 years ago

Thanks, I investigated the issue and its just a bug on vim-lsp's part. I took a closer look at that commit (prabirshrestha/vim-lsp#455) it turns out the fix is quite simple:

A lambda is being passed into timer_start() but that function calls the lambda and passes in a timer handle which is not what the lambda expects. The fix is to have result come from the outer lambda where result is the JSON message.

diff --git a/autoload/lsp.vim b/autoload/lsp.vim
index ee6f568daf0a7c56e0e1d2b9f3ed823b5f336119..25d97670cc137719b98877a2fd51cfb2885315f0 100644
--- a/autoload/lsp.vim
+++ b/autoload/lsp.vim
@@ -194,7 +194,7 @@ function! s:on_text_document_did_save() abort
         " We delay the callback by one loop iteration as calls to ensure_flush
         " can introduce mmap'd file locks that linger on Windows and collide
         " with the second lang server call preventing saves (see #455)
-        call s:ensure_flush(l:buf, l:server_name, {result->timer_start(0, {result->s:call_did_save(l:buf, l:server_name, result, function('s:Noop'))})})
+        call s:ensure_flush(l:buf, l:server_name, {result->timer_start(0, {timer->s:call_did_save(l:buf, l:server_name, result, function('s:Noop'))})})
     endfor
 endfunction

Feel free to PR it to vim-lsp if you want it fixed now or I can PR when I find some time.

Edit: Made a PR for the fix