Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toggling 'termguicolors' changes the colorscheme #54

Closed
gdupras opened this issue Feb 9, 2021 · 22 comments · Fixed by #128
Closed

Toggling 'termguicolors' changes the colorscheme #54

gdupras opened this issue Feb 9, 2021 · 22 comments · Fixed by #128
Labels
bug Something isn't working colortemplate colortemplate issue

Comments

@gdupras
Copy link

gdupras commented Feb 9, 2021

Steps to reproduce bugs

vim --clean
:view $VIMRUNTIME/vimrc_example.vim " Just to see some syntax highlighting.
:colorscheme blue
:set termguicolors " The highlighting changes to something that isn't blue.
:colorscheme " Output is 'blue' but it clearly doesn't look like blue.
:colorscheme blue " Now correctly looks like the blue colorscheme.
:set notermguicolors " The highlighting changes to something that isn't blue.
:colorscheme " Output is 'blue' but it clearly doesn't look like blue.
:colorscheme blue " Now correctly looks like the blue colorscheme.

This affects most (all?) the remade colorschemes but not the original ones.

Vim version: 8.2.2488
Terminal: Konsole
OS: Kubuntu 20.10

@gdupras gdupras changed the title Toggling 'termguicolors' changes the colorscheme back to default Toggling 'termguicolors' changes the colorscheme Feb 9, 2021
@gdupras gdupras changed the title Toggling 'termguicolors' changes the colorscheme Toggling 'termguicolors' changes the colorscheme Feb 9, 2021
@romainl
Copy link
Collaborator

romainl commented Feb 9, 2021

Confirmed with 8.2.2164 in iTerm (true colors) and Terminal (not true colors) on macOS. Thank you for spotting this.

@romainl romainl added the bug Something isn't working label Feb 9, 2021
@lifepillar
Copy link
Contributor

lifepillar commented Feb 9, 2021

Well, that is by design. Color schemes generated by Colortemplate are optimized so that they only define the attributes that apply to the current environment. That is to say, if termguicolors is set, when a color scheme is loaded only guifg/guibg/gui attributes are defined, but not ctermfg/ctermbg/cterm, and vice versa. So, if you change the environment at runtime (why would you do that, btw?), you need to reload the color scheme.

@romainl has already criticized this approach (I can't find the discussion right now, but it might be useful to link it here), and I have promised to address this in Colortemplate: once that is done, rebuilding the color schemes would fix this (no change in the templates will be necessary). Life is getting in the way in this period, but I'll try to use some weekends to work on that.

@gdupras
Copy link
Author

gdupras commented Feb 9, 2021

(why would you do that, btw?)

I wanted to check that the color scheme with and without termguicolors looks the same or is similar enough. I would only do this while trying out new color schemes. Once I settle for one of course I then leave termguicolors alone.

@habamax
Copy link
Collaborator

habamax commented Feb 27, 2021

I wanted to check that the color scheme with and without termguicolors looks the same or is similar enough. I would only do this while trying out new color schemes. Once I settle for one of course I then leave termguicolors alone.

for now you can just reapply the same colorscheme after changing termguicolors

PS. But you know this, as far as I can see (missed it from your post)

@habamax
Copy link
Collaborator

habamax commented Oct 20, 2021

Hi @lifepillar, is there anything from your side on this?

@habamax habamax added the colortemplate colortemplate issue label Oct 20, 2021
@lifepillar
Copy link
Contributor

I am not planning to address this in Colortemplate v2, as I don't think it is a high (or even medium) priority issue (I don't see compelling use cases for changing termguicolors at runtime—what was mentioned above is kind of a niche use case, which can be easily addressed by the minor inconvenience of reloading the color scheme).

Colortemplate v3 (which is not published yet—but I am working on it) will provide more flexibility in the generated code, and will fix this.

@habamax
Copy link
Collaborator

habamax commented Oct 20, 2021

Thanks for the update, @lifepillar!

@romainl would we wait for v3 before asking Bram and team to checkout what we have?

@romainl
Copy link
Collaborator

romainl commented Oct 20, 2021

Vimmers have been living with broken colorschemes for decades so they can probably accept that termguicolors-related annoyance until v3 is ready.

@lifepillar
Copy link
Contributor

lifepillar commented Oct 20, 2021

Note that I have no ETA for v3 yet. The new version will be a complete rewriting in Vim9 script, but the template's syntax won't change, at least not in a way that will affect this project—e.g., it most likely won't support a Neovim directive.

The main advantages of v3 will (hopefully) be speed and a better (and I hope smaller) code base. The generated output will change in a way that will fix this issue, or it will be possible to choose between different “optimization levels” for the generated code: I will ask for feedback when I am ready to ship a usable version.

So, if you feel that the color schemes are now up to date (you have been doing a great job btw!), IMO definitely go ahead and send them to Bram. I think it won't be a problem to send further improvements later on, as with other parts of the runtime.

@habamax
Copy link
Collaborator

habamax commented Jan 23, 2022

@jeanluc2020
Copy link

So, if you change the environment at runtime (why would you do that, btw?), you need to reload the color scheme.

I sometimes start vim, editing a file, and later realize this will be a longer editing session and I want to have my terminal back during that session without jumping in and out of vim or suspending it all the time, so i type ":gui" to continue working on the file.
For me personally, that is something common enough to be annoyed by reloading the colorscheme again.

I suggest adding something like this:
augroup COLORSCHEME
au! GuiEnter * exec "colorscheme " . g:colors_name
augroup END

@lifepillar
Copy link
Contributor

