Griese PR Comments

microsoft · Aug 21, 2020 · 32b9223 · 32b9223
1 parent b343efa
commit 32b9223
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 28 deletions.
diff --git a/src/cascadia/TerminalApp/Profile.cpp b/src/cascadia/TerminalApp/Profile.cpp
@@ -65,7 +65,7 @@ Profile::Profile() :
 }
 
 Profile::Profile(IReference<guid> guid) :
-    _Guid(guid)
+    _Guid(guid ? static_cast<decltype(_Guid)>(guid.Value()) : std::nullopt)
 {
     /* FontWeight is initialized here because the structure won't accept a uint16_t directly */
     winrt::Windows::UI::Text::FontWeight weight;
@@ -205,9 +205,9 @@ Json::Value Profile::GenerateStub() const
     Json::Value stub;
 
     ///// Profile-specific settings /////
-    if (_Guid != nullptr)
+    if (_Guid.has_value())
     {
-        stub[JsonKey(GuidKey)] = winrt::to_string(Utils::GuidToString(_Guid.Value()));
+        stub[JsonKey(GuidKey)] = winrt::to_string(Utils::GuidToString(*_Guid));
     }
 
     stub[JsonKey(NameKey)] = winrt::to_string(_Name);
@@ -245,7 +245,7 @@ winrt::com_ptr<Profile> Profile::FromJson(const Json::Value& json)
 // - true iff the json object has the same `GUID` as we do.
 bool Profile::ShouldBeLayered(const Json::Value& json) const
 {
-    if (_Guid == nullptr)
+    if (!_Guid.has_value())
     {
         return false;
     }
@@ -254,7 +254,7 @@ bool Profile::ShouldBeLayered(const Json::Value& json) const
     // should _definitely_ not layer.
     if (const auto otherGuid{ JsonUtils::GetValueForKey<std::optional<winrt::guid>>(json, GuidKey) })
     {
-        if (otherGuid.value() != _Guid.Value())
+        if (otherGuid.value() != *_Guid)
         {
             return false;
         }
@@ -433,7 +433,7 @@ std::wstring Profile::EvaluateStartingDirectory(const std::wstring& directory)
 //   will _not_ change the profile's GUID.
 void Profile::GenerateGuidIfNecessary() noexcept
 {
-    if (_Guid == nullptr)
+    if (!_Guid.has_value())
     {
         // Always use the name to generate the temporary GUID. That way, across
         // reloads, we'll generate the same static GUID.
@@ -541,14 +541,14 @@ void Profile::BackgroundImageVerticalAlignment(const VerticalAlignment& value) n
 
 bool Profile::HasGuid() const
 {
-    return _Guid != nullptr;
+    return _Guid.has_value();
 }
 
 winrt::guid Profile::Guid() const
 {
     // This can throw if we never had our guid set to a legitimate value.
-    THROW_HR_IF_MSG(E_FAIL, _Guid == nullptr, "Profile._guid always expected to have a value");
-    return _Guid.Value();
+    THROW_HR_IF_MSG(E_FAIL, !_Guid.has_value(), "Profile._guid always expected to have a value");
+    return *_Guid;
 }
 
 void Profile::Guid(winrt::guid guid)
@@ -558,12 +558,12 @@ void Profile::Guid(winrt::guid guid)
 
 bool Profile::HasConnectionType() const
 {
-    return _ConnectionType != nullptr;
+    return _ConnectionType.has_value();
 }
 
 winrt::guid Profile::ConnectionType() const
 {
-    return _ConnectionType.Value();
+    return *_ConnectionType;
 }
 
 void Profile::ConnectionType(winrt::guid conType)

diff --git a/src/cascadia/TerminalApp/Profile.h b/src/cascadia/TerminalApp/Profile.h
@@ -69,6 +69,12 @@ namespace winrt::TerminalApp::implementation
         winrt::guid ConnectionType() const;
         void ConnectionType(winrt::guid conType);
 
+        // BackgroundImageAlignment is 1 setting saved as 2 separate values
+        Windows::UI::Xaml::HorizontalAlignment BackgroundImageHorizontalAlignment() const noexcept;
+        void BackgroundImageHorizontalAlignment(const Windows::UI::Xaml::HorizontalAlignment& value) noexcept;
+        Windows::UI::Xaml::VerticalAlignment BackgroundImageVerticalAlignment() const noexcept;
+        void BackgroundImageVerticalAlignment(const Windows::UI::Xaml::VerticalAlignment& value) noexcept;
+
         GETSET_PROPERTY(hstring, Name, L"Default");
         GETSET_PROPERTY(hstring, Source);
         GETSET_PROPERTY(bool, Hidden, false);
@@ -78,11 +84,11 @@ namespace winrt::TerminalApp::implementation
         GETSET_PROPERTY(CloseOnExitMode, CloseOnExit, CloseOnExitMode::Graceful);
         GETSET_PROPERTY(hstring, TabTitle);
         GETSET_PROPERTY(Windows::Foundation::IReference<Windows::UI::Color>, TabColor);
-        GETSET_PROPERTY(bool, SuppressApplicationTitle);
+        GETSET_PROPERTY(bool, SuppressApplicationTitle, false);
 
         GETSET_PROPERTY(bool, UseAcrylic, false);
         GETSET_PROPERTY(double, AcrylicOpacity, 0.5);
-        GETSET_PROPERTY(Microsoft::Terminal::TerminalControl::ScrollbarState, ScrollState);
+        GETSET_PROPERTY(Microsoft::Terminal::TerminalControl::ScrollbarState, ScrollState, Microsoft::Terminal::TerminalControl::ScrollbarState::Visible);
 
         GETSET_PROPERTY(hstring, FontFace, DEFAULT_FONT_FACE);
         GETSET_PROPERTY(int32_t, FontSize, DEFAULT_FONT_SIZE);
@@ -96,20 +102,10 @@ namespace winrt::TerminalApp::implementation
         GETSET_PROPERTY(Windows::Foundation::IReference<double>, BackgroundImageOpacity);
         GETSET_PROPERTY(Windows::Foundation::IReference<Windows::UI::Xaml::Media::Stretch>, BackgroundImageStretchMode, Windows::UI::Xaml::Media::Stretch::Fill);
 
-    public:
-        // BackgroundImageAlignment is 1 setting saved as 2 separate values
-        Windows::UI::Xaml::HorizontalAlignment BackgroundImageHorizontalAlignment() const noexcept;
-        void BackgroundImageHorizontalAlignment(const Windows::UI::Xaml::HorizontalAlignment& value) noexcept;
-        Windows::UI::Xaml::VerticalAlignment BackgroundImageVerticalAlignment() const noexcept;
-        void BackgroundImageVerticalAlignment(const Windows::UI::Xaml::VerticalAlignment& value) noexcept;
-
-    private:
-        std::optional<std::tuple<Windows::UI::Xaml::HorizontalAlignment, Windows::UI::Xaml::VerticalAlignment>> _BackgroundImageAlignment;
-
         GETSET_PROPERTY(Microsoft::Terminal::TerminalControl::TextAntialiasingMode, AntialiasingMode, Microsoft::Terminal::TerminalControl::TextAntialiasingMode::Grayscale);
-        GETSET_PROPERTY(bool, RetroTerminalEffect);
-        GETSET_PROPERTY(bool, ForceFullRepaintRendering);
-        GETSET_PROPERTY(bool, SoftwareRendering);
+        GETSET_PROPERTY(bool, RetroTerminalEffect, false);
+        GETSET_PROPERTY(bool, ForceFullRepaintRendering, false);
+        GETSET_PROPERTY(bool, SoftwareRendering, false);
 
         GETSET_PROPERTY(hstring, ColorSchemeName, L"Campbell");
         GETSET_PROPERTY(Windows::Foundation::IReference<Windows::UI::Color>, Foreground);
@@ -125,8 +121,9 @@ namespace winrt::TerminalApp::implementation
         GETSET_PROPERTY(uint32_t, CursorHeight, DEFAULT_CURSOR_HEIGHT);
 
     private:
-        Windows::Foundation::IReference<winrt::guid> _Guid = nullptr;
-        Windows::Foundation::IReference<winrt::guid> _ConnectionType = nullptr;
+        std::optional<winrt::guid> _Guid = std::nullopt;
+        std::optional<winrt::guid> _ConnectionType = std::nullopt;
+        std::optional<std::tuple<Windows::UI::Xaml::HorizontalAlignment, Windows::UI::Xaml::VerticalAlignment>> _BackgroundImageAlignment;
 
         static std::wstring EvaluateStartingDirectory(const std::wstring& directory);