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 auto-elevation for a profile, redux #11310

Closed
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
efe42d1
stunned that this remotely works as well as it does
zadjii-msft Sep 2, 2021
abc0265
marginally reduce the risk of a merge conflict
zadjii-msft Sep 2, 2021
974d18a
Merge remote-tracking branch 'origin/main' into dev/migrie/f/632-on-w…
zadjii-msft Sep 22, 2021
8b8de52
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
zadjii-msft Sep 22, 2021
8895e06
these are notes, but they're useless who tf am I kidding
zadjii-msft Sep 27, 2021
9b1e30b
I can't believe how dumb this is (or how well it works)
zadjii-msft Sep 27, 2021
904045b
Tons of comments
zadjii-msft Sep 27, 2021
28bde43
Works with splitting panes again
zadjii-msft Sep 28, 2021
c2b759e
stick these projects in a utils folder because it's getting crowded
zadjii-msft Sep 28, 2021
571b8a9
Merge remote-tracking branch 'origin/dev/migrie/f/non-terminal-conten…
zadjii-msft Sep 29, 2021
4bcdfc3
This should have been in the parent
zadjii-msft Sep 29, 2021
6717b4d
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
zadjii-msft Sep 30, 2021
ab78ad4
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
zadjii-msft Oct 27, 2021
8d7f726
spel
zadjii-msft Oct 27, 2021
88da035
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
zadjii-msft Oct 28, 2021
80fe386
tiny nits
zadjii-msft Oct 28, 2021
7597114
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
zadjii-msft Oct 28, 2021
fdd283d
other pr nits, test comments
zadjii-msft Oct 28, 2021
ebef341
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
zadjii-msft Nov 1, 2021
370a7aa
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
zadjii-msft Nov 10, 2021
3d41cba
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
zadjii-msft Nov 10, 2021
1b4efad
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
zadjii-msft Nov 11, 2021
7a3ec1e
Fix this test
zadjii-msft Nov 11, 2021
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
Prev Previous commit
Next Next commit
Merge branch 'dev/migrie/f/non-terminal-content-elevation-warning' in…
…to dev/migrie/f/632-on-warning-dialog
  • Loading branch information
zadjii-msft committed Nov 10, 2021
commit 370a7aa7932e4cf40563354df4a0c1ed340e38a3
13 changes: 9 additions & 4 deletions src/cascadia/TerminalApp/TabManagement.cpp
Original file line number Diff line number Diff line change
@@ -77,8 +77,10 @@ namespace winrt::TerminalApp::implementation
// We can't go in the other direction (elevated->unelevated)
// unfortunately. This seems to be due to Centennial quirks. It works
Copy link
Member

Choose a reason for hiding this comment

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

Have we filed this in papercuts?

Copy link
Member

Choose a reason for hiding this comment

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

