Enable ctrl+shift to run terminal elevated from context menu #15137
Conversation
| const auto ControlPressed = 1U; | ||
| short control = 0; | ||
| short shift = 0; | ||
| control = GetAsyncKeyState(VK_CONTROL); |
There was a problem hiding this comment.
qq: Why the switch from sync to Async?
There was a problem hiding this comment.
GetKeyState is really intended to be used by an application with a message queue which I dont think is the case here with a shell extension. After some searching and alot of testing different ideas this is where I landed that was reliable.
There was a problem hiding this comment.
IExplorerCommand documentation says "These methods are called on the UI thread", which I think means the thread has a message queue and likely got from that queue the keyboard or mouse events that caused the command to be invoked.
There was a problem hiding this comment.
I would guess that's the UI thread of file explorer?
There was a problem hiding this comment.
Might be a UI thread of some application that uses Common Item Dialog.
There was a problem hiding this comment.
So, this is where it gets weird. When we are in the Windows Terminal package our shell extension gets hosted in a separate process called dllhost.exe!
There was a problem hiding this comment.
Huh. I wonder if the out-of-process execution is intended to ensure that the appx package can be uninstalled or upgraded without killing the shell processes that have called the extension.
This makes me wonder if there's a case where we'll pick up extraneous Ctrl/Shift presses... hmm |
|
Just finally got to our vacation spot (5 hour drive 😫) but I will try to answer more in depth in a bit. But mainly reliability. After adding a second function to check for shift I got really unreliable results. So I did some searching and this way was much more reliable. My thoughts here is that this is a very intentional action by the user and so I feel it is unlikely that some other programs key strokes would get caught up here. |
Hope you had a nice vacation, despite the long drive! It's a good season for it here in the US :D Thanks for the explanation! |
DHowett
changed the title
|
Oh LAWD So I've got a weird one that's either from this PR or the parent.
This PR did just merge so lemme make sure I merge main in again. This may be that mysterious "UWP apps sometimes think ctrl is still held after switching desktops" bug |
hmm. does the same for me. that is strange. Ill try to work out what is happening. |
|
This at least seems to be resolved for me with this PR merged. So the shift check might have helped entirely mitigate, or the switch to GetAsyncKeyState, who knows. I'd also caution that I've had issues with ctrl seemingly remaining held down in UWPs (*cough* onenote) after switching desktops for years. years. So this may not be something trivially solvable. |
Wait, so its good? |
|
I think so. I'll keep my eye out though |
|
Hi. I am the author of issue #14810 (which appears as a reference in the first comment of this issue) where I mentioned the need to be able to run Windows Terminal as admin from the context menu of Windows Explorer using Ctrl + Shift I wanted to thank the people who made this feature possible, as it is a very useful feature for me. Now I don't need to type the path to a folder in an elevated terminal session to run a script, but now I can simply right click on the folder (or an empty space inside the folder), and after opening the context menu I just have to press Ctrl + Shift and click/Enter on the Open in Terminal Preview item to make WT run as admin. By the way, in the release notes for the latest WT preview version, I see a line that says:
So I see that you have not given me credit 😔, since I was the one who came up with the idea of Ctrl + Shift for the context menu. One last thing, I want to report a little bug I have discovered with this new Ctrl+Shift function when I want to open any folder as admin in WT. Here are the steps of the problem:
If I repeat the same steps, but hold Ctrl+Shift 1 second more after clicking on Open in Terminal Preview, and then release the keys at the same time, then this time WT does run as admin. So the bug is that there is a one second delay for WT to run as admin, and this problem only occurs with the new Windows 11 context menu. If I repeat the same steps with the classic context menu, there is no problem, I don't need to hold Ctrl + Shift one more second. That's all. I hope you can reproduce the bug and fix it. |
|
@Dhyfer1 sorry about that! Our typical setup for release notes only detects the person who submitted the code change, rather than the reporter. I'll update the release notes when I am back at my computer to say thanks to you. 🙂 |
|
@DHowett That's fine, thanks. And by the way, when you are in front of your PC, please try the steps I mentioned about the one second delay bug, so that you or someone else can fix it. Good day. |
This pull request adds the requirement for the shift key to be pressed in addition to the control key.
References #14810
Implemented in #14873
This is follow up work from my last pull request that was merged that only required the control key to be pressed to launch the terminal as admin from the shell context menu. After some discussion it was decided that the shift key should be required as well as that is the norm on Windows.
Validation Steps Performed
Tested all combinations of shift+ctrl and verified that the terminal only requests elevation when a shift and control key are pressed together. The shell launches regularly if not.