sample_spell.vim review

Question

sample_spell.vim review

gdupras opened this issue 3 years ago · 18 comments

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:

gVim:

gvim --clean -S colors/blue.vim -S colors/tools/sample_spell.vim
256c:

vim --clean -S colors/blue.vim -S colors/tools/sample_spell.vim
termguicolors:

vim --clean -c "set termguicolors" -S colors/blue.vim -S colors/tools/sample_spell.vim

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

SpellLocal looks like String.

256c

SpellLocal looks like String.

termguicolors

Bad SpellCap and SpellBad.
SpellLocal does not match gVim version.

darkblue.vim

gVim

OK

256c

OK

termguicolors

Bad SpellRare and SpellBad.

delek.vim

gVim

SpellBad looks like Comment.

256c

SpellBad looks like Comment.

termguicolors

Bad SpellCap, SpellLocal, SpellRare, and SpellBad.

desert.vim

gVim

SpellRare looks like String.

256c

SpellRare looks like String.

termguicolors

SpellBad does not match gVim version.

elflord.vim

gVim

SpellCap is too dark.
SpellRare looks like String.

256c

SpellCap is too dark.
SpellRare looks like String.

termguicolors

Bad SpellCap, SpellLocal, SpellRare, and SpellBad.

evening.vim

gVim

OK

256c

OK

termguicolors

Bad SpellCap, SpellLocal, and SpellBad.

industry.vim

gVim

SpellRare looks like String.

256c

SpellRare looks like String.

termguicolors

Bad SpellLocal, SpellRare, and SpellBad.

koehler.vim

gVim

SpellCap is too dark.

256c

SpellCap is too dark.

termguicolors

Bad SpellCap, SpellLocal, SpellRare, and SpellBad.

morning.vim

gVim

SpellCap looks like String.

256c

SpellCap looks like String.
SpellLocal and SpellBad are very similar.

termguicolors

Bad SpellCap, SpellRare, and SpellBad.
SpellLocals does not match gVim version.

murphy.vim

gVim

SpellCap looks like Comment.

256c

SpellCap looks like Comment.

termguicolors

Bad SpellCap, SpellRare, and SpellBad.

pablo.vim

gVim

SpellRare looks like String.

256c

SpellRare looks like String.

termguicolors

Bad SpellLocal, SpellRare, SpellBad.
SpellCap does not match gVim version.

peachpuff.vim

gVim

OK

256c

OK

termguicolors

Bad SpellCap, SpellLocal, and SpellBad.
SpellRare does not match gVim version.

ron.vim

gVim

SpellCap is too dark.

256c

Normal and String look the same
SpellCap is too dark.

termguicolors

Normal and String look the same
Bad SpellCap, SpellLocal, SpellRare, and SpellBad.

shine.vim

gVim

OK

256c

OK

termguicolors

Bad SpellCap, SpellLocal, SpellRare, and SpellBad.

slate.vim

gVim

OK

256c

OK

termguicolors

Bad SpellCap and SpellBad.

torte.vim

gVim

OK

256c

OK

termguicolors

Bad SpellLocal, SpellRare, and SpellBad.
SpellCap does not match gVim version.

zellner.vim

gVim

SpellBad looks like Comment.
SpellRare looks like String.

256c

SpellBad looks like Comment.
SpellRare looks like String.
SpellLocal and SpellBad look very similar.

termguicolors

Bad SpellCap, SpellRare, and SpellBad.
SpellLocal does not match gVim version.

Answer 1 · 2022-04-10T15:09:20.000Z

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

Answer 2 · 2022-04-10T15:56:34.000Z

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

Answer 3 · 2022-04-11T11:31:00.000Z

@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.

Answer 4 · 2022-04-11T13:24:44.000Z

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.

Answer 5 · 2022-04-12T00:22:53.000Z

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:

I have no idea what is causing this.

Answer 6 · 2022-04-12T06:56:21.000Z

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

Before ^L:

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

After ^L:

This is puzzling.

Answer 7 · 2022-04-12T07:49:14.000Z

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:

How it looks in GUI:

How it looks with set termguicolors:

And after ^L:

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:

Hypothesis number 1: something is broken upstream.
Hypothesis number 2: sample_spell.vim is broken.

Answer 8 · 2022-04-12T18:37:01.000Z

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

WWDC16

At startup:

After CTRL-L:

WWDC17

At startup:

After CTRL-L:

Answer 9 · 2022-04-12T19:11:13.000Z

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.

Answer 10 · 2022-04-12T19:24:21.000Z

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

Answer 11 · 2022-04-12T19:28:22.000Z

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"'

Answer 12 · 2022-04-12T19:45:50.000Z

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.

Answer 13 · 2022-04-17T13:14:39.000Z

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

Answer 14 · 2022-04-19T23:12:51.000Z

Yes, I will take a look.

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

Answer 15 · 2022-04-20T05:43:20.000Z

Good news, thanks.

Answer 16 · 2022-05-12T13:51:13.000Z

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:

guisp should probably be made the same color as guifg.

Answer 17 · 2022-05-12T14:58:41.000Z

Indeed it should.

Answer 18 · 2022-05-12T16:56:21.000Z

Last bug fixed in master.