Make Global and Profile settings inheritable

.github/actions/spell-check/dictionary/apis.txt

@@ -21,6 +21,7 @@ ICustom
 IDialog
 IDirect
 IExplorer
+IInheritable
 IMap
 IObject
 IStorage

src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp

@@ -81,6 +81,7 @@ namespace SettingsModelLocalTests
         TEST_METHOD(TestRebindNestedCommand);
 
         TEST_METHOD(TestCopy);
+        TEST_METHOD(TestCloneInheritanceTree);
 
         TEST_CLASS_SETUP(ClassSetup)
         {
@@ -831,7 +832,7 @@ namespace SettingsModelLocalTests
         const auto serialized0Profile = profile0->GenerateStub();
         const auto profile1 = implementation::Profile::FromJson(serialized0Profile);
         VERIFY_IS_FALSE(profile0->HasGuid());
-        VERIFY_IS_FALSE(profile1->HasGuid());
+        VERIFY_IS_TRUE(profile1->HasGuid());
 
         auto settings = winrt::make_self<implementation::CascadiaSettings>();
         settings->_profiles.Append(*profile1);
@@ -1355,6 +1356,7 @@ namespace SettingsModelLocalTests
         const winrt::guid guid1{ ::Microsoft::Console::Utils::GuidFromString(L"{6239a42c-6666-49a3-80bd-e8fdd045185c}") };
         const winrt::guid guid2{ ::Microsoft::Console::Utils::GuidFromString(L"{2C4DE342-38B7-51CF-B940-2309A097F518}") };
         const winrt::guid fakeGuid{ ::Microsoft::Console::Utils::GuidFromString(L"{FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF}") };
+        const winrt::guid autogeneratedGuid{ implementation::Profile::_GenerateGuidForProfile(name3, L"") };
         const std::optional<winrt::guid> badGuid{};
 
         VerifyParseSucceeded(settings0String);
@@ -1366,7 +1368,7 @@ namespace SettingsModelLocalTests
         VERIFY_ARE_EQUAL(guid0, settings->_GetProfileGuidByName(name0));
         VERIFY_ARE_EQUAL(guid1, settings->_GetProfileGuidByName(name1));
         VERIFY_ARE_EQUAL(guid2, settings->_GetProfileGuidByName(name2));
-        VERIFY_ARE_EQUAL(badGuid, settings->_GetProfileGuidByName(name3));
+        VERIFY_ARE_EQUAL(autogeneratedGuid, settings->_GetProfileGuidByName(name3));
         VERIFY_ARE_EQUAL(badGuid, settings->_GetProfileGuidByName(badName));
 
         auto prof0{ settings->FindProfile(guid0) };
@@ -1521,9 +1523,9 @@ namespace SettingsModelLocalTests
         {
             auto settings = winrt::make_self<implementation::CascadiaSettings>(false);
             settings->_ParseJsonString(settings0String, false);
-            VERIFY_IS_TRUE(settings->_userDefaultProfileSettings == Json::Value::null);
+            VERIFY_IS_NULL(settings->_userDefaultProfileSettings);
             settings->_ApplyDefaultsFromUserSettings();
-            VERIFY_IS_FALSE(settings->_userDefaultProfileSettings == Json::Value::null);
+            VERIFY_IS_NOT_NULL(settings->_userDefaultProfileSettings);
             settings->LayerJson(settings->_userSettings);
 
             VERIFY_ARE_EQUAL(guid1String, settings->_globals->UnparsedDefaultProfile());
@@ -1573,9 +1575,9 @@ namespace SettingsModelLocalTests
             VERIFY_ARE_EQUAL(2u, settings->_profiles.Size());
 
             settings->_ParseJsonString(settings0String, false);
-            VERIFY_IS_TRUE(settings->_userDefaultProfileSettings == Json::Value::null);
+            VERIFY_IS_NULL(settings->_userDefaultProfileSettings);
             settings->_ApplyDefaultsFromUserSettings();
-            VERIFY_IS_FALSE(settings->_userDefaultProfileSettings == Json::Value::null);
+            VERIFY_IS_NOT_NULL(settings->_userDefaultProfileSettings);
 
             Log::Comment(NoThrowString().Format(
                 L"Ensure that cmd and powershell don't get their GUIDs overwritten"));
@@ -1692,10 +1694,6 @@ namespace SettingsModelLocalTests
         VERIFY_ARE_EQUAL(L"Terminal.App.UnitTest.1", settings->_profiles.GetAt(1).Source());
         VERIFY_ARE_EQUAL(L"Terminal.App.UnitTest.1", settings->_profiles.GetAt(2).Source());
 
-        VERIFY_IS_TRUE(settings->_profiles.GetAt(0).HasGuid());
-        VERIFY_IS_TRUE(settings->_profiles.GetAt(1).HasGuid());
-        VERIFY_IS_TRUE(settings->_profiles.GetAt(2).HasGuid());
-
         VERIFY_ARE_EQUAL(guid1, settings->_profiles.GetAt(0).Guid());
         VERIFY_ARE_EQUAL(guid1, settings->_profiles.GetAt(1).Guid());
         VERIFY_ARE_EQUAL(guid2, settings->_profiles.GetAt(2).Guid());
@@ -2256,6 +2254,7 @@ namespace SettingsModelLocalTests
         settings->_ParseJsonString(settings1Json, false);
         settings->LayerJson(settings->_userSettings);
         settings->_ValidateSettings();
+        commands = settings->_globals->Commands();
         _logCommandNames(commands);
         VERIFY_ARE_EQUAL(0u, settings->_warnings.Size());
         VERIFY_ARE_EQUAL(0u, commands.Size());
@@ -2350,6 +2349,7 @@ namespace SettingsModelLocalTests
         settings->_ParseJsonString(settings1Json, false);
         settings->LayerJson(settings->_userSettings);
         settings->_ValidateSettings();
+        commands = settings->_globals->Commands();
         _logCommandNames(commands);
         VERIFY_ARE_EQUAL(0u, settings->_warnings.Size());
         VERIFY_ARE_EQUAL(1u, commands.Size());
@@ -2460,4 +2460,126 @@ namespace SettingsModelLocalTests
         copyImpl->_globals->WordDelimiters(L"changed value");
         VERIFY_ARE_NOT_EQUAL(settings->_globals->WordDelimiters(), copyImpl->_globals->WordDelimiters());
     }
+
+    void DeserializationTests::TestCloneInheritanceTree()
+    {
+        const std::string settingsJson{ R"(
+        {
+            "defaultProfile": "{61c54bbd-1111-5271-96e7-009a87ff44bf}",
+            "profiles":
+            {
+                "defaults": {
+                    "name": "PROFILE DEFAULTS"
+                },
+                "list": [
+                    {
+                        "guid": "{61c54bbd-1111-5271-96e7-009a87ff44bf}",
+                        "name": "CMD"
+                    },
+                    {
+                        "guid": "{61c54bbd-2222-5271-96e7-009a87ff44bf}",
+                        "name": "PowerShell"
+                    },
+                    {
+                        "guid": "{61c54bbd-3333-5271-96e7-009a87ff44bf}"
+                    }
+                ]
+            }
+        })" };
+
+        VerifyParseSucceeded(settingsJson);
+
+        auto settings{ winrt::make_self<implementation::CascadiaSettings>() };
+        settings->_ParseJsonString(settingsJson, false);
+        settings->_ApplyDefaultsFromUserSettings();
+        settings->LayerJson(settings->_userSettings);
+        settings->_ValidateSettings();
+
+        const auto copy{ settings->Copy() };
+        const auto copyImpl{ winrt::get_self<implementation::CascadiaSettings>(copy) };
+
+        // test globals
+        VERIFY_ARE_EQUAL(settings->_globals->DefaultProfile(), copyImpl->_globals->DefaultProfile());
+
+        // test profiles
+        VERIFY_ARE_EQUAL(settings->_profiles.Size(), copyImpl->_profiles.Size());
+        VERIFY_ARE_EQUAL(settings->_profiles.GetAt(0).Name(), copyImpl->_profiles.GetAt(0).Name());
+        VERIFY_ARE_EQUAL(settings->_profiles.GetAt(1).Name(), copyImpl->_profiles.GetAt(1).Name());
+        VERIFY_ARE_EQUAL(settings->_profiles.GetAt(2).Name(), copyImpl->_profiles.GetAt(2).Name());
+        VERIFY_ARE_EQUAL(settings->_userDefaultProfileSettings->Name(), copyImpl->_userDefaultProfileSettings->Name());
+
+        // Modifying profile.defaults should...
+        VERIFY_ARE_EQUAL(settings->_userDefaultProfileSettings->HasName(), copyImpl->_userDefaultProfileSettings->HasName());
+        copyImpl->_userDefaultProfileSettings->Name(L"changed value");
+
+        // ...keep the same name for the first two profiles
+        VERIFY_ARE_EQUAL(settings->_profiles.Size(), copyImpl->_profiles.Size());
+        VERIFY_ARE_EQUAL(settings->_profiles.GetAt(0).Name(), copyImpl->_profiles.GetAt(0).Name());
+        VERIFY_ARE_EQUAL(settings->_profiles.GetAt(1).Name(), copyImpl->_profiles.GetAt(1).Name());
+
+        // ...but change the name for the one that inherited it from profile.defaults
+        VERIFY_ARE_NOT_EQUAL(settings->_profiles.GetAt(2).Name(), copyImpl->_profiles.GetAt(2).Name());
+
+        // profile.defaults should be different between the two graphs
+        VERIFY_ARE_EQUAL(settings->_userDefaultProfileSettings->HasName(), copyImpl->_userDefaultProfileSettings->HasName());
+        VERIFY_ARE_NOT_EQUAL(settings->_userDefaultProfileSettings->Name(), copyImpl->_userDefaultProfileSettings->Name());
+
+        Log::Comment(L"Test empty profiles.defaults");
+        const std::string emptyPDJson{ R"(
+        {
+            "defaultProfile": "{61c54bbd-1111-5271-96e7-009a87ff44bf}",
+            "profiles":
+            {
+                "defaults": {
+                },
+                "list": [
+                    {
+                        "guid": "{61c54bbd-2222-5271-96e7-009a87ff44bf}",
+                        "name": "PowerShell"
+                    }
+                ]
+            }
+        })" };
+
+        const std::string missingPDJson{ R"(
+        {
+            "defaultProfile": "{61c54bbd-1111-5271-96e7-009a87ff44bf}",
+            "profiles":
+            [
+                {
+                    "guid": "{61c54bbd-2222-5271-96e7-009a87ff44bf}",
+                    "name": "PowerShell"
+                }
+            ]
+        })" };
+
+        auto verifyEmptyPD = [this](const std::string json) {
+            VerifyParseSucceeded(json);
+
+            auto settings{ winrt::make_self<implementation::CascadiaSettings>() };
+            settings->_ParseJsonString(json, false);
+            settings->_ApplyDefaultsFromUserSettings();
+            settings->LayerJson(settings->_userSettings);
+            settings->_ValidateSettings();
+
+            const auto copy{ settings->Copy() };
+            const auto copyImpl{ winrt::get_self<implementation::CascadiaSettings>(copy) };
+
+            // test optimization: if we don't have profiles.defaults, don't add it to the tree
+            VERIFY_IS_NULL(settings->_userDefaultProfileSettings);
+            VERIFY_ARE_EQUAL(settings->_userDefaultProfileSettings, copyImpl->_userDefaultProfileSettings);
+
+            VERIFY_ARE_EQUAL(settings->Profiles().Size(), 1u);
+            VERIFY_ARE_EQUAL(settings->Profiles().Size(), copyImpl->Profiles().Size());
+
+            // so we should only have one parent, instead of two
+            auto srcProfile{ winrt::get_self<implementation::Profile>(settings->Profiles().GetAt(0)) };
+            auto copyProfile{ winrt::get_self<implementation::Profile>(copyImpl->Profiles().GetAt(0)) };
+            VERIFY_ARE_EQUAL(srcProfile->Parents().size(), 0u);
+            VERIFY_ARE_EQUAL(srcProfile->Parents().size(), copyProfile->Parents().size());
+        };
+
+        verifyEmptyPD(emptyPDJson);
+        verifyEmptyPD(missingPDJson);
+    }
 }

