starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-04-09 15:51:05 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Allow fragments to override the name of new profiles (#12627)

Browse Source 
After this commit we only set the default fields of a profile - primarily the
name field - as late as possible, after layering has already completed.
This ensures that we pick up any modifications from fragments.

* [x] Closes #12520
* [x] I work here
* [x] Tests added/passed

* Add a fragment at
  `%LocalAppData%\Microsoft\Windows Terminal\Fragments\Fragment\fragment.json`
  with
  `{"profiles":[{"updates":"{61c54bbd-c2c6-5271-96e7-009a87ff44bf}","name":"NewName"}]}`
* Windows PowerShell profile is created with the name "NewName" in settings.json ✅

(cherry picked from commit ee83081b64)
This commit is contained in:
Leonard Hecker
2022-03-16 00:06:16 +01:00
committed by Dustin Howett
parent 89fa517735
commit fcdf02b2bf
4 changed files with 49 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp
View File

@@ -1983,7 +1983,7 @@ namespace SettingsModelLocalTests
}
}
// This test ensures GH#11597 doesn't regress.
// This test ensures GH#11597, GH#12520 don't regress.
void DeserializationTests::LoadFragmentsWithMultipleUpdates()
{
static constexpr std::wstring_view fragmentSource{ L"fragment" };
@@ -1991,7 +1991,7 @@ namespace SettingsModelLocalTests
"profiles": [
{
"updates": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}",
"cursorShape": "filledBox"
"name": "NewName"
},
{
"updates": "{0caa0dad-35be-5f56-a8ff-afceeeaa6101}",
@@ -2011,5 +2011,7 @@ namespace SettingsModelLocalTests
VERIFY_IS_FALSE(loader.duplicateProfile);
VERIFY_ARE_EQUAL(3u, loader.userSettings.profiles.size());
// GH#12520: Fragments should be able to override the name of builtin profiles.
VERIFY_ARE_EQUAL(L"NewName", loader.userSettings.profiles[0]->Name());
}
}

2
src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp
View File

@@ -31,6 +31,8 @@ using namespace Microsoft::Console;
// which is why this unsafety wasn't further abstracted away.
winrt::com_ptr<Profile> Model::implementation::CreateChild(const winrt::com_ptr<Profile>& parent)
{
// If you add more fields here, make sure to do the same in
// SettingsLoader::_addUserProfileParent().
auto profile = winrt::make_self<Profile>();
profile->Origin(OriginTag::User);
profile->Name(parent->Name());

2
src/cascadia/TerminalSettingsModel/CascadiaSettings.h
View File

@@ -84,7 +84,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
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<Profile>&& profile, const winrt::guid& guid, ParsedSettings& settings);
static void _addParentProfile(const winrt::com_ptr<implementation::Profile>& profile, ParsedSettings& settings);
void _addUserProfileParent(const winrt::com_ptr<implementation::Profile>& profile);
void _executeGenerator(const IDynamicProfileGenerator& generator);
std::unordered_set<std::wstring_view> _ignoredNamespaces;

52
src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp
View File

@@ -185,7 +185,7 @@ void SettingsLoader::MergeInboxIntoUserSettings()
{
for (const auto& profile : inboxSettings.profiles)
{
_addParentProfile(profile, userSettings);
_addUserProfileParent(profile);
}
}
@@ -314,7 +314,17 @@ void SettingsLoader::FinalizeLayering()
for (const auto& profile : userSettings.profiles)
{
profile->InsertParent(0, userSettings.baseLayerProfile);
// This completes the parenting process that was started in _addUserProfileParent().
profile->_FinalizeInheritance();
if (profile->Origin() == OriginTag::None)
{
// If you add more fields here, make sure to do the same in
// implementation::CreateChild().
profile->Origin(OriginTag::User);
profile->Name(profile->Name());
profile->Hidden(profile->Hidden());
}
}
}
@@ -550,7 +560,7 @@ void SettingsLoader::_parseFragment(const winrt::hstring& source, const std::str
}
else
{
_addParentProfile(fragmentProfile, userSettings);
_addUserProfileParent(fragmentProfile);
}
}
@@ -613,9 +623,9 @@ void SettingsLoader::_appendProfile(winrt::com_ptr<Profile>&& profile, const win
// If the given ParsedSettings instance contains a profile with the given profile's GUID,
// the profile is added as a parent. Otherwise a new child profile is created.
void SettingsLoader::_addParentProfile(const winrt::com_ptr<implementation::Profile>& profile, ParsedSettings& settings)
void SettingsLoader::_addUserProfileParent(const winrt::com_ptr<implementation::Profile>& profile)
{
if (const auto [it, inserted] = settings.profilesByGuid.emplace(profile->Guid(), profile); !inserted)
if (const auto [it, inserted] = userSettings.profilesByGuid.emplace(profile->Guid(), nullptr); !inserted)
{
// If inserted is false, we got a matching user profile with identical GUID.
// --> The generated profile is a parent of the existing user profile.
@@ -623,14 +633,36 @@ void SettingsLoader::_addParentProfile(const winrt::com_ptr<implementation::Prof
}
else
{
// If inserted is true, then this is a generated profile that doesn't exist in the user's settings.
// While emplace() has already created an appropriate entry in .profilesByGuid, we still need to
// add it to .profiles (which is basically a sorted list of .profilesByGuid's values).
// If inserted is true, then this is a generated profile that doesn't exist
// in the user's settings (which makes this branch somewhat unlikely).
//
// When a user modifies a profile they shouldn't modify the (static/constant)
// inbox profile of course. That's why we need to call CreateChild here.
// But we don't need to call _FinalizeInheritance() yet as this is handled by SettingsLoader::FinalizeLayering().
settings.profiles.emplace_back(CreateChild(profile));
// inbox profile of course. That's why we need to create a child.
// And since we previously added the (now) parent profile into profilesByGuid
// we'll have to replace it->second with the (new) child profile.
//
// These additional things are required to complete a (user) profile:
// * A call to _FinalizeInheritance()
// * Every profile should at least have Origin(), Name() and Hidden() set
// They're handled by SettingsLoader::FinalizeLayering() and detected by
// the missing Origin(). Setting these fields as late as possible ensures
// that we pick up the correct, inherited values of all of the child's parents.
//
// If you add more fields here, make sure to do the same in
// implementation::CreateChild().
auto child = winrt::make_self<Profile>();
child->AddLeastImportantParent(profile);
child->Guid(profile->Guid());
// If profile is a dynamic/generated profile, a fragment's
// Source() should have no effect on this user profile.
if (profile->HasSource())
{
child->Source(profile->Source());
}
it->second = child;
userSettings.profiles.emplace_back(std::move(child));
}
}