[VT Mouse Mode] Translate SGR Mouse VT Sequences to MOUSE_EVENT_RECORD #3963

carlos-zamora · 2019-12-14T02:39:36Z

Summary of the Pull Request

Upgrades the InputStateMachineEngine to take SGR Mouse VT Sequences and translate them into MOUSE_EVENT_RECORDS.

References

https://docs.microsoft.com/en-us/windows/console/mouse-event-record-str
https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h3-Extended-coordinates
#376

PR Checklist

Contributes to [Conpty] Add support for mouse input #376
CLA signed.
Tests added/passed
Requires documentation to be updated

Detailed Description of the Pull Request / Additional comments

Modifications to `InputStateMachineEngine`

I introduced various enum types...

CsiIntermediateCodes: our supported intermediate codes. Currently only <
CsiEndCodes: the last code used for SGR Mouse Mode
CsiMouseButtonCodes: which button was pressed. Mutually exclusive. Buttons beyond button 11 are ambiguous.
CsiMouseModifierCodes: bitfield of modifiers active for SGR Mouse Mode.

CsiIntermediateCodes is used first in ActionCsiDispatch to detect the VT Sequence. This kicks off a chain of function calls...

_GetXYPosition(): figure out where the mouse was clicked
_UpdateSGRMouseButtonState: read in what we found and update our internal state
_WriteMouseEvent(): generate an INPUT_RECORD and send it off

Modifications to Testing Suite

read below.
Also, made the test state a globally accessible/modifiable variable.

Validation Steps Performed

Added tests that cover...

button clicks
button clicks with modifiers
mouse movement
mouse movement and entering/exiting a state where multiple buttons were pressed

- make InputEngineTests point out when a record was _not_ generated - add testing for SGR Mouse Input

DHowett-MSFT · 2019-12-16T19:24:58Z

Notes off the dome: we need to make sure the work is tracked to convert a request for ENABLE_MOUSE_INPUT into a request over conpty's output channel that the terminal enter mouse mode.

zadjii-msft · 2020-01-08T16:22:43Z

we need to make sure the work is tracked to convert a request for ENABLE_MOUSE_INPUT into a request over conpty's output channel that the terminal enter mouse mode.

Do we though? (what follows is my literal train of thought. Go ahead and skip to conclusion)

Can conpty just always be listening for mouse events from a terminal? It could continue to just handle mouse as it normally does, only giving the mouse events to the connected client when the client's requested mouse input of a particular form.

When passthrough mode lands, this is a different story, then mouse mode changes will get passed to the terminal, and mouse events will be passed through to the client. This is bad though, because when passthrough mode exists, then the terminal would still be automatically synthesizing SGR sequences to write to the client, that the client hasn't requested yet. Fooey. Telling the passthrough client that it should "enable passthrough, then immediately write a [sequence] to disable mouse from the terminal" is probably also a bad solution.

Conclusion: Okay, so conpty will need to manually emit a [sequence] to the Terminal saying "please enable SGR Mouse mode". Maybe conpty can emit that sequence at startup by default. Or whenever someone enables any VT mouse mode, or ENABLE_MOUSE_INPUT from Win32.

The follow-up tasks being:

Conpty emits a [sequence] on entering any mouse mode to tell terminals that they should synthesize VT Mouse Mode input as SGR sequences (with hover, scroll, etc)
Conhost translates Mouse input into VT from both conpty and the HWND the same way
Terminal can consume [sequence] to synthesize VT mouse mode input

zadjii-msft

I've got a bunch of comments for when you get back 😉

Also get ready - Michael updated all the parsers/adapters to be audit-friendly, which is going to conflict highly with this stuff.

src/terminal/parser/InputStateMachineEngine.cpp

src/terminal/parser/InputStateMachineEngine.hpp

src/terminal/parser/ut_parser/InputEngineTest.cpp

ghost · 2020-01-15T18:08:49Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

zadjii-msft · 2020-01-15T19:58:53Z

shh bot, it's snowpocalypse 2020 in Seattle, give us a break

ghost · 2020-01-22T20:01:14Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

carlos-zamora · 2020-01-28T20:38:24Z

calm down bot... Working on it now and this week

zadjii-msft

I'm 🤏_this_ close to signing off, just want answers to a couple questions

src/terminal/parser/InputStateMachineEngine.cpp

src/terminal/parser/ut_parser/InputEngineTest.cpp

miniksa

Just a couple of things.

src/terminal/parser/InputStateMachineEngine.cpp

src/terminal/parser/InputStateMachineEngine.hpp

src/terminal/parser/ut_parser/InputEngineTest.cpp