src/cascadia/LocalTests_SettingsModel/ProfileTests.cpp

@@ -87,7 +87,7 @@ namespace SettingsModelLocalTests
         // A profile _can_ be layered with itself, though what's the point?
         VERIFY_IS_FALSE(profile3->ShouldBeLayered(profile1Json));
         VERIFY_IS_FALSE(profile3->ShouldBeLayered(profile2Json));
-        VERIFY_IS_FALSE(profile3->ShouldBeLayered(profile3Json));
+        VERIFY_IS_TRUE(profile3->ShouldBeLayered(profile3Json));
     }
 
     void ProfileTests::LayerProfileProperties()
@@ -132,39 +132,41 @@ namespace SettingsModelLocalTests
 
         Log::Comment(NoThrowString().Format(
             L"Layering profile1 on top of profile0"));
-        profile0->LayerJson(profile1Json);
+        auto profile1{ profile0->CreateChild() };
+        profile1->LayerJson(profile1Json);
 
-        VERIFY_IS_NOT_NULL(profile0->Foreground());
-        VERIFY_ARE_EQUAL(til::color(2, 2, 2), til::color{ profile0->Foreground().Value() });
+        VERIFY_IS_NOT_NULL(profile1->Foreground());
+        VERIFY_ARE_EQUAL(til::color(2, 2, 2), til::color{ profile1->Foreground().Value() });
 
