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.

Sign up for GitHub

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

Jump to bottom

Move TerminalSettings from TermApp to TerminalSettingsModel #9318

Merged
9 commits merged into from
Mar 15, 2021
Merged

Move TerminalSettings from TermApp to TerminalSettingsModel #9318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cae4ec7
Move TerminalSettings from TermApp to TSM
carlos-zamora Mar 1, 2021
1e968fd
polish PR for review
carlos-zamora Mar 4, 2021
187a510
Merge branch 'main' into dev/cazamor/sui/move-term-sets
carlos-zamora Mar 4, 2021
fc26768
fix tests (verified)
carlos-zamora Mar 6, 2021
e2fb1e7
Merge branch 'main' into dev/cazamor/sui/move-term-sets
carlos-zamora Mar 8, 2021
0eaf930
add comment for BuildSettings
carlos-zamora Mar 9, 2021
3de9894
address Dustin's PR comments
carlos-zamora Mar 11, 2021
1321d76
a bit more polish
carlos-zamora Mar 12, 2021
f3ac13a
Merge branch 'main' into dev/cazamor/sui/move-term-sets
carlos-zamora Mar 15, 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
1 change: 1 addition & 0 deletions src/cascadia/LocalTests_SettingsModel/SettingsModel.LocalTests.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
<ClCompile Include="CommandTests.cpp" />
<ClCompile Include="DeserializationTests.cpp" />
<ClCompile Include="SerializationTests.cpp" />
<ClCompile Include="TerminalSettingsTests.cpp" />
<ClCompile Include="pch.cpp">
<PrecompiledHeader>Create</PrecompiledHeader>
</ClCompile>
Expand Down
543 changes: 543 additions & 0 deletions src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp
View file
Open in desktop

Large diffs are not rendered by default.

491 changes: 0 additions & 491 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
View file
Open in desktop

Large diffs are not rendered by default.

7 changes: 3 additions & 4 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ namespace winrt::TerminalApp::implementation
if (const auto scheme = _settings.GlobalSettings().ColorSchemes().TryLookup(realArgs.SchemeName()))
{
auto controlSettings = activeControl.Settings().as<TerminalSettings>();
controlSettings->ApplyColorScheme(scheme);
controlSettings.ApplyColorScheme(scheme);
activeControl.UpdateSettings();
args.Handled(true);
}
Expand Down Expand Up @@ -658,9 +658,8 @@ namespace winrt::TerminalApp::implementation
newTerminalArgs = NewTerminalArgs();
}

auto [profileGuid, settings] = TerminalSettings::BuildSettings(_settings,
newTerminalArgs,
*_bindings);
const auto profileGuid{ _settings.GetProfileForArgs(newTerminalArgs) };
const auto settings{ TerminalSettings::CreateWithNewTerminalArgs(_settings, newTerminalArgs, *_bindings) };

// Manually fill in the evaluated profile.
newTerminalArgs.Profile(::Microsoft::Console::Utils::GuidToString(profileGuid));
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppLogic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ namespace winrt::TerminalApp::implementation
}

// Use the default profile to determine how big of a window we need.
const auto [_, settings] = TerminalSettings::BuildSettings(_settings, nullptr, nullptr);
const auto settings{ TerminalSettings::CreateWithNewTerminalArgs(_settings, nullptr, nullptr) };

auto proposedSize = TermControl::GetProposedDimensions(settings, dpi);

