sample_spell.vim review #161

gdupras · 2022-04-10T14:53:03Z

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.

The text was updated successfully, but these errors were encountered:

romainl · 2022-04-10T15:09:20Z

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

romainl · 2022-04-10T15:56:34Z

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 · 2022-04-11T11:31:00Z

@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 · 2022-04-11T13:24:44Z

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 · 2022-04-12T00:22:53Z

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.

romainl · 2022-04-12T06:56:21Z

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.

romainl · 2022-04-12T07:49:14Z

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.

lifepillar · 2022-04-12T18:37:01Z

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:

lifepillar · 2022-04-12T19:11:13Z

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 · 2022-04-12T19:24:21Z

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

lifepillar · 2022-04-12T19:28:22Z

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 · 2022-04-12T19:45:50Z

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 · 2022-04-17T13:14:39Z

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

gdupras · 2022-04-19T23:12:51Z

Yes, I will take a look.

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

romainl · 2022-04-20T05:43:20Z

Good news, thanks.

Deal with some of the issues mentioned in #161

gdupras · 2022-05-12T13:51:13Z

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.

romainl · 2022-05-12T14:58:41Z

Indeed it should.

romainl · 2022-05-12T16:56:21Z

Last bug fixed in master.

romainl added a commit that referenced this issue Apr 11, 2022

Deal with some of the issues mentioned in #161

5a8a46e

lifepillar mentioned this issue Apr 12, 2022

Wrong spellcheck colors with termguicolors (fixable with CTRL-L) vim/vim#10177

Closed

romainl added a commit that referenced this issue Apr 17, 2022

Deal with the rest of the issues mentioned in #161

a7ba842

romainl added a commit that referenced this issue May 11, 2022

Merge pull request #162 from vim/161/spell-review

bee87f9

Deal with some of the issues mentioned in #161

romainl added the bug Something isn't working label May 12, 2022

romainl self-assigned this May 12, 2022

romainl closed this as completed May 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sample_spell.vim review #161

sample_spell.vim review #161

gdupras commented Apr 10, 2022

romainl commented Apr 10, 2022

romainl commented Apr 10, 2022 •

edited

Loading

romainl commented Apr 11, 2022 •

edited

Loading

gdupras commented Apr 11, 2022

gdupras commented Apr 12, 2022

romainl commented Apr 12, 2022

romainl commented Apr 12, 2022 •

edited

Loading

lifepillar commented Apr 12, 2022

lifepillar commented Apr 12, 2022 •

edited

Loading

romainl commented Apr 12, 2022

lifepillar commented Apr 12, 2022 •

edited

Loading

lifepillar commented Apr 12, 2022

romainl commented Apr 17, 2022

gdupras commented Apr 19, 2022

romainl commented Apr 20, 2022

gdupras commented May 12, 2022

romainl commented May 12, 2022

romainl commented May 12, 2022

sample_spell.vim review #161

sample_spell.vim review #161

Comments

gdupras commented Apr 10, 2022

blue.vim

gVim

256c

termguicolors

darkblue.vim

gVim

256c

termguicolors

delek.vim

gVim

256c

termguicolors

desert.vim

gVim

256c

termguicolors

elflord.vim

gVim

256c

termguicolors

evening.vim

gVim

256c

termguicolors

industry.vim

gVim

256c

termguicolors

koehler.vim

gVim

256c

termguicolors

morning.vim

gVim

256c

termguicolors

murphy.vim

gVim

256c

termguicolors

pablo.vim

gVim

256c

termguicolors

peachpuff.vim

gVim

256c

termguicolors

ron.vim

gVim

256c

termguicolors

shine.vim

gVim

256c

termguicolors

slate.vim

gVim

256c

termguicolors

torte.vim

gVim

256c

termguicolors

zellner.vim

gVim

256c

termguicolors

romainl commented Apr 10, 2022

romainl commented Apr 10, 2022 • edited Loading

romainl commented Apr 11, 2022 • edited Loading

gdupras commented Apr 11, 2022

gdupras commented Apr 12, 2022

romainl commented Apr 12, 2022

romainl commented Apr 12, 2022 • edited Loading

lifepillar commented Apr 12, 2022

romainl commented Apr 10, 2022 •

edited

Loading

romainl commented Apr 11, 2022 •

edited

Loading

romainl commented Apr 12, 2022 •

edited

Loading

lifepillar commented Apr 12, 2022 •

edited

Loading

lifepillar commented Apr 12, 2022 •

edited

Loading