[vim/vim] Store extended colors in v:colornames dict. (#8761)
Drew Vogel
Problem solved: Remembering RGB colors is difficult.
Solution: Allow user to (re-)define new color names.
This patch is a follow-up to the discussion on #8502.
A central question in that discussion was how users would put the new v:colornames dict to use and whether a single shared dict was appropriate. My own opinion is that a single shared dict is simultaneously the most approachable and the most usable. I've extended my previous work in an attempt to demonstrate why I think that is the case.
- I've expanded the documentation with examples of how to update the dict in different scenarios.
- I've created a proof-of-concept vim-colorwheel plugin that demonstrates how users might collaborate on color schemes using this shared dict.
- I've fully removed the
rgb.txtfile while retaining the same list of colors.- That special case file has been replaced with a more generic color list script convention. In order to guard against users mistakenly thinking they should remove colors from
v:colornames, thergb.txtcolors are put into a default color list, which is re-executed when a user would have expected thergb.txtcolors to be valid. - I've updated the Haiku color code to use the common gui color code. I've built this on Haiku and conducted simple manual tests but I don't have much expertise with Haiku, so it is possible I've missed some nuance there.
- That special case file has been replaced with a more generic color list script convention. In order to guard against users mistakenly thinking they should remove colors from
Finally, the color name code has been moved into highlight.c as suggested by @brammool. Thanks to Bram, @chrisbra, and @romainl for the helpful feedback on the last PR. I'm looking forward to any feedback you all have on this update/alternative.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/8761
Commit Summary
- Store extended colors in v:colornames dict.
File Changes
- M Filelist (2)
- M READMEdir/README_extra.txt (1)
- M nsis/gvim.nsi (3)
- M runtime/colors/README.txt (3)
- A runtime/colors/lists/csscolors.vim (161)
- A runtime/colors/lists/default.vim (2359)
- M runtime/doc/eval.txt (37)
- M runtime/doc/gui_w32.txt (18)
- M runtime/doc/message.txt (5)
- M runtime/doc/os_haiku.txt (5)
- M runtime/doc/syntax.txt (65)
- M runtime/doc/usr_06.txt (25)
- D runtime/rgb.txt (782)
- M src/Makefile (11)
- M src/evalvars.c (3)
- M src/gui_haiku.cc (159)
- M src/highlight.c (233)
- M src/po/ca.po (3)
- M src/po/da.po (3)
- M src/po/de.po (3)
- M src/po/eo.po (3)
- M src/po/fi.po (3)
- M src/po/fr.po (3)
- M src/po/ga.po (3)
- M src/po/it.po (3)
- M src/po/ja.euc-jp.po (3)
- M src/po/ja.po (3)
- M src/po/ja.sjis.po (3)
- M src/po/ko.UTF-8.po (3)
- M src/po/ko.po (3)
- M src/po/pt_BR.po (3)
- M src/po/ru.cp1251.po (3)
- M src/po/ru.po (3)
- M src/po/sr.po (3)
- M src/po/tr.po (3)
- M src/po/uk.cp1251.po (3)
- M src/po/uk.po (3)
- M src/proto/highlight.pro (6)
- M src/proto/term.pro (2)
- M src/term.c (197)
- M src/testdir/test_highlight.vim (24)
- M src/vim.h (3)
Patch Links:
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
codecov[bot]
Codecov Report
Merging #8761 (c07d147) into master (b033ee2) will decrease coverage by
87.69%.
The diff coverage is1.31%.
❗ Current head c07d147 differs from pull request most recent head 0241aa1. Consider uploading reports for the commit 0241aa1 to get more accurate results
@@ Coverage Diff @@ ## master #8761 +/- ## =========================================== - Coverage 90.15% 2.46% -87.70% =========================================== Files 151 149 -2 Lines 170639 165393 -5246 =========================================== - Hits 153839 4073 -149766 - Misses 16800 161320 +144520
| Flag | Coverage Δ | |
|---|---|---|
| huge-clang-none | ? |
|
| huge-gcc-none | ? |
|
| huge-gcc-testgui | ? |
|
| huge-gcc-unittests | 2.46% <1.31%> (+<0.01%) |
⬆️ |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/highlight.c | 7.62% <0.00%> (-82.85%) |
⬇️ |
| src/memline.c | 0.00% <ø> (-88.77%) |
⬇️ |
| src/term.c | 0.00% <ø> (-83.30%) |
⬇️ |
| src/userfunc.c | 0.18% <0.00%> (-95.10%) |
⬇️ |
| src/version.c | 0.86% <ø> (-91.48%) |
⬇️ |
| src/vim9compile.c | 0.00% <0.00%> (-93.81%) |
⬇️ |
| src/evalvars.c | 5.18% <100.00%> (-91.30%) |
⬇️ |
| src/float.c | 0.00% <0.00%> (-99.22%) |
⬇️ |
| src/gui_gtk_f.c | 0.00% <0.00%> (-97.54%) |
⬇️ |
| src/sound.c | 0.00% <0.00%> (-97.12%) |
⬇️ |
| ... and 144 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 b033ee2...0241aa1. Read the comment docs.
Drew Vogel
@chrisbra I finally had a chance to fix the test failures on this branch. Mind approving the test run again?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Christian Brabandt
of course!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Bram Moolenaar
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?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Drew Vogel
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.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Drew Vogel
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.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Bram Moolenaar
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
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Bram Moolenaar
Setting and restoring 'cpo' will also be needed in csscolors.vim
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Bram Moolenaar
Drew - have you looked into fixing the line continuation?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Drew Vogel
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.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Drew Vogel
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.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Bram Moolenaar
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.