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

single line skipped regions do not highlight properly #49

Closed deeveedoublu closed 3 years ago

deeveedoublu commented 3 years ago

Describe the bug As seen in the screenshot below: code is not properly highlighted when the skipped region only has one line between the preprocessor commands. Line 8 should be highlighted the same as lines 3 & 4.

Screenshots example

Configuration:

Log File:

ccls regions:{'uri': 'file:///src/test.c', 'skippedRanges': [{'end': {'character': 6, 'line': 4}, 'start': {'character': 0, 'line': 1}}, {'end': {'character': 6, 'line': 8}, 'start': {'character': 0, 'line': 6}}]}

Potential Fix: Change the following conditional statement from < to <=. This will properly handle the situation where the start line + 2 is equal to the end line. https://github.com/jackguo380/vim-lsp-cxx-highlight/blob/7c47d39d808118f0ef030b15db28ff3995d91cb6/autoload/lsp_cxx_hl/match.vim#L58-L60

deeveedoublu commented 3 years ago

I noticed similar problems when using:

let g:lsp_cxx_hl_use_nvim_text_props = 1

Which leaves me wondering if there is a difference in the way my version of ccls is reporting the skipped region start and end points that may be different from previous versions.

Screenshots: example_text_prop The last line in any skipped region wasn't being highlighted, regardless of length. Lines 4 & 8 should be highlighted the same as line 3.

Potential Fix: Change the following if statement and for loop to only subtract 1 from e_line instead of subtracting 2. https://github.com/jackguo380/vim-lsp-cxx-highlight/blob/7c47d39d808118f0ef030b15db28ff3995d91cb6/autoload/lsp_cxx_hl/textprop_nvim.vim#L45-L50

phush0 commented 3 years ago

Works for me and nvim

jackguo380 commented 3 years ago

Hi, sorry for getting to this so late.

Your fix does look to be correct, I'm guessing the - 2 was a workaround for some bug in neovim or ccls that got fixed in recent versions.

I pushed the changes to master, I'm guessing it resolves this as I'm just implementing what was already tested. If there's any problems please reopen this issue.