kovidgoyal / kitty

Cross-platform, fast, feature-rich, GPU based terminal
https://sw.kovidgoyal.net/kitty/
GNU General Public License v3.0
22.51k stars 914 forks source link

Errors with neovim when termguicolors is set #160

Closed schaden-freude closed 6 years ago

schaden-freude commented 6 years ago

I am using the onedark colorscheme in neovim. If I set the option termguicolors then kitty borks and throws [PARSE ERROR] Invalid character in CSI: 0x3a, ignoring the sequence repeatedly.

Here is a comparison with terminator(left) and kitty. 2017-11-01-131749_1366x768_scrot

In case I don't set termguicolors in my .vimrc, then it kinda works, but the colors are quite different (I believe the colorscheme then uses 256 colors instead of truecolor.)

kovidgoyal commented 6 years ago

You need to set the correct t_8f and t_8b settings in vim as well. From my .vimrc

            set termguicolors
            let &t_8f = "\e[38;2;%lu;%lu;%lum"
            let &t_8b = "\e[48;2;%lu;%lu;%lum"

IIRC these are the defaults in vim so you are probably changing them in either .vimrc or some plugin.

kovidgoyal commented 6 years ago

Looking at vim help, those are only set if TERM is xterm, so presumably terminator sets term to xterm.

schaden-freude commented 6 years ago

It works for me on vim without setting t_8f and t_8b.

However on neovim, despite setting t_8f and t_8b, I have the same error.

kovidgoyal commented 6 years ago

Presumably neovim does not honor those settings. From the error message kitty prints, neovim is using the colon form of the escape code instead of the semi-colon form. The semi-colon form is the correct form. See http://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Functions-using-CSI-_-ordered-by-the-final-character_s_

You should report the bug to neovim.

kovidgoyal commented 6 years ago

To be precise the semi-colon form is mandated by ISO-8613-3

kovidgoyal commented 6 years ago

And as further evidence, a quick glance at https://gist.github.com/XVilka/8346728#now-supporting-truecolour shows you that all terminals that support truecolor support the semi-colon form and many do not support the colon form at all. So I have no idea why neovim chose to use the colon form.

justinmk commented 6 years ago

Neovim only uses the colon form for terminals that it thinks support it. ISO-8613-3 does not mandate semicolon, in fact it's the opposite:

http://invisible-island.net/xterm/xterm.faq.html

But semicolon is pervasive and frankly I wish we hadn't wasted time trying to support the "correct" (colon) form.

The semi-colon form is the correct form. See http://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Functions-using-CSI-_-ordered-by-the-final-character_s_

From that URL:

ISO-8613-3 can be interpreted in more than one way; xterm allows the semicolons in this control to be replaced by colons (but after the first colon, colons must be used).

DarkDefender commented 6 years ago

It's not too late to remove the colon support and be done with it... ;)

kovidgoyal commented 6 years ago

Really, from just about every reference I have seen ISO-8613 mandates semi-colons. But since no ine is going to pay to get access to the standards documents, who knows :)

Just read the list at https://gist.github.com/XVilka/8346728 every single terminal supports semi-colons and only a subset support colons. There is absolutely no reason to use colons. It's a trivial two line patch for me to add support for colons to kitty, but I'd rather not, since it adds an extra branch to parsing of the escape code for no good reason.

DarkDefender commented 6 years ago

@kovidgoyal Yes, that is what I argued when colon support were added.

I think it's madness to complicate the code when it seems like the de facto standard of semi colons has already been decided. We'll see if rolling back to only supporting semi colons goes better this time.

You should not have to add support for colons.

DarkDefender commented 6 years ago

I've opened up a pull request that should fix this in neovim https://github.com/neovim/neovim/pull/7482

kovidgoyal commented 6 years ago

Thanks :)

justinmk commented 6 years ago

There is absolutely no reason to use colons

