Errors with neovim when termguicolors is set #160
Comments
|
You need to set the correct t_8f and t_8b settings in vim as well. From my .vimrc IIRC these are the defaults in vim so you are probably changing them in either .vimrc or some plugin. |
|
Looking at vim help, those are only set if TERM is xterm, so presumably terminator sets term to xterm. |
|
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. |
|
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. |
|
To be precise the semi-colon form is mandated by ISO-8613-3 |
|
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. |
|
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.
From that URL:
|
|
It's not too late to remove the colon support and be done with it... ;) |
|
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. |
|
@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. |
|
I've opened up a pull request that should fix this in neovim neovim/neovim#7482 |
|
Thanks :) |
That's not exactly true, AFAIU semicolons are ambiguous in some cases. |
|
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_ |
|
Okay so I tried neovim/neovim#7482 and it works fine for me. However, kitty now throws a |
|
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 |
|
And with this commit: 013fd31 you should get more details about the unknown CSI code. |
|
Kitty should ignore unexpected CSI codes, not show anything to normal users. |
|
It does not show anything to normal users. You only see them if you look at its stdout/stderr. |
|
For reference, regarding the colon question see https://bugzilla.gnome.org/show_bug.cgi?id=685759
And https://bugzilla.gnome.org/show_bug.cgi?id=704449 discusses the ambiguous cases:
|
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.
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. |
|
:) 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. |
Would be nice to have, although I see an even much smaller chance for this than for proper introduction of ":".
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.
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.
Exactly. |
|
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. e90aaa8 @XVilka You might want to update your list accordingly. |
|
Awesome, thanks a lot! :) |
|
@kovidgoyal Thanks! We will work on better detection in Neovim, where possible. |
|
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.
|
|
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. |
133ae5e implemented BCE (background color erase). That's fine if the system terminfo claims to support it; but our built-in fallback should not assume it. Per kovidgoyal/kitty#160 (comment) terminal support for BCE seems to be (1) optional and (2) inconsistent. So the built-in terminfos should disable it by default. ref neovim#4210 ref neovim#4421 ref neovim#7035 ref neovim#7337 ref neovim#7381 ref neovim#7425 closes neovim#7618
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 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 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).
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.
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.
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.
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.
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.
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. |
|
@justinmk Tried in neovim from your tui branch. I still run into the same issue sadly. |
|
@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 :) |
|
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. |
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. |
133ae5e implemented BCE (background color erase). That's fine if the system terminfo claims to support it; but our built-in fallback should not assume it. Per kovidgoyal/kitty#160 (comment) terminal support for BCE seems to be (1) optional and (2) inconsistent. So the built-in terminfos should disable it by default. ref neovim#4210 neovim#4421 neovim#7035 neovim#7337 neovim#7381 neovim#7425 neovim#7618
133ae5e implemented BCE (background color erase). It has caused a lot of trouble and gained almost nothing. Per kovidgoyal/kitty#160 (comment) terminal support for BCE seems to be (1) optional and (2) inconsistent. ref neovim#4210 neovim#4421 neovim#7035 neovim#7337 neovim#7381 neovim#7425 neovim#7618
|
@Vxid updated neovim/neovim#7624 to disable BCE almost entirely (for all terminals except real xterm). Can you try it now? |
133ae5e implemented BCE (background color erase). But we can't trust terminfo, so it is safer disable BCE if we are not certain. Per kovidgoyal/kitty#160 (comment) terminal support for BCE seems to be (1) optional and (2) inconsistent. ref neovim#4210 neovim#4421 neovim#7035 neovim#7337 neovim#7381 neovim#7425 neovim#7618
|
@justinmk Still broken I'm afraid...
|
|
@Vxid is $XTERM_VERSION defined? This doesn't make sense. |
|
@justinmk No it is not. $TERM is set to xterm-kitty. By the way I kitty still echoes the following error messages when opening/using neovim: I also just noticed that the problem does not appear when using neovim inside tmux. |
|
@egmontkob Since you also dont like bce, you might be interested in https://github.com/kovidgoyal/kitty/blob/master/protocol-extensions.asciidoc#setting-text-stylescolors-in-arbitrary-regions-of-the-screen which allows setting arbitrary style/color attributes in arbitrary reqions of the screen, by extending the existing DECCARA escape code to work with all SGR sequences. |
|
Hi all, I am still having this issue. As I understand it, neovim still does not support &t_ut, and the fix disabling BCE as long as $XTERM_VERSION is unset does not address this. Are there any known workarounds for the issue? Thanks! |
|
@Aenda Please always mention the exact version of Nvim you're using.
That doesn't make sense.
It's never going to, there's no reason to. |
|
I am running 0.2.2-3.
I am referencing neovim/neovim#7624 which if I am understanding correctly did not stop this issue from occurring. And I only mention &t_ut because it was the other suggested fix in this thread, and is not an option when using neovim. If I have missed a solution to this problem in the thread above, I apologize, and if not, I was just wondering if anyone has discovered a workaround besides just refreshing with ^L when a line disappears. |
It can't stop the issue if you use an old version of Nvim. You must use a version that contains that fix. On GitHub, you can see which tags contain the fix by viewing the commit: only tag there (currently) is "nightly", which means it wasn't released. When it is released, you will need to upgrade, Nvim 0.2.2 will never have the fix. |
|
Sorry - I didn't look closely enough and thought the fix had made it into the November release. I just tried yesterday's neovim nightly build (NVIM v0.2.3-955-g60e96a45b) on the most recent KiTTY build (0.8.4) and the issue still occurs (I used no config file + set relativenumber for neovim and the default config file for KiTTY). Do you have any suggestions? Thanks again for your help and patience. |
|
Does that mean that neovim/neovim#7624 was reverted? That could explain why it's not helping you, @Aenda. |
|
@whitelynx No, that's not correct. |
|
Not sure what has been changed upstream since I last tried building from source, but the 3.0 build fixes this problem for me - thanks! |
|
Hi, I recently migrated to Kitty and have been having the missing line issue, reproducible with the commands in a comment further up:
Is there anything I can do about this? EDIT: Apparently this is a bug in Neovim, so I'll open an issue there. |
I am using the onedark colorscheme in neovim. If I set the option
termguicolorsthen kitty borks and throws[PARSE ERROR] Invalid character in CSI: 0x3a, ignoring the sequencerepeatedly.Here is a comparison with terminator(left) and kitty.
In case I don't set
termguicolorsin my.vimrc, then it kinda works, but the colors are quite different (I believe the colorscheme then uses 256 colors instead of truecolor.)The text was updated successfully, but these errors were encountered: