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

Enable Passthrough for VT Input Mode in ConPty #4856

Merged
2 commits merged into from
Mar 10, 2020

Conversation

carlos-zamora
Copy link
Member

@carlos-zamora carlos-zamora commented Mar 9, 2020

This commit enables passthrough mode for VT Input Mode in ConPty. This
will be used to pass VT Input from Mouse Mode directly to the app on the
other side.

References

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

Detailed Description of the Pull Request / Additional comments

ConHost

  • Set the callback for the InputEngine.
  • Retrieve IsInVirtualTerminalInputMode from the InputBuffer

Adapter (Dispatch)

Retrieve VTInputMode setting from ConHost

Parser

  • Add a callback to passthrough unknown input sequences directly to the
    input queue.
  • If we're in VTInputMode, use the callback

Validation Steps Performed

Tests should still pass.

@carlos-zamora carlos-zamora added Product-Conpty For console issues specifically related to conpty Area-VT Virtual Terminal sequence support labels Mar 9, 2020
@ghost 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
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.

  1. I'm largely blocking on the same thing Dustin is
  2. We still need a function in conhost for "I'm in conpty mode, and the client app turned off VT input mode, I need to reset the Terminal's input mode", right?

@ghost 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
if (isPty)
{
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

god, this API is needlessly verbose ;)

i hope when somebody fixes IsConsolePty to return a bool, they come fix all of these

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.

This same infrastructure is tested w/ statemachine and OSME; I am comfortable shipping this without a test. Mike?

@DHowett-MSFT DHowett-MSFT added the AutoMerge Marked for automatic merge by the bot when requirements are met label Mar 10, 2020
@ghost
Copy link

ghost commented Mar 10, 2020

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.

@ghost ghost merged commit a5297fa into master Mar 10, 2020
@ghost ghost deleted the dev/cazamor/vtmm/dispatch-and-engine-changes branch March 10, 2020 22:07
abhijeetviswam pushed a commit to abhijeetviswam/terminal that referenced this pull request Mar 12, 2020
This commit enables passthrough mode for VT Input Mode in ConPty. This
will be used to pass VT Input from Mouse Mode directly to the app on the
other side.

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

## Detailed Description of the Pull Request / Additional comments

### ConHost
- Set the callback for the InputEngine.
- Retrieve `IsInVirtualTerminalInputMode` from the InputBuffer

### Adapter (Dispatch)
Retrieve `VTInputMode` setting from ConHost

### Parser
- Add a callback to passthrough unknown input sequences directly to the
  input queue.
- If we're in VTInputMode, use the callback

## Validation Steps Performed
Tests should still pass.
DHowett-MSFT pushed a commit that referenced this pull request Mar 13, 2020
## 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 #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
  (#4848)

## Validation Steps Performed
Tests should still pass.
ghost pushed a commit that referenced this pull request Apr 1, 2020
…e's a problem in Win32 quite yet. (#5208)

## Summary of the Pull Request

When conpty is in VT input mode, we pass through all the input we receive. This includes all the other `Action*Dispatch` methods, but missed this one.

## References
* Missed during #4856 
* Discovered during the course of the #4192 review
* #5205 Also investigated part of the issue, but found a different bug.

## PR Checklist
* [x] Doesn't close anything, just related to above things.
* [x] I work here
* [ ] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments
This will fix the <kbd>ctrl+alt+space</kbd> in the Terminal thing mentioned in #4192, but doesn't actually resolve the root cause of that bug (which is tracked in #5205).
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-Conpty For console issues specifically related to conpty
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants