From d7651b27d54a87c5783c0a579af11da9a16a39aa Mon Sep 17 00:00:00 2001 From: Gregory Anders <8965202+gpanders@users.noreply.github.com> Date: Wed, 5 Jun 2024 08:27:56 -0500 Subject: [PATCH] fix(tui): move $COLORTERM check to _defaults.lua (#29197) We currently check $COLORTERM in the TUI process to determine if the terminal supports 24 bit color (truecolor). If $COLORTERM is "truecolor" or "24bit" then we automatically assume that the terminal supports truecolor, but if $COLORTERM is set to any other value we still query the terminal. The `rgb` flag of the UI struct is a boolean which only indicates whether the UI supports truecolor, but does not have a 3rd state that we can use to represent "we don't know if the UI supports truecolor". We currently use `rgb=false` to represent this "we don't know" state, and we use XTGETTCAP and DECRQSS queries to determine at runtime if the terminal supports truecolor. However, if $COLORTERM is set to a value besides "truecolor" or "24bit" (e.g. "256" or "16) that is a clear indication that the terminal _does not_ support truecolor, so it is incorrect to treat `rgb=false` as "we don't know" in that case. Instead, in the TUI process we only check for the terminfo capabilities. This must be done in the TUI process because we do not have access to this information in the core Neovim process when `_defaults.lua` runs. If the TUI cannot determine truecolor support from terminfo alone, we set `rgb=false` to indicate "we don't know if the terminal supports truecolor yet, keep checking". When we get to `_defaults.lua`, we can then check $COLORTERM and only query the terminal if it is unset. This means that users can explicitly opt out of truecolor determination by setting `COLORTERM=256` (or similar) in their environment. --- runtime/lua/vim/_defaults.lua | 10 +++-- src/nvim/tui/tui.c | 14 ++----- test/functional/terminal/tui_spec.lua | 55 +++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 14 deletions(-) diff --git a/runtime/lua/vim/_defaults.lua b/runtime/lua/vim/_defaults.lua index 79fe5a8513..01dcd4bf74 100644 --- a/runtime/lua/vim/_defaults.lua +++ b/runtime/lua/vim/_defaults.lua @@ -470,10 +470,14 @@ do --- response indicates that it does support truecolor enable 'termguicolors', --- but only if the user has not already disabled it. do - if tty.rgb then - -- The TUI was able to determine truecolor support + local colorterm = os.getenv('COLORTERM') + if tty.rgb or colorterm == 'truecolor' or colorterm == '24bit' then + -- The TUI was able to determine truecolor support or $COLORTERM explicitly indicates + -- truecolor support setoption('termguicolors', true) - else + elseif colorterm == nil or colorterm == '' then + -- Neither the TUI nor $COLORTERM indicate that truecolor is supported, so query the + -- terminal local caps = {} ---@type table require('vim.termcap').query({ 'Tc', 'RGB', 'setrgbf', 'setrgbb' }, function(cap, found) if not found then diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c index 650133e6a2..57696b1839 100644 --- a/src/nvim/tui/tui.c +++ b/src/nvim/tui/tui.c @@ -1855,20 +1855,12 @@ static int unibi_find_ext_bool(unibi_term *ut, const char *name) return -1; } -/// Determine if the terminal supports truecolor or not: +/// Determine if the terminal supports truecolor or not. /// -/// 1. If $COLORTERM is "24bit" or "truecolor", return true -/// 2. Else, check terminfo for Tc, RGB, setrgbf, or setrgbb capabilities. If -/// found, return true -/// 3. Else, return false +/// If terminfo contains Tc, RGB, or both setrgbf and setrgbb capabilities, return true. static bool term_has_truecolor(TUIData *tui, const char *colorterm) { - // Check $COLORTERM - if (strequal(colorterm, "truecolor") || strequal(colorterm, "24bit")) { - return true; - } - - // Check for Tc and RGB + // Check for Tc or RGB for (size_t i = 0; i < unibi_count_ext_bool(tui->ut); i++) { const char *n = unibi_get_ext_bool_name(tui->ut, i); if (n && (!strcmp(n, "Tc") || !strcmp(n, "RGB"))) { diff --git a/test/functional/terminal/tui_spec.lua b/test/functional/terminal/tui_spec.lua index efa65b7441..80df336cc4 100644 --- a/test/functional/terminal/tui_spec.lua +++ b/test/functional/terminal/tui_spec.lua @@ -2939,6 +2939,61 @@ describe('TUI', function() end) end) + it('does not query the terminal for truecolor support if $COLORTERM is set', function() + clear() + exec_lua([[ + vim.api.nvim_create_autocmd('TermRequest', { + callback = function(args) + local req = args.data + vim.g.termrequest = req + local xtgettcap = req:match('^\027P%+q([%x;]+)$') + if xtgettcap then + local t = {} + for cap in vim.gsplit(xtgettcap, ';') do + local resp = string.format('\027P1+r%s\027\\', xtgettcap) + vim.api.nvim_chan_send(vim.bo[args.buf].channel, resp) + t[vim.text.hexdecode(cap)] = true + end + vim.g.xtgettcap = t + return true + elseif req:match('^\027P$qm\027\\$') then + vim.g.decrqss = true + end + end, + }) + ]]) + + local child_server = new_pipename() + screen = tt.setup_child_nvim({ + '--listen', + child_server, + '-u', + 'NONE', + '-i', + 'NONE', + }, { + env = { + VIMRUNTIME = os.getenv('VIMRUNTIME'), + -- With COLORTERM=256, Nvim should not query the terminal and should not set 'tgc' + COLORTERM = '256', + TERM = 'xterm-256colors', + }, + }) + + screen:expect({ any = '%[No Name%]' }) + + local child_session = n.connect(child_server) + retry(nil, 1000, function() + local xtgettcap = eval("get(g:, 'xtgettcap', {})") + eq(nil, xtgettcap['Tc']) + eq(nil, xtgettcap['RGB']) + eq(nil, xtgettcap['setrgbf']) + eq(nil, xtgettcap['setrgbb']) + eq(0, eval([[get(g:, 'decrqss')]])) + eq({ true, 0 }, { child_session:request('nvim_eval', '&termguicolors') }) + end) + end) + it('queries the terminal for OSC 52 support', function() clear() exec_lua([[