-        VERIFY_IS_NOT_NULL(profile0->Background());
-        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile0->Background().Value() });
+        VERIFY_IS_NOT_NULL(profile1->Background());
+        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile1->Background().Value() });
 
-        VERIFY_IS_NOT_NULL(profile0->Background());
-        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile0->Background().Value() });
+        VERIFY_IS_NOT_NULL(profile1->Background());
+        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile1->Background().Value() });
 
-        VERIFY_ARE_EQUAL(L"profile1", profile0->Name());
+        VERIFY_ARE_EQUAL(L"profile1", profile1->Name());
 
-        VERIFY_IS_FALSE(profile0->StartingDirectory().empty());
-        VERIFY_ARE_EQUAL(L"C:/", profile0->StartingDirectory());
+        VERIFY_IS_FALSE(profile1->StartingDirectory().empty());
+        VERIFY_ARE_EQUAL(L"C:/", profile1->StartingDirectory());
 
         Log::Comment(NoThrowString().Format(
             L"Layering profile2 on top of (profile0+profile1)"));
-        profile0->LayerJson(profile2Json);
+        auto profile2{ profile1->CreateChild() };
+        profile2->LayerJson(profile2Json);
 
-        VERIFY_IS_NOT_NULL(profile0->Foreground());
-        VERIFY_ARE_EQUAL(til::color(3, 3, 3), til::color{ profile0->Foreground().Value() });
+        VERIFY_IS_NOT_NULL(profile2->Foreground());
+        VERIFY_ARE_EQUAL(til::color(3, 3, 3), til::color{ profile2->Foreground().Value() });
 
