-
Notifications
You must be signed in to change notification settings - Fork 8.5k
EL behavior at end of line #3177
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
Comments
Personally I'm in favor a following the spec (which in this case matches XTerm) and clearing the delayed eol wrap flag for all the commands that require it. At the moment we're only doing it for commands that change the cursor position, so these are some of the ones I think we're missing:
Note that there are other commands that should reset the flag but which we don't yet implement. |
This is also related to #780, where we're keeping all of the world's knowledge on "how we handle deferred EOL and writing characters to the buffer and all that jazz." 😄 |
For a summary of how other terminals handle wrapping see also: https://github.com/mattiase/wraptest/blob/master/results.txt Looking at those results, and noting the way the later VT terminals diverged from the STD 070 spec, I'm now more inclined to accept that we shouldn't be clearing the delayed eol flag when a tab character is received (as @egmontkob was suggesting in issue #3168). But as this requires undoing our default behaviour, it's probably going to be a more complicated fix than the operations that just need the flag clearing added to them. |
When a character is written in the last column of a row, the cursor doesn't move, but instead sets a "delayed EOL wrap" flag. If another character is then output while that flag is still set, the cursor moves to the start of the next line, before writing to the buffer. That flag is supposed to be reset when certain control sequences are executed, but prior to now we haven't always handled that correctly. With this PR, we should be resetting the flag appropriately in all the places that it's expected to be reset. For the most part, I'm following the DEC STD 070 reference, which lists a bunch of operations that are intended to reset the delayed wrap flag: `DECSTBM`, `DECSWL`, `DECDWL`, `DECDHL`, setting `DECCOLM` and `DECOM`, resetting `DECCOLM`, `DECOM`, and `DECAWM`, `CUU`, `CUD`, `CUF`, `CUB`, `CUP`, `HVP`, `BS`, `LF`, `VT`, `FF`, `CR`, `IND`, `RI`, `NEL`, `ECH`, `DCH`, `ICH`, `EL`, `DECSEL`, `DL`, `IL`, `ED`, and `DECSED`. We were already resetting the flag for any of the operations that performed cursor movement, since that always triggers a reset for us. However, I've now also added manual resets in those ops that weren't moving the cursor. Horizontal tabs are a special case, though. Technically the standard says they should reset the flag, but most DEC terminals never followed that rule, and most modern terminals agree that it's best for a tab to leave the flag as it is. Our implementation now does that too. But as mentioned above, we automatically reset the flag on any cursor movement, so the tab operation had to be handled as a special case, saving and restoring the flag when the cursor is updated. Another flaw in our implementation was that we should have been saving and restoring the flag as part of the cursor state in the `DECSC` and `DECRC` operations. That's now been fixed in this PR too. I should also mention there was a change I had to make to the conpty renderer, because it was sometimes using an `EL` sequence while the terminal was in the delayed EOL wrap state. This would reset the flag, and break subsequent output, so I've now added a check to prevent that from happening. ## Validation Steps Performed I've added some unit tests that confirm the operations listed above are now resetting the delayed EOL wrap as expected, and I've expanded the existing `CursorSaveRestore` test to make sure the flag is being saved and restored correctly. I've also manually confirmed that the test case in issue #3177 now matches XTerm's output, and I've confirmed that the results of the wraptest script[^1] now match XTerm's results. [^1]: https://github.com/mattiase/wraptest/ Closes #3177
Environment
Steps to reproduce
Issue an EL (clear to end of line,
\e[K
) when the cursor is at the rightmost position, in "pending wrap" a.k.a. "delayed eol wrap" state. Print further characters. E.g.:Expected behavior
xterm's behavior, and as far as I know the "official" one is that EL clears the "delayed eol wrap" state along with erasing the last column (
9
), resulting in (denoting the right margin by a "left one eighth block"▏
):Some terminals, including VTE, iTerm2, Kitty, Konsole, PuTTY 0.73 intentionally deviate: make EL not clear the "delayed eol wrap" flag and not erase the last character either:
Rationale:
There are several utilities that want to just simply print some text with colors or other attributes, without caring about the terminal width or doing anything more complex terminal driving. If these attributes include a background color too, the behavior is messy across a linewrap, due to the terribly designed bce (background color earse) feature. (Now, VTE, and perhaps some other emulators too, intentionally implement bce differently, as another means of mitigating the problem I'm describing here, but let's leave that to another day.)
As a workaround for the background color problem, several utilities decide to also emit an EL after restoring the background color, to earse the faulty color there. And they often don't realize that this introduces an even more serious issue: at linewrap, a character might get dropped from the output. An example is grep with its default settings. PuTTY 0.73's changelog refers to gcc.
Notes We (VTE) haven't received any bugreport about it, nor did I saw one in Kitty's, Konsole's or iTerm2's bugtracker. Also there's no vttest test case for this.
I do recommend that you follow our behavior here. :)
Actual behavior
The last digit
9
is wiped out by EL for no apparent reason since the "delayed eol wrap" state isn't cleared, the first lettera
is printed in the next line anyway:Or, at 38 columns:
The text was updated successfully, but these errors were encountered: