search

vim / colorschemes

colorschemes for Vim
279 stars 23 forks source link

sample_spell.vim review #161

Closed gdupras closed 2 years ago

gdupras commented 2 years ago

The Normal, String, and Comment groups are the ones that are most often spell checked so the Spell groups should look good on those.

For this review, I used sample_spell.vim as modified in PR #154.

For each colorscheme, I looked at three environments:

The above commands are typed in Konsole, on Kubuntu. &t_Co is 256 by default in this terminal.

There is clearly something very wrong with termguicolors. The other two environments look fine for the most part but will still need some small tweaks.

blue.vim

gVim

blue_gvim

256c

blue_256c

termguicolors

blue_termguicolors

darkblue.vim

gVim

darkblue_gvim

256c

darkblue_256c

termguicolors

darkblue_termguicolors

delek.vim

gVim

delek_gvim

256c

delek_256c

termguicolors

delek_termguicolors

desert.vim

gVim

desert_gvim

256c

desert_256c

termguicolors

desert_termguicolors

elflord.vim

gVim

elflord_gvim

256c

elflord_256c

termguicolors

elflord_terguicolors

evening.vim

gVim

evening_gvim

256c

evening_256c

termguicolors

evening_termguicolors

industry.vim

gVim

industry_gvim

256c

industry_256c

termguicolors

industry_termguicolors

koehler.vim

gVim

koehler_gvim

256c

koehler_256c

termguicolors

koehler_termguicolors

morning.vim

gVim

morning_gvim

256c

morning_256c

termguicolors

morning_termguicolors

murphy.vim

gVim

murphy_gvim

256c

murphy_256c

termguicolors

murphy_termguicolors

pablo.vim

gVim

pablo_gvim

256c

pablo_256c

termguicolors

pablo_termguicolors

peachpuff.vim

gVim

peachpuff_gvim

256c

peachpuff_256c

termguicolors

peachpuff_termguicolors

ron.vim

gVim

ron_gvim

256c

ron_256c

termguicolors

ron_termguicolors

shine.vim

gVim

shine_gvim

256c

shine_256c

termguicolors

shine_termguicolors

slate.vim

gVim

slate_gvim

256c

slate_256c

termguicolors

slate_termguicolors

torte.vim

gVim

torte_gvim

256c

torte_256c

termguicolors

torte_termguicolors

zellner.vim

gVim

zellner_gvim

256c

zellner_256c

termguicolors

zellner_termguicolors

romainl commented 2 years ago

Thank you for this thorough report. I will take a look at it right now.

romainl commented 2 years ago

Let's see… the default colors are:

Case cterm light cterm dark gui light gui dark
Cap lightblue blue X11 blue X11 blue
Local cyan cyan X11 darkcyan X11 cyan
Rare lightmagenta magenta X11 magenta X11 magenta
Bad lightred red X11 red X11 red

I think we should try to stick with those as much as possible and only deviate when strictly necessary. Maybe not the values but… their spirit : blues for cap, cyans for local, magentas for rare, and reds for bad.

FWIW, here are the default values as established in xterm's source:

Name X11 name Hex Equiv 256c index Equiv 256c hex
blue blue2 #0000ee 21 #0000ff
lightblue rgb:5c/5c/ff #5c5cff 63 #5f5fff
cyan cyan3 #00cdcd 44 #00d7d7
magenta magenta3 #cd00cd 164 #d700d7
lightmagenta magenta #ff00ff 201 #ff00ff
red red3 #cd0000 160 #d70000
lightred red #ff0000 196 #ff0000

and the default values as established in rgb.txt:

Name Hex value Equiv 256c index Equiv 256c hex
blue #0000ff 21 #0000ff
cyan #00ffff 51 #00ffff
darkcyan #008b8b 30 #008787
magenta #ff00ff 201 #ff00ff
red #ff0000 196 #ff0000

And here is the default "spelling" palette, normalized for 256c: https://color.hailpixel.com/#5F5FFF,0000FF,00D7D7,008787,FF00FF,D700D7,FF0000,D70000

romainl commented 2 years ago

@gdupras FWIW, I can reproduce the termguicolors issue but it goes away after a ^L or some visual selection so it looks more like a redrawing issue to me than like a real issue with these colorschemes.

Maybe that's something worth investigating in its own right.

gdupras commented 2 years ago

Hmm, weird. I cannot reproduce the termguicolors issue in cmd.exe on Windows 10. I'll have to try again in Konsole later today once I get back home.

gdupras commented 2 years ago

With Konsole, the termguicolors issue does not go away with ^L or with a visual selection.

With a visual section, the problematic Spell groups don't even change colors. Here's an example with blue.vim:

Screenshot_20220411_201725

I have no idea what is causing this.

romainl commented 2 years ago

I also have similar troubles with my own Apprentice in iTerm with termguicolors enabled.

Before ^L:

Capture d’écran 2022-04-12 à 08 51 26

After some random click with the mouse to put the window into focus:

Capture d’écran 2022-04-12 à 08 54 43

After ^L:

Capture d’écran 2022-04-12 à 08 53 21

This is puzzling.

romainl commented 2 years ago

Here is a reduced version of Apprentice with only the strict necessary for testing sample_spell.vim: spellingbee.vim.txt

How it looks in 256c:

Capture d’écran 2022-04-12 à 09 28 08

How it looks in GUI:

Capture d’écran 2022-04-12 à 09 29 16

How it looks with set termguicolors:

Capture d’écran 2022-04-12 à 09 31 01

And after ^L:

Capture d’écran 2022-04-12 à 09 31 39

Note that I recently switched to colortemplate. In case that was relevant, I dug out the last official release of Apprentice (note to self: It's freaking old, I should make a new one), which was written differently, and the result is the same.

FWIW, I also tried with a very popular colorscheme, nord.vim, and spell highlighting is also broken, though slightly differently:

Capture d’écran 2022-04-12 à 09 46 33
lifepillar commented 2 years ago

I can reproduce it in iTerm2 with my own color schemes (which do not visually distinguish among the different spelling errors).

WWDC16

At startup:

Screenshot 2022-04-12 at 20 27 21

After CTRL-L:

Screenshot 2022-04-12 at 20 27 37

WWDC17

At startup:

Screenshot 2022-04-12 at 20 30 41

After CTRL-L:

Screenshot 2022-04-12 at 20 30 57
lifepillar commented 2 years ago

FWIW, a vimrc containing only these lines:

set termguicolors
set spell
call setline(1, 'couleur')

is enough to reproduce the issue with:

vim --clean -S spellingbee.vim -S vimrc

but not with:

vim --clean -S spellingbee.vim -u vimrc

I propose to change the command to run the spelling test to:

vim --clean -S colors/blue.vim -u colors/tools/sample_spell.vim

unless there is a valid reason for the current syntax. Note -u instead of -S.

Edit: Ah, I see, linked groups are not linked if sample_spell.vim is loaded first.

romainl commented 2 years ago

Calling the sample with -u is a good idea. I think a truecolors-enabled variant of sample_spell.vim might be in order.

lifepillar commented 2 years ago

Maybe something like this could be added to sample_spell.vim:

set rtp^=.
execute "colo" $COLORSCHEME

Then call it with:

$ COLORSCHEME=blue vim --clean -u colors/tools/sample_spell.vim

Still not perfect, but just to point out another possibility.

Edit: Or this:

set rtp^=.
execute "colo" c

to be called as follows:

$ vim --clean -u colors/tools/sample_spell.vim --cmd 'let c="blue"'
lifepillar commented 2 years ago

I think a truecolors-enabled variant of sample_spell.vim might be in order.

Now I see that nothing of what I have proposed above prevents the issue when termguicolors is set. I need to look at it with a fresh mind. Maybe it is something worth reporting upstream.

I don't know whether you mean that a different script is needed for termguicolors. I'd try to stick with one script for all use cases.

romainl commented 2 years ago

@gdupras I think I have fixed all the fixable issues. Could you check it out?

gdupras commented 2 years ago

Yes, I will take a look.

The issues with termguicolors have been fixed upstream in 8.2.4775 and 8.2.4777.

romainl commented 2 years ago

Good news, thanks.

gdupras commented 2 years ago

Sorry, I never had time for a full review but I did notice that in koehler.vim, the color of guifg and guisp do not match for SpellCap:

koehler

guisp should probably be made the same color as guifg.

romainl commented 2 years ago

Indeed it should.

romainl commented 2 years ago

Last bug fixed in master.