colorschemes icon indicating copy to clipboard operation
colorschemes copied to clipboard

Validate color schemes with Vim's check_colors script

Open lifepillar opened this issue 3 years ago • 8 comments

I have checked a few color schemes with check_colors.vim, and they do not pass validation. I am not sure what the status of that script is (I am using Vim 8.2.2683), but I think that at least a few of the warnings need to be taken care of. For instance:

  1. should debugBreakpoint, diffAddeddebugPC and debugBreakpoint be defined in a color scheme? If that is the case, then the check made by the script should be updated; otherwise, those highlight groups should be added to all color schemes.
  2. In some cases, some highlight group definitions are missing. Darkblue lacks ColorColumn's guifg, for example. There are other cases.
  3. In some color schemes (elford, peachpuff—there may be others) Normal is not the first highlight group. I do not remember why, but I think that it is important that Normal is defined first (I think it's explained somewhere in Vim's manual).
  4. check_colors also complains about init: No initialization. I think that here it's the script that is at fault (probably, the check for t_Co raises a false positive).

I think that before merging the color schemes, they should validate. This may require updating check_colors.vim as well as performing some minor adjustments to the color schemes.

lifepillar avatar Feb 12 '22 13:02 lifepillar

(I edited your message to help addressing your points by their number)

  1. My opinion is that every highlight group provided by and documented should be definable. If you can do :help hl-debugBreakpoint, then you can do :hi debugBreakpoint ctermbg=….

    I think check_colors.vim should be modified to make the above possible.

  2. Thank you for the heads-up, I'll double-check.

  3. This is due to the Variant layout. It looks like it is fixable in the offending templates but I wonder if it's something that, maybe should be dealt with in colortemplate?

  4. As I mentioned before, the init check only checks for a very strict block and breaks if you add an empty line. This is very brittle and something that should definitely be fixed in the script.

romainl avatar Feb 12 '22 13:02 romainl

Previous discussions regarding check_colors.vim:

  • https://github.com/vim/colorschemes/issues/35
  • https://github.com/vim/colorschemes/issues/31

Current PR:

  • https://github.com/vim/colorschemes/pull/119

romainl avatar Feb 12 '22 13:02 romainl

I think it already could be included. All the finishing touches we can fix along the way.

habamax avatar Feb 12 '22 13:02 habamax

FWIW, many of the failed checks were due to StatusLine being spelled Statusline (lowercase L).

That check is probably too strict because highlight group names are case-insentive.

romainl avatar Feb 12 '22 14:02 romainl

  1. […] I wonder if it's something that, maybe should be dealt with in colortemplate?

Currently, Colortemplate doesn't try to reorder the highlight groups, so the output is always in source order. One difficulty in automating that stems from the fact that, in general, a template my contain arbitrary verbatim code blocks, so the order of declarations in the source is important. The by now mythical v3 will most likely deal with (3), however, by automatically sorting definitions (but not across verbatim blocks, which act as optimization fences).

For now: it should always be possible to put Normal at the top in the template, given that you may switch between variants as many times as you like.

lifepillar avatar Feb 12 '22 15:02 lifepillar

Ah, and I agree with you about (1) and (4), so check_colors.vim should be updated to solve those.

lifepillar avatar Feb 12 '22 15:02 lifepillar

(1) is not an issue now (debugPC and debugBreakpoint are added to check_colors)? Except I couldn't find diffAddeddebugPC in the docs.

habamax avatar Feb 18 '22 12:02 habamax

Likely a typo (should have been just debugPC).

lifepillar avatar Feb 18 '22 17:02 lifepillar