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

Fix the Alternate Scroll Mode when DECCKM enabled #5081

Merged
2 commits merged into from
Mar 23, 2020

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Mar 23, 2020

Summary of the Pull Request

If the Alternate Scroll Mode is enabled, the terminal generates up/down keystrokes when the mouse wheel is scrolled. However, the expected escape sequences for those keys are dependent on the state of the Cursor Keys Mode ( DECCKM), but we haven't taken that into account. This PR updates the alternate scroll implementation to make sure the appropriate sequences are sent for both DECCKM modes.

References

#3321

PR Checklist

  • Closes #xxx
  • 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: #xxx

Detailed Description of the Pull Request / Additional comments

I've simply added a condition in the TerminalInput::_SendAlternateScroll method to send a different pair of sequences dependent on the state of _cursorApplicationMode flag.

Validation Steps Performed

Manually tested in VIM (although that required me enabling the Alternate Scroll Mode myself first). Also added a new unit test in MouseInputTest to confirm the correct sequences were generated for both DECCKM modes.

Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for this.

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Product-Conhost For issues in the Console codebase AutoMerge Marked for automatic merge by the bot when requirements are met labels Mar 23, 2020
@ghost
Copy link

ghost commented Mar 23, 2020

Hello @zadjii-msft!

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.

@ghost ghost merged commit cb3bab4 into microsoft:master Mar 23, 2020
@j4james j4james deleted the fix-alternate-scroll branch March 23, 2020 22:31
This pull request 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 AutoMerge Marked for automatic merge by the bot when requirements are met Product-Conhost For issues in the Console codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants