echasnovski / mini.nvim

Library of 40+ independent Lua modules improving overall Neovim (version 0.8 and higher) experience with minimal effort
MIT License
5.21k stars 189 forks source link

Option to disable setup_termbg_sync() compatibility layer with modern terminals #1128

Closed wroyca closed 2 months ago

wroyca commented 2 months ago

Contributing guidelines

Module(s)

misc

Description

Hello o/

Mini parse_osc11 implementation introduce a bug that breaks terminal background "restore" when following system-wide dark mode preferences:

https://github.com/user-attachments/assets/d085e224-a762-47f6-b16e-0e1f568fffd5

This issue does not occur when using the parse_osc11 function directly from https://github.com/neovim/neovim/blob/0d3f2c4904d552635da07a6e2b4e75520b514940/runtime/lua/vim/_defaults.lua#L430

https://github.com/user-attachments/assets/218497d5-5132-49b3-8041-1198ba6319e6

Neovim version

0.10.0

Steps to reproduce

Using e.g. Fedora 40 in VM

cat << EOF >> ~/.config/kitty/kitty.conf
# BEGIN_KITTY_THEME
include current-theme.conf
# END_KITTY_THEME
EOF

mkdir -p ~/.config/kitty/themes

cat << EOF > ~/.config/kitty/themes/neovim-dark.conf
# vim:ft=kitty

# Basic
foreground              #E0E2EA
background              #14161B
selection_foreground    #E0E2EA
selection_background    #4F5258

# Cursor
cursor                  none
cursor_text_color       background

# Border
active_border_color     #E0E2EA
inactive_border_color   #E0E2EA

# Tab
active_tab_foreground   #E0E2EA
active_tab_background   #14161B
inactive_tab_foreground #C4C6CD
inactive_tab_background #2C2E33

# Palette

# black
color0 #07080d
color8 #4f5258

# red
color1 #ffc0b9
color9 #ffc0b9

# green
color2  #b3f6c0
color10 #b3f6c0

# yellow
color3  #fce094
color11 #fce094

# blue
color4  #a6dbff
color12 #a6dbff

# magenta
color5  #ffcaff
color13 #ffcaff

# cyan
color6  #8cf8f7
color14 #8cf8f7

# white
color7  #eef1f8
color15 #eef1f8
EOF

cat << EOF > ~/.config/kitty/themes/neovim-light.conf
# vim:ft=kitty

# Basic
foreground              #14161B
background              #e0e2ea
selection_foreground    #14161B
selection_background    #9B9EA4

# Cursor
cursor                  none
cursor_text_color       background

# Border
active_border_color     #14161B
inactive_border_color   #14161B

# Tab
active_tab_foreground   #14161B
active_tab_background   #E0E2EA
inactive_tab_foreground #2C2E33
inactive_tab_background #C4C6CD

# Palette

# black
color0 #07080d
color8 #4f5258

# red
color1 #590008
color9 #590008

# green
color2  #005523
color10 #005523

# yellow
color3  #6b5300
color11 #6b5300

# blue
color4  #004c73
color12 #004c73

# magenta
color5  #470045
color13 #470045

# cyan
color6  #007373
color14 #007373

# white
color7  #eef1f8
color15 #eef1f8
EOF

cat << EOF > ~/.local/bin/kitty-daemon
#!/usr/bin/env bash

if ! command -v kitty > /dev/null 2>&1; then
  exit 1
fi

gsettings monitor org.gnome.desktop.interface color-scheme | \
  xargs -I % bash -c "$HOME/.local/bin/kitty-color-scheme" &2>>/dev/null true
EOF

cat << EOF > ~/.local/bin/kitty-color-scheme
#!/usr/bin/env bash

if ! command -v kitty > /dev/null 2>&1; then
  exit 1
fi

mode=`gsettings get org.gnome.desktop.interface color-scheme`

d="Neovim-Dark"
l="Neovim-Light"

case $mode in
  *dark* )
    kitty +kitten themes --reload-in=all $d 2>/dev/null
    ;;
  *default* | *light* )
    kitty +kitten themes --reload-in=all $l 2>/dev/null
    ;;
esac
EOF

chmod +x ~/.local/bin/kitty-daemon
chmod +x ~/.local/bin/kitty-color-scheme

cat << EOF > ~/.config/systemd/user/kitty-daemon.service
[Unit]
Description=Foo

