Codecov Report

Merging #8761 (5adc3a2) into master (844fb64) will increase coverage by 0.02%.
The diff coverage is 67.16%.

❗ Current head 5adc3a2 differs from pull request most recent head 9b8d88b. Consider uploading reports for the commit 9b8d88b to get more accurate results

@@            Coverage Diff             @@
##           master    #8761      +/-   ##
==========================================
+ Coverage   90.10%   90.13%   +0.02%     
==========================================
  Files         151      151              
  Lines      169669   169787     +118     
==========================================
+ Hits       152879   153030     +151     
+ Misses      16790    16757      -33     

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 844fb64...9b8d88b. Read the comment docs.

@chrisbra I finally had a chance to fix the test failures on this branch. Mind approving the test run again?

of course!

For default.vim, instead of calling has_key() for every entry, you can use extend(v:colornames, {dict with values}, 'keep')
That should be much faster.

I notice that the loopup in rgb_table[] just uses a linear search. A binary search would be much better.
It is unrelated to v:colornames, could leave this as a todo item.

Otherwise, I'll leave this open for remarks. Is using v:colornames the best solution?

My original motivation for using has_key in default.vim was to remain vi compatible. I'd be fine using extend there however I tried it recently and it is significantly slower, to the point of causing the CI jobs to time out: https://github.com/dvogel/vim/actions/runs/1238981661.

I've pushed an additional commit that requires uses extend for better performance. The trade-off is that the default colors now require vi-incompatible line continuations. This was at the heart of why the tests were hanging with my first attempt to use extend the Test_background_foreground test relies on the color ivory. Launching that test in nocompatible mode allows it to run without hanging the test suite.

See the runtime files for examples of how the 'cpo' option is changed and restored to allow for line continuation. Basically:

let s:cpo_save = &cpo
set cpo&vim

commands

let &cpo = s:cpo_save
unlet s:cpo_save

Setting and restoring 'cpo' will also be needed in csscolors.vim

Drew - have you looked into fixing the line continuation?

Yeah the color lists included in the patch all use the line continuations with the compatibility flag save/restore pattern you suggested. I haven't had a chance to look into the merge conflicts that arose from the refactor of the highlighting code yet.

Looks like it was just a conflict due to a tiny comment correction I made. I've rebased this branch atop the master branch, squashing my commits in the process.