Lyude / neovim-gtk

gtk ui for neovim
GNU General Public License v3.0
158 stars 10 forks source link

Underlines are red unless a color is explicitly set #10

Closed BoltsJ closed 2 years ago

BoltsJ commented 2 years ago

Describe the bug Unless the guisp color is explicitly set, underlines and undercurls are highlighted red in nvim-gtk, where they inherit the foreground color for terminal nvim. This seems to be a side effect of #3

Expected behavior Inherit the foregroud color unless guisp is set.

Left is nvim-gtk, right is nvim in gnome-terminal: image

Technical information (please complete the following information):

Lyude commented 2 years ago

JFYI - I'm still looking into this, currently though I'm trying to figure out where the actual red color is really coming from. At first glance at the discussion in #3 it seems like this would obviously be:

https://github.com/Lyude/neovim-gtk/blob/cad3a4872419c1f5ba5f3cf31fb1d858f812fbd6/src/highlight.rs#L33

But changing this to COLOR_WHITE doesn't make any difference. I'm still trying to track down where the COLOR_RED is actually coming from, I've at least figured out that it is seemingly likely that we're receiving the value from nvim itself, as I'm able to see the color red coming in to nvim_gtk::shell::State::default_colors_set(). Going to set help needed here in the mean time, just in case someone else comes across this issue and actually knows what might be going on.

jacobmischka commented 2 years ago

Interesting, I assumed that default would be causing it as well. I guess it must a configuration default somewhere, if you see it coming from neovim itself. You can investigate your highlight settings to see if it's being set somewhere with :hi.

Discrepancies between terminal nvim and nvim-gtk are because the latter correctly uses guisp, terminals do not use that setting. A better comparison would be with another GUI wrapper, such as possibly even gvim if your configuration is compatible with stock vim, or another neovim wrapper like gnvim.

Honestly, I think the actual solution is to configure your guisp values correctly instead of relying on defaults, or pick a colorscheme that does that in a way you like by default.

BoltsJ commented 2 years ago

Discrepancies between terminal nvim and nvim-gtk are because the latter correctly uses guisp, terminals do not use that setting.

Gnome Terminal (and I believe any lib-vte based terminal) does correctly use guisp. (See the screenshot in https://github.com/daa84/neovim-gtk/issues/262).

A better comparison would be with another GUI wrapper

neovim-qt and neovide both inherit the underline color from the fg color if it's not explicitly set.

jacobmischka commented 2 years ago

I stand corrected!

The color is indeed coming from nvim via the default_colors_set event. I'm not sure why that is, though, and why other GUIs seem to ignore the special value defined there?

BoltsJ commented 2 years ago

Looking into this more, it looks like nvim-qt actually has the old behavior (ignores guisp for underline) and also uses red for guisp (on undercurl) if its not explicitly set. I also grabbed and compiled gnvim and it behaves the same as this project.

I'll have to poke around with my highlights and figure it out. It's not so bad for the things where I'm defining the foreground colors alongside the underline, but combined highlights are going to be a bit tricky to figure out.

Lyude commented 2 years ago

Looking into this more, it looks like nvim-qt actually has the old behavior (ignores guisp for underline) and also uses red for guisp (on undercurl) if its not explicitly set. I also grabbed and compiled gnvim and it behaves the same as this project.

I'll have to poke around with my highlights and figure it out. It's not so bad for the things where I'm defining the foreground colors alongside the underline, but combined highlights are going to be a bit tricky to figure out.

Yeah… JFYI - I think if that doesn't work, a good next step would probably be to see if we can trace where in nvim's source the red value is coming from using gdb, because I have a feeling this might just be a bug with nvim itself. (if folks aren't familiar with how to do this, I don't mind trying to get to it the next chance I get)

jacobmischka commented 2 years ago

Based on the nvim test suite, I think this is actually intentional behavior: https://github.com/bfredl/neovim/blob/8cca9daa6d80bb9eeda9627a2089134a095d6063/test/functional/ui/screen_basic_spec.lua#L979-L985

Lyude commented 2 years ago

Based on the nvim test suite, I think this is actually intentional behavior: https://github.com/bfredl/neovim/blob/8cca9daa6d80bb9eeda9627a2089134a095d6063/test/functional/ui/screen_basic_spec.lua#L979-L985

Good to know. A possible solution I've been wondering about with this issue is using the ext_termcolors UI option so that we just receive -1 whenever we get unset colors, so that in the absence of a color we can inherit either red by default for undercurl, or inherit the default foreground color for underlines. So I might try messing with this at some point if no one else gets to it first.

Lyude commented 2 years ago

Good to know. A possible solution I've been wondering about with this issue is using the ext_termcolors UI option so that we just receive -1 whenever we get unset colors, so that in the absence of a color we can inherit either red by default for undercurl, or inherit the default foreground color for underlines. So I might try messing with this at some point if no one else gets to it first.

Surprise - turns out the answer is yes, and this seems to fix the issue entirely