[Service]
Type=forking
Environment=\"PATH=$HOME/.local/bin:/usr/local/bin:/usr/bin:/bin\"
ExecStart=$HOME/.local/bin/kitty-daemon
RemainAfterExit=yes

[Install]
WantedBy=default.target
EOF

systemctl --user enable kitty-daemon.service
systemctl --user start kitty-daemon.service

Then run neovim and change desktop to dark or light mode and exit

echasnovski commented 2 months ago

Thanks for the issue!

Mini parse_osc11 implementation introduce a bug that breaks terminal background "restore" when following system-wide dark mode preferences:

Can you, please, clarify what exactly you think is broken? If it is that the color is set back to the light one, then it is by design in order to support wider range of terminal emulators.

This issue does not occur when using the parse_osc11 function directly from https://github.com/neovim/neovim/blob/0d3f2c4904d552635da07a6e2b4e75520b514940/runtime/lua/vim/_defaults.lua#L430

You could not have used parse_osc11 function directly, because it returns incompatible output. Chances are, that the alternative solution uses \027]111\027\\ to restore "original" terminal background color which work well in this scenario. This is not the case with setup_termbg_sync(), which (as said above) is expected.

wroyca commented 2 months ago

Thanks for the issue!

Mini parse_osc11 implementation introduce a bug that breaks terminal background "restore" when following system-wide dark mode preferences:

Can you, please, clarify what exactly you think is broken? If it is that the color is set back to the light one, then it is by design in order to support wider range of terminal emulators.

This issue does not occur when using the parse_osc11 function directly from https://github.com/neovim/neovim/blob/0d3f2c4904d552635da07a6e2b4e75520b514940/runtime/lua/vim/_defaults.lua#L430

You could not have used parse_osc11 function directly, because it returns incompatible output. Chances are, that the alternative solution uses \027]111\027\\ to restore "original" terminal background color which work well in this scenario. This is not the case with setup_termbg_sync(), which (as said above) is expected.

That could be it, indeed, if it return invalid input then it does indeed make sense

If it is that the color is set back to the light one

That'd make sense; would you be open to having a compatibility option that we can opt-out for modern terminals that can handle the initial request properly?

wroyca commented 2 months ago

Sorry for the noise. This can either be closed or converted to a feature request. :)

echasnovski commented 2 months ago

would you be open to having a compatibility option that we can opt-out for modern terminals that can handle the initial request properly?

Sorry, no. I don't changing default terminal color is a frequent enough use case to warrant adding extra configuration. If this is annoying enough for you, I think it would be better to just use custom autocommands:

vim.api.nvim_create_autocmd({ "UIEnter", "ColorScheme" }, {
  callback = function()
    local normal = vim.api.nvim_get_hl(0, { name = "Normal" })
    if not normal.bg then return end
    io.write(string.format("\027]11;#%06x\027\\", normal.bg))
  end,
})

vim.api.nvim_create_autocmd("UILeave", {
  callback = function() io.write("\027]111\027\\") end,
})

Closing as both not an issue and not planned.

wroyca commented 2 months ago

I don't changing default terminal color is a frequent enough use case to warrant adding extra configuration

This is not about changing the default terminal color, this is about how it break every terminal that follow OS dark-light preference, this is non-trivial imo :thinking:

echasnovski commented 2 months ago

This is not about changing the default terminal color, this is about how it break every terminal that follow OS dark-light preference, this is non-trivial imo 🤔

The effect of following OS dark-light preference in terminal emulator is to effectively change its color scheme (which means changing its "default" background color). If using "\027]111\027\" OSC won't help, then there are even more reasons to not support this use case.

wroyca commented 2 months ago

If using "\027]111\027\" OSC won't help, then there are even more reasons to not support this use case.

Are we misunderstanding each other? Just to be clear, \027]111\027\ does work properly—which is basically what I'm asking for here: a way to send it instead of sending, e.g.

    H.termbg_init = H.termbg_init or bg_init
    local reset = function() io.write('\027]11;' .. H.termbg_init .. '\007') end

where H.termbg_init is (e.g.) #162430

echasnovski commented 2 months ago

Are we misunderstanding each other? Just to be clear, \027]111\027\ does work properly

Then my suggestion is to use it inside custom autocommands instead of setup_termbg_sync().

I don't see it as a good idea based solely on this use case to introduce configuration for an approach with less terminal emulator coverage, which should also be implemented, documented (introducing unnecessary confusion), and tested.