Conversation
zadjii-msft
left a comment
zadjii-msft
left a comment
There was a problem hiding this comment.
what comes down must come back up
i love it
|
@msftbot make sure @miniksa signs off |
ghost
added
the
AutoMerge
|
Hello @DHowett-MSFT! Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:
If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you". |
|
Just want to make sure: Control events can be sketchy business here in Terminal land |
DHowett-MSFT
left a comment
DHowett-MSFT
left a comment
There was a problem hiding this comment.
This seems sane to me, I'm just worried about the repercussions on ^C generation 😄
The Ctrl+C chord in regular conhost event dispatch land always comes with the pair of down and up. It's actually really weird to only be doing half of it. As far as I can tell, this is what we should have been doing the whole time (until proven otherwise by some mysterious future bug that may or may not exist.) |
ghost
merged commit 
## Summary of the Pull Request
Users were not able to intercept Ctrl-C input using `$Host.UI.RawUI.ReadKey("IncludeKeyUp")`, because we weren't sending a Ctrl-C KeyUp event. This PR simply adds a KeyUp event alongside the existing KeyDown.
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes #1894
* [x] CLA signed.
* [x] Tests added/passed
<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
The repro script in #1894 now works, both options for `ReadKey`: `IncludeKeyUp` and `IncludeKeyDown` work fine.
|
🎉 Handy links: |
ghost
mentioned this pull request
5 participants
Summary of the Pull Request
Users were not able to intercept Ctrl-C input using
$Host.UI.RawUI.ReadKey("IncludeKeyUp"), because we weren't sending a Ctrl-C KeyUp event. This PR simply adds a KeyUp event alongside the existing KeyDown.PR Checklist
Validation Steps Performed
The repro script in #1894 now works, both options for
ReadKey:IncludeKeyUpandIncludeKeyDownwork fine.