-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Build and ship an actual binary named wt that just launches WT #6860
Conversation
Due to a shell limitation, Ctrl+Shift+Enter will not launch Windows Terminal as Administrator. This is caused by the app execution alias and the actual targeted executable not having the same name. In addition, PowerShell has an issue detecting app execution aliases as GUI/TUI applications. When you run wt from PowerShell, the shell will wait for WT to exit before returning to the prompt. Having a shim that immediately re-executes WindowsTerminal and then returns handily knocks this issue out (as the process that PS was waiting for exits immediately.) This could cause a regression for anybody who tries to capture the PID of wt.exe. Our process tree is not an API, and we have offered no consistency guarantee on it. Fixes #4645 (PowerShell waits for wt) Fixes #6625 (Can't launch as admin using C-S-enter)
src/cascadia/wt/shim.cpp
Outdated
|
||
// Go! | ||
wil::unique_process_information pi; | ||
return CreateProcessW(module.c_str(), cmdline.data(), nullptr, nullptr, FALSE, 0, nullptr, nullptr, &si, &pi); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, createprocess returns 1 on success; definitely don't do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider !!CreateProcessW
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hate the hellscape we live in. I'm waiting for an answer before giving the ✔️
src/cascadia/wt/shim.cpp
Outdated
|
||
// Go! | ||
wil::unique_process_information pi; | ||
return CreateProcessW(module.c_str(), cmdline.data(), nullptr, nullptr, FALSE, 0, nullptr, nullptr, &si, &pi); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, the way this is, won't the commandline windowsterminal.exe
processes be windownterminal.exe wt.exe new-tab ...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So actually, no! The first argument to CreateProcess is the actual executable to run, and the second is its entire commandline. That includes the first argument, usually the program name, that comes in as argv[0]
.
There are some tools (usually linux ones) that detect what identity they were launched as and do something different depending on what it was. Busybox is a great example. It’s one binary that contains sh, ls, rm, cat, ... and at runtime decides which one to be based on how you invoked it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, all that is possible because the first argument, usually the program name, can vary and is detectable by the application. Because of that, though, it must be specified when creating a process!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep OK.
Hello @DHowett! Because this pull request has the 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 (
|
🎉 Handy links: |
Due to a shell limitation, Ctrl+Shift+Enter will not launch Windows
Terminal as Administrator. This is caused by the app execution alias and
the actual targeted executable not having the same name.
In addition, PowerShell has an issue detecting app execution aliases as
GUI/TUI applications. When you run wt from PowerShell, the shell will
wait for WT to exit before returning to the prompt. Having a shim that
immediately re-executes WindowsTerminal and then returns handily knocks
this issue out (as the process that PS was waiting for exits
immediately.)
This could cause a regression for anybody who tries to capture the PID
of wt.exe. Our process tree is not an API, and we have offered no
consistency guarantee on it.
VALIDATION
Tested manual launch in a number of different scenarios:
Fixes #4645 (PowerShell waits for wt)
Fixes #6625 (Can't launch as admin using C-S-enter)