-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
xterm-addon-serialize taint the background of next line when line wrap + scroll happen #3102
Comments
Imho the serializer tests only for null content atm (and tries to be smart about nullish cells), what is missing here is the text attributes check for every cell, that would have to be recorded as well. Imho the issue would vanish, if the serializer does likewise here - use ECH + some cursor repositioning to correctly update attribs for nullish cells. Note that you have to use ECH + cursor repositioning, as directly writing of '\x00' would be swallowed as padding by the parser and not write null cells or advance the cursor. About adding a line delimiting style directive: |
|
Getting cell styles should be possible as single pass, there is no need for costly look-ahead during serialize. Think of it like this:
Imho thats it basically, no need for complicated read-ahead BG evaluation. Note that the ECH + CUF patch for null cells above will not work correctly anymore, if the buffer content should be printed nicely on a terminal with different dimensions (as both sequences stop at the rightmost cell). I would ignore that in a first iteration, and see later, if this really should be addressed by the serializer. (There are several options to deal with that, from interim resizing to writing an active deserializer, which can request the dimensions of the target and alter the input accordingly.) |
There is another problem for that. That means the first So a conditional check need to be added to the serialize algorithm to prevent the first n rows from being messed up by non-exist BCE background. But that probably does not matter. |
BCE is basically not needed above in my list - it is just a minor optimization approach later on to get filling null cells to the right pretty quick. You can completely ignore it, and just go with the last found attrib BG data for filling null cells to the right. Yeah, Maybe its simpler to just ignore it during serialization. Yes an early wrapping null cell between wide chars is a real bug in xterm.js as described in the other issue. Tthere is no way to decide on serializer level, whether its genuine data or just there from early wrapping. |
This should have been fixed by #3101. I forgot to tag this. |
Details
Steps to reproduce
echo -e "\x1b[44m\x1b[86X\x1b[87C1\x1b[m\r\n\r\n"
Write back to terminal
option of SerializeAddonNote
It seems this can't be solved without insert
\x1b[m
at line end when the last cell has background.But do this will break all current tests?
Or you need to do a costly look ahead to next line to check whether a reset is required or not.
The text was updated successfully, but these errors were encountered: