Fix interpretation of DECSTBM margin parameters

[j4james]

Summary of the Pull Request

When setting the scroll margins with the DECSTBM escape sequence, some of the edge cases are interpreted incorrectly. If the top margin is equal to the bottom margin, or the bottom margin is greater than the screen height, the command should be ignored, but at the moment it isn't. This PR fixes those cases.

Tested manually and with additional adapter unit tests.

PR Checklist

• Closes Incorrect interpretation of DECSTBM parameters #1849
• 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: Incorrect interpretation of DECSTBM parameters #1849

Detailed Description of the Pull Request / Additional comments

This is a slight rewrite of the AdaptDispatch::_DoSetTopBottomScrollingMargins method which I think should be a little easier to follow.

• Step 1: Translate the default (zero) values - default top is 1, and default bottom is the screen height.
• Step 2: Check whether the values are valid, i.e. top is less than bottom, and bottom is less than or equal to the screen height.
• Step 3a: If valid, check whether the range covers the full height of the screen, in which case the margins are cleared (i.e. set to 0,0).
• Step 3b: If valid but not full screen, subtract 1 from the values to translate from the VT 1-based coordinate system to the internal 0-based coordinates.

Validation Steps Performed

I've added a few more unit tests to the ScrollMarginsTest in adapterTest.cpp to validate the specific edge cases that were previously incorrect: the top margin being equal to the bottom margin, the top margin being out of range, and the bottom margin being out of range. There was also a weird range of 1,0 which should have cleared the margins but didn't.

While adding these tests, I noticed a couple of the existing tests weren't correct. They were meant to be testing whether certain full screen ranges would clear the margins, but the expected values weren't correctly set, and the tested ranges weren't covering the full screen. So while the tests passed, they weren't actually testing what they claimed to be. This PR also includes fixes for those tests.

[miniksa]

[ghost]

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

Handy links:

• Release Notes
• Store Download