Synthesize VT mouse events and add mouse support to Terminal by carlos-zamora · Pull Request #4859 · microsoft/terminal

carlos-zamora · 2020-03-09T23:30:21Z

Summary of the Pull Request

Make TerminalControl synthesize mouse events and Terminal send them to
the TerminalInput's MouseInput module.

The implementation here takes significant inspiration from how we handle
KeyEvents.

References

Closes #545 - VT Mouse Mode (Terminal)
References #376 - VT Mouse Mode (ConPty)

TerminalControl

_TrySendMouseEvent attempts to send a mouse event via TermInput.
Similar to _TrySendKeyEvent
Use the above function to try and send the mouse event before
deciding to modify the selection

TerminalApi

Hookup (re)setting the various modes to handle VT Input
Terminal is always in VT Input mode (important for Enable Passthrough for VT Input Mode in ConPty #4856)

TerminalDispatch

Hookup (re)setting the various modes to handle VT Input

TerminalInput

Convert the mouse input position from viewport position to buffer
position
Then send it over to the MouseInput in TerminalInput to actually do it
(Move MouseInput from TermAdapter to TermInput #4848)

Validation Steps Performed

Tests should still pass.

src/cascadia/TerminalControl/TermControl.cpp

src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp

zadjii-msft

I'll hold signing off till we get some of the points outlined in that mail thread cleaned up but here's some of the other feedback I have

src/cascadia/TerminalCore/Terminal.cpp

src/cascadia/TerminalCore/TerminalApi.cpp

src/cascadia/TerminalCore/TerminalDispatch.cpp

leonMSFT

A couple of comments, but this looks pretty cool 😎

src/cascadia/TerminalControl/TermControl.cpp

DHowett-MSFT · 2020-03-12T00:05:22Z

Okay, I played with a build of this extensively and only found two issues. One isn't even a bug, it's just ~ ~ weird ~ ~.

(the bug) if you launch mc and un-focus the window, your first click-drag inside the window will begin a selection and send a mouse event. All click-drags after that will extend the selection and send mouse events.
(the weirdo) if you're using standard X10 coordinate mode, which can only address cells up to 94x94, clicking outside of the 94x94 rectangle in the top left of the terminal causes normal mouse events to fire!

2 is a weird case and the encoding is not often used, i bet, so we can ignore it.

DHowett-MSFT · 2020-03-12T00:39:30Z

Also, yes, gotta figure out how that "revert" commit got in here.

zadjii-msft

These are definitely nits, so fix them and I'll sign off on this too. I know Dustin mentioned some other bugs, but IMO we can just fix them in post. They seem pretty trivial comparatively. Let's file them, and get these all in

src/cascadia/TerminalCore/TerminalDispatch.cpp

src/cascadia/TerminalControl/TermControl.cpp

DHowett-MSFT

LET'S DO THIS.

DHowett-MSFT · 2020-03-12T22:22:18Z

do not automerge this -- it is not into master.

)" This reverts commit 62e424f.

…nges (#4845)"" This reverts commit 83c8f83.

ghost · 2020-03-12T23:28:20Z

Hello @DHowett-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.

DHowett-MSFT · 2020-03-13T00:24:09Z

/azp run

azure-pipelines · 2020-03-13T00:24:18Z

Azure Pipelines successfully started running 1 pipeline(s).

issues addressed; carlos worked with Leon on this

DHowett-MSFT · 2020-03-13T00:44:14Z

old status cannot be cleared.

carlos-zamora added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Product-Terminal The new Windows Terminal. labels Mar 9, 2020

carlos-zamora requested a review from zadjii-msft March 9, 2020 23:30

DHowett-MSFT suggested changes Mar 9, 2020

View reviewed changes

src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved

src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Show resolved Hide resolved

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

zadjii-msft reviewed Mar 10, 2020

View reviewed changes

src/cascadia/TerminalCore/Terminal.cpp Outdated Show resolved Hide resolved

src/cascadia/TerminalCore/TerminalApi.cpp Outdated Show resolved Hide resolved

src/cascadia/TerminalCore/TerminalDispatch.cpp Outdated Show resolved Hide resolved

leonMSFT previously requested changes Mar 10, 2020

View reviewed changes

src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved

src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved

src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved

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

carlos-zamora force-pushed the dev/cazamor/vtmm/wt-synthesis branch 2 times, most recently from 98a2fbf to 7184a01 Compare March 10, 2020 23:15

carlos-zamora force-pushed the dev/cazamor/vtmm/mouse-input branch from 97916aa to 863fab4 Compare March 11, 2020 21:11

carlos-zamora mentioned this pull request Mar 11, 2020

Move MouseInput from TermAdapter to TermInput #4848

Merged

carlos-zamora force-pushed the dev/cazamor/vtmm/wt-synthesis branch from 7184a01 to 125e502 Compare March 11, 2020 21:25

zadjii-msft reviewed Mar 12, 2020

View reviewed changes

src/cascadia/TerminalCore/TerminalDispatch.cpp Outdated Show resolved Hide resolved

src/cascadia/TerminalControl/TermControl.cpp Outdated Show resolved Hide resolved

zadjii-msft approved these changes Mar 12, 2020

View reviewed changes

DHowett-MSFT reviewed Mar 12, 2020

View reviewed changes

src/cascadia/TerminalControl/TermControl.cpp Show resolved Hide resolved

zadjii-msft mentioned this pull request Mar 12, 2020

Terminal: add support for showing/hiding the cursor (DECSET/DECRST 25, DECTCEM) #3093

Closed

DHowett-MSFT approved these changes Mar 12, 2020

View reviewed changes

DHowett-MSFT requested a review from leonMSFT March 12, 2020 22:22

ghost closed this Mar 12, 2020

DHowett-MSFT reopened this Mar 12, 2020

DHowett-MSFT changed the base branch from dev/cazamor/vtmm/mouse-input to master March 12, 2020 22:29

carlos-zamora added 5 commits March 12, 2020 15:33

Synthesize VT Mouse Events

30710b1

Revert "Update the dragbar when the titlebar content size changes (#4845

796446a

)" This reverts commit 62e424f.

clean up TrySendMouseEvent and other PR comments

7c532e5

fix some errors and PR comments

d04effe

fix capture issue

31153ce

carlos-zamora added 3 commits March 12, 2020 15:33

Revert "Revert "Update the dragbar when the titlebar content size cha…

fea90a7

…nges (#4845)"" This reverts commit 83c8f83.

fix a few miscellaneous things. Still no fix for the drag bug.

b483c66

Properly mix selection and VTMM story

5c720be

carlos-zamora force-pushed the dev/cazamor/vtmm/wt-synthesis branch from 6e5c928 to 5c720be Compare March 12, 2020 22:36

DHowett-MSFT added the AutoMerge Marked for automatic merge by the bot when requirements are met label Mar 12, 2020

DHowett-MSFT changed the title ~~Synthesize VT Mouse Events~~ Synthesize VT mouse events from the terminal control Mar 12, 2020

DHowett-MSFT changed the title ~~Synthesize VT mouse events from the terminal control~~ Synthesize VT mouse events and add mouse support to Terminal Mar 12, 2020

fix SA

a861356

DHowett-MSFT merged commit ae71dce into master Mar 13, 2020

DHowett-MSFT deleted the dev/cazamor/vtmm/wt-synthesis branch March 13, 2020 00:44

sosnik mentioned this pull request Dec 13, 2020

Allow mouse clicks to reposition cursor by emitting cursor key sequences #8573

Open

Conversation

carlos-zamora commented Mar 9, 2020 • edited by DHowett-MSFT Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the Pull Request

References

TerminalControl

TerminalApi

TerminalDispatch

TerminalInput

Validation Steps Performed

Uh oh!

Uh oh!

Uh oh!

zadjii-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leonMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DHowett-MSFT commented Mar 12, 2020

Uh oh!

DHowett-MSFT commented Mar 12, 2020

Uh oh!

zadjii-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DHowett-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

DHowett-MSFT commented Mar 12, 2020

Uh oh!

ghost commented Mar 12, 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.

Uh oh!

DHowett-MSFT commented Mar 13, 2020

Uh oh!

azure-pipelines bot commented Mar 13, 2020

Uh oh!

DHowett-MSFT commented Mar 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

carlos-zamora commented Mar 9, 2020 •

edited by DHowett-MSFT

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.