use projected type + other PR comments

microsoft · Sep 8, 2020 · 641e1c7 · 641e1c7
1 parent c3df3d8
commit 641e1c7
Show file tree

Hide file tree

Showing 10 changed files with 87 additions and 80 deletions.
diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp
@@ -320,7 +320,8 @@ namespace winrt::TerminalApp::implementation
                 if (auto activeControl = activeTab->GetActiveTerminalControl())
                 {
                     auto controlSettings = activeControl.Settings();
-                    if (_settings->ApplyColorScheme(controlSettings, realArgs.SchemeName()))
+                    const auto settingsImpl{ winrt::get_self<implementation::CascadiaSettings>(_settings) };
+                    if (settingsImpl->ApplyColorScheme(controlSettings, realArgs.SchemeName()))
                     {
                         activeControl.UpdateSettings(controlSettings);
                         args.Handled(true);

diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp
@@ -165,6 +165,17 @@ namespace winrt::TerminalApp::implementation
         return nullptr;
     }
 
+    // Method Description:
+    // - Returns the settings currently in use by the entire Terminal application.
+    // Throws:
+    // - HR E_INVALIDARG if the app isn't up and running.
+    const TerminalApp::CascadiaSettings AppLogic::CurrentAppSettings()
+    {
+        auto appLogic{ ::winrt::TerminalApp::implementation::AppLogic::Current() };
+        THROW_HR_IF_NULL(E_INVALIDARG, appLogic);
+        return appLogic->GetSettings();
+    }
+
     AppLogic::AppLogic() :
         _dialogLock{},
         _loadedInitialSettings{ false },
@@ -238,7 +249,7 @@ namespace winrt::TerminalApp::implementation
         // so this setting is overridden to false no matter what the preference is.
         if (_isUwp)
         {
-            _settings->GlobalSettings().ShowTabsInTitlebar(false);
+            _settings.GlobalSettings().ShowTabsInTitlebar(false);
         }
 
         _root->SetSettings(_settings, false);
@@ -256,14 +267,14 @@ namespace winrt::TerminalApp::implementation
         });
         _root->Create();
 
-        _ApplyTheme(_settings->GlobalSettings().Theme());
+        _ApplyTheme(_settings.GlobalSettings().Theme());
         _ApplyStartupTaskStateChange();
 
         TraceLoggingWrite(
             g_hTerminalAppProvider,
             "AppCreated",
             TraceLoggingDescription("Event emitted when the application is started"),
-            TraceLoggingBool(_settings->GlobalSettings().ShowTabsInTitlebar(), "TabsInTitlebar"),
+            TraceLoggingBool(_settings.GlobalSettings().ShowTabsInTitlebar(), "TabsInTitlebar"),
             TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
             TelemetryPrivacyDataTag(PDT_ProductAndServicePerformance));
     }
