diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 449d19b6b74..d5f58be99dc 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -306,7 +306,7 @@ namespace winrt::TerminalApp::implementation }); _root->Create(); - _ApplyTheme(_settings.GlobalSettings().Theme()); + _RefreshThemeRoutine(); _ApplyStartupTaskStateChange(); TraceLoggingWrite( @@ -895,36 +895,25 @@ namespace winrt::TerminalApp::implementation // this stops us from reloading too many times or too quickly. fire_and_forget AppLogic::_DispatchReloadSettings() { - static constexpr auto FileActivityQuiesceTime{ std::chrono::milliseconds(50) }; - if (!_settingsReloadQueued.exchange(true)) + if (_settingsReloadQueued.exchange(true)) { - co_await winrt::resume_after(FileActivityQuiesceTime); - _ReloadSettings(); - _settingsReloadQueued.store(false); + co_return; } - } - fire_and_forget AppLogic::_LoadErrorsDialogRoutine() - { - co_await winrt::resume_foreground(_root->Dispatcher()); + auto weakSelf = get_weak(); - const winrt::hstring titleKey = USES_RESOURCE(L"ReloadJsonParseErrorTitle"); - const winrt::hstring textKey = USES_RESOURCE(L"ReloadJsonParseErrorText"); - _ShowLoadErrorsDialog(titleKey, textKey, _settingsLoadedResult); - } - - fire_and_forget AppLogic::_ShowLoadWarningsDialogRoutine() - { + co_await winrt::resume_after(std::chrono::milliseconds(100)); co_await winrt::resume_foreground(_root->Dispatcher()); - _ShowLoadWarningsDialog(); + if (auto self{ weakSelf.get() }) + { + _ReloadSettings(); + _settingsReloadQueued.store(false); + } } - fire_and_forget AppLogic::_RefreshThemeRoutine() + void AppLogic::_RefreshThemeRoutine() { - co_await winrt::resume_foreground(_root->Dispatcher()); - - // Refresh the UI theme _ApplyTheme(_settings.GlobalSettings().Theme()); } @@ -959,39 +948,26 @@ namespace winrt::TerminalApp::implementation return; } - auto weakThis{ get_weak() }; - co_await winrt::resume_foreground(_root->Dispatcher(), CoreDispatcherPriority::Normal); - if (auto page{ weakThis.get() }) - { - StartupTaskState state; - bool tryEnableStartupTask = _settings.GlobalSettings().StartOnUserLogin(); - StartupTask task = co_await StartupTask::GetAsync(StartupTaskName); + const auto tryEnableStartupTask = _settings.GlobalSettings().StartOnUserLogin(); + const auto task = co_await StartupTask::GetAsync(StartupTaskName); - state = task.State(); - switch (state) - { - case StartupTaskState::Disabled: + switch (task.State()) + { + case StartupTaskState::Disabled: + if (tryEnableStartupTask) { - if (tryEnableStartupTask) - { - co_await task.RequestEnableAsync(); - } - break; + co_await task.RequestEnableAsync(); } - case StartupTaskState::DisabledByUser: + break; + case StartupTaskState::DisabledByUser: + // TODO: GH#6254: define UX for other StartupTaskStates + break; + case StartupTaskState::Enabled: + if (!tryEnableStartupTask) { - // TODO: GH#6254: define UX for other StartupTaskStates - break; - } - case StartupTaskState::Enabled: - { - if (!tryEnableStartupTask) - { - task.Disable(); - } - break; - } + task.Disable(); } + break; } } CATCH_LOG(); @@ -1009,12 +985,15 @@ namespace winrt::TerminalApp::implementation if (FAILED(_settingsLoadedResult)) { - _LoadErrorsDialogRoutine(); + const winrt::hstring titleKey = USES_RESOURCE(L"ReloadJsonParseErrorTitle"); + const winrt::hstring textKey = USES_RESOURCE(L"ReloadJsonParseErrorText"); + _ShowLoadErrorsDialog(titleKey, textKey, _settingsLoadedResult); return; } - else if (_settingsLoadedResult == S_FALSE) + + if (_settingsLoadedResult == S_FALSE) { - _ShowLoadWarningsDialogRoutine(); + _ShowLoadWarningsDialog(); } // Here, we successfully reloaded the settings, and created a new diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h index 50b7b95f09d..99b8640e5a5 100644 --- a/src/cascadia/TerminalApp/AppLogic.h +++ b/src/cascadia/TerminalApp/AppLogic.h @@ -124,18 +124,14 @@ namespace winrt::TerminalApp::implementation ::TerminalApp::AppCommandlineArgs _appArgs; ::TerminalApp::AppCommandlineArgs _settingsAppArgs; - int _ParseArgs(winrt::array_view& args); static TerminalApp::FindTargetWindowResult _doFindTargetWindow(winrt::array_view args, const Microsoft::Terminal::Settings::Model::WindowingMode& windowingBehavior); void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey, HRESULT settingsLoadedResult); void _ShowLoadWarningsDialog(); bool _IsKeyboardServiceEnabled(); - void _ShowKeyboardServiceDisabledDialog(); - fire_and_forget _LoadErrorsDialogRoutine(); - fire_and_forget _ShowLoadWarningsDialogRoutine(); - fire_and_forget _RefreshThemeRoutine(); + void _RefreshThemeRoutine(); fire_and_forget _ApplyStartupTaskStateChange(); void _OnLoaded(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 18afb5962a2..ee26b0c9707 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -92,31 +92,26 @@ namespace winrt::TerminalApp::implementation } } - winrt::fire_and_forget TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI) + void TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI) { _settings = settings; - auto weakThis{ get_weak() }; - co_await winrt::resume_foreground(Dispatcher()); - if (auto page{ weakThis.get() }) - { - // Make sure to _UpdateCommandsForPalette before - // _RefreshUIForSettingsReload. _UpdateCommandsForPalette will make - // sure the KeyChordText of Commands is updated, which needs to - // happen before the Settings UI is reloaded and tries to re-read - // those values. - _UpdateCommandsForPalette(); - CommandPalette().SetActionMap(_settings.ActionMap()); + // Make sure to _UpdateCommandsForPalette before + // _RefreshUIForSettingsReload. _UpdateCommandsForPalette will make + // sure the KeyChordText of Commands is updated, which needs to + // happen before the Settings UI is reloaded and tries to re-read + // those values. + _UpdateCommandsForPalette(); + CommandPalette().SetActionMap(_settings.ActionMap()); - if (needRefreshUI) - { - _RefreshUIForSettingsReload(); - } - - // Upon settings update we reload the system settings for scrolling as well. - // TODO: consider reloading this value periodically. - _systemRowsToScroll = _ReadSystemRowsToScroll(); + if (needRefreshUI) + { + _RefreshUIForSettingsReload(); } + + // Upon settings update we reload the system settings for scrolling as well. + // TODO: consider reloading this value periodically. + _systemRowsToScroll = _ReadSystemRowsToScroll(); } void TerminalPage::Create() @@ -1830,7 +1825,7 @@ namespace winrt::TerminalApp::implementation // This includes update the settings of all the tabs according // to their profiles, update the title and icon of each tab, and // finally create the tab flyout - winrt::fire_and_forget TerminalPage::_RefreshUIForSettingsReload() + void TerminalPage::_RefreshUIForSettingsReload() { // Re-wire the keybindings to their handlers, as we'll have created a // new AppKeyBindings object. @@ -1885,17 +1880,10 @@ namespace winrt::TerminalApp::implementation tabImpl->SetActionMap(_settings.ActionMap()); } - auto weakThis{ get_weak() }; - - co_await winrt::resume_foreground(Dispatcher()); - // repopulate the new tab button's flyout with entries for each // profile, which might have changed - if (auto page{ weakThis.get() }) - { - _UpdateTabWidthMode(); - _CreateNewTabFlyout(); - } + _UpdateTabWidthMode(); + _CreateNewTabFlyout(); // Reload the current value of alwaysOnTop from the settings file. This // will let the user hot-reload this setting, but any runtime changes to diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 9de1ded9b74..f7bf1f1ce18 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -54,7 +54,7 @@ namespace winrt::TerminalApp::implementation // put it in our inheritance graph. https://github.com/microsoft/microsoft-ui-xaml/issues/3331 STDMETHODIMP Initialize(HWND hwnd); - winrt::fire_and_forget SetSettings(Microsoft::Terminal::Settings::Model::CascadiaSettings settings, bool needRefreshUI); + void SetSettings(Microsoft::Terminal::Settings::Model::CascadiaSettings settings, bool needRefreshUI); void Create(); @@ -67,8 +67,6 @@ namespace winrt::TerminalApp::implementation winrt::hstring ApplicationDisplayName(); winrt::hstring ApplicationVersion(); - winrt::hstring ThirdPartyNoticesLink(); - winrt::fire_and_forget CloseWindow(); void ToggleFocusMode(); @@ -296,7 +294,7 @@ namespace winrt::TerminalApp::implementation winrt::Microsoft::Terminal::Control::TermControl _InitControl(const winrt::Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings, const winrt::Microsoft::Terminal::TerminalConnection::ITerminalConnection& connection); - winrt::fire_and_forget _RefreshUIForSettingsReload(); + void _RefreshUIForSettingsReload(); void _SetNonClientAreaColors(const Windows::UI::Color& selectedTabColor); void _ClearNonClientAreaColors(); diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index fc256a9f41f..7807f648e01 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -65,13 +65,11 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // - settings - the new settings source // Return value: // - - fire_and_forget MainPage::UpdateSettings(Model::CascadiaSettings settings) + void MainPage::UpdateSettings(const Model::CascadiaSettings& settings) { _settingsSource = settings; _settingsClone = settings.Copy(); - co_await winrt::resume_foreground(Dispatcher()); - // Deduce information about the currently selected item IInspectable selectedItemTag; auto menuItems{ SettingsNav().MenuItems() }; @@ -83,34 +81,43 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } } - // remove all profile-related NavViewItems by populating a std::vector - // with the ones we want to keep. - // NOTE: menuItems.Remove() causes an out-of-bounds crash. Using ReplaceAll() - // gets around this crash. - std::vector menuItemsSTL; - for (const auto& item : menuItems) - { - if (const auto& navViewItem{ item.try_as() }) - { - if (const auto& tag{ navViewItem.Tag() }) - { - if (tag.try_as()) + // We'll remove a bunch of items and iterate over it twice. + // --> Copy it into an STL vector to simplify our code and reduce COM overhead. + std::vector menuItemsSTL(menuItems.Size(), nullptr); + menuItems.GetMany(0, menuItemsSTL); + + // We want to refresh the list of profiles in the NavigationView. + // In order to add profiles we can use _InitializeProfilesList(); + // But before we can do that we have to remove existing profiles first of course. + // This "erase-remove" idiom will achieve just that. + menuItemsSTL.erase( + std::remove_if( + menuItemsSTL.begin(), + menuItemsSTL.end(), + [](const auto& item) -> bool { + if (const auto& navViewItem{ item.try_as() }) { - // don't add NavViewItem pointing to a Profile - continue; - } - else if (const auto& stringTag{ tag.try_as() }) - { - if (stringTag == addProfileTag) + if (const auto& tag{ navViewItem.Tag() }) { - // don't add the "Add Profile" item - continue; + if (tag.try_as()) + { + // don't add NavViewItem pointing to a Profile + return true; + } + if (const auto& stringTag{ tag.try_as() }) + { + if (stringTag == addProfileTag) + { + // don't add the "Add Profile" item + return true; + } + } } } - } - } - menuItemsSTL.emplace_back(item); - } + return false; + }), + menuItemsSTL.end()); + menuItems.ReplaceAll(menuItemsSTL); // Repopulate profile-related menu items @@ -123,7 +130,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // refresh the current page using the SelectedItem data we collected before the refresh if (selectedItemTag) { - for (const auto& item : menuItems) + for (const auto& item : menuItemsSTL) { if (const auto& menuItem{ item.try_as() }) { @@ -138,7 +145,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // found the one that was selected before the refresh SettingsNav().SelectedItem(item); _Navigate(*stringTag); - co_return; + return; } } } @@ -151,7 +158,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation // found the one that was selected before the refresh SettingsNav().SelectedItem(item); _Navigate(*profileTag); - co_return; + return; } } } diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.h b/src/cascadia/TerminalSettingsEditor/MainPage.h index 677b7c6d133..401c2576eb0 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.h +++ b/src/cascadia/TerminalSettingsEditor/MainPage.h @@ -13,7 +13,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation MainPage() = delete; MainPage(const Model::CascadiaSettings& settings); - fire_and_forget UpdateSettings(Model::CascadiaSettings settings); + void UpdateSettings(const Model::CascadiaSettings& settings); void OpenJsonKeyDown(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::KeyRoutedEventArgs const& args); void OpenJsonTapped(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::Input::TappedRoutedEventArgs const& args);