Skip to content

Commit

Permalink
[ainulindale] "fix" hot reload
Browse files Browse the repository at this point in the history 
  Doesn't work with multiple windows open, but doesn't do _nothing_
  • Loading branch information
@zadjii-msft
zadjii-msft committed Feb 3, 2023
1 parent 17057d1 commit 427a4a5
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 48 deletions.
24 changes: 19 additions & 5 deletions src/cascadia/TerminalApp/AppLogic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "../inc/WindowingBehavior.h"
#include "AppLogic.g.cpp"
#include "FindTargetWindowResult.g.cpp"
#include "SettingsLoadEventArgs.h"

#include <LibraryResources.h>
#include <WtExeUtils.h>
Expand Down Expand Up @@ -256,10 +257,10 @@ namespace winrt::TerminalApp::implementation
return E_INVALIDARG;
}

_warnings.clear();
_warnings.Clear();
for (uint32_t i = 0; i < newSettings.Warnings().Size(); i++)
{
_warnings.push_back(newSettings.Warnings().GetAt(i));
_warnings.Append(newSettings.Warnings().GetAt(i));
}

_hasSettingsStartupActions = false;
Expand All @@ -279,12 +280,12 @@ namespace winrt::TerminalApp::implementation
}
else
{
_warnings.push_back(SettingsLoadWarnings::FailedToParseStartupActions);
_warnings.Append(SettingsLoadWarnings::FailedToParseStartupActions);
}
}

_settings = std::move(newSettings);
hr = _warnings.empty() ? S_OK : S_FALSE;
hr = (_warnings.Size()) == 0 ? S_OK : S_FALSE;
}
catch (const winrt::hresult_error& e)
{
Expand Down Expand Up @@ -486,6 +487,14 @@ namespace winrt::TerminalApp::implementation
// const winrt::hstring textKey = USES_RESOURCE(L"ReloadJsonParseErrorText");
// _ShowLoadErrorsDialog(titleKey, textKey, _settingsLoadedResult);
// return;

auto ev = winrt::make_self<SettingsLoadEventArgs>(true,
static_cast<uint64_t>(_settingsLoadedResult),
_settingsLoadExceptionText,
_warnings,
_settings);
_SettingsChangedHandlers(*this, *ev);
return;
}
}

Expand All @@ -508,7 +517,12 @@ namespace winrt::TerminalApp::implementation
_ApplyStartupTaskStateChange();
_ProcessLazySettingsChanges();

_SettingsChangedHandlers(*this, _settings);
auto ev = winrt::make_self<SettingsLoadEventArgs>(!initialLoad,
_settingsLoadedResult,
_settingsLoadExceptionText,
_warnings,
_settings);
_SettingsChangedHandlers(*this, *ev);
}

// Method Description:
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/AppLogic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace winrt::TerminalApp::implementation
TerminalApp::TerminalWindow CreateNewWindow();

winrt::TerminalApp::ContentManager ContentManager();
TYPED_EVENT(SettingsChanged, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
TYPED_EVENT(SettingsChanged, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::SettingsLoadEventArgs);

private:
bool _isUwp{ false };
Expand All @@ -88,7 +88,7 @@ namespace winrt::TerminalApp::implementation
std::shared_ptr<ThrottledFuncTrailing<>> _reloadSettings;
til::throttled_func_trailing<> _reloadState;

std::vector<Microsoft::Terminal::Settings::Model::SettingsLoadWarnings> _warnings;
winrt::Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::SettingsLoadWarnings> _warnings{ winrt::multi_threaded_vector<Microsoft::Terminal::Settings::Model::SettingsLoadWarnings>() };

// These fields invoke _reloadSettings and must be destroyed before _reloadSettings.
// (C++ destroys members in reverse-declaration-order.)
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppLogic.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ namespace TerminalApp

Windows.Foundation.Collections.IMapView<Microsoft.Terminal.Control.KeyChord, Microsoft.Terminal.Settings.Model.Command> GlobalHotkeys();

event Windows.Foundation.TypedEventHandler<Object, Object> SettingsChanged;
event Windows.Foundation.TypedEventHandler<Object, SettingsLoadEventArgs> SettingsChanged;

}
}
30 changes: 30 additions & 0 deletions src/cascadia/TerminalApp/SettingsLoadEventArgs.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

#pragma once

#include "SettingsLoadEventArgs.g.h"
#include <inc/cppwinrt_utils.h>
namespace winrt::TerminalApp::implementation
{
struct SettingsLoadEventArgs : SettingsLoadEventArgsT<SettingsLoadEventArgs>
{
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<Microsoft::Terminal::Settings::Model::SettingsLoadWarnings>, Warnings, nullptr);
WINRT_PROPERTY(Microsoft::Terminal::Settings::Model::CascadiaSettings, NewSettings, nullptr);

public:
SettingsLoadEventArgs(bool reload,
uint64_t result,
const winrt::hstring& exceptionText,
const winrt::Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::SettingsLoadWarnings>& warnings,
const Microsoft::Terminal::Settings::Model::CascadiaSettings& newSettings) :
_Reload{ reload },
_Result{ result },
_ExceptionText{ exceptionText },
_Warnings{ warnings },
_NewSettings{ newSettings } {};
};
}
69 changes: 35 additions & 34 deletions src/cascadia/TerminalApp/TerminalWindow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "TerminalWindow.h"
#include "../inc/WindowingBehavior.h"
#include "TerminalWindow.g.cpp"
#include "SettingsLoadEventArgs.g.cpp"

#include <LibraryResources.h>
#include <WtExeUtils.h>
Expand Down Expand Up @@ -410,7 +411,8 @@ namespace winrt::TerminalApp::implementation
// - contentKey: The key to use to lookup the content text from our resources.
void TerminalWindow::_ShowLoadErrorsDialog(const winrt::hstring& titleKey,
const winrt::hstring& contentKey,
HRESULT settingsLoadedResult)
HRESULT settingsLoadedResult,
const winrt::hstring& exceptionText)
{
auto title = GetLibraryResourceString(titleKey);
auto buttonText = RS_(L"Ok");
Expand All @@ -429,13 +431,12 @@ namespace winrt::TerminalApp::implementation

if (FAILED(settingsLoadedResult))
{
// TODO! _settingsLoadExceptionText needs to get into the TerminalWindow somehow

// if (!_settingsLoadExceptionText.empty())
// {
// warningsTextBlock.Inlines().Append(_BuildErrorRun(_settingsLoadExceptionText, ::winrt::Windows::UI::Xaml::Application::Current().as<::winrt::TerminalApp::App>().Resources()));
// warningsTextBlock.Inlines().Append(Documents::LineBreak{});
// }
if (!exceptionText.empty())
{
warningsTextBlock.Inlines().Append(_BuildErrorRun(exceptionText,
winrt::WUX::Application::Current().as<::winrt::TerminalApp::App>().Resources()));
warningsTextBlock.Inlines().Append(Documents::LineBreak{});
}
}

// Add a note that we're using the default settings in this case.
Expand All @@ -460,7 +461,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()
void TerminalWindow::_ShowLoadWarningsDialog(const Windows::Foundation::Collections::IVector<SettingsLoadWarnings>& warnings)
{
auto title = RS_(L"SettingsValidateErrorTitle");
auto buttonText = RS_(L"Ok");
Expand All @@ -471,18 +472,16 @@ namespace winrt::TerminalApp::implementation
// Make sure the lines of text wrap
warningsTextBlock.TextWrapping(TextWrapping::Wrap);

// TODO! warnings need to get into here somehow
//
// for (const auto& warning : _warnings)
// {
// // Try looking up the warning message key for each warning.
// const auto warningText = _GetWarningText(warning);
// if (!warningText.empty())
// {
// warningsTextBlock.Inlines().Append(_BuildErrorRun(warningText, ::winrt::Windows::UI::Xaml::Application::Current().as<::winrt::TerminalApp::App>().Resources()));
// warningsTextBlock.Inlines().Append(Documents::LineBreak{});
// }
// }
for (const auto& warning : warnings)
{
// Try looking up the warning message key for each warning.
const auto warningText = _GetWarningText(warning);
if (!warningText.empty())
{
warningsTextBlock.Inlines().Append(_BuildErrorRun(warningText, winrt::WUX::Application::Current().as<::winrt::TerminalApp::App>().Resources()));
warningsTextBlock.Inlines().Append(Documents::LineBreak{});
}
}

Controls::ContentDialog dialog;
dialog.Title(winrt::box_value(title));
Expand Down Expand Up @@ -739,20 +738,25 @@ namespace winrt::TerminalApp::implementation
_RequestedThemeChangedHandlers(*this, Theme());
}

void TerminalWindow::UpdateSettings(const HRESULT settingsLoadedResult, const CascadiaSettings& settings)
winrt::fire_and_forget TerminalWindow::UpdateSettings(winrt::TerminalApp::SettingsLoadEventArgs args)
{
if (FAILED(settingsLoadedResult))
co_await wil::resume_foreground(_root->Dispatcher());

if (FAILED(args.Result()))
{
const winrt::hstring titleKey = USES_RESOURCE(L"ReloadJsonParseErrorTitle");
const winrt::hstring textKey = USES_RESOURCE(L"ReloadJsonParseErrorText");
_ShowLoadErrorsDialog(titleKey, textKey, settingsLoadedResult);
return;
_ShowLoadErrorsDialog(titleKey,
textKey,
gsl::narrow_cast<HRESULT>(args.Result()),
args.ExceptionText());
co_return;
}
else if (settingsLoadedResult == S_FALSE)
else if (args.Result() == S_FALSE)
{
_ShowLoadWarningsDialog();
_ShowLoadWarningsDialog(args.Warnings());
}
_settings = settings;
_settings = args.NewSettings();
// Update the settings in TerminalPage
_root->SetSettings(_settings, true);
_RefreshThemeRoutine();
Expand Down Expand Up @@ -1160,13 +1164,10 @@ namespace winrt::TerminalApp::implementation
}
// TODO! Arg should be a SettingsLoadEventArgs{ result, warnings, error, settings}
void TerminalWindow::UpdateSettingsHandler(const winrt::IInspectable& /*sender*/,
const winrt::IInspectable& arg)
const winrt::TerminalApp::SettingsLoadEventArgs& args)
{
if (const auto& settings{ arg.try_as<CascadiaSettings>() })
{
this->UpdateSettings(S_OK, settings);
_root->SetSettings(_settings, true);
}
UpdateSettings(args);
// _root->SetSettings(_settings, true);
}

////////////////////////////////////////////////////////////////////////////
Expand Down
12 changes: 8 additions & 4 deletions src/cascadia/TerminalApp/TerminalWindow.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "SystemMenuChangeArgs.g.h"

#include "TerminalPage.h"
#include "SettingsLoadEventArgs.h"

#include <inc/cppwinrt_utils.h>
#include <ThrottledFunc.h>
Expand Down Expand Up @@ -47,7 +48,7 @@ namespace winrt::TerminalApp::implementation

void Quit();

void UpdateSettings(const HRESULT settingsLoadedResult, const Microsoft::Terminal::Settings::Model::CascadiaSettings& settings);
winrt::fire_and_forget UpdateSettings(winrt::TerminalApp::SettingsLoadEventArgs args);

bool HasCommandlineArguments() const noexcept;
// bool HasSettingsStartupActions() const noexcept;
Expand Down Expand Up @@ -108,7 +109,7 @@ namespace winrt::TerminalApp::implementation
void DismissDialog();

Microsoft::Terminal::Settings::Model::Theme Theme();
void UpdateSettingsHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& arg);
void UpdateSettingsHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::TerminalApp::SettingsLoadEventArgs& arg);

// Normally, WindowName and WindowId would be
// WINRT_OBSERVABLE_PROPERTY's, but we want them to raise
Expand Down Expand Up @@ -163,8 +164,11 @@ namespace winrt::TerminalApp::implementation

TerminalApp::ContentManager _manager{ nullptr };

void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey, HRESULT settingsLoadedResult);
void _ShowLoadWarningsDialog();
void _ShowLoadErrorsDialog(const winrt::hstring& titleKey,
const winrt::hstring& contentKey,
HRESULT settingsLoadedResult,
const winrt::hstring& exceptionText);
void _ShowLoadWarningsDialog(const Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::SettingsLoadWarnings>& warnings);
bool _IsKeyboardServiceEnabled();

void _RefreshThemeRoutine();
Expand Down
11 changes: 11 additions & 0 deletions src/cascadia/TerminalApp/TerminalWindow.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ namespace TerminalApp
SystemMenuItemHandler Handler { get; };
};

[default_interface] runtimeclass SettingsLoadEventArgs
{
Boolean Reload { get; };
UInt64 Result { get; };
IVector<Microsoft.Terminal.Settings.Model.SettingsLoadWarnings> Warnings { get; };
String ExceptionText { get; };

Microsoft.Terminal.Settings.Model.CascadiaSettings NewSettings { get; };

};

// See IDialogPresenter and TerminalPage's DialogPresenter for more
// information.
[default_interface] runtimeclass TerminalWindow : IDirectKeyListener, IDialogPresenter, IWindowProperties, Windows.UI.Xaml.Data.INotifyPropertyChanged
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/WindowsTerminal/AppHost.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1436,8 +1436,10 @@ void AppHost::_updateTheme()
}

void AppHost::_HandleSettingsChanged(const winrt::Windows::Foundation::IInspectable& /*sender*/,
const winrt::Windows::Foundation::IInspectable& /*args*/)
const winrt::TerminalApp::SettingsLoadEventArgs& /*args*/)
{
// We don't need to call in to windowLogic here - it has its own SettingsChanged handler

_setupGlobalHotkeys();

// TODO! tray icon
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/WindowsTerminal/AppHost.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class AppHost
winrt::fire_and_forget _setupGlobalHotkeys();
winrt::fire_and_forget _createNewTerminalWindow(winrt::Microsoft::Terminal::Settings::Model::GlobalSummonArgs args);
void _HandleSettingsChanged(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::Foundation::IInspectable& args);
const winrt::TerminalApp::SettingsLoadEventArgs& args);

void _IsQuakeWindowChanged(const winrt::Windows::Foundation::IInspectable& sender,
const winrt::Windows::Foundation::IInspectable& args);
Expand Down

1 comment on commit 427a4a5

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@check-spelling-bot Report

🔴 Please review

See the 📜action log for details.

Unrecognized words (9)

Acn
APeasant
connectecd
onarch
taht
tearout
TOODO
unzoom
wehn

Previously acknowledged words that are now absent CLA demoable everytime Hirots inthread reingest unmark :arrow_right:
To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the git@github.com:microsoft/terminal.git repository
on the dev/migrie/oop/3/quenta-silmarillion branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/4087287375/attempts/1'
Errors (1)

See the 📜action log for details.

❌ Errors Count
❌ forbidden-pattern 1

See ❌ Event descriptions for more information.

✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Please sign in to comment.