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

Break everything out of App except Xaml platform init #3465

Merged
merged 4 commits into from
Nov 7, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/App.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace winrt::TerminalApp::implementation
Initialize();
}

static AppLogic& Logic()
AppLogic App::Logic()
{
static AppLogic logic;
miniksa marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Why is this not a member of App? You could wait to instantiate it until this is first called.

Copy link
Member Author

@miniksa miniksa Nov 7, 2019

Choose a reason for hiding this comment

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

@DHowett-MSFT, as far as I can see, the magic static is safer than the alternative as it cannot accidentally be referenced from elsewhere in the class without being allocated and will only be allocated deferred to the first call.

Doing what Mike says opens us up to a null check on the instance variable, theoretically. But this is indeed magic and maybe we shouldn't magic?

return logic;
Expand All @@ -48,7 +48,7 @@ namespace winrt::TerminalApp::implementation
auto content = Window::Current().Content();
if (content == nullptr)
{
auto& logic = Logic();
auto logic = Logic();
logic.LoadSettings();
logic.Create();

Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/App.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ namespace winrt::TerminalApp::implementation
App();
void OnLaunched(Windows::ApplicationModel::Activation::LaunchActivatedEventArgs const&);

TerminalApp::AppLogic Logic();

private:
bool _isUwp = false;
};
Expand Down
8 changes: 6 additions & 2 deletions src/cascadia/TerminalApp/App.idl
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import "../AppLogic.idl";

namespace TerminalApp
{
// ADD ARBITRARY APP LOGIC TO APPLOGIC.IDL, NOT HERE.
// This is for XAML platform setup only.
// ADD ARBITRARY APP LOGIC TO APPLOGIC.IDL, NOT HERE.
// This is for XAML platform setup only.
[default_interface] runtimeclass App : Microsoft.Toolkit.Win32.UI.XamlHost.XamlApplication
{
App();

AppLogic Logic { get; };
}
}
31 changes: 15 additions & 16 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ using namespace ::Microsoft::Console::Types;

AppHost::AppHost() noexcept :
_app{},
_logic{},
_window{ nullptr }
{
miniksa marked this conversation as resolved.
Show resolved Hide resolved
_useNonClientArea = _logic.GetShowTabsInTitlebar();
_useNonClientArea = _app.Logic().GetShowTabsInTitlebar();

if (_useNonClientArea)
{
_window = std::make_unique<NonClientIslandWindow>(_logic.GetRequestedTheme());
_window = std::make_unique<NonClientIslandWindow>(_app.Logic().GetRequestedTheme());
}
else
{
Expand Down Expand Up @@ -69,30 +68,30 @@ void AppHost::Initialize()
// Register our callbar for when the app's non-client content changes.
// This has to be done _before_ App::Create, as the app might set the
// content in Create.
_logic.SetTitleBarContent({ this, &AppHost::_UpdateTitleBarContent });
_app.Logic().SetTitleBarContent({ this, &AppHost::_UpdateTitleBarContent });
}

// Register the 'X' button of the window for a warning experience of multiple
// tabs opened, this is consistent with Alt+F4 closing
_window->WindowCloseButtonClicked([this]() { _logic.WindowCloseButtonClicked(); });
_window->WindowCloseButtonClicked([this]() { _app.Logic().WindowCloseButtonClicked(); });

// Add an event handler to plumb clicks in the titlebar area down to the
// application layer.
_window->DragRegionClicked([this]() { _logic.TitlebarClicked(); });
_window->DragRegionClicked([this]() { _app.Logic().TitlebarClicked(); });

_logic.RequestedThemeChanged({ this, &AppHost::_UpdateTheme });
_logic.ToggleFullscreen({ this, &AppHost::_ToggleFullscreen });
_app.Logic().RequestedThemeChanged({ this, &AppHost::_UpdateTheme });
_app.Logic().ToggleFullscreen({ this, &AppHost::_ToggleFullscreen });

_logic.Create();
_app.Logic().Create();

_logic.TitleChanged({ this, &AppHost::AppTitleChanged });
_logic.LastTabClosed({ this, &AppHost::LastTabClosed });
_app.Logic().TitleChanged({ this, &AppHost::AppTitleChanged });
_app.Logic().LastTabClosed({ this, &AppHost::LastTabClosed });

_window->UpdateTitle(_logic.Title());
_window->UpdateTitle(_app.Logic().Title());

// Set up the content of the application. If the app has a custom titlebar,
// set that content as well.
_window->SetContent(_logic.GetRoot());
_window->SetContent(_app.Logic().GetRoot());
_window->OnAppInitialized();
}

Expand Down Expand Up @@ -137,10 +136,10 @@ void AppHost::LastTabClosed(const winrt::Windows::Foundation::IInspectable& /*se
// - None
void AppHost::_HandleCreateWindow(const HWND hwnd, RECT proposedRect, winrt::TerminalApp::LaunchMode& launchMode)
{
launchMode = _logic.GetLaunchMode();
launchMode = _app.Logic().GetLaunchMode();

// Acquire the actual intial position
winrt::Windows::Foundation::Point initialPosition = _logic.GetLaunchInitialPositions(proposedRect.left, proposedRect.top);
winrt::Windows::Foundation::Point initialPosition = _app.Logic().GetLaunchInitialPositions(proposedRect.left, proposedRect.top);
proposedRect.left = gsl::narrow_cast<long>(initialPosition.X);
proposedRect.top = gsl::narrow_cast<long>(initialPosition.Y);

Expand Down Expand Up @@ -188,7 +187,7 @@ void AppHost::_HandleCreateWindow(const HWND hwnd, RECT proposedRect, winrt::Ter
proposedRect.top = monitorInfo.rcWork.top;
}

auto initialSize = _logic.GetLaunchDimensions(dpix);
auto initialSize = _app.Logic().GetLaunchDimensions(dpix);

const short islandWidth = Utils::ClampToShortMax(
static_cast<long>(ceil(initialSize.X)), 1);
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/WindowsTerminal/AppHost.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ class AppHost

std::unique_ptr<IslandWindow> _window;
winrt::TerminalApp::App _app;
winrt::TerminalApp::AppLogic _logic;

void _HandleCreateWindow(const HWND hwnd, RECT proposedRect, winrt::TerminalApp::LaunchMode& launchMode);
void _UpdateTitleBarContent(const winrt::Windows::Foundation::IInspectable& sender,
Expand Down