diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index b7e0eae91a..6a0d456142 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -392,7 +392,7 @@ namespace winrt::TerminalApp::implementation auto ev = winrt::make_self(true, static_cast(_settingsLoadedResult), _settingsLoadExceptionText, - warnings, + warnings.GetView(), _settings); SettingsChanged.raise(*this, *ev); return; @@ -424,7 +424,7 @@ namespace winrt::TerminalApp::implementation auto ev = winrt::make_self(!initialLoad, _settingsLoadedResult, _settingsLoadExceptionText, - warnings, + warnings.GetView(), _settings); SettingsChanged.raise(*this, *ev); } @@ -491,7 +491,7 @@ namespace winrt::TerminalApp::implementation auto ev = winrt::make_self(false, _settingsLoadedResult, _settingsLoadExceptionText, - warnings, + warnings.GetView(), _settings); auto window = winrt::make_self(*ev, _contentManager); diff --git a/src/cascadia/TerminalApp/SettingsLoadEventArgs.h b/src/cascadia/TerminalApp/SettingsLoadEventArgs.h index e094810784..f9436f95ae 100644 --- a/src/cascadia/TerminalApp/SettingsLoadEventArgs.h +++ b/src/cascadia/TerminalApp/SettingsLoadEventArgs.h @@ -12,14 +12,14 @@ namespace winrt::TerminalApp::implementation WINRT_PROPERTY(bool, Reload, false); WINRT_PROPERTY(uint64_t, Result, S_OK); WINRT_PROPERTY(winrt::hstring, ExceptionText, L""); - WINRT_PROPERTY(winrt::Windows::Foundation::Collections::IVector, Warnings, nullptr); + WINRT_PROPERTY(winrt::Windows::Foundation::Collections::IVectorView, Warnings, nullptr); WINRT_PROPERTY(Microsoft::Terminal::Settings::Model::CascadiaSettings, NewSettings, nullptr); public: SettingsLoadEventArgs(bool reload, uint64_t result, winrt::hstring exceptionText, - winrt::Windows::Foundation::Collections::IVector warnings, + winrt::Windows::Foundation::Collections::IVectorView warnings, Microsoft::Terminal::Settings::Model::CascadiaSettings newSettings) : _Reload{ reload }, _Result{ result }, diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 9a73591556..48e4e2ee1c 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -4165,6 +4165,13 @@ namespace winrt::TerminalApp::implementation } }); + sui.ShowLoadWarningsDialog([weakThis{ get_weak() }](auto&& /*s*/, const Windows::Foundation::Collections::IVectorView& warnings) { + if (auto page{ weakThis.get() }) + { + page->ShowLoadWarningsDialog.raise(*page, warnings); + } + }); + return *settingsContent; } diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index 9837f9fe61..b7194f7ab4 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -187,6 +187,7 @@ namespace winrt::TerminalApp::implementation til::typed_event OpenSystemMenu; til::typed_event QuitRequested; til::typed_event ShowWindowChanged; + til::typed_event> ShowLoadWarningsDialog; til::typed_event RequestMoveContent; til::typed_event RequestReceiveContent; diff --git a/src/cascadia/TerminalApp/TerminalPage.idl b/src/cascadia/TerminalApp/TerminalPage.idl index 2788cedd10..f1bd4fbc66 100644 --- a/src/cascadia/TerminalApp/TerminalPage.idl +++ b/src/cascadia/TerminalApp/TerminalPage.idl @@ -97,6 +97,7 @@ namespace TerminalApp event Windows.Foundation.TypedEventHandler OpenSystemMenu; event Windows.Foundation.TypedEventHandler ShowWindowChanged; + event Windows.Foundation.TypedEventHandler > ShowLoadWarningsDialog; event Windows.Foundation.TypedEventHandler RequestMoveContent; event Windows.Foundation.TypedEventHandler RequestReceiveContent; diff --git a/src/cascadia/TerminalApp/TerminalWindow.cpp b/src/cascadia/TerminalApp/TerminalWindow.cpp index ff3a66acae..e64eefed55 100644 --- a/src/cascadia/TerminalApp/TerminalWindow.cpp +++ b/src/cascadia/TerminalApp/TerminalWindow.cpp @@ -219,6 +219,7 @@ namespace winrt::TerminalApp::implementation _root->Initialized({ get_weak(), &TerminalWindow::_pageInitialized }); _root->WindowSizeChanged({ get_weak(), &TerminalWindow::_WindowSizeChanged }); _root->RenameWindowRequested({ get_weak(), &TerminalWindow::_RenameWindowRequested }); + _root->ShowLoadWarningsDialog({ get_weak(), &TerminalWindow::_ShowLoadWarningsDialog }); _root->Create(); AppLogic::Current()->SettingsChanged({ get_weak(), &TerminalWindow::UpdateSettingsHandler }); @@ -476,7 +477,7 @@ namespace winrt::TerminalApp::implementation // validating the settings. // - Only one dialog can be visible at a time. If another dialog is visible // when this is called, nothing happens. See ShowDialog for details - void TerminalWindow::_ShowLoadWarningsDialog(const Windows::Foundation::Collections::IVector& warnings) + void TerminalWindow::_ShowLoadWarningsDialog(const IInspectable&, const Windows::Foundation::Collections::IVectorView& warnings) { auto title = RS_(L"SettingsValidateErrorTitle"); auto buttonText = RS_(L"Ok"); @@ -536,7 +537,7 @@ namespace winrt::TerminalApp::implementation } else if (settingsLoadedResult == S_FALSE) { - _ShowLoadWarningsDialog(_initialLoadResult.Warnings()); + _ShowLoadWarningsDialog(nullptr, _initialLoadResult.Warnings()); } } @@ -822,7 +823,7 @@ namespace winrt::TerminalApp::implementation } else if (args.Result() == S_FALSE) { - _ShowLoadWarningsDialog(args.Warnings()); + _ShowLoadWarningsDialog(nullptr, args.Warnings()); } else if (args.Result() == S_OK) { diff --git a/src/cascadia/TerminalApp/TerminalWindow.h b/src/cascadia/TerminalApp/TerminalWindow.h index 6e37e17947..40abbce9ce 100644 --- a/src/cascadia/TerminalApp/TerminalWindow.h +++ b/src/cascadia/TerminalApp/TerminalWindow.h @@ -189,7 +189,7 @@ namespace winrt::TerminalApp::implementation const winrt::hstring& contentKey, HRESULT settingsLoadedResult, const winrt::hstring& exceptionText); - void _ShowLoadWarningsDialog(const Windows::Foundation::Collections::IVector& warnings); + void _ShowLoadWarningsDialog(const IInspectable& sender, const Windows::Foundation::Collections::IVectorView& warnings); bool _IsKeyboardServiceEnabled(); diff --git a/src/cascadia/TerminalApp/TerminalWindow.idl b/src/cascadia/TerminalApp/TerminalWindow.idl index a8fa0c97c5..f1a6918120 100644 --- a/src/cascadia/TerminalApp/TerminalWindow.idl +++ b/src/cascadia/TerminalApp/TerminalWindow.idl @@ -30,7 +30,7 @@ namespace TerminalApp { Boolean Reload { get; }; UInt64 Result { get; }; - IVector Warnings { get; }; + IVectorView Warnings { get; }; String ExceptionText { get; }; Microsoft.Terminal.Settings.Model.CascadiaSettings NewSettings { get; }; diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index 054c8480cc..ec22d6d356 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -590,7 +590,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void MainPage::SaveButton_Click(const IInspectable& /*sender*/, const RoutedEventArgs& /*args*/) { _settingsClone.LogSettingChanges(false); - _settingsClone.WriteSettingsToDisk(); + if (!_settingsClone.WriteSettingsToDisk()) + { + ShowLoadWarningsDialog.raise(*this, _settingsClone.Warnings()); + } } void MainPage::ResetButton_Click(const IInspectable& /*sender*/, const RoutedEventArgs& /*args*/) diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.h b/src/cascadia/TerminalSettingsEditor/MainPage.h index 582f8813b1..234c4281a7 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.h +++ b/src/cascadia/TerminalSettingsEditor/MainPage.h @@ -45,6 +45,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation Windows::Foundation::Collections::IObservableVector Breadcrumbs() noexcept; til::typed_event OpenJson; + til::typed_event> ShowLoadWarningsDialog; private: Windows::Foundation::Collections::IObservableVector _breadcrumbs; diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.idl b/src/cascadia/TerminalSettingsEditor/MainPage.idl index 0f5a6e49b8..adda68f381 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.idl +++ b/src/cascadia/TerminalSettingsEditor/MainPage.idl @@ -36,6 +36,7 @@ namespace Microsoft.Terminal.Settings.Editor void UpdateSettings(Microsoft.Terminal.Settings.Model.CascadiaSettings settings); event Windows.Foundation.TypedEventHandler OpenJson; + event Windows.Foundation.TypedEventHandler > ShowLoadWarningsDialog; // Due to the aforementioned bug, we can't use IInitializeWithWindow _here_ either. // Let's just smuggle the HWND in as a UInt64 :| diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h index e91c43885e..33202d1df8 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h @@ -127,7 +127,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation winrt::Windows::Foundation::Collections::IObservableVector AllProfiles() const noexcept; winrt::Windows::Foundation::Collections::IObservableVector ActiveProfiles() const noexcept; Model::ActionMap ActionMap() const noexcept; - void WriteSettingsToDisk(); + bool WriteSettingsToDisk(); Json::Value ToJson() const; Model::Profile ProfileDefaults() const; Model::Profile CreateNewProfile(); diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl b/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl index 2fa41941d6..bddea4fe59 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl @@ -23,7 +23,7 @@ namespace Microsoft.Terminal.Settings.Model CascadiaSettings(String userJSON, String inboxJSON); CascadiaSettings Copy(); - void WriteSettingsToDisk(); + Boolean WriteSettingsToDisk(); void LogSettingChanges(Boolean isJsonLoad); String Hash { get; }; diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index 0d7f05730c..4c34eb0cd9 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -1056,15 +1056,7 @@ try // settings string back to the file. if (mustWriteToDisk) { - try - { - settings->WriteSettingsToDisk(); - } - catch (...) - { - LOG_CAUGHT_EXCEPTION(); - settings->_warnings.Append(SettingsLoadWarnings::FailedToWriteToSettings); - } + settings->WriteSettingsToDisk(); } else { @@ -1354,7 +1346,7 @@ winrt::hstring CascadiaSettings::DefaultSettingsPath() // - // Return Value: // - -void CascadiaSettings::WriteSettingsToDisk() +bool CascadiaSettings::WriteSettingsToDisk() { const auto settingsPath = _settingsPath(); @@ -1364,18 +1356,28 @@ void CascadiaSettings::WriteSettingsToDisk() wbuilder.settings_["indentation"] = " "; wbuilder.settings_["precision"] = 6; // prevent values like 1.1000000000000001 - FILETIME lastWriteTime{}; - const auto styledString{ Json::writeString(wbuilder, ToJson()) }; - til::io::write_utf8_string_to_file_atomic(settingsPath, styledString, &lastWriteTime); - - _hash = _calculateHash(styledString, lastWriteTime); - - // Persists the default terminal choice - // GH#10003 - Only do this if _currentDefaultTerminal was actually initialized. - if (_currentDefaultTerminal) + try { - DefaultTerminal::Current(_currentDefaultTerminal); + FILETIME lastWriteTime{}; + const auto styledString{ Json::writeString(wbuilder, ToJson()) }; + til::io::write_utf8_string_to_file_atomic(settingsPath, styledString, &lastWriteTime); + + _hash = _calculateHash(styledString, lastWriteTime); + + // Persists the default terminal choice + // GH#10003 - Only do this if _currentDefaultTerminal was actually initialized. + if (_currentDefaultTerminal) + { + DefaultTerminal::Current(_currentDefaultTerminal); + } } + catch (...) + { + LOG_CAUGHT_EXCEPTION(); + _warnings.Append(SettingsLoadWarnings::FailedToWriteToSettings); + return false; + } + return true; } #ifndef NDEBUG diff --git a/src/inc/til/io.h b/src/inc/til/io.h index 6f917f0d0b..9e28b99630 100644 --- a/src/inc/til/io.h +++ b/src/inc/til/io.h @@ -256,7 +256,11 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" // renaming one is (supposed to be) atomic. // Wait... "supposed to be"!? Well it's technically not always atomic, // but it's pretty darn close to it, so... better than nothing. - std::filesystem::rename(tmpPath, resolvedPath); + std::filesystem::rename(tmpPath, resolvedPath, ec); + if (ec) + { + THROW_WIN32_MSG(ec.value(), "failed to write to file"); + } } } // io } // til