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
This commit is contained in:
Dustin L. Howett
2025-09-16 16:08:45 -05:00
committed by GitHub
parent 1926c4601c
commit e80aadd98b

View File

@@ -559,6 +559,7 @@ bool SettingsLoader::AddDynamicProfileFolders()
folderEntry->Inlining(FolderEntryInlining::Auto);
folderEntry->RawEntries(winrt::single_threaded_vector<Model::NewTabMenuEntry>({ *matchProfilesEntry }));
// NewTabMenu is guaranteed to exist by FixupUserSettings, which runs before this fixup.
userSettings.globals->NewTabMenu().Append(folderEntry.as<Model::NewTabMenuEntry>());
state->SSHFolderGenerated(true);
return true;
@@ -689,6 +690,16 @@ bool SettingsLoader::FixupUserSettings()
fixedUp = true;
}
// Terminal 1.24
// Ensure that the user always has a newTabMenu. We used to do this last, after
// resolving all of the new tab menu entries, but there was no conceivable reason
// that it should happen so late.
if (!userSettings.globals->HasNewTabMenu())
{
userSettings.globals->NewTabMenu(winrt::single_threaded_vector<Model::NewTabMenuEntry>({ Model::RemainingProfilesEntry{} }));
// This one does not need to be written back to the settings file immediately, it can wait until we write one for another reason.
}
return fixedUp;
}
@@ -1263,8 +1274,8 @@ try
// DisableDeletedProfiles returns true whenever we encountered any new generated/dynamic profiles.
// Similarly FixupUserSettings returns true, when it encountered settings that were patched up.
mustWriteToDisk |= loader.DisableDeletedProfiles();
mustWriteToDisk |= loader.AddDynamicProfileFolders();
mustWriteToDisk |= loader.FixupUserSettings();
mustWriteToDisk |= loader.AddDynamicProfileFolders();
// If this throws, the app will catch it and use the default settings.
const auto settings = winrt::make_self<CascadiaSettings>(std::move(loader));
@@ -1745,16 +1756,6 @@ void CascadiaSettings::_resolveNewTabMenuProfiles() const
{
remainingProfilesEntry.Profiles(remainingProfiles);
}
// If the configuration does not have a "newTabMenu" field, GlobalAppSettings
// will return a default value containing just a "remainingProfiles" entry. However,
// this value is regenerated on every "get" operation, so the effect of setting
// the remaining profiles above will be undone. So only in the case that no custom
// value is present in GlobalAppSettings, we will store the modified default value.
if (!_globals->HasNewTabMenu())
{
_globals->NewTabMenu(entries);
}
}
// Method Description: