Fix fragments that update other profiles (#11343)
`SettingsLoader::_parse` used to skip profiles which didn't have either a "guid" or "name" field, due to #9962. This is however wrong for fragment loading, as fragments can alternatively use an "updates" field instead of guid/name. `SettingsLoader::_parse` was updated to allow profiles with this alternative field during fragment loading. ## PR Checklist * [x] Closes #11331 * [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": [ { "updates": "{574e775e-4f2a-5b96-ac1e-a2962a402336}", "background": "#FFD700" } ] } ```
This commit is contained in:
parent
5542e727d0
commit
2d583fc860
|
@ -553,9 +553,11 @@ Model::Profile CascadiaSettings::GetProfileForArgs(const Model::NewTerminalArgs&
|
||||||
FindProfile(GlobalSettings().DefaultProfile()) :
|
FindProfile(GlobalSettings().DefaultProfile()) :
|
||||||
ProfileDefaults();
|
ProfileDefaults();
|
||||||
}
|
}
|
||||||
|
else
|
||||||
// For compatibility with the stable version's behavior, return the default by GUID in all other cases.
|
{
|
||||||
return FindProfile(GlobalSettings().DefaultProfile());
|
// For compatibility with the stable version's behavior, return the default by GUID in all other cases.
|
||||||
|
return FindProfile(GlobalSettings().DefaultProfile());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Method Description:
|
// Method Description:
|
||||||
|
|
|
@ -67,9 +67,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
|
||||||
static void _rethrowSerializationExceptionWithLocationInfo(const JsonUtils::DeserializationError& e, const std::string_view& settingsString);
|
static void _rethrowSerializationExceptionWithLocationInfo(const JsonUtils::DeserializationError& e, const std::string_view& settingsString);
|
||||||
static Json::Value _parseJSON(const std::string_view& content);
|
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 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;
|
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);
|
void _appendProfile(winrt::com_ptr<implementation::Profile>&& profile, ParsedSettings& settings);
|
||||||
static void _addParentProfile(const 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);
|
void _executeGenerator(const IDynamicProfileGenerator& generator);
|
||||||
|
|
|
@ -201,7 +201,7 @@ void SettingsLoader::FindFragmentsAndMergeIntoUserSettings()
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
const auto content = ReadUTF8File(fragmentExt.path());
|
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)
|
for (const auto& fragmentProfile : fragmentSettings.profiles)
|
||||||
{
|
{
|
||||||
|
@ -418,17 +418,6 @@ const Json::Value& SettingsLoader::_getJSONValue(const Json::Value& json, const
|
||||||
return Json::Value::nullSingleton();
|
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
|
// We treat userSettings.profiles as an append-only array and will
|
||||||
// append profiles into the userSettings as necessary in this function.
|
// append profiles into the userSettings as necessary in this function.
|
||||||
// _userProfileCount stores the number of profiles that were in userJSON during construction.
|
// _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.
|
// 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 json = content.empty() ? Json::Value{ Json::ValueType::objectValue } : _parseJSON(content);
|
||||||
const auto& profilesObject = _getJSONValue(json, ProfilesKey);
|
const auto& profilesObject = _getJSONValue(json, ProfilesKey);
|
||||||
|
@ -492,28 +481,39 @@ void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source
|
||||||
|
|
||||||
for (const auto& profileJson : profilesArray)
|
for (const auto& profileJson : profilesArray)
|
||||||
{
|
{
|
||||||
if (_isValidProfileObject(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())
|
||||||
{
|
{
|
||||||
auto profile = Profile::FromJson(profileJson);
|
profile->Source(source);
|
||||||
profile->Origin(origin);
|
}
|
||||||
|
|
||||||
// The Guid() generation below depends on the value of Source().
|
// The Guid() getter generates one from Name() and Source() if none exists otherwise.
|
||||||
// --> Provide one if we got one.
|
// We want to ensure that every profile has a GUID no matter what, not just to
|
||||||
if (!source.empty())
|
// cache the value, but also to make them consistently identifiable later on.
|
||||||
{
|
if (!profile->HasGuid())
|
||||||
profile->Source(source);
|
{
|
||||||
}
|
if (profile->HasName())
|
||||||
|
|
||||||
// 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())
|
|
||||||
{
|
{
|
||||||
profile->Guid(profile->Guid());
|
profile->Guid(profile->Guid());
|
||||||
}
|
}
|
||||||
|
else if (!updatesKeyAllowed || profile->Updates() == winrt::guid{})
|
||||||
_appendProfile(std::move(profile), settings);
|
{
|
||||||
|
// 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);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue