Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate color schemes with Vim's check_colors script #130

Open
lifepillar opened this issue Feb 12, 2022 · 8 comments
Open

Validate color schemes with Vim's check_colors script #130

lifepillar opened this issue Feb 12, 2022 · 8 comments

Comments

@lifepillar
Copy link
Contributor

lifepillar commented Feb 12, 2022

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.

@romainl
Copy link
Collaborator

romainl commented Feb 12, 2022

(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
Copy link
Collaborator

romainl commented Feb 12, 2022

@habamax
Copy link
Collaborator

habamax commented Feb 12, 2022

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

@romainl
Copy link
Collaborator

romainl commented Feb 12, 2022

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.

@lifepillar
Copy link
Contributor Author

  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
Copy link
Contributor Author

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

@habamax
Copy link
Collaborator

habamax commented Feb 18, 2022

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

@lifepillar
Copy link
Contributor Author

Likely a typo (should have been just debugPC).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants