Fix "normal" highlighting with termguicolors #1344
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nuko8
reviewed
Dec 30, 2016
| @@ -8765,6 +8765,8 @@ hl_combine_attr(int char_attr, int prim_attr) | |||
| else | |||
| { | |||
| vim_memset(&new_en, 0, sizeof(new_en)); | |||
| new_en.ae_u.cterm.bg_rgb = INVALCOLOR; | |||
| new_en.ae_u.cterm.fg_rgb = INVALCOLOR; | |||
There was a problem hiding this comment.
Thank you for the detailed bug report and proposing the patch for that.
Looks some CI tests about the builds not supporting termguicolors failed. To fix that, how about enclosing those two new lines like this?
#ifdef FEAT_TERMGUICOLORS
(your new code)
#endifThere was a problem hiding this comment.
Yes you're quite right!
The 'uninitialised' value for RGB values is INVALCOLOR, but new entries in the cterm_attr_table were initialised to 0, meaning black. Fixes vim#1343
puremourning
force-pushed
the
fix-hl-termguicolors
branch
from
07c5745
to
c825124
Compare
dmerejkowsky
pushed a commit
to dmerejkowsky/vim
that referenced
this pull request
Jan 4, 2017
Initialize to INVALCOLOR instead of zero. (Ben Jackson, closes vim#1344)
desvp
pushed a commit
to desvp/vim
that referenced
this pull request
May 30, 2017
Problem: Normal colors are wrong with 'termguicolors'. Solution: Initialize to INVALCOLOR instead of zero. (Ben Jackson, closes vim#1344)
ckelsel
added a commit
to ckelsel/neovim
that referenced
this pull request
Sep 28, 2017
Problem: Normal colors are wrong with 'termguicolors'. Solution: Initialize to INVALCOLOR instead of zero. (Ben Jackson, closes vim/vim#1344) vim/vim@0cdb72a
ckelsel
added a commit
to ckelsel/neovim
that referenced
this pull request
Oct 7, 2017
Problem: Normal colors are wrong with 'termguicolors'. Solution: Initialize to INVALCOLOR instead of zero. (Ben Jackson, closes vim/vim#1344) vim/vim@0cdb72a
justinmk
pushed a commit
to neovim/neovim
that referenced
this pull request
Oct 7, 2017
see also #7082 Problem: Normal colors are wrong with 'termguicolors'. Solution: Initialize to INVALCOLOR instead of zero. (Ben Jackson, closes vim/vim#1344) vim/vim@0cdb72a
guns
added a commit
to guns/vim
that referenced
this pull request
Nov 22, 2017
* origin/master: (64 commits) patch 8.0.0150: completion for :filter does not skip the pattern patch 8.0.0149: :earlier does not work after reading the undo file patch 8.0.0148: wrong indent in C preprocessor with line continuation patch 8.0.0147: searchpair() fails when 'magic' is off patch 8.0.0146: termguicolors uses wrong colors on MS-Windows with ConEmu patch 8.0.0145: running tests on MS-Windows is noisy Only install coveralls when used. Better solution to make coveralls work again. Tune travis config for coverage with gcc only. Another attempt to make coveralls work. Attempt to make coverage working again. patch 8.0.0144: when using MSVC the GvimExt directory is cleaned twice patch 8.0.0143: line number of current buffer in getbufinfo() is wrong patch 8.0.0142 Problem: Normal colors are wrong with 'termguicolors'. Solution: Initialize to INVALCOLOR instead of zero. (Ben Jackson, closes vim#1344) Updated runtime files. patch 8.0.0141 Problem: Nested function test fails on AppVeyor. Solution: Disable the test on Windows for now. patch 8.0.0140 Problem: Pasting inserted text in Visual mode does not work properly. (Matthew Malcomson) Solution: Stop Visual mode before stuffing the inserted text. (Christian Brabandt, from neovim vim#5709) patch 8.0.0139 Problem: Warning for unused argument. Solution: Add UNUSED. patch 8.0.0138 Problem: Small build fails. Solution: Add #ifdef. patch 8.0.0137 Problem: When 'maxfuncdepth' is set above 200 the nesting is limited to 200. (Brett Stahlman) Solution: Allow for Ex command recursion depending on 'maxfuncdepth'. ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The 'uninitialised' value for RGB values is
INVALCOLOR, but new entries in thecterm_attr_tablewere initialised to 0, meaning black.Fixes #1343
Testing
I have tested this fixes the reported issue, and I have run
make testsuccessfully. However I haven't done extensive regression testing and I have equally not written any new tests. The former I will do in parallel, the latter because I wasn't able to find an obvious way to test this as it pertains to what is put on the screen. If there are any suggestions, I'm happy to write tests.A final test (less canonical) was to install the
vim-solarized8plugin and:set termguicolors,:set cursorline,:colorscheme solarized8_dark, then:so $VIMRUNTIME/syntax/hitest.vim. Before, a number of entries would randomly render in black, now they render correctly. It was using this colorscheme that I noticed the original issue.Otherwise, the repro steps on the attached issue can be used to verify.