Expand Down
5 changes: 1 addition & 4 deletions src/cascadia/TerminalApp/Pane.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,10 +644,7 @@ void Pane::UpdateSettings(const TerminalSettings& settings, const GUID& profile)
{
// Update the parent of the control's settings object (and not the object itself) so
// that any overrides made by the control don't get affected by the reload
auto child = winrt::get_self<winrt::TerminalApp::implementation::TerminalSettings>(_control.Settings());
auto parent = winrt::get_self<winrt::TerminalApp::implementation::TerminalSettings>(settings);
child->ClearParents();
child->InsertParent(0, parent->get_strong());
_control.Settings().as<TerminalSettings>().SetParent(settings);
_control.UpdateSettings();
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/Pane.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class Pane : public std::enable_shared_from_this<Pane>
void ClearActive();
void SetActive();

void UpdateSettings(const winrt::TerminalApp::TerminalSettings& settings,
void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::TerminalSettings& settings,
const GUID& profile);
void ResizeContent(const winrt::Windows::Foundation::Size& newSize);
void Relayout();
Expand Down
7 changes: 0 additions & 7 deletions src/cascadia/TerminalApp/TerminalAppLib.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,6 @@
</ClInclude>
<ClInclude Include="Pane.h" />
<ClInclude Include="ColorHelper.h" />
<ClInclude Include="TerminalSettings.h">
<DependentUpon>TerminalSettings.idl</DependentUpon>
</ClInclude>
<ClInclude Include="pch.h" />
<ClInclude Include="ShortcutActionDispatch.h">
<DependentUpon>ShortcutActionDispatch.idl</DependentUpon>
Expand Down Expand Up @@ -205,9 +202,6 @@
<ClCompile Include="Pane.LayoutSizeNode.cpp" />
<ClCompile Include="ColorHelper.cpp" />
<ClCompile Include="DebugTapConnection.cpp" />
<ClCompile Include="TerminalSettings.cpp">
<DependentUpon>TerminalSettings.idl</DependentUpon>
</ClCompile>
<ClCompile Include="pch.cpp">
<PrecompiledHeader>Create</PrecompiledHeader>
</ClCompile>
Expand Down Expand Up @@ -287,7 +281,6 @@
</Midl>
<Midl Include="FilteredCommand.idl" />
<Midl Include="EmptyStringVisibilityConverter.idl" />
<Midl Include="TerminalSettings.idl" />
</ItemGroup>
<!-- ========================= Misc Files ======================== -->
<ItemGroup>
Expand Down
11 changes: 1 addition & 10 deletions src/cascadia/TerminalApp/TerminalAppLib.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@
<ClCompile Include="ColorHelper.cpp" />
<ClCompile Include="DebugTapConnection.cpp" />
<ClCompile Include="Utils.cpp" />
<ClCompile Include="TerminalSettings.cpp">
<Filter>settings</Filter>
</ClCompile>
<ClCompile Include="Jumplist.cpp" />
<ClCompile Include="Tab.cpp">
<Filter>tab</Filter>
Expand Down Expand Up @@ -70,10 +67,7 @@
<ClInclude Include="Commandline.h" />
<ClInclude Include="DebugTapConnection.h" />
<ClInclude Include="ColorHelper.h" />
<ClInclude Include="TerminalSettings.h">
<Filter>settings</Filter>
</ClInclude>
<ClInclude Include="Jumplist.h" />
<ClInclude Include="Jumplist.h" />
<ClInclude Include="Tab.h">
<Filter>tab</Filter>
</ClInclude>
Expand Down Expand Up @@ -113,9 +107,6 @@
<Filter>settings</Filter>
</Midl>
<Midl Include="IDirectKeyListener.idl" />
<Midl Include="TerminalSettings.idl">
<Filter>settings</Filter>
</Midl>
<Midl Include="ITab.idl">
<Filter>tab</Filter>
</Midl>
Expand Down
24 changes: 13 additions & 11 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,11 +757,12 @@ namespace winrt::TerminalApp::implementation
// Arguments:
// - newTerminalArgs: An object that may contain a blob of parameters to
// control which profile is created and with possible other
// configurations. See TerminalSettings::BuildSettings for more details.
// configurations. See TerminalSettings::CreateWithNewTerminalArgs for more details.
void TerminalPage::_OpenNewTab(const NewTerminalArgs& newTerminalArgs)
try
{
auto [profileGuid, settings] = TerminalSettings::BuildSettings(_settings, newTerminalArgs, *_bindings);
Copy link
Member

Choose a reason for hiding this comment

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

wow crazy how many times we were actually using the guid in the return value of this function too. Shame WinRT won't let us return the pair. Shame.

Copy link
Member

Choose a reason for hiding this comment

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

I really don't love that this responsibility moved into TSM but ok.

const auto profileGuid{ _settings.GetProfileForArgs(newTerminalArgs) };
const auto settings{ TerminalSettings::CreateWithNewTerminalArgs(_settings, newTerminalArgs, *_bindings) };

_CreateNewTabFromSettings(profileGuid, settings);

Expand Down Expand Up @@ -805,7 +806,7 @@ namespace winrt::TerminalApp::implementation
// currently displayed, it will be shown.
// Arguments:
// - settings: the TerminalSettings object to use to create the TerminalControl with.
void TerminalPage::_CreateNewTabFromSettings(GUID profileGuid, TerminalApp::TerminalSettings settings)
void TerminalPage::_CreateNewTabFromSettings(GUID profileGuid, TerminalSettings settings)
{
// Initialize the new tab

Expand Down Expand Up @@ -940,7 +941,7 @@ namespace winrt::TerminalApp::implementation
// Return value:
// - the desired connection
TerminalConnection::ITerminalConnection TerminalPage::_CreateConnectionFromSettings(GUID profileGuid,
TerminalApp::TerminalSettings settings)
TerminalSettings settings)
{
const auto profile = _settings.FindProfile(profileGuid);

Expand Down Expand Up @@ -1299,7 +1300,7 @@ namespace winrt::TerminalApp::implementation
const auto& profileGuid = tab.GetFocusedProfile();
if (profileGuid.has_value())
{
const auto settings{ winrt::make<TerminalSettings>(_settings, profileGuid.value(), *_bindings) };
const auto settings{ TerminalSettings::CreateWithProfileByID(_settings, profileGuid.value(), *_bindings) };
const auto workingDirectory = tab.GetActiveTerminalControl().WorkingDirectory();
const auto validWorkingDirectory = !workingDirectory.empty();
if (validWorkingDirectory)
Expand Down Expand Up @@ -1858,7 +1859,7 @@ namespace winrt::TerminalApp::implementation

try
{
TerminalApp::TerminalSettings controlSettings;
TerminalSettings controlSettings{ nullptr };
GUID realGuid;
bool profileFound = false;

Expand All @@ -1868,7 +1869,7 @@ namespace winrt::TerminalApp::implementation
if (current_guid)
{
profileFound = true;
controlSettings = { winrt::make<TerminalSettings>(_settings, current_guid.value(), *_bindings) };
controlSettings = TerminalSettings::CreateWithProfileByID(_settings, current_guid.value(), *_bindings);
const auto workingDirectory = focusedTab->GetActiveTerminalControl().WorkingDirectory();
const auto validWorkingDirectory = !workingDirectory.empty();
if (validWorkingDirectory)
Expand All @@ -1892,7 +1893,8 @@ namespace winrt::TerminalApp::implementation
}
if (!profileFound)
{
std::tie(realGuid, controlSettings) = TerminalSettings::BuildSettings(_settings, newTerminalArgs, *_bindings);
realGuid = _settings.GetProfileForArgs(newTerminalArgs);
controlSettings = TerminalSettings::CreateWithNewTerminalArgs(_settings, newTerminalArgs, *_bindings);
}

const auto controlConnection = _CreateConnectionFromSettings(realGuid, controlSettings);
Expand Down Expand Up @@ -2546,9 +2548,9 @@ namespace winrt::TerminalApp::implementation
_RemoveTabViewItem(tabViewItem);
}

TermControl TerminalPage::_InitControl(const TerminalApp::TerminalSettings& settings, const ITerminalConnection& connection)
TermControl TerminalPage::_InitControl(const TerminalSettings& settings, const ITerminalConnection& connection)
{
return TermControl{ *(winrt::get_self<TerminalSettings>(settings)->CreateChild()), connection };
return TermControl{ TerminalSettings::CreateWithParent(settings), connection };
}

// Method Description:
Expand All @@ -2572,7 +2574,7 @@ namespace winrt::TerminalApp::implementation
{
// This can throw an exception if the profileGuid does
// not belong to an actual profile in the list of profiles.
auto settings{ winrt::make<TerminalSettings>(_settings, profileGuid, *_bindings) };
auto settings{ TerminalSettings::CreateWithProfileByID(_settings, profileGuid, *_bindings) };

for (auto tab : _tabs)
{
Expand Down
7 changes: 3 additions & 4 deletions src/cascadia/TerminalApp/TerminalPage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "TerminalPage.g.h"
#include "TerminalTab.h"
#include "AppKeyBindings.h"
#include "TerminalSettings.h"

#include <winrt/Microsoft.Terminal.TerminalControl.h>

Expand Down Expand Up @@ -152,8 +151,8 @@ namespace winrt::TerminalApp::implementation
void _CreateNewTabFlyout();
void _OpenNewTabDropdown();
void _OpenNewTab(const Microsoft::Terminal::Settings::Model::NewTerminalArgs& newTerminalArgs);
void _CreateNewTabFromSettings(GUID profileGuid, TerminalApp::TerminalSettings settings);
winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _CreateConnectionFromSettings(GUID profileGuid, TerminalApp::TerminalSettings settings);
void _CreateNewTabFromSettings(GUID profileGuid, Microsoft::Terminal::Settings::Model::TerminalSettings settings);
winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection _CreateConnectionFromSettings(GUID profileGuid, Microsoft::Terminal::Settings::Model::TerminalSettings settings);

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

Expand Down Expand Up @@ -248,7 +247,7 @@ namespace winrt::TerminalApp::implementation

void _Find();

winrt::Microsoft::Terminal::TerminalControl::TermControl _InitControl(const winrt::TerminalApp::TerminalSettings& settings,
winrt::Microsoft::Terminal::TerminalControl::TermControl _InitControl(const winrt::Microsoft::Terminal::Settings::Model::TerminalSettings& settings,
const winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection& connection);

winrt::fire_and_forget _RefreshUIForSettingsReload();
Expand Down
Loading