Fix fragments that update other profiles

src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

@@ -553,10 +553,12 @@ Model::Profile CascadiaSettings::GetProfileForArgs(const Model::NewTerminalArgs&
                    FindProfile(GlobalSettings().DefaultProfile()) :
                    ProfileDefaults();
     }
-
+    else
+    {
         // For compatibility with the stable version's behavior, return the default by GUID in all other cases.
         return FindProfile(GlobalSettings().DefaultProfile());
     }
+}
 
 // Method Description:
 // - Helper to get a profile given a name that could be a guid or an actual name.

src/cascadia/TerminalSettingsModel/CascadiaSettings.h

@@ -67,9 +67,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         static void _rethrowSerializationExceptionWithLocationInfo(const JsonUtils::DeserializationError& e, const std::string_view& settingsString);
         static Json::Value _parseJSON(const std::string_view& content);
         static const Json::Value& _getJSONValue(const Json::Value& json, const std::string_view& key) noexcept;
-        static bool _isValidProfileObject(const Json::Value& profileJson);
         gsl::span<const winrt::com_ptr<implementation::Profile>> _getNonUserOriginProfiles() const;
-        void _parse(const OriginTag origin, const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings);
+        void _parse(const OriginTag origin, const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings, bool updatesKeyAllowed = false);
         void _appendProfile(winrt::com_ptr<implementation::Profile>&& profile, ParsedSettings& settings);
         static void _addParentProfile(const winrt::com_ptr<implementation::Profile>& profile, ParsedSettings& settings);
         void _executeGenerator(const IDynamicProfileGenerator& generator);

src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp

@@ -201,7 +201,7 @@ void SettingsLoader::FindFragmentsAndMergeIntoUserSettings()
                 try
                 {
                     const auto content = ReadUTF8File(fragmentExt.path());
-                    _parse(OriginTag::Fragment, source, content, fragmentSettings);
+                    _parse(OriginTag::Fragment, source, content, fragmentSettings, true);
 
                     for (const auto& fragmentProfile : fragmentSettings.profiles)
                     {
@@ -418,17 +418,6 @@ const Json::Value& SettingsLoader::_getJSONValue(const Json::Value& json, const
     return Json::Value::nullSingleton();
 }
 
-// Returns true if the given Json::Value looks like a profile.
-// We introduced a bug (GH#9962, fixed in GH#9964) that would result in one or
-// more nameless, guid-less profiles being emitted into the user's settings file.
-// Those profiles would show up in the list as "Default" later.
-bool SettingsLoader::_isValidProfileObject(const Json::Value& profileJson)
-{
-    return profileJson.isObject() &&
-           (profileJson.isMember(NameKey.data(), NameKey.data() + NameKey.size()) || // has a name (can generate a guid)
-            profileJson.isMember(GuidKey.data(), GuidKey.data() + GuidKey.size())); // or has a guid
-}
-
 // We treat userSettings.profiles as an append-only array and will
 // append profiles into the userSettings as necessary in this function.
 // _userProfileCount stores the number of profiles that were in userJSON during construction.
@@ -443,7 +432,7 @@ gsl::span<const winrt::com_ptr<Profile>> SettingsLoader::_getNonUserOriginProfil
 }
 
 // Parses the given JSON string ("content") and fills a ParsedSettings instance with it.
-void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings)
+void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings, bool updatesKeyAllowed)
 {
     const auto json = content.empty() ? Json::Value{ Json::ValueType::objectValue } : _parseJSON(content);
     const auto& profilesObject = _getJSONValue(json, ProfilesKey);
@@ -491,8 +480,6 @@ void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source
         settings.profilesByGuid.reserve(size);
 
         for (const auto& profileJson : profilesArray)
-        {
-            if (_isValidProfileObject(profileJson))
         {
             auto profile = Profile::FromJson(profileJson);
             profile->Origin(origin);
@@ -508,15 +495,28 @@ void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source
             // We want to ensure that every profile has a GUID no matter what, not just to
             // cache the value, but also to make them consistently identifiable later on.
             if (!profile->HasGuid())
+            {
+                if (profile->HasName())
                 {
                     profile->Guid(profile->Guid());
                 }
+                else if (!updatesKeyAllowed || profile->Updates() == winrt::guid{})
+                {
+                    // We introduced a bug (GH#9962, fixed in GH#9964) that would result in one or
+                    // more nameless, guid-less profiles being emitted into the user's settings file.
+                    // Those profiles would show up in the list as "Default" later.
+                    //
+                    // Fragments however can contain an alternative "updates" key, which works similar to the "guid".
+                    // If updatesKeyAllowed is true (see FindFragmentsAndMergeIntoUserSettings) we permit
+                    // such Guid-less, Name-less profiles as long as they have a valid Updates field.
+                    continue;
+                }
+            }
 
             _appendProfile(std::move(profile), settings);
         }
     }
 }
-}
 
 // Adds a profile to the ParsedSettings instance. Takes ownership of the profile.
 // It ensures no duplicate GUIDs are added to the ParsedSettings instance.