That's not exactly true, AFAIU semicolons are ambiguous in some cases.

kovidgoyal commented 6 years ago

I dont know of any such cases. In fact, all other CSI codes that need to be parsed by a terminal emulator use semi-colons. Using colons for this one variant makes no sense. Run your eye over this list and you will see there are no colons anywhere: http://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Functions-using-CSI-_-ordered-by-the-final-character_s_

schaden-freude commented 6 years ago

Okay so I tried neovim/neovim#7482 and it works fine for me.

However, kitty now throws a [PARSE ERROR] Unknown CSI code: 0x74 each time neovim is opened. This doesn't happen with vim.

kovidgoyal commented 6 years ago

That just means neovim is using the CSI code to manipulate the graphical window. kitty does not support that. Indeed, since I always use tiling window managers, kitty cannot support that, as it has no way to control its size/position. I dont know why neovim is using that code. Perhaps to query the window title? You'd have to ask @justinmk

kovidgoyal commented 6 years ago

And with this commit: https://github.com/kovidgoyal/kitty/commit/013fd31493100d9c41b656734778ecee7914b41f

you should get more details about the unknown CSI code.

justinmk commented 6 years ago

Kitty should ignore unexpected CSI codes, not show anything to normal users.

kovidgoyal commented 6 years ago

It does not show anything to normal users. You only see them if you look at its stdout/stderr.

justinmk commented 6 years ago

For reference, regarding the colon question see https://bugzilla.gnome.org/show_bug.cgi?id=685759

We overlooked the detail about ":" as a subparameter delimiter (documented in 5.4.t2 in ECMA-48). Some discussion in KDE in mid-2006 led Lars Doelle to discuss the issue with me. Lars' initial concern dealt with the fact that a sequence such as CSI 38 ; 5 ; 1 m violated the principle that SGR parameters could be used in any order. Further discussion (see KDE #107487) resolved that the standard expected that the sequence would look like CSI 38 ; 5 : 1 m which still violates that principle, since the "5:1" parameter has to follow the "38" to be useful.

This function accepts either format (per request by Paul Leonerd Evans). It also accepts CSI 38 : 5 : 1 m according to Lars' original assumption.

And https://bugzilla.gnome.org/show_bug.cgi?id=704449 discusses the ambiguous cases:

accepting ':' instead of ';' as the delimiter after CSI 38/48, something we should do too. The problem with semicolon is that it's supposed to delimit properties that are independent from each other and take no additional parameters. E.g. vte currently doesn't recognize ^[[38;2;1;3;4 as a true color escape sequence (rgb:1/3/4), but instead it switches to bold(1), italic(3) and underline(4) since it has no clue how many parameters 38;2 is supposed to take.

egmontkob commented 6 years ago

It's a trivial two line patch for me to add support for colons to kitty,

I don't know anything about kitty's internals, so maybe it's really only 2 lines of code, but I have a bit of a doubt here. At least in VTE, it was significantly more complicated than that.

The catch is: You cannot handle ":" the same way as you do ";", that would give us two equivalent separators for no good reason at all and the core problem would remain unsolved. The point is that ":" must only be accepted as a separator between subparameters, and must be refused as a separator at the outer level, that is, between independent attributes.

but I'd rather not, since it adds an extra branch to parsing of the escape code for no good reason.

The very reason is to avoid the mess we're currently having if there's ever again an extension, let's say "38;6;1;2;3;4;5". We can't foresee how many parameters it'll take, and as such, cannot tell where the list continues with other, independent values. Going instead with a syntax like "38:6:1:2:3;4;5" you can already tell where "38:6:" ends and you can already tell that "1" is not bold/bright, "2" is not dim, "3" is not italic, but 4 is underline and 5 is blink; without having any clue whatsoever what the heck "38:6" will mean in the future and how many parameters it'll take. As such, with this syntax you can safely ignore the bits that aren't (yet) supported, and at the same time, correctly process the rest.

Therefore I blame all the terminal emulators that support ";" and not ":" for keeping this mess alive for future extensions, and not putting an end to it now. (Needless to say, it was messed up by whoever first thought it was a good idea to take semicolon-separated subparameters.) Of course, it would only work if all terminal emulators supported ":", or at least a critical majority of them supported so that apps wouldn't revert for the easy solution of ";" but would rather insist on using ":" and would push the remaining terminal emulators to change their behavior. Given that kitty is by far not the only emulator not supporting colon, I don't see a reasonable chance for this to happen, I'm afraid we'll stick with those stupid semicolons and will face this mess again whenever some new extension is added.

kovidgoyal commented 6 years ago

:) I see we are coming from very different places. To me, SGR codes are a legacy mess (and this is not the fault of the codes themselves, but of history, and differing implementations in the age before frictionless communication). Generally, better to start in a clean namespace with an escape code design that is well documented (with documentation that is not behind a paywall) and efficient to parse and that comes with a test suite.

As for the actual problem you point out. If you really wanted to extend the SGR codes further you simply specify that parameters after 38, 48 and 58 (for colored underlines) have special semantics. Then you can extend the other codes to your hearts desire. Of course, this complicates parser implementations, but it is no worse than specifying that the colon has special semantics different from the semi-colon, either.

The semi-colon horse has already left the barn. There is no practical way anyone can change all terminal implementations at this point, not to mention the actual terminal emulators in use by users.

So, to re-iterate, in the world we have right now, semi-colons work everywhere, colons work in some places.

egmontkob commented 6 years ago

Generally, better to start in a clean namespace with an escape code design that is well documented (with documentation that is not behind a paywall) and efficient to parse and that comes with a test suite.

Would be nice to have, although I see an even much smaller chance for this than for proper introduction of ":".

The semi-colon horse has already left the barn.

For true colors, yup, it has. For potential future extensions, not necessarily. Although I can't foresee any potential extension (as subcommands of 38 and 48), and indeed the motivation in various terminal emulators' developers is damn low.

There is no practical way anyone can change all terminal implementations at this point, not to mention the actual terminal emulators in use by users.

Ideally, if everyone agreed that we should make an effort here, or if there was some common standard that released a new version which marked ";" as obsolete in favor of ":", it could reasonable be done. Of course you'd still need to wait for a few years before switching the behavior of apps. It's quite hypothetical, I know that (unfortunately – if you ask me) it's most likely not going to happen.

So, to re-iterate, in the world we have right now, semi-colons work everywhere, colons work in some places.

Exactly.

kovidgoyal commented 6 years ago

Well, for my part, to be a good citizen, I have added colon support to the SGR parsing code in kitty. It has the good side-effect of making kitty robust against apps like neovim that implement colon output based on fragile capability detection heuristics. https://github.com/kovidgoyal/kitty/commit/e90aaa847047c4dd680d8d92ccb5ac30f25e46e7

@XVilka You might want to update your list accordingly.

egmontkob commented 6 years ago

Awesome, thanks a lot! :)

justinmk commented 6 years ago

@kovidgoyal Thanks! We will work on better detection in Neovim, where possible.

XVilka commented 6 years ago

@kovidgoyal thanks, updated!

ik9999 commented 6 years ago

Im having the same problem with neovim:

[PARSE ERROR] Unknown CSI code: 't' with start_modifier: '' and end_modifier: '' and parameters: '0 159 45'
[PARSE ERROR] Unknown CSI code: 't' with start_modifier: '' and end_modifier: '' and parameters: '27 159 45'
[PARSE ERROR] Unknown CSI code: 't' with start_modifier: '' and end_modifier: '' and parameters: '27 159 45'
[PARSE ERROR] Unknown CSI code: 't' with start_modifier: '' and end_modifier: '' and parameters: '27 159 45'
[PARSE ERROR] Unknown CSI code: 't' with start_modifier: '' and end_modifier: '' and parameters: '27 159 45'