Ok, this has come up a few times already… I was not aware of :gui (is there an end to Vim's knowledge?)

So, the solution here is to build the color schemes so that GUI and terminal definitions are both applied, regardless of the values of termguicolors. I was planning to address it for v3, but it's clear that it is needed now. I hope be able to work on that during the weekend. I'll keep you updated.

@romainl
Copy link
Collaborator

romainl commented Jan 24, 2022

Well, there's also the case of Vim honouring g:terminal_ansi_colors even when not in GUI and termguicolors is disabled. Isn't there a line to draw in the sand between what can/should be done at the colorscheme level and what should be fixed in Vim?

IMO…

  • [COLORSCHEME] gui* attributes should be defined unconditionally (this should fix the :gui problem innocuously).
  • [COLORSCHEME] cterm* attributes should be defined against &t_Co: 0-15 if < 256 and 16-255 if >= 256.
  • [COLORSCHEME] g:terminal_ansi_colors should be defined unconditionally.
  • [VIM CORE] g:terminal_ansi_colors should only be honoured by Vim in the documented cases.
  • [VIM CORE] Startup sequence should be the same after $ gvim foo.txt and after :gui.

@habamax
Copy link
Collaborator

habamax commented Jan 24, 2022

For the last 2 we have to create issues against vim repo I guess

lifepillar added a commit to lifepillar/vim-colortemplate that referenced this issue Feb 8, 2022
Some users have complained that setting `termguicolors` *after* loading
the color scheme does not make GUI colors immediately available: rather,
reloading the color scheme is required. Initially, I thought that there
was no sensible use case for setting `termguicolors` after loading
a color scheme, but that is not the case apparently. The most compelling
reason for having GUI colors defined in all sufficiently capable
terminals is Vim's `:gui` command. This is what user jeanluc2020
reported [here](vim/colorschemes#54):

>I sometimes start vim, editing a file, and later realize this will be
>a longer editing session and I want to have my terminal back during that
>session without jumping in and out of vim or suspending it all the time,
>so i type ":gui" to continue working on the file.

This commit implements a quick fix that should solve this issue. This
solution is suboptimal, however, because `gui` and `cterm` definitions
are still kept separate. Ideally, one would want to define each
highlight group only once. But then, is it necessary to define `cterm`
attributes when running in the GUI?
lifepillar added a commit to lifepillar/vim-colortemplate that referenced this issue Feb 8, 2022
Some users have complained that setting `termguicolors` *after* loading
the color scheme does not make GUI colors immediately available: rather,
reloading the color scheme is required. Initially, I thought that there
was no sensible use case for setting `termguicolors` after loading
a color scheme (except for testing purposes), but that is not the case
apparently. The most compelling reason for having GUI colors defined in
all terminals is Vim's `:gui` command. This is what user jeanluc2020
reported [here](vim/colorschemes#54):

>I sometimes start vim, editing a file, and later realize this will be
>a longer editing session and I want to have my terminal back during that
>session without jumping in and out of vim or suspending it all the time,
>so i type ":gui" to continue working on the file.

This commit addresses this issue by defining GUI colors unconditionally
in all environments; cterm and term attributes are instead defined
according to the value of t_Co, as usual.
@lifepillar
Copy link
Contributor

lifepillar commented Feb 8, 2022

The current Colortemplate's master fixes this issue. As proposed by @romainl, gui attributes and g:terminal_ansi_colors are now defined unconditionally.

@lifepillar
Copy link
Contributor

Regarding g:terminal_ansi_colors in particular, please tell me what to do: is it ok to always have that variable defined?

@romainl
Copy link
Collaborator

romainl commented Feb 9, 2022

@lifepillar in theory yes, as it is supposed to be a harmless, passive, global variable. But I found out recently that, at least in some builds, attempts were made by Vim to interpret those colors in situations where it shouldn't, leading to unexpected results.

For that reason I would suggest putting g:terminal_ansi_colors behind a guard that prevents it from being defined outside of the documented scenarios, which are:

  • I am in a GUI Vim,
  • I am running Vim in a terminal emulator, Vim supports termguicolors, and I have enabled it.

This is what I have in Apprentice:

if (has('termguicolors') && &termguicolors) || has('gui_running')
  let g:terminal_ansi_colors = [...]
endif

With that guard in place and the gui attributes set unconditionally, we should be good.

@romainl
Copy link
Collaborator

romainl commented Feb 13, 2022

@jeanluc2020 If you have time, could you try again with the master branch?

@jeanluc2020
Copy link

jeanluc2020 commented Feb 13, 2022

@jeanluc2020 If you have time, could you try again with the master branch?

Thanks, looks much better now, switching from vim inside an xterm to gvim via :gui now works perfectly.

@lifepillar
Copy link
Contributor

Sorry to hijack a closed thread, but regarding this:

at least in some builds, attempts were made by Vim to interpret those colors in situations where it shouldn't, leading to unexpected results.

I haven't been able to reproduce it. I'm using Terminal.app, and /Applications/MacVim.app/Contents/bin/vim (v9.0.1276) launched from a terminal session, but even if my colorscheme unconditionally sets g:terminal_ansi_colors to sixteen copies of the same color, colors in Vim's terminal look just fine:

Apple Terminal MacVim TUI

Here, the shown colors are those from the underlying terminal (which are not the colors of the active colorscheme), as expected. g:terminal_ansi_colors is set to all #11AAFF and it's clearly not used, as expected. termguicolors is off, of course, because Terminal.app does not support it.

Am I missing something?

I am trying to understand whether guarding g:terminal_ansi_colors as suggested above is something that Colortemplate should do.

@lifepillar
Copy link
Contributor

Or perhaps, vim/vim@b2b3acb did fix it?

@romainl
Copy link
Collaborator

romainl commented Jun 2, 2023

@lifepillar I can't reproduce the bug either so it must have been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working colortemplate colortemplate issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants