diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index e16ecbaae..e2ee89b82 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -185,8 +185,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation inline bool _IsClosing() const noexcept { +#ifndef NDEBUG // _closing isn't atomic and may only be accessed from the main thread. - assert(Dispatcher().HasThreadAccess()); + if (const auto dispatcher = Dispatcher()) + { + assert(dispatcher.HasThreadAccess()); + } +#endif return _closing; } diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index bcefe5b77..9d6e5c1f8 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -155,15 +155,14 @@ winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings CascadiaSettings:: const auto hardcodedDefaultGuid = resultPtr->GlobalSettings().DefaultProfile(); std::optional fileData = _ReadUserSettings(); - const bool foundFile = fileData.has_value(); // Make sure the file isn't totally empty. If it is, we'll treat the file // like it doesn't exist at all. - const bool fileHasData = foundFile && !fileData.value().empty(); + const bool fileHasData = fileData && !fileData->empty(); bool needToWriteFile = false; if (fileHasData) { - resultPtr->_ParseJsonString(fileData.value(), false); + resultPtr->_ParseJsonString(*fileData, false); } // Load profiles from dynamic profile generators. _userSettings should be @@ -204,6 +203,35 @@ winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings CascadiaSettings:: _CatchRethrowSerializationExceptionWithLocationInfo(resultPtr->_userSettingsString); } + // Let's say a user doesn't know that they need to write `"hidden": true` in + // order to prevent a profile from showing up (and a settings UI doesn't exist). + // Naturally they would open settings.json and try to remove the profile object. + // This section of code recognizes if a profile was seen before and marks it as + // `"hidden": true` by default and thus ensures the behavior the user expects: + // Profiles won't show up again after they've been removed from settings.json. + { + const auto state = winrt::get_self(ApplicationState::SharedInstance()); + auto generatedProfiles = state->GeneratedProfiles(); + bool generatedProfilesChanged = false; + + for (auto profile : resultPtr->_allProfiles) + { + if (generatedProfiles.emplace(profile.Guid()).second) + { + generatedProfilesChanged = true; + } + else if (profile.Origin() != OriginTag::User) + { + profile.Hidden(true); + } + } + + if (generatedProfilesChanged) + { + state->GeneratedProfiles(generatedProfiles); + } + } + // After layering the user settings, check if there are any new profiles // that need to be inserted into their user settings file. needToWriteFile = resultPtr->_AppendDynamicProfilesToUserSettings() || needToWriteFile; @@ -352,7 +380,6 @@ void CascadiaSettings::_LoadDynamicProfiles() } } - const GUID nullGuid{ 0 }; for (auto& generator : _profileGenerators) { const std::wstring generatorNamespace{ generator->GetNamespace() }; @@ -711,7 +738,7 @@ bool CascadiaSettings::_AppendDynamicProfilesToUserSettings() wbuilder.settings_["indentation"] = " "; wbuilder.settings_["enableYAMLCompatibility"] = true; // suppress spaces around colons - auto isInJsonObj = [](const auto& profile, const auto& json) { + static const auto isInJsonObj = [](const auto& profile, const auto& json) { for (auto profileJson : _GetProfilesJsonObject(json)) { if (profileJson.isObject()) @@ -745,8 +772,16 @@ bool CascadiaSettings::_AppendDynamicProfilesToUserSettings() for (const auto& profile : _allProfiles) { - // Skip profiles that are in the user settings or the default settings. - if (isInJsonObj(profile, _userSettings) || isInJsonObj(profile, _defaultSettings)) + // Skip profiles that are: + // * hidden + // Because when a user manually removes profiles from settings.json, + // we mark them as hidden in LoadAll(). Adding those profiles right + // back into settings.json would feel confusing, while the + // profile that was just erased is added right back. + // * in the user settings or the default settings + // Because we don't want to add profiles which are already + // in the settings.json (explicitly or implicitly). + if (profile.Hidden() || isInJsonObj(profile, _userSettings) || isInJsonObj(profile, _defaultSettings)) { continue; }