justinmk commented 6 years ago

@ik9999 unknown CSI codes are not a problem unless they actually cause a problem. Log messages are just log messages.

ik9999 commented 6 years ago

I see the black random squares on a screen. Anyway, solved with TERM=xterm-kitty.

kovidgoyal commented 6 years ago

@justinmk I am curious to know what neovim is using those codes for. That appears to be https://vt100.net/docs/vt510-rm/DECSLPP.html Is neovim trying to resize the window? Should it really be doing that?

mtreca commented 6 years ago

I'm experiencing the same issue in both vim and neovim, either when re-centering the cursor using zz or sometimes when inserting a newline, even without any plugin or vimrc (vim -u NONE). I used a different colorscheme to show the issue more clearly. See below (from left to right):

2017-11-22-104308_1920x1080_scrot

Redrawing the screen using CTRL-L removes the blocks, but it is annoying to do so, especially since it sometimes hides entire lines of text in the buffer.

I have the following lines in (n)vimrc:

if $TERM == "xterm-kitty"
    let &t_8f = "\e[38;2;%lu;%lu;%lum"
    let &t_8b = "\e[48;2;%lu;%lu;%lum"
endif

and term in my kitty.conf is set to xterm-kitty

ik9999 commented 6 years ago

@vxid Im not sure about term variable in kitty.conf, try to run nvim like this

TERM=xterm-kitty nvim

This helped me.

mtreca commented 6 years ago

@ik9999 Thanks for the suggestion. Unfortunately it doesn't help.

kovidgoyal commented 6 years ago

Add this to your .vimrc

        " vim hardcodes background color erase even if the terminfo file does
        " not contain bce (not to mention that libvte based terminals
        " incorrectly contain bce in their terminfo files). This causes
        " incorrect background rendering when using a color theme with a
        " background color.
        let &t_ut=''
mtreca commented 6 years ago

Thanks for the quick feedback. Setting this option completely fixes my issues with Vim. However, using Neovim, I still get redrawing issues hiding lines in the current buffer:

2017-11-22-121419_1920x1080_scrot

One of the lines is completely hidden until I redraw the screen with CTRL-L. The issue is that when I use a colorscheme matching my terminal colors, it is impossible to know if a line is hidden or not until I redraw.

justinmk commented 6 years ago

Nvim doesn't support the t_xx options. We will need to work around this internally.

What I don't understand is why kitty doesn't like background color erase.

kovidgoyal commented 6 years ago

Why does nvim like bce? Implementing bce means that erasing requires setting values on each cell individually. Without it, erase is a simple memset(0). The correct way to implement a color scheme with a background color is to use the control codes to change the default background/foreground colors. For instance, from my .vimrc

    " Set the terminal default background and foreground colors, thereby
    " improving performance by not needing to set these colors on empty cells.
    hi Normal guifg=NONE guibg=NONE ctermfg=NONE ctermbg=NONE
    let &t_ti = &t_ti . "\033]10;#f6f3e8\007\033]11;#242424\007"
    let &t_te = &t_te . "\033]110\007\033]111\007"

This means that the client program such as nvim can use the simple erase command and the terminal emulator such as kitty can use memset(0) to do the erasing.

kovidgoyal commented 6 years ago

Also, nvim should only use bce if the terminfo file advertises that the terminal supports it, which kitty's does not.

justinmk commented 6 years ago

Also, nvim should only use bce if the terminfo file advertises that the terminal supports it, which kitty's does not.

Some logging in tui.c shows that Nvim is working correctly. Question for the reports above, is whether kitty terminfo file is available. E.g. on a SSH session.

mtreca commented 6 years ago

I managed to narrow the issue down. :set relativenumber in Neovim seems to cause the issue. Steps to reproduce:

