Provide support for CSI REP control code

[keynslug]

References:

• https://invisible-island.net/xterm/ctlseqs/ctlseqs.html
• GNOME/vte@0656a579
• https://bugzilla.gnome.org/show_bug.cgi?id=787701
• https://www.man7.org/linux/man-pages/man5/terminfo.5.html

I recently got bitten by the lack of support while evaluating sysdig ncurses-based CLI which for some reason thinks that I'm running on xterm-1003 compatible terminal emulator. As though this is probably bug on their side I reckoned that the support in kitty would be good to have either way.

Thoughts?

[kovidgoyal]

I am fine with adding support for this escape code, however the implementation has an issue. If the previous character was the last in a line it wont work. You need to special case x == 0 and decrement y after checking y > margin_top.

[kovidgoyal]

Also why is the repeat limited to the number of places left in the current line? It should probably be capped to 65535 to match VTE and xterm.

[keynslug]

Also why is the repeat limited to the number of places left in the current line? It should probably be capped to 65535 to match VTE and xterm.

To be honest I was adhering to current libvte logic where they cap repetitions up to remaining row length.

I am fine with adding support for this escape code, however the implementation has an issue. If the previous character was the last in a line it wont work. You need to special case x == 0 and decrement y after checking y > margin_top.

As for that you're probably right but it's not clear then what should terminal do if: 1) the previous character was the last in a line above, and 2) that line is already off screen, for example due to scrolling or due to the fact that screen is 1 row high (which probably never happens but helps to illustrate an edge case). I could probably track last entered character in the similar way as libvte does this but I'm happy to hear your thoughts.

[kovidgoyal]

On Fri, May 29, 2020 at 04:43:41AM -0700, Andrew Mayorov wrote: > Also why is the repeat limited to the number of places left in the current line? It should probably be capped to 65535 to match VTE and xterm. To be honest I was adhering to current libvte logic where they [cap repetitions up to remaining row length](https://github.com/GNOME/vte/blob/f69eadf4fa48bbf8b45c36d9a9b71fbf6049557c/src/vteseq.cc#L6739).

Quoting the standard: REP is used to indicate that the preceding character in the data stream, if it is a graphic character (represented by one or more bit combinations) including SPACE, is to be repeated n times, where n equals the value of Pn. If the character preceding REP is a control function or part of a control function, the effect of REP is not defined by this Standard. That's all ECMA 48 has to say on the subject. No mention of truncating to current line length. Seems strange to go out of one's way to do something that is not mentioned in the standard. A cap to prevent DoS I can understand and think is good, but I see no reason to cap to remaining cells in line.

> I am fine with adding support for this escape code, however the implementation has an issue. If the previous character was the last in a line it wont work. You need to special case x == 0 and decrement y after checking y > margin_top. As for that you're probably right but it's not clear then what should terminal do if: 1) the previous character was the last in a line above, and 2) that line is already off screen, for example due to scrolling or due to the fact that screen is 1 row high (which probably never happens but helps to illustrate an edge case). I could probably track _last entered character_ in the similar way as libvte does this but I'm happy to hear your thoughts.

If the line is offscreen, we ignore it. That's a far less common edge case than the last character causing a wrap. Note that the previous line could only scroll off the screen on a one row terminal or if there was some control character sent, or the terminal window was resized, in which case the spec says behavior is undefined.

[keynslug]

That's all ECMA 48 has to say on the subject. No mention of truncating to current line length. Seems strange to go out of one's way to do something that is not mentioned in the standard. A cap to prevent DoS I can understand and think is good, but I see no reason to cap to remaining cells in line.

I see your point. Dropped the truncation.

Note that the previous line could only scroll off the screen on a one row terminal or if there was some control character sent, or the terminal window was resized, in which case the spec says behavior is undefined.

Well, doesn't spec talk about data stream here? Which is (I believe) not related to the screen contents, which in turn means that to follow the spec we must keep track of the last graphic character seen in that stream. Am I misinterpreting something?

[kovidgoyal]

On Fri, May 29, 2020 at 10:42:41AM -0700, Andrew Mayorov wrote: > That's all ECMA 48 has to say on the subject. No mention of truncating to current line length. Seems strange to go out of one's way to do something that is not mentioned in the standard. A cap to prevent DoS I can understand and think is good, but I see no reason to cap to remaining cells in line. I see your point. Dropped the truncation. > Note that the previous line could only scroll off the screen on a one row terminal or if there was some control character sent, or the terminal window was resized, in which case the spec says behavior is undefined. Well, doesn't spec talks about data stream here? Which is (I believe) not related to the screen contents, which in turn means that to follow the spec we must keep track of the last graphic character seen in that stream. Am I misinterpreting something?

Yes, it does and to be strictly spec compliant one must indeed track the last received character, however, functionally, except for a very rare edge cases involving single line terminals and also combining characters, what you are doing is identical. If in the future it turns out that we are overlooking something, we can always add tracking of the last received character pretty easily, however, I'd prefer to avoid extra state if at all possible.