sainnhe / gruvbox-material

Gruvbox with Material Palette
MIT License
1.96k stars 166 forks source link

Add YcmErrorText and YcmWarningText highlight #161

Closed xpac27 closed 1 year ago

xpac27 commented 1 year ago

Adding missing hl for YouCompleteMe virtual text.

This makes virtual text displayed next to warning and errors colored yellow or red when virtual text is enabled.

2022-12-16_19:53:27

sainnhe commented 1 year ago

Thanks! But there is something needs to be improved.

  1. Add ycm to help doc

https://github.com/sainnhe/gruvbox-material/blob/dca3fbce664de8d52ed5fcfbc0f0bc09d2f8a560/doc/gruvbox-material.txt#L422-L439

  1. Use hi groups defined here:

https://github.com/sainnhe/gruvbox-material/blob/dca3fbce664de8d52ed5fcfbc0f0bc09d2f8a560/colors/gruvbox-material.vim#L452-L462

antoineco commented 1 year ago

Another comment: LSP virtual texts are grey by default, and colors are opt-in via an option.

https://github.com/sainnhe/gruvbox-material/blob/dca3fbce664de8d52ed5fcfbc0f0bc09d2f8a560/doc/gruvbox-material.txt#L381-L385

I think it would be great to make Ycm's behavior consistent with the rest of the theme.

xpac27 commented 1 year ago

@sainnhe I addressed both your suggestions.

Here's the result by default: 2022-12-20_12:47:56

And here's the result with let g:gruvbox_material_diagnostic_virtual_text = 'colored': 2022-12-20_12:48:27

xpac27 commented 1 year ago

@antoineco the text on my firs screenshot is yellow because of a local change I made to my scheme :innocent: sorry for the confusion. My last 2 screenshots above show the actual behavior with g:gruvbox_material_diagnostic_text_highlight = 1. Changing this setting to zero (or removing it) removes the background color. I believe this corresponds to the description.

Here's an other example enabling that setting and looking at both a warning and an error. They are highlighted in yellow and red. However both underlines show as yellow. I'm not sure if this is expected or not :thinking: 2022-12-20_12:59:23

sainnhe commented 1 year ago

LGTM, too. Thanks!