When using page as scrollback_pager, if the last character on a line has ANSI formatting, the formatting continues onto following lines

[s1341]

When using page (https://github.com/I60R/page) as the scollback_pager, I encounter the following issue:

If the last character on the line has ANSI styling/colors/background, when switching into scrollback_pager mode, the styling continues onto the following lines, until another explicit ANSI styling is encountered.

For example, see the following screenshot (python):

When switching to scrollback_pager mode, we get the following:

Note that the prompt on the line following the word 'start' is in red. It should not be!

I think that this issue is a result of the fact that we retrieve history/scrollback with ANSI formatting on a line by line basis, and the code at https://github.com/kovidgoyal/kitty/blob/master/kitty/line.c#L284 checks whether a new formatting (SGR) is required to be written to the stream. But that check only works within the line, not between consecutive lines. I'm not sure how to fix this... If you give me some guidance I can try for a PR to fix.

[kovidgoyal]

Simplest way would be to have line_as_ansi take a pointer to a prev cell, which can be NULL. Have the various call sites to that function do the appropriate thing.

[s1341]

This is still not working as expected. See the below screenshot:

I can't figure out why sometimes the next line is still highlighted (see grouping 3), or why we have the remainder of the line highlighted (as in grouping 4 and 5). Can you point me in the right direction for a fix?

[kovidgoyal]

Use less and you should be fine.

[s1341]

Yeah. But I'd like to properly resolve the bug... Do you have suggestions as to how to debug/fix?

[kovidgoyal]

There is no bug in kitty as far as I can see. You can confirm that by writing a simple script that dumps stdin to a temp file and then passes it to the pager. Set that script as your pager. Then trigger the problem and inspect the temp file using a hex editor and see if the output is as expected. Or simply cat the temp file in a new terminal and see how it displays.

[s1341]

Ok. I believe I just confirmed that it IS a bug in kitty. Here's what I did:

As you suggested, I created a simple script:

import sys
import select

f = open("/tmp/txt", "wb")
while True:
    if select.select([sys.__stdin__.buffer], [], [], 0) == ([sys.__stdin__.buffer], [], []):
        text = sys.__stdin__.buffer.read()
        sys.__stdout__.buffer.write(text)
        sys.__stdout__.buffer.flush()
        f.write(text)
        f.flush()

I then set that script to be my scrollback_pager.

The result (/tmp/txt) of running some python to create green background lines is this:

You can clearly see that the 'world' line in each cluster has a green background, even though it shouldn't according to the ANSI string printed.

The above is without my patch for this PR. With it, I get:

You can see that the 6th cluster is still exhibiting the problem.

What do you think?

[kovidgoyal]

Your patch has already been merged, I meant there is no longer a bug, after that patch. You seem to be still having a bug in one random case, I dont really see how that's possible, but recreate the bug, and use --dump-bytes to create a dump of the input used to create the bug and attach that and I will have a look.

[s1341]

Attached the dump-bytes output. bytes.log

[kovidgoyal]

Those bytes already show the issue you mention when catted. I need the bytes from before you run the pager. i.e. create the input which would have caused then issue when the pager is run and post that, without actually running the pager.

[s1341]

Oh sorry. Here you go. bytes.log

[kovidgoyal]

https://github.com/kovidgoyal/kitty/commit/bc222af2e2c76dec516e66ac96e1bce043be8987

[s1341]

That appears to have fixed it. Thanks a lot!

[s1341]

Ok. Still experiencing one artifact. The following:

Renders like this:

I've attached the dump-bytes output for this kind of issue. dump2.log

[kovidgoyal]

that's a bug in the pager. I cannot replicate using the following steps:

1) Run: kitty -o scrollback_pager=/t/filter sh -c "cat /t/dump2.log; read" 2) Run the pager, which writes the data is received to /tmp/txt 3) Run: kitty sh -c "cat /tmp/txt; read"

Output does not show artifact

[s1341]

Ok. I'll open a bug for this on the pager github. Thanks for your help running this down.

[s1341]

Ok. So I discussed with some of the people over in #neovim on freenode. They helped clarify some things.

It seems that if I do something similar to what you did above, with the following 'filter':

#!/usr/bin/env python
import sys
import select

f = open("/tmp/txt", "wb")
while True:
    if select.select([sys.__stdin__.buffer], [], [], 0) == ([sys.__stdin__.buffer], [], []):
        text = sys.__stdin__.buffer.read()
        sys.__stdout__.buffer.write(text)
        sys.__stdout__.buffer.flush()
        f.write(text)
        f.flush()

It seems that the output I get is NOT exactly what is expected. Examining the output with cat -e shows the following for one of the groupings:

^[[38:5:28mIn [^[[39m15^[[38:5:28m]: ^[[38:5:83mprint^[[39m(u"\u001b[42mhello\u001b[0m\nh\u001b[42mello\u001b[0m,world\nhi \u001b[42mthe\u001b[0mre")                                $
^[[42mhello$
^[[49mh^[[42mello^[[49m,world$
hi ^[[42mthe^[[49mre$
$

Note that kitty appears to be outputing SGR 49 instead of a 'clear formatting' (SGR 0). Perhaps this is the source of my issue?

<LeoNerd> No, 49 is background reset, 0 is overall reset; but the only thing in effect is background, so either is fine
14:46:44 ⇐ aeonzh1 quit (~aeonzh@2a01:4b00:82bb:4700:d929:7666:f62f:d433) Ping timeout: 252 seconds
14:46:47 <LeoNerd> The issue is the ordering - reset vs. linefeed
14:47:09 <LeoNerd> IF you reset *first* then the pen is back in default state when the linefeed happens, so it doesn't fill the background of the new line in this different colour
14:47:31 <LeoNerd> But if you linefeed while the background colour is still set, it will fill the cells of the newly-created line because of that scroll with that colour

Or perhaps the above is the issue?

[kovidgoyal]

49m means reset to default background color, https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_parameters

the pager is presumably misinterpreting it. 0m means reset all formatting.

[s1341]

Yeah I understood that after I posted. Is it perhaps the ordering of the newline and the SGR 49, as suggested by LeoNerd.

[kovidgoyal]

A newline most definitely should not fill a line with the current background color. That is done only by terminals which support bce and indicate so in the terminfo files. kitty does not support bce and says so in its terminfo. The pager needs to respect that. https://invisible-island.net/ncurses/ncurses.faq.html#bce_mismatches

[kovidgoyal]

And https://github.com/kovidgoyal/kitty/issues/160#issuecomment-346470545

[s1341]

hrm. was a fix upstreamed in neovim for this issue?

[kovidgoyal]

Neovim improved its heuristics to detect bce support a long time ago. As of 3.0 I think.

[s1341]

I'm running git master neovim, so I'm not sure why I'm still dealing with this issue... Maybe I need to update my libvte...

[kovidgoyal]

It's probable that this has nothing to do with nvim. The pager presumably reads and interprets ANSI contrl sequences and converts them into formatting commands for the nvim engine. The code to do thatis likely assuming bce without consulting terminfo.

[s1341]

The pager I’m using, page, is just neovim’s built in :terminal. The ANSI parsing is done by libvterm. I updated the libvterm I used to build neovim and I have not observed the issue since. I’ll verify tomorrow.

[s1341]

FYI https://github.com/neovim/neovim/issues/10922