@@ -310,7 +321,7 @@ namespace winrt::TerminalApp::implementation
         // details here, but it does have the desired effect.
         // It's not enough to set the theme on the dialog alone.
         auto themingLambda{ [this](const Windows::Foundation::IInspectable& sender, const RoutedEventArgs&) {
-            auto theme{ _settings->GlobalSettings().Theme() };
+            auto theme{ _settings.GlobalSettings().Theme() };
             auto element{ sender.try_as<winrt::Windows::UI::Xaml::FrameworkElement>() };
             while (element)
             {
@@ -400,7 +411,8 @@ namespace winrt::TerminalApp::implementation
         // Make sure the lines of text wrap
         warningsTextBlock.TextWrapping(TextWrapping::Wrap);
 
-        const auto& warnings = _settings->GetWarnings();
+        const auto settingsImpl{ winrt::get_self<implementation::CascadiaSettings>(_settings) };
+        const auto& warnings = settingsImpl->GetWarnings();
         for (const auto& warning : warnings)
         {
             // Try looking up the warning message key for each warning.
@@ -483,7 +495,8 @@ namespace winrt::TerminalApp::implementation
         }
 
         // Use the default profile to determine how big of a window we need.
-        const auto [_, settings] = _settings->BuildSettings(nullptr);
+        const auto settingsImpl{ winrt::get_self<implementation::CascadiaSettings>(_settings) };
+        const auto [_, settings] = settingsImpl->BuildSettings(nullptr);
 
         auto proposedSize = TermControl::GetProposedDimensions(settings, dpi);
 
@@ -492,7 +505,7 @@ namespace winrt::TerminalApp::implementation
         // GH#2061 - If the global setting "Always show tab bar" is
         // set or if "Show tabs in title bar" is set, then we'll need to add
         // the height of the tab bar here.
-        if (_settings->GlobalSettings().ShowTabsInTitlebar())
+        if (_settings.GlobalSettings().ShowTabsInTitlebar())
         {
             // If we're showing the tabs in the titlebar, we need to use a
             // TitlebarControl here to calculate how much space to reserve.
@@ -506,7 +519,7 @@ namespace winrt::TerminalApp::implementation
             titlebar.Measure({ SHRT_MAX, SHRT_MAX });
             proposedSize.Height += (titlebar.DesiredSize().Height) * scale;
         }
-        else if (_settings->GlobalSettings().AlwaysShowTabs())
+        else if (_settings.GlobalSettings().AlwaysShowTabs())
         {
             // Otherwise, let's use a TabRowControl to calculate how much extra
             // space we'll need.
@@ -544,7 +557,7 @@ namespace winrt::TerminalApp::implementation
 
         // GH#4620/#5801 - If the user passed --maximized or --fullscreen on the
         // commandline, then use that to override the value from the settings.
-        const auto valueFromSettings = _settings->GlobalSettings().LaunchMode();
+        const auto valueFromSettings = _settings.GlobalSettings().LaunchMode();
         const auto valueFromCommandlineArgs = _appArgs.GetLaunchMode();
         return valueFromCommandlineArgs.has_value() ?
                    valueFromCommandlineArgs.value() :
@@ -569,7 +582,7 @@ namespace winrt::TerminalApp::implementation
             LoadSettings();
         }
 
-        const auto initialPosition{ _settings->GlobalSettings().InitialPosition() };
+        const auto initialPosition{ _settings.GlobalSettings().InitialPosition() };
         return {
             initialPosition.X ? initialPosition.X.Value() : defaultInitialX,
             initialPosition.Y ? initialPosition.Y.Value() : defaultInitialY
@@ -584,7 +597,7 @@ namespace winrt::TerminalApp::implementation
             LoadSettings();
         }
 
-        return _settings->GlobalSettings().Theme();
+        return _settings.GlobalSettings().Theme();
     }
 
     bool AppLogic::GetShowTabsInTitlebar()
@@ -595,7 +608,7 @@ namespace winrt::TerminalApp::implementation
             LoadSettings();
         }
 
-        return _settings->GlobalSettings().ShowTabsInTitlebar();
+        return _settings.GlobalSettings().ShowTabsInTitlebar();
     }
 
     // Method Description:
@@ -616,9 +629,9 @@ namespace winrt::TerminalApp::implementation
         try
         {
             auto newSettings = _isUwp ? CascadiaSettings::LoadUniversal() : CascadiaSettings::LoadAll();
-            _settings.copy_from(winrt::get_self<CascadiaSettings>(newSettings));
+            _settings = newSettings;
 
-            auto settingsImpl = _settings.as<CascadiaSettings>();
+            const auto settingsImpl{ winrt::get_self<implementation::CascadiaSettings>(_settings) };
             const auto& warnings = settingsImpl->GetWarnings();
             hr = warnings.size() == 0 ? S_OK : S_FALSE;
         }
@@ -677,7 +690,7 @@ namespace winrt::TerminalApp::implementation
 
         if (FAILED(_settingsLoadedResult))
         {
-            _settings.copy_from(winrt::get_self<CascadiaSettings>(CascadiaSettings::LoadDefaults()));
+            _settings = CascadiaSettings::LoadDefaults();
         }
 
         auto end = std::chrono::high_resolution_clock::now();
@@ -773,7 +786,7 @@ namespace winrt::TerminalApp::implementation
         co_await winrt::resume_foreground(_root->Dispatcher());
 
         // Refresh the UI theme
-        _ApplyTheme(_settings->GlobalSettings().Theme());
+        _ApplyTheme(_settings.GlobalSettings().Theme());
     }
 
     fire_and_forget AppLogic::_ApplyStartupTaskStateChange()
@@ -792,7 +805,7 @@ namespace winrt::TerminalApp::implementation
         if (auto page{ weakThis.get() })
         {
             StartupTaskState state;
-            bool tryEnableStartupTask = _settings->GlobalSettings().StartOnUserLogin();
+            bool tryEnableStartupTask = _settings.GlobalSettings().StartOnUserLogin();
             StartupTask task = co_await StartupTask::GetAsync(StartupTaskName);
 
             state = task.State();
@@ -859,7 +872,7 @@ namespace winrt::TerminalApp::implementation
     // - Returns a pointer to the global shared settings.
     [[nodiscard]] TerminalApp::CascadiaSettings AppLogic::GetSettings() const noexcept
     {
-        return *_settings;
+        return _settings;
     }
 
     // Method Description:

diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h
@@ -16,6 +16,7 @@ namespace winrt::TerminalApp::implementation
     {
     public:
         static AppLogic* Current() noexcept;
+        static const TerminalApp::CascadiaSettings CurrentAppSettings();
 
         AppLogic();
         ~AppLogic() = default;
@@ -68,7 +69,7 @@ namespace winrt::TerminalApp::implementation
         // updated in _ApplyTheme. The root currently is _root.
         winrt::com_ptr<TerminalPage> _root{ nullptr };
 
-        winrt::com_ptr<CascadiaSettings> _settings{ nullptr };
+        TerminalApp::CascadiaSettings _settings{ nullptr };
 
         HRESULT _settingsLoadedResult;
         winrt::hstring _settingsLoadExceptionText{};

diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp
@@ -364,8 +364,7 @@ void CascadiaSettings::_ReorderProfilesToMatchUserSettingsOrder()
 // - <none>
 void CascadiaSettings::_RemoveHiddenProfiles()
 {
-    uint32_t i = 0;
-    while (i < _profiles.Size())
+    for (uint32_t i{}; i < _profiles.Size();)
     {
         if (_profiles.GetAt(i).Hidden())
         {
@@ -701,7 +700,7 @@ std::string CascadiaSettings::_ApplyFirstRunChangesToSettingsTemplate(std::strin
     std::string finalSettings{ settingsTemplate };
 
     std::wstring defaultProfileGuid{ DEFAULT_WINDOWS_POWERSHELL_GUID };
-    if (const auto psCoreProfileGuid{ _GetProfileGuidByName(PowershellCoreProfileGenerator::GetPreferredPowershellProfileName().data()) })
+    if (const auto psCoreProfileGuid{ _GetProfileGuidByName(hstring{ PowershellCoreProfileGenerator::GetPreferredPowershellProfileName() }) })
     {
         defaultProfileGuid = Utils::GuidToString(*psCoreProfileGuid);
     }

diff --git a/src/cascadia/TerminalApp/CascadiaSettings.h b/src/cascadia/TerminalApp/CascadiaSettings.h
@@ -60,7 +60,7 @@ namespace winrt::TerminalApp::implementation
     {
     public:
         CascadiaSettings();
-        CascadiaSettings(const bool addDynamicProfiles);
+        explicit CascadiaSettings(const bool addDynamicProfiles);
 
         static TerminalApp::CascadiaSettings LoadDefaults();
         static TerminalApp::CascadiaSettings LoadAll();

diff --git a/src/cascadia/TerminalApp/CascadiaSettings.idl b/src/cascadia/TerminalApp/CascadiaSettings.idl
@@ -1,22 +1,16 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import "TerminalSettings.idl";
 import "GlobalAppSettings.idl";
 import "Profile.idl";
 
 namespace TerminalApp
 {
     [default_interface] runtimeclass CascadiaSettings {
-        CascadiaSettings();
-        CascadiaSettings(Boolean addDynamicProfiles);
-
         static CascadiaSettings LoadDefaults();
         static CascadiaSettings LoadAll();
         static CascadiaSettings LoadUniversal();
 
-        TerminalSettings BuildSettings(Guid profileGuid);
-
         GlobalAppSettings GlobalSettings { get; };
 
         Windows.Foundation.Collections.IObservableVector<Profile> Profiles { get; };
@@ -25,8 +19,5 @@ namespace TerminalApp
 
         Profile FindProfile(Guid profileGuid);
         ColorScheme GetColorSchemeForProfile(Guid profileGuid);
-
-        Boolean ApplyColorScheme(Microsoft.Terminal.TerminalControl.IControlSettings settings, String schemeName);
-
     }
 }
diff --git a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp
@@ -264,7 +264,7 @@ winrt::TerminalApp::CascadiaSettings CascadiaSettings::LoadUniversal()
     // If this throws, the app will catch it and use the default settings
     resultPtr->_ValidateSettings();
 
-    return resultPtr.as<winrt::TerminalApp::CascadiaSettings>();
+    return *resultPtr;
 }
 
 // Function Description:
@@ -620,7 +620,7 @@ void CascadiaSettings::_LayerOrCreateProfile(const Json::Value& profileJson)
             }
 
             profile->LayerJson(profileJson);
-            _profiles.Append(profile.as<winrt::TerminalApp::Profile>());
+            _profiles.Append(*profile);
         }
     }
 }

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
@@ -319,9 +319,7 @@ void Pane::_ControlConnectionStateChangedHandler(const TermControl& /*sender*/,
         return;
     }
 
-    const auto appLogic{ winrt::TerminalApp::implementation::AppLogic::Current() };
-    THROW_HR_IF_NULL(E_INVALIDARG, appLogic);
-    auto settings = appLogic->GetSettings();
+    const auto settings{ winrt::TerminalApp::implementation::AppLogic::CurrentAppSettings() };
     auto paneProfile = settings.FindProfile(_profile.value());
     if (paneProfile)
     {