Should not set the foreground color for undercurl highlights for every terminal

[saviocmc]

First of all, thank you all very much for this great color scheme! I use everywhere.

Now, digging in this repo I noticed this comment here in the helper function that sets the highlight groups:

" Falls back to coloring foreground group on terminals because
" nearly all do not support undercurl

And as the comment says, the code searches for the 'gui_running' flag and as it is false [for terminals], it sets the foreground color of the highlight as the color defined for the undercurl:

if l:special[0] !=# 'NONE' && l:fg[0] ==# 'NONE' && !has('gui_running')
  let l:fg[0] = l:special[0]
  let l:fg[1] = l:special[1]
endif

This code was committed 3 and a half years ago and back there sure terminals did not support undercurls, but today this is no longer the case. Several terminal emulator have full support for colored undercurls, like kitty, iTerm2 and all VTE based terminals, like GNOME Terminal, XFCE Terminal, Tilda, Tilix, Sakura, ...and the list goes on.

Some others like alacritty does not have full support, but gracefully falls back to underlines.

Not having the foreground colored can put vim/neovim on pair with modern IDE's for visualizing undercurls like for spell error or LSP servers diagnostics warnings/errors.

As a hack, I commented out the if statement referenced above in my local copy, and as result I get the desired colored undercurls without changing the syntax highlight of the text/code.

I could not find yet a reliable way to replace the if condition to test for the specific support of the terminal for undercurls, as seams it is not yet standardized, but I think a way should be found as a large number of terminals already have full support.

Thanks again for all the work on Dracula.

[benknoble]

Removing the conditional does manage to break other things for me (not sure what the relation is, though): my custom statusline colors suddenly stop working (they are all just links to Dracula groups).

So I'm not sure what the fix is. There is no reliable way to detect undercurl support, AFAIK, since it's all based on terminfo drawing codes.

[benknoble]

That would be t_Cs and t_Ce for vim; note that these are considered non-standard extensions.

[saviocmc]

Removing the conditional does manage to break other things for me (not sure what the relation is, though): my custom statusline colors suddenly stop working (they are all just links to Dracula groups).

That seems very weird... This conditional should affect only highlights with undercurls, currently DraculaErrorLine, DraculaWarnLine and DraculaInfoLine and others that links to this three. I also use links of Dracula groups in my statusline and did not notice anything unusual. What terminal emulator do you use?

I`m gonna show prints of what this highlights looks like with and without this conditional to me on kitty with spell check enabled:

With the conditional

Without the conditional

I think the second one is the desirable behavior, not overwriting the foreground.

So I'm not sure what the fix is. There is no reliable way to detect undercurl support, AFAIK, since it's all based on terminfo drawing codes.

I realise there is not yet a way do safely detect the support automatically, so I propose as a fix to add a new g: variable (like g:dracula_undercurl_full_support, for example) so the user can set the desired behavior. Also this could be set as has('gui_running') by default, preserving the current default behavior. What do you think? Should I submit a PR implementing this solution?

[benknoble]

I saw the problem on alacritty. Didn't test terminal.app.

I realise there is not yet a way do safely detect the support automatically, so I propose as a fix to add a new g: variable (like g:dracula_undercurl_full_support, for example) so the user can set the desired behavior. Also this could be set as has('gui_running') by default, preserving the current default behavior. What do you think? Should I submit a PR implementing this solution?

That's not… unreasonable… though avoiding a proliferation of config variables for a colorscheme might be nice. Go ahead with the PR—we can have more conversation that way about what works and what doesn't.

[saviocmc]

I saw the problem on alacritty.

Alacritty does not support color independent underline/undercurl yet, but it is under discussion. Currently it uses the foreground color as the underline color. So perhaps for Alacritty the default behavior makes more sense.

(Same file of the above screenshots on Alacritty without the conditional)

I should submit the PR soon.