New colorschemes' t_Co detection is subtly broken for GVim

[ychin]

204 added new t_Co detection logic, and it seems to be subtly broken for Gvim. In GVim, t_Co only gets set after GUI initialization (done in https://github.com/vim/vim/commit/acc770a10f6510), so these colorschemes would only have proper behavior if you do something like color desert in the gvimrc instead of vimrc. If you just set the colorscheme only at the vimrc, t_Co would either be whatever the terminal reported (e.g. 256), or empty if you started GVim without going through a terminal (e.g. double clicking on the binary in the GUI).

Previously (before #204 was implemented), the logic for "empty t_Co" was to treat it as -1, which somehow worked ok (I'm not sure if it was intentional though) for these colorschemes. Now, the logic explicitly sets it to 0, which definitely does not work ok as it falls into the t_Co >= 0 check in the end.

You can repro this if you add colorscheme desert to a vimrc (but not gvimrc), and then launch gvim from the GUI (e.g. "Files" in Ubuntu, Finder in macOS, etc) instead of going through a terminal.

---

Proposed solution

Check has('gui_running') and treat it separately. You can check that GUI is running and explicitly set s:t_Co to 16 million. This way you don't have to rely on the Vim feature of setting t_Co when GUI is running and much less fragile that way.

Do note that I looked around at some popular Vim color schemes and they either have manual "gui_running" check or manually set t_Co to a high number.

---

Alternative solution (just set this in gvimrc)

Just tell people to set their colorscheme in gvimrc. I personally dislike this because to a lot of people this isn't a GUI-specific preference, and it's only GUI-specific because of the implementation-specific way of how Vim initializes the t_Co value etc. This isn't a thing that most users will do because now they will have to set their color scheme both in vimrc and gvimrc. Most people won't even know they are supposed to do this.

Alternative solution (fix this in Vim)

Somehow, fix Vim so that it sets t_Co before GUI init, so that it's always set to 16 million right off the bat. I'm not opposed to this in a way because it seems this will help other color schemes as well, but maybe it breaks the order of operations a little.

[habamax]

@lifepillar, it turned out not the bullet proof, I will try to come up with a better detection.

@ychin thanks for reporting.

Could you please change desert colorscheme, replacing t_Co detection with:

let s:t_Co = exists('&t_Co') && !has("gui_running") ? (&t_Co ?? 0) : -1

[habamax]

The next also works for me:

let s:t_Co = exists('&t_Co') ? (&t_Co ?? -1) : -1

[habamax]

Having colorscheme desert in vimrc running GVim...

variant 1

let s:t_Co = exists('&t_Co') ? (&t_Co ?? -1) : -1

• GUI -- works
• termguicolors -- works
• 256 -- works
• 16 -- works
• 8 -- works
• 0 -- works

Fails if you add set t_Co=0 before colorscheme desert -- endofbuffer is cleared and terminal is cleared.

variant 2

let s:t_Co = exists('&t_Co') && !has("gui_running") ? (&t_Co ?? 0) : -1

• GUI -- works
• termguicolors -- works
• 256 -- works
• 16 -- works
• 8 -- works
• 0 -- works

Survives set t_Co=0 before colorscheme desert.

[habamax]

@ychin I have regenerated colorschemes in a different branch, could you please check it?

@neutaaaaan, @romainl please check them too.

https://github.com/vim/colorschemes/tree/fix/tco-detection

[ychin]

@habamax The new gui_running check works for me and it looks correct now.

Just an aside, I'm not really up to date with the discussions here but what does set t_Co=0 accomplish? That should only matter for terminal Vim right (for dealing with bad terminal detection)? For GUI t_Co should always be 16 million colors and the user can always simply disable syntax highlighting or color schemes.

[habamax]

Just an aside, I'm not really up to date with the discussions here but what does set t_Co=0 accomplish? That should only matter for terminal Vim right (for dealing with bad terminal detection)? For GUI t_Co should always be 16 million colors and the user can always simply disable syntax highlighting or color schemes.

It makes no sense indeed, but if you set it to 0 without gui check, it would softly break colorscheme if let s:t_Co = exists('&t_Co') ? (&t_Co ?? -1) : -1 would be used as t_Co detection.

[habamax]

It makes no sense indeed, but if you set it to 0 without gui check

I mean, if it is possible to set and break, somebody would set and break it :)