Add support for VT100 Auto Wrap Mode (DECAWM)

[j4james]

Summary of the Pull Request

This adds support for the 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, and with some new unit tests.

PR Checklist

• Closes Add support for VT100 Auto Wrap Mode (DECAWM) #3826
• CLA signed. If not, go over here and sign the CLA
• Tests added/passed
• Requires documentation to be updated
• 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: Add support for VT100 Auto Wrap Mode (DECAWM) #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.

[miniksa]

Just one minor request comment before I'll sign.
Overall, I think this is fine. Thank you.

I want to get this in before I work on the boogaloo stream-writing fixes (#780) that are intended to replace WriteCharsLegacy. I'm a bit nervous about changing WriteCharsLegacy and AdjustCursorPosition here because they've historically been bug factories (which is why #780 is a thing). But I accept your justifications and mitigation of risk by scoping it to VT here.

[j4james]

I want to get this in before I work on the boogaloo stream-writing fixes (#780) that are intended to replace WriteCharsLegacy.

If #780 is likely to happen soonish, I'm actually wondering now if we're better off leaving this PR for the time being, and then later reimplement it on top of the new stream writer? At that point, none of this code would be needed in WriteCharsLegacy and AdjustCursorPosition, so we'd just be pulling it all out again. I don't mind either way.

[miniksa]

I want to get this in before I work on the boogaloo stream-writing fixes (#780) that are intended to replace WriteCharsLegacy.

If #780 is likely to happen soonish, I'm actually wondering now if we're better off leaving this PR for the time being, and then later reimplement it on top of the new stream writer? At that point, none of this code would be needed in WriteCharsLegacy and AdjustCursorPosition, so we'd just be pulling it all out again. I don't mind either way.

Let's bring it in because it will guide the way I work. And I think these in conjunction with your C0 proposal will make #780 more self-evident.

[miniksa]

K looks good.

[j4james]

(╯‵□′)╯︵┻━┻

[miniksa]

(╯‵□′)╯︵┻━┻

Going to try to re-run/merge until success.

[miniksa]

OK after I busted #4171 by clicking update branch, I don't really want to do it here. It looks like the build cannot pull the merge result branch. Can you update your branch, @j4james?

[j4james]

I thought I had it working, but it looks like I'm going to need to merge again or rebase. I have to go to sleep now, and I have work tomorrow, but will try again tomorrow evening or over the weekend.

[miniksa]

The merge looked super easy so I did it in the web editor. Marking AutoMerge. If build succeeds, we're good to go.

[ghost]

Hello @miniksa!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[j4james]

The merge looked super easy so I did it in the web editor. Marking AutoMerge. If build succeeds, we're good to go.

I think the web editor screws up line endings, which is something the code formatter now checks, and I suspect that is why it's failing.

[j4james]

@miniksa Is it OK if I just force-push a rebase? I don't know how else to fix things at this point.

[DHowett-MSFT]

@j4james that'll be fine. Sorry about that 😄

[miniksa]

@j4james that'll be fine. Sorry about that 😄

Yes, it's fine. Sorry, everyone stood in my office for like 45 minutes BSing so I didn't see this as it came in.

Also, apologies that I keep learning bad merging lessons on your branches.

[j4james]

FYI, the current build failure is just the unit tests timing out, which seems to happen quite often in the CI build. I don't think there's actually anything wrong with the code itself.

[DHowett-MSFT]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[miniksa]

FYI, the current build failure is just the unit tests timing out, which seems to happen quite often in the CI build. I don't think there's actually anything wrong with the code itself.

Yeah , looking at this in #4384

[miniksa]

WELP.

[ghost]

🎉Windows Terminal Preview v0.9.433.0 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download

[DHowett-MSFT]

🎉 Once again, thanks for the contribution!

This pull request was included in a set of conhost changes that was just
released with Windows Insider Build 19603.