Skip to content
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

EL behavior at end of line #3177

Closed
egmontkob opened this issue Oct 13, 2019 · 3 comments · Fixed by #14936
Closed

EL behavior at end of line #3177

egmontkob opened this issue Oct 13, 2019 · 3 comments · Fixed by #14936
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase
Milestone

Comments

@egmontkob
Copy link

Environment

Windows build number: 10.0.18362.0
Windows Terminal version (if applicable): 0.5.2762.0

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.:

printf "%$(tput cols)s\e[K%s\n" 0123456789 abcdef

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" ):

          012345678a▏
bcdef               ▏

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:

          0123456789▏
abcdef              ▏

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 letter a is printed in the next line anyway:

          012345678 ▏
abcdef              ▏

Or, at 38 columns:

echo 'The quick brown fox jumps over the lazy dog.' | GREP_COLORS='ms=1;91' grep z

The quick brown fox jumps over the la ▏
y dog.                                ▏
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 13, 2019
@j4james
Copy link
Collaborator

j4james commented Oct 14, 2019

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:

  • ECH (erase character)
  • DCH (delete character)
  • ICH (insert character)
  • EL (erase in line)
  • ED (erase in display)

Note that there are other commands that should reset the flag but which we don't yet implement.

@DHowett-MSFT DHowett-MSFT added Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Help Wanted We encourage anyone to jump in on these. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 14, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 14, 2019
@DHowett-MSFT DHowett-MSFT added this to the Console Backlog milestone Oct 14, 2019
@DHowett-MSFT
Copy link
Contributor

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." 😄

@j4james
Copy link
Collaborator

j4james commented Oct 15, 2019

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.

@zadjii-msft zadjii-msft modified the milestones: Console Backlog, Backlog Jan 4, 2022
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Feb 28, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Mar 4, 2023
DHowett pushed a commit that referenced this issue Mar 4, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Help Wanted We encourage anyone to jump in on these. In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Conhost For issues in the Console codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants