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

Add support for ctrl+click on the dropdown to launch elevated #12209

Merged
merged 3 commits into from
Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -709,16 +709,14 @@ namespace winrt::TerminalApp::implementation
// This might cause a UAC prompt. The elevation is performed on a
// background thread, as to not block the UI thread.
// Arguments:
// - elevate: If true, launch the new Terminal elevated using `runas`
// - newTerminalArgs: A NewTerminalArgs describing the terminal instance
// that should be spawned. The Profile should be filled in with the GUID
// of the profile we want to launch.
// Return Value:
// - <none>
// Important: Don't take the param by reference, since we'll be doing work
// on another thread.
fire_and_forget TerminalPage::_OpenNewWindow(const bool elevate,
const NewTerminalArgs newTerminalArgs)
fire_and_forget TerminalPage::_OpenNewWindow(const NewTerminalArgs newTerminalArgs)
{
// Hop to the BG thread
co_await winrt::resume_background();
Expand All @@ -745,9 +743,8 @@ namespace winrt::TerminalApp::implementation
SHELLEXECUTEINFOW seInfo{ 0 };
seInfo.cbSize = sizeof(seInfo);
seInfo.fMask = SEE_MASK_NOASYNC;
// `runas` will cause the shell to launch this child process elevated.
// `open` will just run the executable normally.
seInfo.lpVerb = elevate ? L"runas" : L"open";
seInfo.lpVerb = L"open";
seInfo.lpFile = exePath.c_str();
seInfo.lpParameters = cmdline.c_str();
seInfo.nShow = SW_SHOWNORMAL;
Expand Down Expand Up @@ -781,7 +778,7 @@ namespace winrt::TerminalApp::implementation

// Manually fill in the evaluated profile.
newTerminalArgs.Profile(::Microsoft::Console::Utils::GuidToString(profile.Guid()));
_OpenNewWindow(false, newTerminalArgs);
_OpenNewWindow(newTerminalArgs);
actionArgs.Handled(true);
}

Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@
<data name="NewWindowRun.Text" xml:space="preserve">
<value>Shift+Click to open a new window</value>
</data>
<data name="ElevatedRun.Text" xml:space="preserve">
<value>Ctrl+Click to open as administrator</value>
</data>
<data name="WindowCloseButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
<value>Close</value>
</data>
Expand Down
34 changes: 28 additions & 6 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ namespace winrt::TerminalApp::implementation
_newTabButton.Click([weakThis{ get_weak() }](auto&&, auto&&) {
if (auto page{ weakThis.get() })
{
page->_OpenNewTerminal(NewTerminalArgs());
page->_OpenNewTerminalViaDropdown(NewTerminalArgs());
}
});
_newTabButton.Drop([weakThis{ get_weak() }](Windows::Foundation::IInspectable const&, winrt::Windows::UI::Xaml::DragEventArgs e) {
Expand Down Expand Up @@ -347,7 +347,7 @@ namespace winrt::TerminalApp::implementation

NewTerminalArgs args;
args.StartingDirectory(winrt::hstring{ path.wstring() });
this->_OpenNewTerminal(args);
this->_OpenNewTerminalViaDropdown(args);

TraceLoggingWrite(
g_hTerminalAppProvider,
Expand Down Expand Up @@ -731,13 +731,18 @@ namespace winrt::TerminalApp::implementation
auto newWindowRun = WUX::Documents::Run();
newWindowRun.Text(RS_(L"NewWindowRun/Text"));
newWindowRun.FontStyle(FontStyle::Italic);
auto elevatedRun = WUX::Documents::Run();
elevatedRun.Text(RS_(L"ElevatedRun/Text"));
elevatedRun.FontStyle(FontStyle::Italic);

auto textBlock = WUX::Controls::TextBlock{};
textBlock.Inlines().Append(newTabRun);
textBlock.Inlines().Append(WUX::Documents::LineBreak{});
textBlock.Inlines().Append(newPaneRun);
textBlock.Inlines().Append(WUX::Documents::LineBreak{});
textBlock.Inlines().Append(newWindowRun);
textBlock.Inlines().Append(WUX::Documents::LineBreak{});
textBlock.Inlines().Append(elevatedRun);

auto toolTip = WUX::Controls::ToolTip{};
toolTip.Content(textBlock);
Expand All @@ -747,7 +752,7 @@ namespace winrt::TerminalApp::implementation
if (auto page{ weakThis.get() })
{
NewTerminalArgs newTerminalArgs{ profileIndex };
page->_OpenNewTerminal(newTerminalArgs);
page->_OpenNewTerminalViaDropdown(newTerminalArgs);
}
});
newTabFlyout.Items().Append(profileMenuItem);
Expand Down Expand Up @@ -841,7 +846,7 @@ namespace winrt::TerminalApp::implementation
_newTabButton.Flyout().ShowAt(_newTabButton);
}

void TerminalPage::_OpenNewTerminal(const NewTerminalArgs newTerminalArgs)
void TerminalPage::_OpenNewTerminalViaDropdown(const NewTerminalArgs newTerminalArgs)
{
// if alt is pressed, open a pane
const CoreWindow window = CoreWindow::GetForCurrentThread();
Expand All @@ -857,12 +862,21 @@ namespace winrt::TerminalApp::implementation
WI_IsFlagSet(lShiftState, CoreVirtualKeyStates::Down) ||
WI_IsFlagSet(rShiftState, CoreVirtualKeyStates::Down) };

const auto ctrlState{ window.GetKeyState(VirtualKey::Control) };
const auto rCtrlState = window.GetKeyState(VirtualKey::RightControl);
const auto lCtrlState = window.GetKeyState(VirtualKey::LeftControl);
const auto ctrlPressed{ WI_IsFlagSet(ctrlState, CoreVirtualKeyStates::Down) ||
WI_IsFlagSet(rCtrlState, CoreVirtualKeyStates::Down) ||
WI_IsFlagSet(lCtrlState, CoreVirtualKeyStates::Down) };
Comment on lines +868 to +870
Copy link
Member

Choose a reason for hiding this comment

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

I always found this code curious... Reading the docs again:

The Ctrl key or button. This is the general Ctrl case, applicable to key layouts with only one Ctrl key or that do not need to differentiate between left Ctrl and right Ctrl keystrokes.

It seems to me as if this is enough:

const auto ctrlPressed = WI_IsFlagSet(window.GetKeyState(VirtualKey::Control), CoreVirtualKeyStates::Down);

(Same for Shift and Menu.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly, I don't know. I've been copy pasting that same structure of code for years since the first Terminal prototypes. No idea if it's really all necessary.

Copy link
Member

Choose a reason for hiding this comment

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

let's file a followup to clean these up, k?


// Check for DebugTap
bool debugTap = this->_settings.GlobalSettings().DebugFeaturesEnabled() &&
WI_IsFlagSet(lAltState, CoreVirtualKeyStates::Down) &&
WI_IsFlagSet(rAltState, CoreVirtualKeyStates::Down);

if (shiftPressed && !debugTap)
const bool dispatchToElevatedWindow = ctrlPressed && !IsElevated();

if ((shiftPressed || dispatchToElevatedWindow) && !debugTap)
{
// Manually fill in the evaluated profile.
if (newTerminalArgs.ProfileIndex() != nullptr)
Expand All @@ -874,7 +888,15 @@ namespace winrt::TerminalApp::implementation
newTerminalArgs.Profile(::Microsoft::Console::Utils::GuidToString(profile.Guid()));
}
}
this->_OpenNewWindow(false, newTerminalArgs);

if (dispatchToElevatedWindow)
{
_OpenElevatedWT(newTerminalArgs);
}
else
{
_OpenNewWindow(newTerminalArgs);
}
}
else
{
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,9 @@ namespace winrt::TerminalApp::implementation
void _CreateNewTabFromPane(std::shared_ptr<Pane> pane);
winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _CreateConnectionFromSettings(Microsoft::Terminal::Settings::Model::Profile profile, Microsoft::Terminal::Settings::Model::TerminalSettings settings);

winrt::fire_and_forget _OpenNewWindow(const bool elevate, const Microsoft::Terminal::Settings::Model::NewTerminalArgs newTerminalArgs);
winrt::fire_and_forget _OpenNewWindow(const Microsoft::Terminal::Settings::Model::NewTerminalArgs newTerminalArgs);

void _OpenNewTerminal(const Microsoft::Terminal::Settings::Model::NewTerminalArgs newTerminalArgs);
void _OpenNewTerminalViaDropdown(const Microsoft::Terminal::Settings::Model::NewTerminalArgs newTerminalArgs);

bool _displayingCloseDialog{ false };
void _SettingsButtonOnClick(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs);
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalSettingsModel/ActionArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
}
}

if (Elevate())
{
ss << fmt::format(L"elevate: {}, ", Elevate().Value());
}

auto s = ss.str();
if (s.empty())
{
Expand Down