Fix SGR mouse movement reports #18864
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DHowett
approved these changes
May 2, 2025
Member
DHowett
left a comment
DHowett
left a comment
There was a problem hiding this comment.
This feels completely correct. Thanks for maintaining our compatibilty and correctness :)
Member
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
lhecker
approved these changes
May 2, 2025
Member
|
I've tried to kick the CLA robot... let's see if it works? |
Member
|
IT WORKS |
DHowett
pushed a commit
that referenced
this pull request
May 5, 2025
According to the documentation, the final character of an SGR mouse report is meant to be `M` for a button press and `m` for a button release. However it isn't clear what the final character should be for motion events, and we were using an `m` if there weren't any buttons held down at the time, while all other terminals used an `M`, regardless of the button state. This PR updates our implementation to match what everyone else is doing, since our interpretation of the spec was causing problems for some apps. ## Validation Steps Performed I've manually tested the new behavior in Vttest, and confirmed that our mouse reports now match Xterm more closely, and I've tested with v0.42.0 of Zellij, which was previous glitching badly in Windows Terminal, but now works correctly. I've also updated our unit tests for the SGR mouse mode to reflect the correct report format. Closes #18712 (cherry picked from commit 865f5e5) Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgZ9_Ew Service-Version: 1.23
DHowett
pushed a commit
that referenced
this pull request
May 5, 2025
According to the documentation, the final character of an SGR mouse report is meant to be `M` for a button press and `m` for a button release. However it isn't clear what the final character should be for motion events, and we were using an `m` if there weren't any buttons held down at the time, while all other terminals used an `M`, regardless of the button state. This PR updates our implementation to match what everyone else is doing, since our interpretation of the spec was causing problems for some apps. I've manually tested the new behavior in Vttest, and confirmed that our mouse reports now match Xterm more closely, and I've tested with v0.42.0 of Zellij, which was previous glitching badly in Windows Terminal, but now works correctly. I've also updated our unit tests for the SGR mouse mode to reflect the correct report format. Closes #18712 (cherry picked from commit 865f5e5) Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgZ9_E0 Service-Version: 1.22
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to the documentation, the final character of an SGR mouse
report is meant to be
Mfor a button press andmfor a buttonrelease. However it isn't clear what the final character should be for
motion events, and we were using an
mif there weren't any buttonsheld down at the time, while all other terminals used an
M, regardlessof the button state.
This PR updates our implementation to match what everyone else is doing,
since our interpretation of the spec was causing problems for some apps.
Validation Steps Performed
I've manually tested the new behavior in Vttest, and confirmed that our
mouse reports now match Xterm more closely, and I've tested with v0.42.0
of Zellij, which was previous glitching badly in Windows Terminal, but
now works correctly.
I've also updated our unit tests for the SGR mouse mode to reflect the
correct report format.
Closes #18712