diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h index 39ae689e4..d25e7373c 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h @@ -44,6 +44,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation winrt::com_ptr baseLayerProfile; std::vector> profiles; std::unordered_map> profilesByGuid; + + void clear(); }; struct SettingsLoader @@ -63,12 +65,23 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation bool duplicateProfile = false; private: + struct JsonSettings + { + Json::Value root; + const Json::Value& colorSchemes; + const Json::Value& profileDefaults; + const Json::Value& profilesList; + }; + static std::pair _lineAndColumnFromPosition(const std::string_view& string, const size_t position); 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; gsl::span> _getNonUserOriginProfiles() const; - void _parse(const OriginTag origin, const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings, bool updatesKeyAllowed = false); + void _parse(const OriginTag origin, const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings); + void _parseFragment(const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings); + static JsonSettings _parseJson(const std::string_view& content); + static winrt::com_ptr _parseProfile(const OriginTag origin, const winrt::hstring& source, const Json::Value& profileJson); void _appendProfile(winrt::com_ptr&& profile, ParsedSettings& settings); static void _addParentProfile(const winrt::com_ptr& profile, ParsedSettings& settings); void _executeGenerator(const IDynamicProfileGenerator& generator); diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index 8720ad8c3..f667b9ede 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -80,6 +80,14 @@ static std::filesystem::path buildPath(const std::wstring_view& lhs, const std:: return { std::move(buffer) }; } +void ParsedSettings::clear() +{ + globals = {}; + baseLayerProfile = {}; + profiles.clear(); + profilesByGuid.clear(); +} + // This is a convenience method used by the CascadiaSettings constructor. // It runs some basic settings layering without relying on external programs or files. // This makes it suitable for most unit tests. @@ -199,7 +207,7 @@ void SettingsLoader::FindFragmentsAndMergeIntoUserSettings() try { const auto content = ReadUTF8File(fragmentExt.path()); - _parse(OriginTag::Fragment, source, content, fragmentSettings, true); + _parseFragment(source, content, fragmentSettings); for (const auto& fragmentProfile : fragmentSettings.profiles) { @@ -430,92 +438,131 @@ gsl::span> 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, bool updatesKeyAllowed) +// This function is to be used for user settings files. +void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings) { - const auto json = content.empty() ? Json::Value{ Json::ValueType::objectValue } : _parseJSON(content); - const auto& profilesObject = _getJSONValue(json, ProfilesKey); - const auto& defaultsObject = _getJSONValue(profilesObject, DefaultSettingsKey); - const auto& profilesArray = profilesObject.isArray() ? profilesObject : _getJSONValue(profilesObject, ProfilesListKey); + const auto json = _parseJson(content); + + settings.clear(); - // globals { - settings.globals = GlobalAppSettings::FromJson(json); + settings.globals = GlobalAppSettings::FromJson(json.root); - if (const auto& schemes = _getJSONValue(json, SchemesKey)) + for (const auto& schemeJson : json.colorSchemes) { - for (const auto& schemeJson : schemes) + if (const auto scheme = ColorScheme::FromJson(schemeJson)) { - if (schemeJson.isObject()) - { - if (const auto scheme = ColorScheme::FromJson(schemeJson)) - { - settings.globals->AddColorScheme(*scheme); - } - } + settings.globals->AddColorScheme(*scheme); } } } - // profiles.defaults { - settings.baseLayerProfile = Profile::FromJson(defaultsObject); + settings.baseLayerProfile = Profile::FromJson(json.profileDefaults); // Remove the `guid` member from the default settings. // That will hyper-explode, so just don't let them do that. settings.baseLayerProfile->ClearGuid(); settings.baseLayerProfile->Origin(OriginTag::ProfilesDefaults); } - // profiles.list { - const auto size = profilesArray.size(); - - // NOTE: This function is supposed to *replace* the contents of ParsedSettings. Don't break this promise. - // SettingsLoader::FindFragmentsAndMergeIntoUserSettings relies on this. - settings.profiles.clear(); + const auto size = json.profilesList.size(); settings.profiles.reserve(size); - - settings.profilesByGuid.clear(); settings.profilesByGuid.reserve(size); - for (const auto& profileJson : profilesArray) + for (const auto& profileJson : json.profilesList) { - auto profile = Profile::FromJson(profileJson); - profile->Origin(origin); - - // The Guid() generation below depends on the value of Source(). - // --> Provide one if we got one. - if (!source.empty()) + auto profile = _parseProfile(origin, source, profileJson); + // GH#9962: Discard Guid-less, Name-less profiles. + if (profile->HasGuid()) { - profile->Source(source); + _appendProfile(std::move(profile), settings); } - - // The Guid() getter generates one from Name() and Source() if none exists otherwise. - // 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); } } } +// Just like _parse, but is to be used for fragment files, which don't support anything but color +// schemes and profiles. Additionally this function supports profiles which specify an "updates" key. +void SettingsLoader::_parseFragment(const winrt::hstring& source, const std::string_view& content, ParsedSettings& settings) +{ + const auto json = _parseJson(content); + + settings.clear(); + + { + settings.globals = winrt::make_self(); + + for (const auto& schemeJson : json.colorSchemes) + { + try + { + if (const auto scheme = ColorScheme::FromJson(schemeJson)) + { + settings.globals->AddColorScheme(*scheme); + } + } + CATCH_LOG() + } + } + + { + const auto size = json.profilesList.size(); + settings.profiles.reserve(size); + settings.profilesByGuid.reserve(size); + + for (const auto& profileJson : json.profilesList) + { + try + { + auto profile = _parseProfile(OriginTag::Fragment, source, profileJson); + // GH#9962: Discard Guid-less, Name-less profiles, but... + // allow ones with an Updates field, as those are special for fragments. + if (profile->HasGuid() || profile->Updates() != winrt::guid{}) + { + _appendProfile(std::move(profile), settings); + } + } + CATCH_LOG() + } + } +} + +SettingsLoader::JsonSettings SettingsLoader::_parseJson(const std::string_view& content) +{ + auto root = content.empty() ? Json::Value{ Json::ValueType::objectValue } : _parseJSON(content); + const auto& colorSchemes = _getJSONValue(root, SchemesKey); + const auto& profilesObject = _getJSONValue(root, ProfilesKey); + const auto& profileDefaults = _getJSONValue(profilesObject, DefaultSettingsKey); + const auto& profilesList = profilesObject.isArray() ? profilesObject : _getJSONValue(profilesObject, ProfilesListKey); + return JsonSettings{ std::move(root), colorSchemes, profileDefaults, profilesList }; +} + +// Just a common helper function between _parse and _parseFragment. +// Parses a profile and ensures it has a Guid if possible. +winrt::com_ptr SettingsLoader::_parseProfile(const OriginTag origin, const winrt::hstring& source, const Json::Value& profileJson) +{ + auto profile = Profile::FromJson(profileJson); + profile->Origin(origin); + + // The Guid() generation below depends on the value of Source(). + // --> Provide one if we got one. + if (!source.empty()) + { + profile->Source(source); + } + + // If none exists. the Guid() getter generates one from Name() and optionally 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() && profile->HasName()) + { + profile->Guid(profile->Guid()); + } + + return profile; +} + // Adds a profile to the ParsedSettings instance. Takes ownership of the profile. // It ensures no duplicate GUIDs are added to the ParsedSettings instance. void SettingsLoader::_appendProfile(winrt::com_ptr&& profile, ParsedSettings& settings)