Skip to content

Commit

Permalink
Merge branch 'dev/migrie/oop/3/ainulindale' into dev/migrie/oop/3/val…
Browse files Browse the repository at this point in the history 
…aquenta
  • Loading branch information
@zadjii-msft
zadjii-msft committed Feb 7, 2023
2 parents 1b4f1ef + 2822c36 commit f27db59
Show file tree
Hide file tree
Showing 21 changed files with 248 additions and 139 deletions.
27 changes: 22 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,15 @@ 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;

_settings.ExpandCommands();

hr = (_warnings.Size()) == 0 ? S_OK : S_FALSE;
}
catch (const winrt::hresult_error& e)
{
Expand Down Expand Up @@ -486,6 +490,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 +520,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
5 changes: 3 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,8 @@ 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 +89,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 } {};
};
}
87 changes: 5 additions & 82 deletions src/cascadia/TerminalApp/TerminalPage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,40 +98,12 @@ namespace winrt::TerminalApp::implementation
return S_OK;
}

// Function Description:
// - Recursively check our commands to see if there's a keybinding for
// exactly their action. If there is, label that command with the text
// corresponding to that key chord.
// - Will recurse into nested commands as well.
// Arguments:
// - settings: The settings who's keybindings we should use to look up the key chords from
// - commands: The list of commands to label.
static void _recursiveUpdateCommandKeybindingLabels(CascadiaSettings settings,
IMapView<winrt::hstring, Command> commands)
{
for (const auto& nameAndCmd : commands)
{
const auto& command = nameAndCmd.Value();
if (command.HasNestedCommands())
{
_recursiveUpdateCommandKeybindingLabels(settings, command.NestedCommands());
}
else
{
// If there's a keybinding that's bound to exactly this command,
// then get the keychord and display it as a
// part of the command in the UI.
// We specifically need to do this for nested commands.
const auto keyChord{ settings.ActionMap().GetKeyBindingForAction(command.ActionAndArgs().Action(), command.ActionAndArgs().Args()) };
command.RegisterKey(keyChord);
}
}
}

void TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI)
winrt::fire_and_forget TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI)
{
_settings = settings;

co_await wil::resume_foreground(Dispatcher());

// Make sure to _UpdateCommandsForPalette before
// _RefreshUIForSettingsReload. _UpdateCommandsForPalette will make
// sure the KeyChordText of Commands is updated, which needs to
Expand Down Expand Up @@ -2976,50 +2948,6 @@ namespace winrt::TerminalApp::implementation
}
}

// This is a helper to aid in sorting commands by their `Name`s, alphabetically.
static bool _compareSchemeNames(const ColorScheme& lhs, const ColorScheme& rhs)
{
std::wstring leftName{ lhs.Name() };
std::wstring rightName{ rhs.Name() };
return leftName.compare(rightName) < 0;
}

// Method Description:
// - Takes a mapping of names->commands and expands them
// Arguments:
// - <none>
// Return Value:
// - <none>
IMap<winrt::hstring, Command> TerminalPage::_ExpandCommands(IMapView<winrt::hstring, Command> commandsToExpand,
IVectorView<Profile> profiles,
IMapView<winrt::hstring, ColorScheme> schemes)
{
auto warnings{ winrt::single_threaded_vector<SettingsLoadWarnings>() };

std::vector<ColorScheme> sortedSchemes;
sortedSchemes.reserve(schemes.Size());

for (const auto& nameAndScheme : schemes)
{
sortedSchemes.push_back(nameAndScheme.Value());
}
std::sort(sortedSchemes.begin(),
sortedSchemes.end(),
_compareSchemeNames);

auto copyOfCommands = winrt::single_threaded_map<winrt::hstring, Command>();
for (const auto& nameAndCommand : commandsToExpand)
{
copyOfCommands.Insert(nameAndCommand.Key(), nameAndCommand.Value());
}

Command::ExpandCommands(copyOfCommands,
profiles,
{ sortedSchemes },
warnings);

return copyOfCommands;
}
// Method Description:
// - Repopulates the list of commands in the command palette with the
// current commands in the settings. Also updates the keybinding labels to
Expand All @@ -3030,15 +2958,10 @@ namespace winrt::TerminalApp::implementation
// - <none>
void TerminalPage::_UpdateCommandsForPalette()
{
auto copyOfCommands = _ExpandCommands(_settings.GlobalSettings().ActionMap().NameMap(),
_settings.ActiveProfiles().GetView(),
_settings.GlobalSettings().ColorSchemes());

_recursiveUpdateCommandKeybindingLabels(_settings, copyOfCommands.GetView());

// Update the command palette when settings reload
const auto& expanded{ _settings.GlobalSettings().ActionMap().ExpandedCommands() };
auto commandsCollection = winrt::single_threaded_vector<Command>();
for (const auto& nameAndCommand : copyOfCommands)
for (const auto& nameAndCommand : expanded)
{
commandsCollection.Append(nameAndCommand.Value());
}
Expand Down
6 changes: 1 addition & 5 deletions src/cascadia/TerminalApp/TerminalPage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ namespace winrt::TerminalApp::implementation
// put it in our inheritance graph. https://github.com/microsoft/microsoft-ui-xaml/issues/3331
STDMETHODIMP Initialize(HWND hwnd);

void SetSettings(Microsoft::Terminal::Settings::Model::CascadiaSettings settings, bool needRefreshUI);
winrt::fire_and_forget SetSettings(Microsoft::Terminal::Settings::Model::CascadiaSettings settings, bool needRefreshUI);

void Create();

Expand Down Expand Up @@ -298,10 +298,6 @@ namespace winrt::TerminalApp::implementation
void _UpdateCommandsForPalette();
void _SetBackgroundImage(const winrt::Microsoft::Terminal::Settings::Model::IAppearanceConfig& newAppearance);

static winrt::Windows::Foundation::Collections::IMap<winrt::hstring, Microsoft::Terminal::Settings::Model::Command> _ExpandCommands(Windows::Foundation::Collections::IMapView<winrt::hstring, Microsoft::Terminal::Settings::Model::Command> commandsToExpand,
Windows::Foundation::Collections::IVectorView<Microsoft::Terminal::Settings::Model::Profile> profiles,
Windows::Foundation::Collections::IMapView<winrt::hstring, Microsoft::Terminal::Settings::Model::ColorScheme> schemes);

void _DuplicateFocusedTab();
void _DuplicateTab(const TerminalTab& tab);

Expand Down
77 changes: 41 additions & 36 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,22 +738,31 @@ 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))
_settings = args.NewSettings();
// Update the settings in TerminalPage
_root->SetSettings(_settings, true);

co_await wil::resume_foreground(_root->Dispatcher());

// Bubble the notification up to the AppHost, now that we've updated our _settings.
_SettingsChangedHandlers(*this, args);

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;
// Update the settings in TerminalPage
_root->SetSettings(_settings, true);
_RefreshThemeRoutine();
}
void TerminalWindow::_OpenSettingsUI()
Expand Down Expand Up @@ -1160,13 +1168,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
Loading

1 comment on commit f27db59

@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 (5)

APeasant
applogic
connectecd
onarch
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 [email protected]:microsoft/terminal.git repository
on the dev/migrie/oop/3/valaquenta 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/4119066044/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.