-        VERIFY_IS_NOT_NULL(profile0->Background());
-        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile0->Background().Value() });
+        VERIFY_IS_NOT_NULL(profile2->Background());
+        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile2->Background().Value() });
 
-        VERIFY_IS_NOT_NULL(profile0->SelectionBackground());
-        VERIFY_ARE_EQUAL(til::color(2, 2, 2), til::color{ profile0->SelectionBackground().Value() });
+        VERIFY_IS_NOT_NULL(profile2->SelectionBackground());
+        VERIFY_ARE_EQUAL(til::color(2, 2, 2), til::color{ profile2->SelectionBackground().Value() });
 
-        VERIFY_ARE_EQUAL(L"profile2", profile0->Name());
+        VERIFY_ARE_EQUAL(L"profile2", profile2->Name());
 
-        VERIFY_IS_FALSE(profile0->StartingDirectory().empty());
-        VERIFY_ARE_EQUAL(L"C:/", profile0->StartingDirectory());
+        VERIFY_IS_FALSE(profile2->StartingDirectory().empty());
+        VERIFY_ARE_EQUAL(L"C:/", profile2->StartingDirectory());
     }
 
     void ProfileTests::LayerProfileIcon()

src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp

@@ -499,6 +499,7 @@ namespace TerminalAppLocalTests
 
         const std::string settings0String{ R"(
         {
+            "defaultProfile": "profile5",
             "profiles": [
                 {
                     "name" : "profile0",

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -752,17 +752,10 @@ namespace winrt::TerminalApp::implementation
 
         TerminalConnection::ITerminalConnection connection{ nullptr };
 
-        winrt::guid connectionType{};
+        winrt::guid connectionType = profile.ConnectionType();
         winrt::guid sessionGuid{};
 
-        const auto hasConnectionType = profile.HasConnectionType();
-        if (hasConnectionType)
-        {
-            connectionType = profile.ConnectionType();
-        }
-
-        if (hasConnectionType &&
-            connectionType == TerminalConnection::AzureConnection::ConnectionType() &&
+        if (connectionType == TerminalConnection::AzureConnection::ConnectionType() &&
             TerminalConnection::AzureConnection::IsAzureConnectionAvailable())
         {
             // TODO GH#4661: Replace this with directly using the AzCon when our VT is better

src/cascadia/TerminalApp/TerminalSettings.cpp

@@ -94,10 +94,7 @@ namespace winrt::TerminalApp::implementation
 
         _Commandline = profile.Commandline();
 
-        if (!profile.StartingDirectory().empty())
-        {
-            _StartingDirectory = profile.EvaluatedStartingDirectory();
-        }
+        _StartingDirectory = profile.EvaluatedStartingDirectory();
 
         // GH#2373: Use the tabTitle as the starting title if it exists, otherwise
         // use the profile name

src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp

@@ -244,7 +244,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     {
         auto copy{ winrt::make_self<ActionAndArgs>() };
         copy->_Action = _Action;
-        copy->_Args = _Args.Copy();
+        copy->_Args = _Args ? _Args.Copy() : IActionArgs{ nullptr };
         return copy;
     }