Errors when opening files using some hilights due to a bug in the X function
tasn opened this issue · 14 comments
Describe the bug
There are errors when opening files using some of the hilight directives. The problem is that the <SID>X function has a bug with how it handles guifg/bg and none.
Take a look at https://github.com/danilo-augusto/vim-afterglow/blob/master/colors/afterglow.vim#L232
If you pass "none" to this function (as the case in SignColumn), you'll get guibg=#none instead of the correct guibg=none. I guess the value for ctermbg is also wrong, just in a different way.
The correct thing would be to fix this function to handle none correctly.
To Reproduce
Open a file that uses SignColumn or many other highlight directives, and you'll get an error.
hint: Waiting for your editor to close the file... Error detected while processing function <SNR>27_X:
line 5:
E254: Cannot allocate colour #none
Expected behavior
Should work.
You are right, those functions expect only hexadecimals (in order to prepend a hashtag), not named colors or none.
I think a more straightforward solution than tweaking the function is simply replacing the "none"s by empty strings ;)
Since we don't use named colors in these places, it's just the calls with "none" that cause us trouble apparently. What do you think?
Hope it is fixed now. Please re-open if necessary.
This patch added slight modifications to very particular highlight groups. For example, TODO/FIXME/XXX appear in yellow background, since apparently this is the default after a cleared highlight. I think I will address this separately. This was "programming by coincidence", which should be avoided ;)
You were right ;)
adapting the method is better after all
thanks! new PR made already. I will merge it and next time you :PlugUpdate it should be all normal again :)
Great, thanks!
Still broken with the same issue... This time it's complaining about none rather than #none.
Upon further inspection, it seems that none is not a legal value for guifg/bg (though it is for ctermfg/bg)!
The correct values for guifg/bg are fg and bg respectively.
Ok, I will take a look. It's strange, I never see these errors. What are the parameters you use?
Not using any parameters. Just using this and a few plugins, and it happens when I run git commit.
It's very easy to reproduce what I'm talking about though. Just open gvim and run hi SomeHighlightName guifg=none and you'll get the error. It should be fg and bg instead of none for gvim, as I said. It's a very simple patch.
I don't have gvim. I try to reproduce it on iTerm + Neovim by issuing :hi Normal guifg=none and nothing happens. I've seen that even if I write "nne" instead of none nothing appears or happens, not even a warning. I will download gvim and try to reproduce it later, using vim instead of neovim. Is it Vim 8?
Yeah it's Vim 8. I'm getting the error in the commandline as well as the gui.
Thanks for taking a look!
Bug found
Indeed it was only working in Neovim since I added "none" in multiple places!
It's turns out "none" is not an officially supported option in Vim. Only "NONE" is supported.
So thank you a lot for opening the issue!
About regressions
Most Vim plugins are currently developed in a vacuum, without much of a framework to test functionalities, like tox for Python, which allows for automated testing for many Python versions and so on.
However, I will now systematically test at least for neovim + vim 8 + macvim (which works like gvim I think).
IMO this will dramatically reduce the number of possible regressions made by a release.
Minor improvement
I also detected a problem for the option let g:afterglow_inherit_background=1 in GUI's (MacVim in my case): putting "NONE" here will make it fallback to a default that is non existent (white in most cases). In the terminal, the ANSI colors of the terminal itself are used, but in the GUI there is no such thing.
And I think it's very upsetting to write set background=dark, open MacVim/gVim and see a white shiny background. For this reason, the plugin will now automatically detect when there's a GUI and if this option is set to 1, it will be ignored and a warning will be stored in the messages. :)
Thanks!
Could you confirm it has finally fixed your problem? I will wait for a confirmation before closing the issue this time.
Works! Thanks! :)
Excellent!