ghost · 2020-02-03T22:20:21Z

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.

ghost · 2020-02-13T18:02:32Z

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

Handy links:

zadjii-msft mentioned this pull request Dec 16, 2019

[Terminal] VT Mouse Support #545

Closed

carlos-zamora added 4 commits December 16, 2019 11:08

SGR Mouse Mode for InputStateMachine

629b43e

Testing Modifications and Additions:

cc092de

- make InputEngineTests point out when a record was _not_ generated - add testing for SGR Mouse Input

Test and feature polish

ae3e8a4

make sure we always update the state. fix static analysis error

e20291e

carlos-zamora force-pushed the dev/cazamor/vt-mouse-mode branch from 0e3259b to e20291e Compare December 16, 2019 19:08

zadjii-msft added Area-Input Related to input processing (key presses, mouse, etc.) Product-Conpty For console issues specifically related to conpty labels Dec 16, 2019

zadjii-msft requested changes Jan 8, 2020

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jan 8, 2020

ghost added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jan 15, 2020

ghost removed the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jan 15, 2020

ghost added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jan 22, 2020

ghost removed the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jan 28, 2020

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jan 28, 2020

carlos-zamora added 2 commits January 29, 2020 14:05

merge master

dbed160

PR Changes

d4542bf

carlos-zamora mentioned this pull request Jan 30, 2020

InputEngineTest::RoundTripTest Fails #4405

Open

zadjii-msft reviewed Jan 30, 2020

View reviewed changes

src/terminal/parser/InputStateMachineEngine.cpp Show resolved Hide resolved

src/terminal/parser/InputStateMachineEngine.cpp Outdated Show resolved Hide resolved

src/terminal/parser/ut_parser/InputEngineTest.cpp Show resolved Hide resolved

zadjii-msft added the Needs-Second It's a PR that needs another sign-off label Jan 30, 2020

ghost requested review from miniksa, DHowett-MSFT and leonMSFT January 30, 2020 15:05

miniksa requested changes Jan 30, 2020

View reviewed changes

ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jan 30, 2020

carlos-zamora added 3 commits January 31, 2020 10:53

PR Comments

ab40735

caught off guard by the formatter

e60003e

scrolling tests and bugfix

11286f5

zadjii-msft approved these changes Jan 31, 2020

View reviewed changes

miniksa approved these changes Feb 3, 2020

View reviewed changes

miniksa added AutoMerge Marked for automatic merge by the bot when requirements are met and removed Needs-Second It's a PR that needs another sign-off labels Feb 3, 2020

ghost merged commit 5c1b407 into master Feb 3, 2020

ghost deleted the dev/cazamor/vt-mouse-mode branch February 3, 2020 22:20

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[VT Mouse Mode] Translate SGR Mouse VT Sequences to MOUSE_EVENT_RECORD #3963

[VT Mouse Mode] Translate SGR Mouse VT Sequences to MOUSE_EVENT_RECORD #3963

carlos-zamora commented Dec 14, 2019 •

edited

Loading

DHowett-MSFT commented Dec 16, 2019

zadjii-msft commented Jan 8, 2020 •

edited

Loading

zadjii-msft left a comment

ghost commented Jan 15, 2020

zadjii-msft commented Jan 15, 2020

ghost commented Jan 22, 2020

carlos-zamora commented Jan 28, 2020

zadjii-msft left a comment

miniksa left a comment

ghost commented Feb 3, 2020

ghost commented Feb 13, 2020

[VT Mouse Mode] Translate SGR Mouse VT Sequences to MOUSE_EVENT_RECORD #3963

[VT Mouse Mode] Translate SGR Mouse VT Sequences to MOUSE_EVENT_RECORD #3963

Conversation

carlos-zamora commented Dec 14, 2019 • edited Loading

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Modifications to InputStateMachineEngine

Modifications to Testing Suite

Validation Steps Performed

DHowett-MSFT commented Dec 16, 2019

zadjii-msft commented Jan 8, 2020 • edited Loading

zadjii-msft left a comment

Choose a reason for hiding this comment

ghost commented Jan 15, 2020

zadjii-msft commented Jan 15, 2020

ghost commented Jan 22, 2020

carlos-zamora commented Jan 28, 2020

zadjii-msft left a comment

Choose a reason for hiding this comment

miniksa left a comment

Choose a reason for hiding this comment

ghost commented Feb 3, 2020

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 commented Feb 13, 2020

carlos-zamora commented Dec 14, 2019 •

edited

Loading

Modifications to `InputStateMachineEngine`

zadjii-msft commented Jan 8, 2020 •

edited

Loading

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.