Open pi-rho opened 1 year ago
Hi, sorry I didn’t get around to your bug report. Thank you for the PR. I do have some unreleased changes which include a fix (at least a partial one) for that… but I never finished and shipped them. To make amends I was going to just accept the PR even if it doesn’t look entirely as I’d prefer, so at least some fix is shipped, and leave my own preferences for later when I come back to it.
But the more I look at the patch, the more questions I have unfortunately. 🙁
You are obviously right about the t_Co
check.
But about &termguicolors
I’m not so sure. Is it possible for a terminal to advertise only (say) 16 colors yet still support full RGB? That is to say, is it even necessary to check that specifically, or is it just redundant with the t_Co
check?
Also, making the loading of the plugin conditional on the current value of a variable seems like a problem. If the user currently has termguicolors
reset but the terminal has RGB support, shouldn’t the user be able to set termguicolors
and have CSS Color start working – without requiring a restart of Vim? (Well, by that token, the t_Co
check is a problem too. Hmm.)
I don’t feel confident that I have the whole picture reasoned out yet.
As for the css_color#disabled()
part of the patch – a predicate like that makes total sense of course, but it needs a different name because there are already css_color#enable()
and css_color#disable()
functions which do something totally different. Also v:true
and v:false
were not available in Vim 7.0, which this plugin still supports, so it’d have to be just 0 and 1. Anyway, this seems more like a job for a variable, similar to the customary g:loaded_someplugin
variable many plugins have, though loaded
is not the right term here either. I’ll have to think about that one too.
I’ve pushed 950e80352b325ff26d3b0faf95b29e301c200f7d with a minimal fix for the original issue regarding t_Co
to at least get that part of the issue unstuck, since I won’t have time to come back to this until May.
Yes, it is possible. A few examples being alacritty and kitty. Some software blows up if colors
or t_Co
is greater than 256. Another terminfo attribute to look at is pairs
(not sure what the termcap version is called). Alacritty ships both alacritty
and alacritty-direct
.
$ infocmp alacritty -1x | rg 'colors|pairs' # colors = decimal 256
colors#0x100,
pairs#0x7fff,
$ infocmp alacritty-direct -1x | rg 'colors|pairs' # colors > 256 decimal
colors#0x1000000,
pairs#0x7fff,
The terminfo that ships with kitty sets colors
to 256. Both terminals are well capable of using truecolor (24bit). Vim's termguicolors
will use t_8f
and t_8b
values to figure out how to send the 24bit color to the terminal. I think that would be an indicator that the user has a truecolor terminal.
:) when I first started debugging my setup, trying to find why css_colors wasn't working, it took a bit to figure out that it was disabled. I wrote the css_color#disabled()
trying to follow your style. Any kind of indicator seems fine to me.
A few examples being alacritty and kitty.
Have I understood you correctly that those both advertise 256 colors or more? If yes then they would not be examples because what I’m asking is whether there is a terminal where
t_Co
is 16, or 88, or something else less than 256Vim's
termguicolors
will uset_8f
andt_8b
values to figure out how to send the 24bit color to the terminal. I think that would be an indicator that the user has a truecolor terminal.
Meaning that if t_8f
and t_8b
are present, this means the terminal supports RGB colors, and otherwise it doesn’t? I suppose that would be on par with the t_Co
condition.
I wonder if it would be necessary to combine that with a has('terminguicolors')
to ensure that Vim has terminal RGB support, or if it would be redundant because a Vim without terminguicolors
wouldn’t even fetch the t_8f
/t_8b
values in the first place.
I should look at the Vim source but I have to cut this short for now.
https://github.com/ap/vim-css-color/blob/5687a7978bc80263cd03d0a667c2f56890cfb940/autoload/css_color.vim#L6
You disable the plugin if
&t_Co != 256
. This excludes terminals where&t_Co > 256
and doesn't considertermguicolors
. An easy fix would be to use&t_Co >= 256
or include an additional combined test of(has('termguicolors') && &termguicolors)
prior to the&t_Co
test. An additional suggestion is to define another empty function along the lines ofcss_color#i_am_disabled()
so it's obvious. :)Thank you for a very useful plugin.