srcery-colors / srcery-vim

Srcery is a dark color scheme with clearly defined contrasting colors and a slightly earthy tone.
https://srcery.sh
Other
824 stars 60 forks source link

Undercurls stopped working #93

Closed Eternex49 closed 1 year ago

Eternex49 commented 1 year ago

As the title says, undercurls have stopped working. I think this has been going on for a very long time, but I didn't notice it because in the last year I haven't been using nvim a lot. I'm using the 1.0.0 release, which I modified to make the lsp highlight groups use undercurl, instead of underline (I think that setting the undercurl variable didn't work):

call s:HL('DiagnosticUnderlineError', s:bright_red, s:none, s:undercurl)
call s:HL('DiagnosticUnderlineWarn', s:bright_yellow, s:none, s:undercurl)
call s:HL('DiagnosticUnderlineInfo', s:bright_green, s:none, s:undercurl)
call s:HL('DiagnosticUnderlineHint', s:bright_cyan, s:none, s:undercurl)

I'm pretty sure this worked. Has Neovim changed something with respect to the Lsp Highlight groups?

roosta commented 1 year ago

Hi @Eternex49, thanks for reporting. Have you tried the latest version of srcery? the vim.org package is lagging a bit behind.

Also I don't use lsp, but I get undercurl to work fine elsewhere. I'll take a look at see if the lsp groups have changed...

Eternex49 commented 1 year ago

I've updated to the latest version, as you said. Now, the lsp errors and warnings have a white underlining. Searching the repository for lsp, you can see that there are no mentions of it.

roosta commented 1 year ago

Can you check if undercurl works elsewhere?

Eternex49 commented 1 year ago

I'm having trouble to find another program which uses it. The terminfo database, though, has the escape sequences, so undercurl should be supported.

roosta commented 1 year ago

I meant if it worked elsewhere in vim? Are you using it in the terminal? and if so do you use a multiplexer like tmux?

Eternex49 commented 1 year ago

I've tested the terminal with some escape sequences, and undercurl does indeed work. I'm using Neovim in the terminal without a multiplexer (tested this case, and the result is the same). Also, the underline has the same color of the word above it, instead of being, for example, red in case of an error. Could be irrelevant, but there's no mention of the lsp highlight groups in the codebase, whilst in the older versions (before the refactoring) those were explicitly employed.

roosta commented 1 year ago

Don't think the highlight groups matter, the plugin probably links some sane defaults, but could you confirm that undercurl works/doesn't work elsewhere in vim? Spell checking for example should produce undercurl errors.

Eternex49 commented 1 year ago

Spell checking does, indeed, produce undercurls. Also, Tokyonight produces undercurls too. So the problem is definitely within srcery-vim. I think the issue lies within the absence of the lsp highlight groups, which absolutely matter since they are what defines the highlighting behavior.

Eternex49 commented 1 year ago

If I may, the easiest solution would just be to copy-paste the Lsp highlight groups, like they existed before the big refactoring which split the codebase into multiple files. In the current codebase undercurl is set for ALE, Coc and spellcheck's highlight groups.

roosta commented 1 year ago

Yeah, you're right. I wasn't sure if it was a lsp specific issue or a vim wide kinda issue. Check out this pr, will this work?

Eternex49 commented 1 year ago

Yeah, that'll work. It'd be nice if, with the srcery_undercurl variable active, one would get undercurls instead of underlines.

roosta commented 1 year ago

I changed underline to undercurl for lsp, makes more sense in my mind to have errors as undercurl.

I'm gonna merge as is, setting up fallback to underline is not something I could find a good example of in other themes. Underline and undercurl seems linked, and if one works the other probably will too. Either way it would require more thought than I've put in this PR at least.

If you're ok with the latest changes I'm merging.

Eternex49 commented 1 year ago

Absolutely!

Eternex49 commented 1 year ago

Sorry, undercurls don't work because the highlight groups have changed. These are the correct ones:

if has('nvim')
  hi! link DiagnosticError SrceryBrightRed
  hi! link DiagnosticWarn SrceryBrightYellow
  hi! link DiagnosticInfo SrceryBrightGreen
  hi! link DiagnosticHint SrceryBrightCyan
  call s:HL('DiagnosticUnderlineError', s:bright_red, s:none, s:undercurl)
  call s:HL('DiagnosticUnderlineWarn', s:bright_yellow, s:none, s:undercurl)
  call s:HL('DiagnosticUnderlineInfo', s:bright_green, s:none, s:undercurl)
  call s:HL('DiagnosticUnderlineHint', s:bright_cyan, s:none, s:undercurl)
endif

There also appears to be another realted highlight group: DiagnosticOk (and the associated DiagnosticUnderlineOk). Should it be added?

roosta commented 1 year ago

I've setup another PR, where I account for the new hl groups, and also sets up backward compatibility.

Not sure about the DiagnosticOk, I don't use lsp, so not sure how that would look, but maybe you could try it and if you find something you like let me know?

Eternex49 commented 1 year ago

From what I've seen, DiagnosticOk seems to be extremely niche and not even Tokyonight employs it, so it's practically unnecessary. The latest highlight groups, in the new PR, should containDiagnostic and not Diagnostics; then it should be ok.