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

Add support for VT100 Auto Wrap Mode (DECAWM) #3826

Closed
j4james opened this issue Dec 3, 2019 · 2 comments · Fixed by #3943
Closed

Add support for VT100 Auto Wrap Mode (DECAWM) #3826

j4james opened this issue Dec 3, 2019 · 2 comments · Fixed by #3943
Labels
Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@j4james
Copy link
Collaborator

j4james commented Dec 3, 2019

Description of the new feature/enhancement

The DECAWM escape sequence is a DEC private mode which controls whether characters wrap at the end of the line. It corresponds, more or less, with the existing console ENABLE_WRAP_AT_EOL_OUTPUT flag.

I'm not sure how widely it's used, but it's supported by most terminal emulators I've tested, and it's required to pass the Test of screen features in Vttest.

Proposed technical implementation details

Since we already have the ENABLE_WRAP_AT_EOL_OUTPUT flag, it should just be a matter of hooking up the mode change escape sequence to toggle that flag. However, there is one slight catch - disabling the ENABLE_WRAP_AT_EOL_OUTPUT flag doesn't actually work when WC_DELAY_EOL_WRAP is set (which is always the case in VT mode), so that'll need to be fixed first.

@j4james j4james added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Dec 3, 2019
@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 Dec 3, 2019
@DHowett-MSFT DHowett-MSFT added Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Dec 5, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Dec 5, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Dec 5, 2019
@DHowett-MSFT DHowett-MSFT added this to the 21H1 milestone Dec 5, 2019
@DHowett-MSFT
Copy link
Contributor

Yup, this sounds good to me. I'd be interested to see how it lands with ...EOL as well. 😄

@ghost ghost added the In-PR This issue has a related PR label Dec 13, 2019
@ghost ghost closed this as completed in #3943 Feb 4, 2020
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Feb 4, 2020
ghost pushed a commit that referenced this issue Feb 4, 2020
## Summary of the Pull Request

This adds support for the [`DECAWM`](https://vt100.net/docs/vt510-rm/DECAWM) private mode escape sequence, which controls whether or not the output wraps to the next line when the cursor reaches the right edge of the screen. Tested manually, with [Vttest](https://invisible-island.net/vttest/), and with some new unit tests.

## PR Checklist
* [x] Closes #3826
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #3826

## Detailed Description of the Pull Request / Additional comments

The idea was to repurpose the existing `ENABLE_WRAP_AT_EOL_OUTPUT` mode, but the problem with that was it didn't work in VT mode - specifically, disabling it didn't prevent the wrapping from happening. This was because in VT mode the `WC_DELAY_EOL_WRAP` behaviour takes affect, and that bypasses the usual codepath where `ENABLE_WRAP_AT_EOL_OUTPUT` is checked,

To fix this, I had to add additional checks in the `WriteCharsLegacy` function (7dbefe0) to make sure the `WC_DELAY_EOL_WRAP` mode is only activated when `ENABLE_WRAP_AT_EOL_OUTPUT`  is also set.

Once that was fixed, though, another issue came to light: the `ENABLE_WRAP_AT_EOL_OUTPUT` mode doesn't actually work as documented. According to the docs, "if this mode is disabled, the last character in the row is overwritten with any subsequent characters". What actually happens is the cursor jumps back to the position at the start of the write, which could be anywhere on the line.

This seems completely broken to me, but I've checked in the Windows XP, and it has the same behaviour, so it looks like that's the way it has always been. So I've added a fix for this (9df9849), but it is only applied in VT mode.

Once that basic functionality was in place, though, we just needed a private API in the `ConGetSet` interface to toggle the mode, and then that API could be called from the `AdaptDispatch` class when the `DECAWM` escape sequence was received.

One last thing was to reenable the mode in reponse to a `DECSTR` soft reset. Technically the auto wrap mode was disabled by default on many of the DEC terminals, and some documentation suggests that `DECSTR` should reset it to that state, But most modern terminals (including XTerm) expect the wrapping to be enabled by default, and `DECSTR` reenables that state, so that's the behaviour I've copied.

## Validation Steps Performed

I've add a state machine test to confirm the `DECAWM` escape is dispatched correctly, and a screen buffer test to make sure the output is wrapped or clamped as appropriate for the two states.

I've also confirmed that the "wrap around" test is now working correctly in the _Test of screen features_ in Vttest.
@ghost
Copy link

ghost commented Feb 13, 2020

🎉This issue was addressed in #3943, which has now been successfully released as Windows Terminal Preview v0.9.433.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants