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

Advertise directcolor support in Neovim terminal #24717

Closed
hpjansson opened this issue Aug 15, 2023 · 8 comments · Fixed by #24763
Closed

Advertise directcolor support in Neovim terminal #24717

hpjansson opened this issue Aug 15, 2023 · 8 comments · Fixed by #24763
Labels
enhancement feature request terminal built-in :terminal or :shell

Comments

@hpjansson
Copy link

hpjansson commented Aug 15, 2023

Problem

The Neovim terminal (:terminal) does not appear to advertise directcolor support anywhere when termguicolors is set. This makes it impossible for CLI programs to unambiguously determine whether directcolor sequences can be used when running in the Nvim terminal.

Steps to reproduce:

  1. Install nvim.
  2. Install a program that uses terminal colors, e.g. chafa, from its master branch.
  3. unset COLORTERM NVIM_TUI_ENABLE_TRUE_COLOR (the former may not be present, and it's my understanding that the latter is deprecated).
  4. export TERM=vte. This step probably doesn't matter as long as you indicate an outer terminal that has directcolor support.
  5. Start nvim.
  6. Issue :terminal in Nvim and hit i.
  7. Print something that issues SGR codes, e.g. chafa test.png.
  8. chafa has no way to determine whether the Nvim terminal supports directcolor, so will only use 256 colors. If you try to use directcolor, the output will look corrupted.
  9. Exit Nvim.
  10. Add set termguicolors to init.vim.
  11. Repeat steps 5-8. Observe that chafa will still use only 256 colors, because it can't determine from the environment that directcolor support is now available.
  12. Issue chafa -c full test.png and observe that correct directcolor output is possible if forced.

Additionally, the inner TERM is set to xterm-256color in both cases and there appears to be no other environment variables indicating directcolor support. COLORTERM almost does the job, but Neovim doesn't set it unconditionally.

Expected behavior

Neovim should make it possible for CLI programs to differentiate 256-color from directcolor :terminal environments. The exact mechanism is of lesser importance, but I guess it could either export a different TERM string or set some other environment variable. The goal is to produce optimal output in Neovim without extra prompting from the user.

It could also be resolved with some policy, e.g. "it's the user's responsibility to ensure COLORTERM is set".

A quick test with vim 9.0.1632 showed that it exports COLORS=16777216 when outer TERM=xterm-direct2.

@hpjansson hpjansson added the enhancement feature request label Aug 15, 2023
@zeertzjq zeertzjq added the terminal built-in :terminal or :shell label Aug 15, 2023
@gpanders

This comment was marked as outdated.

@gpanders

This comment was marked as outdated.

gpanders added a commit to gpanders/neovim that referenced this issue Aug 17, 2023
If the outer terminal claims to support truecolor through the $COLORTERM
environment variable, then it should not be modified by Nvim just
because 'termguicolors' is unset.

If an application running in :terminal writes a direct color sequence
(i.e. CSI 38 ; 2 ; <r> ; <g> ; <b> m) that sequence is forwarded
directly to the outer terminal emulator, where it is displayed in
truecolor if the terminal supports it, regardless of the value of
'termguicolors'. The only affect that 'termguicolors' has on colors
for applications in :terminal is how the 16 ANSI colors are displayed
(when 'termguicolors' is set, the ANSI colors are set with
g:terminal_color_x; otherwise, they are forwarded directly to the outer
terminal).

Ref: neovim#13315

Fixes: neovim#24717
@gpanders
Copy link
Member

gpanders commented Aug 17, 2023

The 'termguicolors' setting shouldn't affect direct color sequences in the terminal emulator at all.

This may not actually be true, looking into it further.

@hpjansson In any case, this issue can be closed because Nvim does advertise direct color support via the COLORTERM variable. If the outer terminal defines $COLORTERM and Nvim has 'termguicolors' set, then $COLORTERM will be set to truecolor in :terminal. If 'termguicolors' is unset then $COLORTERM will be 256.

@hpjansson
Copy link
Author

Great, so the resolution is that it's the user's responsibility, then. Thanks for the clarification.

@gpanders
Copy link
Member

Great, so the resolution is that it's the user's responsibility, then.

No I don't think so, unless I'm misunderstanding. From your issue:

Neovim should make it possible for CLI programs to differentiate 256-color from directcolor :terminal environments. The exact mechanism is of lesser importance, but I guess it could either export a different TERM string or set some other environment variable.

It does do this, via the COLORTERM variable. The user does not need to set this themselves. It is done automatically. The application can just check the value of this variable.

@hpjansson
Copy link
Author

neovim/src/nvim/eval/funcs.c

Lines 3985 to 3991 in 88887a7

// Set COLORTERM to "truecolor" if termguicolors is set and 256
// otherwise, but only if it was set in the parent terminal at all
dictitem_T *dv = tv_dict_find(env, S_LEN("COLORTERM"));
if (dv) {
tv_dict_item_remove(env, dv);
tv_dict_add_str(env, S_LEN("COLORTERM"), p_tgc ? "truecolor" : "256");
}

The problem is with "only if it was set in the parent terminal at all". Unfortunately, that can't always be relied on. E.g. openssh preserves TERM but strips COLORTERM from the environment. Then nvim sets TERM to xterm-256color, and now neither env var is usable. The session would look like this:

local$ echo $TERM $COLORTERM
vte-direct truecolor
local$ ssh remote
remote$ echo $TERM $COLORTERM
vte-direct
remote$ nvim
# :terminal
remote-nvim$ echo $TERM $COLORTERM
xterm-256color

The user can work around it either by setting COLORTERM in their remote shell profile or adding it to sshd's AcceptEnv list, if they have the permissions to do so.

@gpanders
Copy link
Member

gpanders commented Aug 17, 2023

I'm trying to think of an example where setting $COLORTERM regardless of whether or not it is set in the parent terminal would be bad.

If $COLORTERM is not set in the parent terminal, it might mean that the terminal does not support 24-bit color. But it could also just mean that the terminal doesn't set it, or the user is in an SSH session (as you said).

If the user has 'termguicolors' set then we should trust them that 24 bit color is supported. If it isn't, then the colors in Nvim are going to look bad already anyway, so using direct colors in the terminal emulator won't make it any worse.

I didn't see any discussion in #13315 around the reasoning for only setting $COLORTERM if it was already set in the parent terminal, so I'm not sure if there are any gotchas I'm not thinking of.

But we could try it out and see what (if anything) breaks.

@gpanders gpanders reopened this Aug 17, 2023
gpanders added a commit to gpanders/neovim that referenced this issue Aug 17, 2023
$COLORTERM is set in the terminal emulator based on the value of
'termguicolors' ("truecolor" if &tgc is set, 256 otherwise), but ONLY if
$COLORTERM is also set in the parent terminal emulator.

This is an unnecessary restriction that can cause issues in some cases.
For instance, $COLORTERM is stripped by default by OpenSSH, so is not
present in an SSH session. The terminal emulator still supports 24 bit
color, so the presence of $COLORTERM is not a very reliable indicator.

Instead, setting it unconditionally based on 'termguicolors' uses the
user's own preferences to infer if 24-bit color is supported. If
'termguicolors' is set in a terminal that does not support true color
then the colors in Nvim will already look bad. Enabling them for
applications in the terminal emulator will not make it any worse.

Fixes: neovim#24717
@hpjansson
Copy link
Author

Thanks a lot, @gpanders!

From f_termopen() it looks like the inner terminal sets TERM=xterm-256color always, so COLORTERM is only really needed when the user wants directcolor on top of that. If it's left unset in 256-color mode, that's fine from my point of view.

gpanders added a commit to gpanders/neovim that referenced this issue Aug 21, 2023
$COLORTERM is set in the terminal emulator based on the value of
'termguicolors' ("truecolor" if &tgc is set, 256 otherwise), but ONLY if
$COLORTERM is also set in the parent terminal emulator.

This is an unnecessary restriction that can cause issues in some cases.
For instance, $COLORTERM is stripped by default by OpenSSH, so is not
present in an SSH session. The terminal emulator still supports 24 bit
color, so the presence of $COLORTERM is not a very reliable indicator.

Instead, setting it unconditionally based on 'termguicolors' uses the
user's own preferences to infer if 24-bit color is supported. If
'termguicolors' is set in a terminal that does not support true color
then the colors in Nvim will already look bad. Enabling them for
applications in the terminal emulator will not make it any worse.

Do not set COLORTERM to 256 if 'termguicolors' is not enabled, because
the underlying terminal may not support 256 colors. $TERM is set to
xterm-256color by default, which is sufficient to allow applications
running in :terminal to determine 256 color support.

Fixes: neovim#24717
gpanders added a commit to gpanders/neovim that referenced this issue Aug 21, 2023
$COLORTERM is set in the terminal emulator based on the value of
'termguicolors' ("truecolor" if &tgc is set, 256 otherwise), but ONLY if
$COLORTERM is also set in the parent terminal emulator.

This is an unnecessary restriction that can cause issues in some cases.
For instance, $COLORTERM is stripped by default by OpenSSH, so is not
present in an SSH session. The terminal emulator still supports 24 bit
color, so the lack of $COLORTERM is not a reliable indicator. When an
application runs in Nvim's :terminal it thus has no way to know whether
or not true color is supported.

Instead, setting it unconditionally based on 'termguicolors' uses the
user's own preferences to infer if 24-bit color is supported, rather
than depending on the (unreliable) presence of $COLORTERM. If
'termguicolors' is set in a terminal that does not support true color
then the colors in Nvim will already look bad. Enabling them for
applications in the terminal emulator will not make it any worse.

If 'termguicolors' is not set then the value of $COLORTERM from the
parent terminal (if any) is forwarded to Nvim's :terminal.

Fixes: neovim#24717
gpanders added a commit that referenced this issue Aug 21, 2023
$COLORTERM is set in the terminal emulator based on the value of
'termguicolors' ("truecolor" if &tgc is set, 256 otherwise), but ONLY if
$COLORTERM is also set in the parent terminal emulator.

This is an unnecessary restriction that can cause issues in some cases.
For instance, $COLORTERM is stripped by default by OpenSSH, so is not
present in an SSH session. The terminal emulator still supports 24 bit
color, so the lack of $COLORTERM is not a reliable indicator. When an
application runs in Nvim's :terminal it thus has no way to know whether
or not true color is supported.

Instead, setting it unconditionally based on 'termguicolors' uses the
user's own preferences to infer if 24-bit color is supported, rather
than depending on the (unreliable) presence of $COLORTERM. If
'termguicolors' is set in a terminal that does not support true color
then the colors in Nvim will already look bad. Enabling them for
applications in the terminal emulator will not make it any worse.

If 'termguicolors' is not set then the value of $COLORTERM from the
parent terminal (if any) is forwarded to Nvim's :terminal.

Fixes: #24717
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature request terminal built-in :terminal or :shell
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants