Skip to content

Commit e80aadd

Browse files
authored
Move newTabMenu creation to Settings fixups (#19353)
Some of the other settings fixups require there to be a valid NewTabMenu, rather than just a temporary object. Since the resolving all the menu entries after loading already forces the user to have a `newTabMenu`, let's just codify it as a real fixup. I've moved the SSH folder fixup after the settings fixup because it relies on there being a NTM. I decided not to make this fixup write back to the user's settings. There are a couple reasons for this, all of which are flimsy. - There are a number of tests that test fixup behavior, especially those around actions, which would need to be updated for this new mandatory key. I did not think it proper to add `newTabMenu` to ten unrelated tests that only contain actions (for example.) - We actually don't currently have mandatory keys. But this one was always being added anyway, in a later phase... - It's consistent with the existing behavior. Closes #19356
1 parent 1926c46 commit e80aadd

File tree

1 file changed

+12
-11
lines changed

1 file changed

+12
-11
lines changed

src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,7 @@ bool SettingsLoader::AddDynamicProfileFolders()
559559
folderEntry->Inlining(FolderEntryInlining::Auto);
560560
folderEntry->RawEntries(winrt::single_threaded_vector<Model::NewTabMenuEntry>({ *matchProfilesEntry }));
561561

562+
// NewTabMenu is guaranteed to exist by FixupUserSettings, which runs before this fixup.
562563
userSettings.globals->NewTabMenu().Append(folderEntry.as<Model::NewTabMenuEntry>());
563564
state->SSHFolderGenerated(true);
564565
return true;
@@ -689,6 +690,16 @@ bool SettingsLoader::FixupUserSettings()
689690
fixedUp = true;
690691
}
691692

693+
// Terminal 1.24
694+
// Ensure that the user always has a newTabMenu. We used to do this last, after
695+
// resolving all of the new tab menu entries, but there was no conceivable reason
696+
// that it should happen so late.
697+
if (!userSettings.globals->HasNewTabMenu())
698+
{
699+
userSettings.globals->NewTabMenu(winrt::single_threaded_vector<Model::NewTabMenuEntry>({ Model::RemainingProfilesEntry{} }));
700+
// This one does not need to be written back to the settings file immediately, it can wait until we write one for another reason.
701+
}
702+
692703
return fixedUp;
693704
}
694705

@@ -1263,8 +1274,8 @@ try
12631274
// DisableDeletedProfiles returns true whenever we encountered any new generated/dynamic profiles.
12641275
// Similarly FixupUserSettings returns true, when it encountered settings that were patched up.
12651276
mustWriteToDisk |= loader.DisableDeletedProfiles();
1266-
mustWriteToDisk |= loader.AddDynamicProfileFolders();
12671277
mustWriteToDisk |= loader.FixupUserSettings();
1278+
mustWriteToDisk |= loader.AddDynamicProfileFolders();
12681279

12691280
// If this throws, the app will catch it and use the default settings.
12701281
const auto settings = winrt::make_self<CascadiaSettings>(std::move(loader));
@@ -1745,16 +1756,6 @@ void CascadiaSettings::_resolveNewTabMenuProfiles() const
17451756
{
17461757
remainingProfilesEntry.Profiles(remainingProfiles);
17471758
}
1748-
1749-
// If the configuration does not have a "newTabMenu" field, GlobalAppSettings
1750-
// will return a default value containing just a "remainingProfiles" entry. However,
1751-
// this value is regenerated on every "get" operation, so the effect of setting
1752-
// the remaining profiles above will be undone. So only in the case that no custom
1753-
// value is present in GlobalAppSettings, we will store the modified default value.
1754-
if (!_globals->HasNewTabMenu())
1755-
{
1756-
_globals->NewTabMenu(entries);
1757-
}
17581759
}
17591760

17601761
// Method Description:

0 commit comments

Comments
 (0)