Fix all fragments not loading when one is badly formed (#11346)
This commit introduces a number of poor abstractions to split `SettingsLoader::_parse` into `_parse` for content in the format of the user's settings.json and `_parseFragment` which is specialized for fragment files. The latter suppresses exceptions and supports the "updates" key for profiles. ## PR Checklist * [x] Closes #11330 * [x] I work here * [x] Tests added/passed ## Validation Steps Performed * Wrote the following to `%LOCALAPPDATA%\Microsoft\Windows Terminal\Fragments\test\test.json`: ```json { "profiles": [ { "name": "bad", "unfocusedAppearance": "" }, { "name": "good" }, { "updates": "{574e775e-4f2a-5b96-ac1e-a2962a402336}", "background": "#333" } ] } ``` * Ensured that "bad" is ignored ✔️ * Ensured that "good" shows up and works ✔️ * Ensured that the pwsh profile has a gray background ✔️
This commit is contained in:
parent
85f067403d
commit
e5293b7814
|
@ -44,6 +44,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
|||
winrt::com_ptr<implementation::Profile> baseLayerProfile;
|
||||
std::vector<winrt::com_ptr<implementation::Profile>> profiles;
|
||||
std::unordered_map<winrt::guid, winrt::com_ptr<implementation::Profile>> 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<size_t, size_t> _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<const winrt::com_ptr<implementation::Profile>> _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<implementation::Profile> _parseProfile(const OriginTag origin, const winrt::hstring& source, const Json::Value& profileJson);
|
||||
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);
|
||||
|
|
|
@ -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<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, 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<GlobalAppSettings>();
|
||||
|
||||
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<Profile> 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>&& profile, ParsedSettings& settings)
|
||||
|
|
Loading…
Reference in New Issue