(or written BODGY here... cuz that's a representation of cut workarounds...)

// unpackaged, but not packaged.

_CreateNewTabWithProfileAndSettings(profile, settings, existingConnection);
//
// This call to _MakePane won't return nullptr, we already checked that
// case above with the _maybeElevate call.
_CreateNewTabFromPane(_MakePane(newTerminalArgs, false, existingConnection));

const uint32_t tabCount = _tabs.Size();
const bool usedManualProfile = (newTerminalArgs != nullptr) &&
@@ -250,8 +252,11 @@ namespace winrt::TerminalApp::implementation
// - pane: The pane to use as the root.
void TerminalPage::_CreateNewTabFromPane(std::shared_ptr<Pane> pane)
{
auto newTabImpl = winrt::make_self<TerminalTab>(pane);
_InitializeTab(newTabImpl);
if (pane)
{
auto newTabImpl = winrt::make_self<TerminalTab>(pane);
_InitializeTab(newTabImpl);
}
}

// Method Description:
152 changes: 29 additions & 123 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
@@ -884,6 +884,13 @@ namespace winrt::TerminalApp::implementation
else
{
const auto newPane = _MakePane(newTerminalArgs);
// If the newTerminalArgs caused us to open an elevated window
// instead of creating a pane, it may have returned nullptr. Just do
// nothing then.
if (!newPane)
{
return;
}
if (altPressed && !debugTap)
{
this->_SplitPane(SplitDirection::Automatic,
@@ -1821,32 +1828,6 @@ namespace winrt::TerminalApp::implementation
std::shared_ptr<Pane> newPane)
{
const auto focusedTab{ _GetFocusedTabImpl() };

// Do nothing if no TerminalTab is focused
if (!focusedTab)
{
// GH#632
// If there's no tab, but we're requesting a elevated split,
// we still want to toss that over to the elevated window. This is
// for something like
//
// `wtd nt -p "elevated cmd" ; sp -p "elevated cmd"`
//
// We should just make two elevated tabs for that.

// Note: If the action was a "duplicate pane" action, but there's no
// existing tab to duplicate, then we can't resolve the profile from
// the existing pane. So in that scenario, we won't be able to toss
// anything over to the elevated window.
TerminalSettingsCreateResult controlSettings{ TerminalSettings::CreateWithNewTerminalArgs(_settings,
newTerminalArgs,
*_bindings) };
Profile profile{ _settings.GetProfileForArgs(newTerminalArgs) };
_maybeElevate(newTerminalArgs, controlSettings, profile);

return;
}

_SplitPane(*focusedTab, splitDirection, splitSize, newPane);
}

@@ -1864,6 +1845,14 @@ namespace winrt::TerminalApp::implementation
const float splitSize,
std::shared_ptr<Pane> newPane)
{
// If the caller is calling us with the return value of _MakePane
// directly, it's possible that nullptr was returned, if the connections
// was supposed to be launched in an elevated window. In that case, do
// nothing here. We don't have a pane with which to create the split.
if (!newPane)
{
return;
}
const float contentWidth = ::base::saturated_cast<float>(_tabContent.ActualWidth());
const float contentHeight = ::base::saturated_cast<float>(_tabContent.ActualHeight());
const winrt::Windows::Foundation::Size availableSpace{ contentWidth, contentHeight };
@@ -1890,102 +1879,7 @@ namespace winrt::TerminalApp::implementation
{
if (const auto control = _GetActiveControl())
{
profile = tab.GetFocusedProfile();
if (profile)
{
// TODO GH#5047 If we cache the NewTerminalArgs, we no longer need to do this.
profile = GetClosestProfileForDuplicationOfProfile(profile);
controlSettings = TerminalSettings::CreateWithProfile(_settings, profile, *_bindings);
const auto workingDirectory = tab.GetActiveTerminalControl().WorkingDirectory();
const auto validWorkingDirectory = !workingDirectory.empty();
if (validWorkingDirectory)
{
controlSettings.DefaultSettings().StartingDirectory(workingDirectory);
}
}
// TODO: GH#5047 - In the future, we should get the Profile of
// the focused pane, and use that to build a new instance of the
// settings so we can duplicate this tab/pane.
//
// Currently, if the profile doesn't exist anymore in our
// settings, we'll silently do nothing.
//
// In the future, it will be preferable to just duplicate the
// current control's settings, but we can't do that currently,
// because we won't be able to create a new instance of the
// connection without keeping an instance of the original Profile
// object around.
}
if (!profile)
{
profile = _settings.GetProfileForArgs(newTerminalArgs);
controlSettings = TerminalSettings::CreateWithNewTerminalArgs(_settings, newTerminalArgs, *_bindings);
}

// Try to handle auto-elevation
if (_maybeElevate(newTerminalArgs, controlSettings, profile))
{
return;
}

const auto controlConnection = _CreateConnectionFromSettings(profile, controlSettings.DefaultSettings());

const float contentWidth = ::base::saturated_cast<float>(_tabContent.ActualWidth());
const float contentHeight = ::base::saturated_cast<float>(_tabContent.ActualHeight());
const winrt::Windows::Foundation::Size availableSpace{ contentWidth, contentHeight };

auto realSplitType = splitDirection;
if (realSplitType == SplitDirection::Automatic)
{
realSplitType = tab.PreCalculateAutoSplit(availableSpace);
}

const auto canSplit = tab.PreCalculateCanSplit(realSplitType, splitSize, availableSpace);
if (!canSplit)
{
return;
}

auto newControl = _InitControl(controlSettings, controlConnection);
WUX::Controls::UserControl controlToAdd{ newControl };

const auto& cmdline{ controlSettings.DefaultSettings().Commandline() };
const auto doAdminWarning{ _shouldPromptForCommandline(cmdline) };
if (doAdminWarning)
{
auto warningControl{ winrt::make_self<implementation::AdminWarningPlaceholder>(newControl, cmdline) };
warningControl->PrimaryButtonClicked({ get_weak(), &TerminalPage::_adminWarningPrimaryClicked });
warningControl->CancelButtonClicked({ get_weak(), &TerminalPage::_adminWarningCancelClicked });
controlToAdd = *warningControl;
}

// Hookup our event handlers to the new terminal
_RegisterTerminalEvents(newControl);

_UnZoomIfNeeded();

tab.SplitPane(realSplitType, splitSize, profile, controlToAdd);

// After GH#6586, the control will no longer focus itself
// automatically when it's finished being laid out. Manually focus
// the control here instead.
if (_startupState == StartupState::Initialized)
{
if (const auto& activeControl{ _GetActiveControl() })
{
activeControl.Focus(FocusState::Programmatic);
}
}

if (doAdminWarning)
{
// We know this is safe - we literally just added the
// AdminWarningPlaceholder as the controlToAdd like 20 lines up.
//
// Focus the warning here. The LayoutUpdated within the dialog
// itself isn't good enough. That, for some reason, fires _before_
// the dialog is in the UI tree, which is useless for us.
controlToAdd.try_as<implementation::AdminWarningPlaceholder>()->FocusOnLaunch();
control.Focus(FocusState::Programmatic);
}
}
}
@@ -2584,7 +2478,13 @@ namespace winrt::TerminalApp::implementation
// a duplicate of the currently focused pane
// - existingConnection: optionally receives a connection from the outside
// world instead of attempting to create one
std::shared_ptr<Pane> TerminalPage::_MakePane(const NewTerminalArgs& newTerminalArgs, const bool duplicate, TerminalConnection::ITerminalConnection existingConnection)
// Return Value:
// - If the newTerminalArgs required us to open the pane as a new elevated
// connection, then we'll return nullptr. Otherwise, we'll return a new
// Pane for this connection.
std::shared_ptr<Pane> TerminalPage::_MakePane(const NewTerminalArgs& newTerminalArgs,
const bool duplicate,
TerminalConnection::ITerminalConnection existingConnection)
{
TerminalSettingsCreateResult controlSettings{ nullptr };
Profile profile{ nullptr };
@@ -2615,6 +2515,12 @@ namespace winrt::TerminalApp::implementation
controlSettings = TerminalSettings::CreateWithNewTerminalArgs(_settings, newTerminalArgs, *_bindings);
}

// Try to handle auto-elevation
if (_maybeElevate(newTerminalArgs, controlSettings, profile))
{
return nullptr;
}

auto connection = existingConnection ? existingConnection : _CreateConnectionFromSettings(profile, controlSettings.DefaultSettings());
if (existingConnection)
{
3 changes: 2 additions & 1 deletion src/cascadia/TerminalSettingsModel/MTSMSettings.h
Original file line number Diff line number Diff line change
@@ -73,7 +73,8 @@ Author(s):
X(hstring, Icon, "icon", L"\uE756") \
X(CloseOnExitMode, CloseOnExit, "closeOnExit", CloseOnExitMode::Graceful) \
X(hstring, TabTitle, "tabTitle") \
X(Model::BellStyle, BellStyle, "bellStyle", BellStyle::Audible)
X(Model::BellStyle, BellStyle, "bellStyle", BellStyle::Audible) \
X(bool, Elevate, "elevate", false)

#define MTSM_FONT_SETTINGS(X) \
X(hstring, FontFace, "face", DEFAULT_FONT_FACE) \
7 changes: 5 additions & 2 deletions src/cascadia/TerminalSettingsModel/Profile.cpp
Original file line number Diff line number Diff line change
@@ -176,8 +176,11 @@ void Profile::LayerJson(const Json::Value& json)
JsonUtils::GetValueForKey(json, PaddingKey, _Padding, JsonUtils::OptionalConverter<hstring, JsonUtils::PermissiveStringConverter<std::wstring>>{});

JsonUtils::GetValueForKey(json, TabColorKey, _TabColor);
JsonUtils::GetValueForKey(json, BellStyleKey, _BellStyle);
JsonUtils::GetValueForKey(json, ElevateKey, _Elevate);

#define PROFILE_SETTINGS_LAYER_JSON(type, name, jsonKey, ...) \
JsonUtils::GetValueForKey(json, jsonKey, _##name);
MTSM_PROFILE_SETTINGS(PROFILE_SETTINGS_LAYER_JSON)
#undef PROFILE_SETTINGS_LAYER_JSON

if (json.isMember(JsonKey(UnfocusedAppearanceKey)))
{
19 changes: 4 additions & 15 deletions src/cascadia/TerminalSettingsModel/Profile.h
Original file line number Diff line number Diff line change
@@ -119,21 +119,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
INHERITABLE_SETTING(Model::Profile, guid, Guid, _GenerateGuidForProfile(Name(), Source()));
INHERITABLE_SETTING(Model::Profile, hstring, Padding, DEFAULT_PADDING);

INHERITABLE_SETTING(Model::Profile, hstring, Commandline, L"%SystemRoot%\\System32\\cmd.exe");
INHERITABLE_SETTING(Model::Profile, hstring, StartingDirectory);

INHERITABLE_SETTING(Model::Profile, Microsoft::Terminal::Control::TextAntialiasingMode, AntialiasingMode, Microsoft::Terminal::Control::TextAntialiasingMode::Grayscale);
INHERITABLE_SETTING(Model::Profile, bool, ForceFullRepaintRendering, false);
INHERITABLE_SETTING(Model::Profile, bool, SoftwareRendering, false);

INHERITABLE_SETTING(Model::Profile, int32_t, HistorySize, DEFAULT_HISTORY_SIZE);
INHERITABLE_SETTING(Model::Profile, bool, SnapOnInput, true);
INHERITABLE_SETTING(Model::Profile, bool, AltGrAliasing, true);

INHERITABLE_SETTING(Model::Profile, Model::BellStyle, BellStyle, BellStyle::Audible);
INHERITABLE_SETTING(Model::Profile, bool, Elevate, false);

INHERITABLE_SETTING(Model::Profile, Model::IAppearanceConfig, UnfocusedAppearance, nullptr);
#define PROFILE_SETTINGS_INITIALIZE(type, name, jsonKey, ...) \
INHERITABLE_SETTING(Model::Profile, type, name, ##__VA_ARGS__)
MTSM_PROFILE_SETTINGS(PROFILE_SETTINGS_INITIALIZE)
#undef PROFILE_SETTINGS_INITIALIZE

private:
Model::IAppearanceConfig _DefaultAppearance{ winrt::make<AppearanceConfig>(weak_ref<Model::Profile>(*this)) };
You are viewing a condensed version of this merge commit. You can view the full changes here.