nvim -u NONE
:set relativenumber
iddd<ESC>
yy12p
o<CR>

The above is enough to make a line disappear.

Note that there is no issue with set number.

As for the terminfo file, it is available. I use kitty-git from the AUR but I was able to reproduce the issue building directly from source.

egmontkob commented 6 years ago

nvim should only use bce if the terminfo file advertises that the terminal supports it

That's absolutely correct!

Why does nvim like bce?

bce is a truly problematic (mis)feature, whoever designed it must have been out of their minds. I can perfectly understand why certain terminal emulators (kitty, screen, tmux) prefer not to have it.

See VTE bug 754596 for my opinion, and for how (and why) we intentionally defer from xterm to hopefully suck less.

Quick summary:

Back to \e[K: This, in combination with bce, is required for an app (that wishes to have a custom background color, and not via OSC 11 which I'll again get back to shortly) to control how trailing spaces are copy-pasted. As far as I know, screen drawing libraries (ncurses, slang) don't allow you to control this. Apps not relying on ncurses (such as vim, neovim, less, joe etc.) might do this probably though. I'm not a (neo)vim user so I can't tell if it's done correctly there, but it could. By correctly I mean that copy-pasting should carry trailing spaces exactly as present in the file. Without bce, an app has two choices: either use OSC 11, or explicitly fill up with spaces which will always be copy-pasted then. One could of course argue that copy-pasting code with the mouse is problematic anyways in a couple of other ways (e.g. TAB characters, lines that don't fit in the current width etc.) so that not properly copy-pasting trailing whitespaces might not be a big deal.

Another problematic corner case with \e[K along with bce is: What to do when (after some explicit cursor positioning and bgcolor change and \e[K) the "rest" of the line (after the last character that is to be copy-pasted) contains multiple colors? Should this be supported? Also, what to do on rewrap-on-resize if the emulator supports that?

The correct way to implement a color scheme with a background color is to use the control codes to change the default background/foreground colors [...] \033]11;#242424\007

I firmly disagree with this.

This might be a nice approach if we only cared about alternate screen apps, we could be certain that the user didn't use these escape sequences to set their favorite background color (e.g. to have a slightly random, different background for each tab) or there would be a way to stack (push/pop) colors, and also we could afford not to care about apps that potentially crash and don't properly clean up after themselves (okay, a crashing app is problematic in many other ways too so let's leave this out).

Many people prefer their editors not to switch to full screen, so that their contents remain visible after they quit. Any palette-like approach (like OSC 10/11) break here big time, quitting the app recolors the remains of the screen. Also while using the app, scrolling back produces incorrect colors for the previous contents in the scrollback. (xterm allows you to scroll back with the mouse even if the alternate screen is present, and see the normal screen's history. So even if neovim switches to the alternate screen, scrolling back in xterm would be broken.) Any approach where an app is designed to (as a side effect) retroactively change the color of any previous output is broken beyond repair. If an app wants to paint its area blue, it should paint its area blue, and not recolor everything that's ever been printed to the terminal. For me, OSC 4/10/11 only makes a tiny bit of sense to be used from users' own hacks in .bashrc and alike.

OSC 11 can only be used if you're planning to have a single background across your entire app. Lines of different colors (e.g. the top and bottom bars of Midnight Commander) can't be handled this way, they have to be manually filled with spaces or bce'd all the way through. If some lines need to be treated that way, there's not much point having an alternate method too.

Plus, OSC 11 requires an absolute RGB value to use rather than (what most apps would like to have) an index from the 256-color palette. Otherwise, how could an app be confident that the RGB default background and palette-based foregrounds and one-off backgrounds look nicely together for everyone? At least requires to avoid using the first 16 colors which typically differ across systems and users, and stick with the additional 240 (which aren't typically modified) or direct RGB colors.

Another minor difference is that most terminal emulators have a thin border around the cell grid, and OSC 11 changes the color of that while painting all the cells with the given background doesn't. Here one could argue that OSC 11's approach looks better.

And let's also mention that OSC 4/10/11/etc. are not supported by plenty of terminal emulators.

Anyway, my whole point is that this palette-like approach is so utterly brain-damaged in general. I've written about it on some other bug entries / forum posts too. It's disappointing to see how much the terminal emulation world is resisting to get over it, while desktops have left behind the palette-mode about 20 years ago, and in some ways, the 8 bit charsets -> UTF-8 transition was something analoguous too (there shouldn't be any global palette setting effecting every pixel or every character, everything should go down individually to each pixel or each character).

Implementing bce means that erasing requires setting values on each cell individually. Without it, erase is a simple memset(0).

Ease of implementation to this extreme (memset vs. for-loop) shouldn't matter at all in any such discussion about this truly problematic area, and I do sincerely hope this wasn't your primary reason for not supporting bce :-)


So what could be done to fix this mess?

justinmk commented 6 years ago

@egmontkob and @kovidgoyal thanks for the insights. Nvim's built-in terminfo definitions assumed BCE. https://github.com/neovim/neovim/pull/7624 changes that, @vxid can you try it?

kovidgoyal commented 6 years ago

On Wed, Nov 22, 2017 at 08:55:36PM +0000, egmontkob wrote:

The correct way to implement a color scheme with a background color is to use the control codes to change the default background/foreground colors [...] \033]11;#242424\007

I firmly disagree with this.

This might be a nice approach if we only cared about alternate screen apps, we could be certain that the user didn't use these escape sequences to set their favorite background color (e.g. to have a slightly random, different background for each tab) or there would be a way to stack (push/pop) colors, and also we could afford not to care about apps that potentially crash and don't properly clean up after themselves (okay, a crashing app is problematic in many other ways too so let's leave this out).

IME only full screen apps need to set default background colors. Crashing apps can break all sorts of things, colors is the least of them. As for apps that dont clean up after themselves, handling that is not a terminal emulators responsibility, and indeed is generally impossible to do.

As for overwriting colors the user has set via OSC 11, that is trivially handled by querying the terminal emulator for the current value of OSC 11 and restoring to that instead of null on exit.

Many people prefer their editors not to switch to full screen, so that their contents remain visible after they quit. Any palette-like approach (like OSC 10/11) break here big time, quitting the app recolors the remains of the screen. Also while using the app, scrolling back produces incorrect colors for the previous contents in the scrollback. (xterm allows you to scroll back with the mouse even if the alternate screen is present, and see the normal screen's history. So even if neovim switches to the alternate screen, scrolling back in xterm would be broken.) Any approach where an app is designed to (as a side effect) retroactively change the color of any previous output is broken beyond repair. If an app wants to paint its area blue, it should paint its area blue, and not recolor everything that's ever been printed to the terminal. For me, OSC 4/10/11 only makes a tiny bit of sense to be used from users' own hacks in .bashrc and alike.

That some terminal emulators decide to allow scrollback in alternate screen mode, is a bug in those terminal emulators. Not a reason not to use OSC 11.

OSC 11 can only be used if you're planning to have a single background across your entire app. Lines of different colors (e.g. the top and bottom bars of Midnight Commander) can't be handled this way, they have to be manually filled with spaces or bce'd all the way through. If some lines need to be treated that way, there's not much point having an alternate method too.

Yes, but transmitting a screen full of spaces vs a line full of them is a big performance difference on a slow network. Not to mention that a line with spaces is semantically different than an empty line. This affects copying via mouse selection, for example.

Plus, OSC 11 requires an absolute RGB value to use rather than (what most apps would like to have) an index from the 256-color palette. Or, how could an app be confident that the RGB default background and palette-based foregrounds and one-off backgrounds look nicely together for everyone? At least requires to avoid using the first 16 colors which typically differ across systems and users, and stick with the additional 240 (which aren't typically modified) or direct RGB colors.

Sure, I have no objection to extending OSC 11 to allow palette colors.

Another minor difference is that most terminal emulators have a thin border around the cell grid, and OSC 11 changes the color of that while painting all the cells with the given background doesn't. Here one could argue that OSC 11's approach looks better.

OSC 11 should not change that, and does not change it in kitty.

And let's also mention that OSC 4/10/11/etc. are not supported by plenty of terminal emulators.

Well, tons of things are not supported by tons of emulators. If we wait for widespread support, we will never progress.

Anyway, my whole point is that this palette-like approach is so utterly brain-damaged in general. I've written about it on some other bug entries / forum posts too. It's disappointing to see how much the terminal emulation world is resisting to get over it, while desktops have left behind the palette-mode about 20 years ago, and in some ways, the 8 bit charsets -> UTF-8 transition was something analoguous too (there shouldn't be any global palette setting effecting every pixel or every character, everything should go down individually to each pixel or each character).

I agree, palette based colors are far from perfect, but they are better than bce for many reasons, some of which you point out. And they are better than filling screens with spaces, for performance reasons and semantic reasons.

So if you wish to propose a better alternative by all means do so, I might even be willing to implement it in kitty, but in the world we have right now, OSC 11 is the best way for fullscreen apps, such as nvim to do themes with background colors.

Implementing bce means that erasing requires setting values on each cell individually. Without it, erase is a simple memset(0).

Ease of implementation to this extreme (memset vs. for-loop) shouldn't matter at all in any such discussion about this truly problematic area, and I do sincerely hope this wasn't your primary reason for not supporting bce :-)

It's not ease of implementation, it's performance. kitty cares about performance, and part of that caring is not implementing stupid features that are bad for performance.

  • Redesign this whole thing with buy-in from key people (already tried, didn't succeed). E.g. introduce a new capability which only does bce at a \e[K, not in any other cases. Start using this new capability and retire bce. Or introduce a new character that is a "printable, single-width, but not-to-be-copy-pasted space". Or...???

Another possible alternative is to use erase-keeping-attributes.

kovidgoyal commented 6 years ago

And here is my proposal for a better way to fix this:

A new escape code that combines SGR codes with a cell range specification, so you can easily set/reset any attribute on any block of cells with only a few bytes transmitted. With this code, erasing the screen would mean, doing a normal erase and then sending the code to set the background color on all cells.

egmontkob commented 6 years ago

IME only full screen apps need to set default background colors.

My point here was that full screen apps don't necessarily switch to the alternate screen. An approach that doesn't work in the latter case just doesn't sound right.

Another point of mine was the top and bottom lines of Midnight Commander. To push it even further, try mcedit with multiple files in randomly scattered windows (click on the [*] at the upper right and move the subwindows with mouse). Or even just a single file, with a different background color showing when the 80-color right margin is exceeded. Which color should be the terminal's default, and why should this concept exist at all? We'd still want semantically correct copy-pasting or trailing whitespaces, no matter if the text exceeds to the right margin on not.

The notion of one certain color being the "default" one does not necessarily exist (and does not exist in these cases) and should not be forced on apps. Let apps decide which cell has which color, that's all.

I'm not a (neo)vim user but I suppose (neo)vim has something similar too. I'd be surprised if it didn't support something that mcedit does.

The concept of a configurable per-terminal default background is clearly insufficient to address these use cases. The concept of a configurable per-row default background is probably sufficient, and this is kinda-sorta what bce brings in semantically (along with a handful of bad behavior).

Another question is: What should tmux-like software do upon OCS 11?? It cannot directly forward that to the emulator, as that would badly influence other panes as well. It could maybe convert them to SGR RGB and paint each cell with spaces accordingly (if it can be sure that RGB is supported by the emulator).

As for overwriting colors the user has set via OSC 11, that is trivially handled by querying the terminal emulator for the current value of OSC 11

Querying the terminal emulator is anything but trivial, in fact, it's a similarly huge pain point as the palette approach. See https://unix.stackexchange.com/a/390797/53656.

Well, tons of things are not supported by tons of emulators. If we wait for widespread support, we will never progress.

But terminal emulators all adding their own extensions, causing them to diverge more and more, requiring apps to be tested on each and have tons of terminal-specific extensions/quirks/etc. isn't the right approach either. Something is more deeply fundamentally broken here.

So if you wish to propose a better alternative by all means do so, I might even be willing to implement it in kitty,

I'd love to cooperate towards a new solution if I saw any chance of that getting widespread (that is, not VTE and Kitty only). As mentioned, I've actually raised this issue with the key person (xterm/ncurses/tereminfo maintainer) who could help the most to move forward, and whose cooperation is necessary, since without terminfo support you won't be getting anywhere. I failed to get his buy-in or make any progress.

but in the world we have right now, OSC 11 is the best way for fullscreen apps, such as nvim to do themes with background colors.

For the reasons I outlined above, I still disagree with this.

Anyway, I think we agree that all the current approaches have significant drawbacks. Which one is worse and which one is better (read: less bad) is probably a matter of personal taste.

It's not ease of implementation, it's performance. kitty cares about performance, and part of that caring is not implementing stupid features that are bad for performance.

I totally buy your argument that bce is a stupid feature :-) I don't buy the performance one; I mean if it wasn't a stupid one then even if performance was crucial, a memset vs. for-loop couldn't make such a difference to ditch a good feature for it. bce is not a good one, we agree on it, and for this reason (and not the performance one) I totally accept and respect your decision of not supporting it.

Another possible alternative is to use erase-keeping-attributes. [...] A new escape code that combines SGR codes with a cell range specification

I guess I like the former approach better. Anyway, without buy-in from key people, unfortunately I'm not interested in working towards a solution. I just wanted to show a broader picture of the existing problems. As for VTE, a solution would require to eventually ditch bce and hence stop piggybacking on TERM=xterm-256color, deviating from most of the graphical emulators out there. Without seeing most of the major terminal emulators and screen libraries cooperating towards a unified good solution, I don't see a point in VTE diverging further away from the rest and being the "odd" one. (And by the way I probably wouldn't even get approval from the project's main developer.)

Don't get me wrong, I would LOVE to see all key players in the game cooperating, designing and implementing a good solution! Alas, the two of us is nowhere near sufficient for this.

mtreca commented 6 years ago

@justinmk Tried in neovim from your tui branch. I still run into the same issue sadly.

kovidgoyal commented 6 years ago

@egmontkob I feel for you. A major reason I developed kitty was so I dont have to depend on the stagnant terminal ecosystem for the tools I use. I'm afraid if we are always going to be waiting for "everyone to co-operate" we are going to be waiting forever. The way forward is to get buy in not from the maintainers of terminfo/ncurses, but instead from the developers of key terminal applications. Once major applications start using features not blessed by terminfo, the rest of the ecosystem will follow.

Fortunately, since I write most of the sophisticated terminal applications that I use (apart from vim and zsh) this is easy for me :)

kovidgoyal commented 6 years ago

Oh and I know you hate querying terminal emulators, but we have to agree to disagree there. For instance, none of your objections apply to this use case. If the terminal emulator has not responded with the pre-existing OSC 11 value by the time the application quits, it simply resets to null instead. Since this is actually almost never going to happen in practice, it is a non-issue.

egmontkob commented 6 years ago

A major reason I developed kitty was so I dont have to depend on the stagnant terminal ecosystem for the tools I use.

We have indeed different motives, goals, ideas, and this leads to most of the disagreement between us. You're much more free to go wherever you want to and experiment, I'm more stuck with the current state of things. It'll be interesting to see where this takes us in the long run.