From 4d212cec1f42788030800285d222277e643bfe11 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 9 Oct 2020 21:54:45 -0700 Subject: [PATCH 1/4] Implement CascadiaSettings::Copy() --- .../DeserializationTests.cpp | 85 ++++++++++++++ .../TerminalSettingsModel/ActionAndArgs.cpp | 8 ++ .../TerminalSettingsModel/ActionAndArgs.h | 1 + .../TerminalSettingsModel/ActionArgs.h | 104 ++++++++++++++++++ .../TerminalSettingsModel/ActionArgs.idl | 2 + .../CascadiaSettings.cpp | 23 ++++ .../TerminalSettingsModel/CascadiaSettings.h | 3 +- .../CascadiaSettings.idl | 1 + .../TerminalSettingsModel/ColorScheme.cpp | 12 ++ .../TerminalSettingsModel/ColorScheme.h | 1 + .../TerminalSettingsModel/Command.cpp | 26 ++++- src/cascadia/TerminalSettingsModel/Command.h | 3 +- .../GlobalAppSettings.cpp | 47 ++++++++ .../TerminalSettingsModel/GlobalAppSettings.h | 1 + .../TerminalSettingsModel/KeyMapping.cpp | 11 ++ .../TerminalSettingsModel/KeyMapping.h | 1 + .../TerminalSettingsModel/Profile.cpp | 43 ++++++++ src/cascadia/TerminalSettingsModel/Profile.h | 1 + 18 files changed, 366 insertions(+), 7 deletions(-) diff --git a/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp b/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp index 283646a6f02..7d76f9da6ca 100644 --- a/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp @@ -79,6 +79,8 @@ namespace SettingsModelLocalTests TEST_METHOD(TestUnbindNestedCommand); TEST_METHOD(TestRebindNestedCommand); + TEST_METHOD(TestCopy); + TEST_CLASS_SETUP(ClassSetup) { InitializeJsonReader(); @@ -2347,4 +2349,87 @@ namespace SettingsModelLocalTests } } + void DeserializationTests::TestCopy() + { + const std::string settingsJson{ R"( + { + "defaultProfile": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}", + "initialCols": 50, + "profiles": + [ + { + "guid": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}", + "name": "Custom Profile", + "fontFace": "Cascadia Code" + } + ], + "schemes": + [ + { + "name": "Campbell, but for a test", + "foreground": "#CCCCCC", + "background": "#0C0C0C", + "cursorColor": "#FFFFFF", + "black": "#0C0C0C", + "red": "#C50F1F", + "green": "#13A10E", + "yellow": "#C19C00", + "blue": "#0037DA", + "purple": "#881798", + "cyan": "#3A96DD", + "white": "#CCCCCC", + "brightBlack": "#767676", + "brightRed": "#E74856", + "brightGreen": "#16C60C", + "brightYellow": "#F9F1A5", + "brightBlue": "#3B78FF", + "brightPurple": "#B4009E", + "brightCyan": "#61D6D6", + "brightWhite": "#F2F2F2" + } + ], + "actions": + [ + { "command": "openSettings", "keys": "ctrl+," }, + { "command": { "action": "openSettings", "target": "defaultsFile" }, "keys": "ctrl+alt+," }, + + { + "name": { "key": "SetColorSchemeParentCommandName" }, + "commands": [ + { + "iterateOn": "schemes", + "name": "${scheme.name}", + "command": { "action": "setColorScheme", "colorScheme": "${scheme.name}" } + } + ] + } + ] + })" }; + + VerifyParseSucceeded(settingsJson); + + auto settings{ winrt::make_self() }; + settings->_ParseJsonString(settingsJson, false); + settings->LayerJson(settings->_userSettings); + settings->_ValidateSettings(); + + const auto copy{ settings->Copy() }; + const auto copyImpl{ winrt::get_self(copy) }; + + // test globals + VERIFY_ARE_EQUAL(settings->_globals->DefaultProfile(), copyImpl->_globals->DefaultProfile()); + + // test profiles + VERIFY_ARE_EQUAL(settings->_profiles.Size(), copyImpl->_profiles.Size()); + VERIFY_ARE_EQUAL(settings->_profiles.GetAt(0).Name(), copyImpl->_profiles.GetAt(0).Name()); + + // test schemes + const auto schemeName{ L"Campbell, but for a test" }; + VERIFY_ARE_EQUAL(settings->_globals->_colorSchemes.Size(), copyImpl->_globals->_colorSchemes.Size()); + VERIFY_ARE_EQUAL(settings->_globals->_colorSchemes.HasKey(schemeName), copyImpl->_globals->_colorSchemes.HasKey(schemeName)); + + // test actions + VERIFY_ARE_EQUAL(settings->_globals->_keymap->_keyShortcuts.size(), copyImpl->_globals->_keymap->_keyShortcuts.size()); + VERIFY_ARE_EQUAL(settings->_globals->_commands.Size(), copyImpl->_globals->_commands.Size()); + } } diff --git a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp index 91b1c29dd4d..2cf9c7febbc 100644 --- a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp @@ -240,6 +240,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } } + com_ptr ActionAndArgs::Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Action = _Action; + copy->_Args = _Args.Copy(); + return copy; + } + winrt::hstring ActionAndArgs::GenerateName() const { // Use a magic static to initialize this map, because we won't be able diff --git a/src/cascadia/TerminalSettingsModel/ActionAndArgs.h b/src/cascadia/TerminalSettingsModel/ActionAndArgs.h index a23879c6183..7bea215d7be 100644 --- a/src/cascadia/TerminalSettingsModel/ActionAndArgs.h +++ b/src/cascadia/TerminalSettingsModel/ActionAndArgs.h @@ -19,6 +19,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation ActionAndArgs(ShortcutAction action, IActionArgs args) : _Action{ action }, _Args{ args } {}; + com_ptr Copy() const; hstring GenerateName() const; diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h index 209fef24c24..636cca76c53 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.h +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.h @@ -89,6 +89,16 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation JsonUtils::GetValueForKey(json, ProfileKey, args->_Profile); return *args; } + Model::NewTerminalArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Commandline = _Commandline; + copy->_StartingDirectory = _StartingDirectory; + copy->_TabTitle = _TabTitle; + copy->_ProfileIndex = _ProfileIndex; + copy->_Profile = _Profile; + return *copy; + } }; struct CopyTextArgs : public CopyTextArgsT @@ -121,6 +131,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation JsonUtils::GetValueForKey(json, CopyFormattingKey, args->_CopyFormatting); return { *args, {} }; } + + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_SingleLine = _SingleLine; + copy->_CopyFormatting = _CopyFormatting; + return copy.as(); + } }; struct NewTabArgs : public NewTabArgsT @@ -149,6 +167,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation args->_TerminalArgs = NewTerminalArgs::FromJson(json); return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_TerminalArgs = _TerminalArgs.Copy(); + return copy.as(); + } }; struct SwitchToTabArgs : public SwitchToTabArgsT @@ -179,6 +203,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation JsonUtils::GetValueForKey(json, TabIndexKey, args->_TabIndex); return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_TabIndex = _TabIndex; + return copy.as(); + } }; struct ResizePaneArgs : public ResizePaneArgsT @@ -214,6 +244,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return { *args, {} }; } } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Direction = _Direction; + return copy.as(); + } }; struct MoveFocusArgs : public MoveFocusArgsT @@ -249,6 +285,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return { *args, {} }; } } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Direction = _Direction; + return copy.as(); + } }; struct AdjustFontSizeArgs : public AdjustFontSizeArgsT @@ -277,6 +319,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation JsonUtils::GetValueForKey(json, AdjustFontSizeDelta, args->_Delta); return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Delta = _Delta; + return copy.as(); + } }; struct SendInputArgs : public SendInputArgsT @@ -308,6 +356,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Input = _Input; + return copy.as(); + } }; struct SplitPaneArgs : public SplitPaneArgsT @@ -347,6 +401,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation JsonUtils::GetValueForKey(json, SplitModeKey, args->_SplitMode); return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_SplitStyle = _SplitStyle; + copy->_TerminalArgs = _TerminalArgs.Copy(); + copy->_SplitMode = _SplitMode; + return copy.as(); + } }; struct OpenSettingsArgs : public OpenSettingsArgsT @@ -375,6 +437,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation JsonUtils::GetValueForKey(json, TargetKey, args->_Target); return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Target = _Target; + return copy.as(); + } }; struct SetColorSchemeArgs : public SetColorSchemeArgsT @@ -407,6 +475,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_SchemeName = _SchemeName; + return copy.as(); + } }; struct SetTabColorArgs : public SetTabColorArgsT @@ -438,6 +512,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_TabColor = _TabColor; + return copy.as(); + } }; struct RenameTabArgs : public RenameTabArgsT @@ -466,6 +546,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation JsonUtils::GetValueForKey(json, TitleKey, args->_Title); return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Title = _Title; + return copy.as(); + } }; struct ExecuteCommandlineArgs : public ExecuteCommandlineArgsT @@ -500,6 +586,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation } return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Commandline = _Commandline; + return copy.as(); + } }; struct CloseOtherTabsArgs : public CloseOtherTabsArgsT @@ -528,6 +620,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation JsonUtils::GetValueForKey(json, IndexKey, args->_Index); return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Index = _Index; + return copy.as(); + } }; struct CloseTabsAfterArgs : public CloseTabsAfterArgsT @@ -556,6 +654,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation JsonUtils::GetValueForKey(json, IndexKey, args->_Index); return { *args, {} }; } + IActionArgs Copy() const + { + auto copy{ winrt::make_self() }; + copy->_Index = _Index; + return copy.as(); + } }; } diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.idl b/src/cascadia/TerminalSettingsModel/ActionArgs.idl index 407dbd6d4cb..7cb075e7cea 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.idl +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.idl @@ -7,6 +7,7 @@ namespace Microsoft.Terminal.Settings.Model { Boolean Equals(IActionArgs other); String GenerateName(); + IActionArgs Copy(); }; interface IActionEventArgs @@ -48,6 +49,7 @@ namespace Microsoft.Terminal.Settings.Model [default_interface] runtimeclass NewTerminalArgs { NewTerminalArgs(); NewTerminalArgs(Int32 profileIndex); + NewTerminalArgs Copy(); String Commandline; String StartingDirectory; diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp index b54c2daea20..0110c4babdf 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp @@ -66,6 +66,29 @@ CascadiaSettings::CascadiaSettings(winrt::hstring json) : _ValidateSettings(); } +winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings CascadiaSettings::Copy() const +{ + // dynamic profile generators added by default + auto settings{ winrt::make_self() }; + settings->_globals = _globals->Copy(); + for (const auto profile : _profiles) + { + auto profImpl{ winrt::get_self(profile) }; + settings->_profiles.Append(*profImpl->Copy()); + } + for (auto warning : _warnings) + { + settings->_warnings.Append(warning); + } + settings->_loadError = _loadError; + settings->_deserializationErrorMessage = _deserializationErrorMessage; + settings->_userSettingsString = _userSettingsString; + settings->_userSettings = _userSettings; + settings->_defaultSettings = _defaultSettings; + settings->_userDefaultProfileSettings = _userDefaultProfileSettings; + return *settings; +} + // Method Description: // - Finds a profile that matches the given GUID. If there is no profile in this // settings object that matches, returns nullptr. diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h index 410710b90b4..65aed31eb32 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h @@ -60,15 +60,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation CascadiaSettings(); explicit CascadiaSettings(const bool addDynamicProfiles); CascadiaSettings(hstring json); + Model::CascadiaSettings Copy() const; static Model::CascadiaSettings LoadDefaults(); static Model::CascadiaSettings LoadAll(); static Model::CascadiaSettings LoadUniversal(); Model::GlobalAppSettings GlobalSettings() const; - Windows::Foundation::Collections::IObservableVector Profiles() const noexcept; - Model::KeyMapping KeyMap() const noexcept; static com_ptr FromJson(const Json::Value& json); diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl b/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl index 09779be7700..8394a9ba145 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl @@ -9,6 +9,7 @@ namespace Microsoft.Terminal.Settings.Model { [default_interface] runtimeclass CascadiaSettings { CascadiaSettings(String json); + CascadiaSettings Copy(); static CascadiaSettings LoadDefaults(); static CascadiaSettings LoadAll(); diff --git a/src/cascadia/TerminalSettingsModel/ColorScheme.cpp b/src/cascadia/TerminalSettingsModel/ColorScheme.cpp index 78c2da9e0b4..1daf81f6671 100644 --- a/src/cascadia/TerminalSettingsModel/ColorScheme.cpp +++ b/src/cascadia/TerminalSettingsModel/ColorScheme.cpp @@ -57,6 +57,18 @@ ColorScheme::ColorScheme(winrt::hstring name, Color defaultFg, Color defaultBg, { } +winrt::com_ptr ColorScheme::Copy() const +{ + auto scheme{ winrt::make_self() }; + scheme->_Name = _Name; + scheme->_Foreground = _Foreground; + scheme->_Background = _Background; + scheme->_SelectionBackground = _SelectionBackground; + scheme->_CursorColor = _CursorColor; + scheme->_table = _table; + return scheme; +} + // Method Description: // - Create a new instance of this class from a serialized JsonObject. // Arguments: diff --git a/src/cascadia/TerminalSettingsModel/ColorScheme.h b/src/cascadia/TerminalSettingsModel/ColorScheme.h index 87c69c9fba4..d5abd4ac813 100644 --- a/src/cascadia/TerminalSettingsModel/ColorScheme.h +++ b/src/cascadia/TerminalSettingsModel/ColorScheme.h @@ -34,6 +34,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation public: ColorScheme(); ColorScheme(hstring name, Windows::UI::Color defaultFg, Windows::UI::Color defaultBg, Windows::UI::Color cursorColor); + com_ptr Copy() const; static com_ptr FromJson(const Json::Value& json); bool ShouldBeLayered(const Json::Value& json) const; diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp index c060886b6a6..9fa5136470a 100644 --- a/src/cascadia/TerminalSettingsModel/Command.cpp +++ b/src/cascadia/TerminalSettingsModel/Command.cpp @@ -33,19 +33,38 @@ static constexpr std::string_view SchemeNameToken{ "${scheme.name}" }; namespace winrt::Microsoft::Terminal::Settings::Model::implementation { - Command::Command() + Command::Command() : + _subcommands{ winrt::single_threaded_map() } { _setAction(nullptr); } + com_ptr Command::Copy() const + { + auto command{ winrt::make_self() }; + command->_Name = _Name; + command->_Action = _Action; + command->_KeyChordText = _KeyChordText; + command->_Icon = _Icon; + command->_IterateOn = _IterateOn; + + command->_originalJson = _originalJson; + for (auto kv : _subcommands) + { + const auto subcmd{ winrt::get_self(kv.Value()) }; + command->_subcommands.Insert(kv.Key(), *subcmd->Copy()); + } + return command; + } + IMapView Command::NestedCommands() { - return _subcommands ? _subcommands.GetView() : nullptr; + return _subcommands.Size() > 0 ? _subcommands.GetView() : nullptr; } bool Command::HasNestedCommands() { - return _subcommands ? _subcommands.Size() > 0 : false; + return _subcommands.Size() > 0; } // Function Description: // - attempt to get the name of this command from the provided json object. @@ -135,7 +154,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation if (const auto nestedCommandsJson{ json[JsonKey(CommandsKey)] }) { // Initialize our list of subcommands. - result->_subcommands = winrt::single_threaded_map(); auto nestedWarnings = Command::LayerJson(result->_subcommands, nestedCommandsJson); // It's possible that the nested commands have some warnings warnings.insert(warnings.end(), nestedWarnings.begin(), nestedWarnings.end()); diff --git a/src/cascadia/TerminalSettingsModel/Command.h b/src/cascadia/TerminalSettingsModel/Command.h index 01579f1ac9a..77ccb356e65 100644 --- a/src/cascadia/TerminalSettingsModel/Command.h +++ b/src/cascadia/TerminalSettingsModel/Command.h @@ -36,6 +36,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation struct Command : CommandT { Command(); + com_ptr Copy() const; static winrt::com_ptr FromJson(const Json::Value& json, std::vector& warnings); @@ -62,7 +63,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation private: Json::Value _originalJson; - Windows::Foundation::Collections::IMap _subcommands{ nullptr }; + Windows::Foundation::Collections::IMap _subcommands; static std::vector _expandCommand(Command* const expandable, Windows::Foundation::Collections::IVectorView profiles, diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp index b3eb1ad0e8d..f751bffd347 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp @@ -63,6 +63,53 @@ GlobalAppSettings::GlobalAppSettings() : _colorSchemes = winrt::single_threaded_map(); } +winrt::com_ptr GlobalAppSettings::Copy() const +{ + auto globals{ winrt::make_self() }; + globals->_InitialRows = _InitialRows; + globals->_InitialCols = _InitialCols; + globals->_AlwaysShowTabs = _AlwaysShowTabs; + globals->_ShowTitleInTitlebar = _ShowTitleInTitlebar; + globals->_ConfirmCloseAllTabs = _ConfirmCloseAllTabs; + globals->_Theme = _Theme; + globals->_TabWidthMode = _TabWidthMode; + globals->_ShowTabsInTitlebar = _ShowTabsInTitlebar; + globals->_WordDelimiters = _WordDelimiters; + globals->_CopyOnSelect = _CopyOnSelect; + globals->_CopyFormatting = _CopyFormatting; + globals->_WarnAboutLargePaste = _WarnAboutLargePaste; + globals->_WarnAboutMultiLinePaste = _WarnAboutMultiLinePaste; + globals->_InitialPosition = _InitialPosition; + globals->_LaunchMode = _LaunchMode; + globals->_SnapToGridOnResize = _SnapToGridOnResize; + globals->_ForceFullRepaintRendering = _ForceFullRepaintRendering; + globals->_SoftwareRendering = _SoftwareRendering; + globals->_ForceVTInput = _ForceVTInput; + globals->_DebugFeaturesEnabled = _DebugFeaturesEnabled; + globals->_StartOnUserLogin = _StartOnUserLogin; + globals->_AlwaysOnTop = _AlwaysOnTop; + globals->_UseTabSwitcher = _UseTabSwitcher; + globals->_DisableAnimations = _DisableAnimations; + + globals->_unparsedDefaultProfile = _unparsedDefaultProfile; + globals->_defaultProfile = _defaultProfile; + globals->_keymap = _keymap->Copy(); + std::for_each(_keybindingsWarnings.begin(), _keybindingsWarnings.end(), [globals](auto& warning) { globals->_keybindingsWarnings.push_back(warning); }); + + for (auto kv : _colorSchemes) + { + const auto schemeImpl{ winrt::get_self(kv.Value()) }; + globals->_colorSchemes.Insert(kv.Key(), *schemeImpl->Copy()); + } + + for (auto kv : _commands) + { + const auto commandImpl{ winrt::get_self(kv.Value()) }; + globals->_commands.Insert(kv.Key(), *commandImpl->Copy()); + } + return globals; +} + winrt::Windows::Foundation::Collections::IMapView GlobalAppSettings::ColorSchemes() noexcept { return _colorSchemes.GetView(); diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h index 39eb18f6549..8b2dbf5fdce 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h @@ -34,6 +34,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { public: GlobalAppSettings(); + com_ptr Copy() const; Windows::Foundation::Collections::IMapView ColorSchemes() noexcept; void AddColorScheme(const Model::ColorScheme& scheme); diff --git a/src/cascadia/TerminalSettingsModel/KeyMapping.cpp b/src/cascadia/TerminalSettingsModel/KeyMapping.cpp index 91838eab09f..52e23f23ea8 100644 --- a/src/cascadia/TerminalSettingsModel/KeyMapping.cpp +++ b/src/cascadia/TerminalSettingsModel/KeyMapping.cpp @@ -4,6 +4,7 @@ #include "pch.h" #include "KeyMapping.h" #include "KeyChordSerialization.h" +#include "ActionAndArgs.h" #include "KeyMapping.g.cpp" @@ -12,6 +13,16 @@ using namespace winrt::Microsoft::Terminal::TerminalControl; namespace winrt::Microsoft::Terminal::Settings::Model::implementation { + winrt::com_ptr KeyMapping::Copy() const + { + auto keymap{ winrt::make_self() }; + std::for_each(_keyShortcuts.begin(), _keyShortcuts.end(), [keymap](auto& kv) { + const auto actionAndArgsImpl{ winrt::get_self(kv.second) }; + keymap->_keyShortcuts[kv.first] = *actionAndArgsImpl->Copy(); + }); + return keymap; + } + Microsoft::Terminal::Settings::Model::ActionAndArgs KeyMapping::TryLookup(KeyChord const& chord) const { const auto result = _keyShortcuts.find(chord); diff --git a/src/cascadia/TerminalSettingsModel/KeyMapping.h b/src/cascadia/TerminalSettingsModel/KeyMapping.h index 5918b8b9686..aba32936855 100644 --- a/src/cascadia/TerminalSettingsModel/KeyMapping.h +++ b/src/cascadia/TerminalSettingsModel/KeyMapping.h @@ -52,6 +52,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation struct KeyMapping : KeyMappingT { KeyMapping() = default; + com_ptr Copy() const; Model::ActionAndArgs TryLookup(TerminalControl::KeyChord const& chord) const; uint64_t Size() const; diff --git a/src/cascadia/TerminalSettingsModel/Profile.cpp b/src/cascadia/TerminalSettingsModel/Profile.cpp index d3419d34255..0dee925a91d 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.cpp +++ b/src/cascadia/TerminalSettingsModel/Profile.cpp @@ -67,6 +67,49 @@ Profile::Profile(guid guid) : { } +winrt::com_ptr Profile::Copy() const +{ + auto profile{ winrt::make_self() }; + profile->_Name = _Name; + profile->_Source = _Source; + profile->_Hidden = _Hidden; + profile->_Icon = _Icon; + profile->_CloseOnExit = _CloseOnExit; + profile->_TabTitle = _TabTitle; + profile->_TabColor = _TabColor; + profile->_SuppressApplicationTitle = _SuppressApplicationTitle; + profile->_UseAcrylic = _UseAcrylic; + profile->_AcrylicOpacity = _AcrylicOpacity; + profile->_ScrollState = _ScrollState; + profile->_FontFace = _FontFace; + profile->_FontSize = _FontSize; + profile->_FontWeight = _FontWeight; + profile->_Padding = _Padding; + profile->_Commandline = _Commandline; + profile->_StartingDirectory = _StartingDirectory; + profile->_BackgroundImagePath = _BackgroundImagePath; + profile->_BackgroundImageOpacity = _BackgroundImageOpacity; + profile->_BackgroundImageStretchMode = _BackgroundImageStretchMode; + profile->_AntialiasingMode = _AntialiasingMode; + profile->_RetroTerminalEffect = _RetroTerminalEffect; + profile->_ForceFullRepaintRendering = _ForceFullRepaintRendering; + profile->_SoftwareRendering = _SoftwareRendering; + profile->_ColorSchemeName = _ColorSchemeName; + profile->_Foreground = _Foreground; + profile->_Background = _Background; + profile->_SelectionBackground = _SelectionBackground; + profile->_CursorColor = _CursorColor; + profile->_HistorySize = _HistorySize; + profile->_SnapOnInput = _SnapOnInput; + profile->_AltGrAliasing = _AltGrAliasing; + profile->_CursorShape = _CursorShape; + profile->_CursorHeight = _CursorHeight; + profile->_Guid = _Guid; + profile->_ConnectionType = _ConnectionType; + profile->_BackgroundImageAlignment = _BackgroundImageAlignment; + return profile; +} + // Method Description: // - Generates a Json::Value which is a "stub" of this profile. This stub will // have enough information that it could be layered with this profile. diff --git a/src/cascadia/TerminalSettingsModel/Profile.h b/src/cascadia/TerminalSettingsModel/Profile.h index eec8d1dda9a..1d28c7e19fa 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.h +++ b/src/cascadia/TerminalSettingsModel/Profile.h @@ -44,6 +44,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation public: Profile(); Profile(guid guid); + com_ptr Copy() const; Json::Value GenerateStub() const; static com_ptr FromJson(const Json::Value& json); From d3057d80f96cae77b145f2a68900ca8db18d0e9c Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 12 Oct 2020 09:26:20 -0700 Subject: [PATCH 2/4] subcmd is a word --- .github/actions/spell-check/dictionary/apis.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/spell-check/dictionary/apis.txt b/.github/actions/spell-check/dictionary/apis.txt index 2ce6c20c31f..ea6482584ee 100644 --- a/.github/actions/spell-check/dictionary/apis.txt +++ b/.github/actions/spell-check/dictionary/apis.txt @@ -45,6 +45,7 @@ RSHIFT rx serializer SIZENS +subcmd spsc STDCPP syscall From 526e9a4f43e818f66d34f99a99240d86f5193d04 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 16 Oct 2020 13:30:13 -0700 Subject: [PATCH 3/4] Address PR comments --- .../actions/spell-check/dictionary/apis.txt | 1 - .../DeserializationTests.cpp | 5 ++++ .../TerminalSettingsModel/ActionArgs.h | 30 +++++++++---------- .../TerminalSettingsModel/Command.cpp | 22 ++++++++------ src/cascadia/TerminalSettingsModel/Command.h | 6 ++-- .../GlobalAppSettings.cpp | 2 +- 6 files changed, 37 insertions(+), 29 deletions(-) diff --git a/.github/actions/spell-check/dictionary/apis.txt b/.github/actions/spell-check/dictionary/apis.txt index ea6482584ee..2ce6c20c31f 100644 --- a/.github/actions/spell-check/dictionary/apis.txt +++ b/.github/actions/spell-check/dictionary/apis.txt @@ -45,7 +45,6 @@ RSHIFT rx serializer SIZENS -subcmd spsc STDCPP syscall diff --git a/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp b/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp index 7d76f9da6ca..43d457c0fad 100644 --- a/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp +++ b/src/cascadia/LocalTests_SettingsModel/DeserializationTests.cpp @@ -2431,5 +2431,10 @@ namespace SettingsModelLocalTests // test actions VERIFY_ARE_EQUAL(settings->_globals->_keymap->_keyShortcuts.size(), copyImpl->_globals->_keymap->_keyShortcuts.size()); VERIFY_ARE_EQUAL(settings->_globals->_commands.Size(), copyImpl->_globals->_commands.Size()); + + // Test that changing the copy should not change the original + VERIFY_ARE_EQUAL(settings->_globals->WordDelimiters(), copyImpl->_globals->WordDelimiters()); + copyImpl->_globals->WordDelimiters(L"changed value"); + VERIFY_ARE_NOT_EQUAL(settings->_globals->WordDelimiters(), copyImpl->_globals->WordDelimiters()); } } diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.h b/src/cascadia/TerminalSettingsModel/ActionArgs.h index 636cca76c53..148ad73a3f6 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.h +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.h @@ -137,7 +137,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation auto copy{ winrt::make_self() }; copy->_SingleLine = _SingleLine; copy->_CopyFormatting = _CopyFormatting; - return copy.as(); + return *copy; } }; @@ -171,7 +171,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_TerminalArgs = _TerminalArgs.Copy(); - return copy.as(); + return *copy; } }; @@ -207,7 +207,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_TabIndex = _TabIndex; - return copy.as(); + return *copy; } }; @@ -248,7 +248,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Direction = _Direction; - return copy.as(); + return *copy; } }; @@ -289,7 +289,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Direction = _Direction; - return copy.as(); + return *copy; } }; @@ -323,7 +323,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Delta = _Delta; - return copy.as(); + return *copy; } }; @@ -360,7 +360,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Input = _Input; - return copy.as(); + return *copy; } }; @@ -407,7 +407,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation copy->_SplitStyle = _SplitStyle; copy->_TerminalArgs = _TerminalArgs.Copy(); copy->_SplitMode = _SplitMode; - return copy.as(); + return *copy; } }; @@ -441,7 +441,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Target = _Target; - return copy.as(); + return *copy; } }; @@ -479,7 +479,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_SchemeName = _SchemeName; - return copy.as(); + return *copy; } }; @@ -516,7 +516,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_TabColor = _TabColor; - return copy.as(); + return *copy; } }; @@ -550,7 +550,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Title = _Title; - return copy.as(); + return *copy; } }; @@ -590,7 +590,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Commandline = _Commandline; - return copy.as(); + return *copy; } }; @@ -624,7 +624,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Index = _Index; - return copy.as(); + return *copy; } }; @@ -658,7 +658,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Index = _Index; - return copy.as(); + return *copy; } }; } diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp index 9fa5136470a..d9f7dcd3ef1 100644 --- a/src/cascadia/TerminalSettingsModel/Command.cpp +++ b/src/cascadia/TerminalSettingsModel/Command.cpp @@ -33,8 +33,7 @@ static constexpr std::string_view SchemeNameToken{ "${scheme.name}" }; namespace winrt::Microsoft::Terminal::Settings::Model::implementation { - Command::Command() : - _subcommands{ winrt::single_threaded_map() } + Command::Command() { _setAction(nullptr); } @@ -49,22 +48,26 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation command->_IterateOn = _IterateOn; command->_originalJson = _originalJson; - for (auto kv : _subcommands) + if (HasNestedCommands()) { - const auto subcmd{ winrt::get_self(kv.Value()) }; - command->_subcommands.Insert(kv.Key(), *subcmd->Copy()); + command->_subcommands = winrt::single_threaded_map(); + for (auto kv : NestedCommands()) + { + const auto subCmd{ winrt::get_self(kv.Value()) }; + command->_subcommands.Insert(kv.Key(), *subCmd->Copy()); + } } return command; } - IMapView Command::NestedCommands() + IMapView Command::NestedCommands() const { - return _subcommands.Size() > 0 ? _subcommands.GetView() : nullptr; + return _subcommands ? _subcommands.GetView() : nullptr; } - bool Command::HasNestedCommands() + bool Command::HasNestedCommands() const { - return _subcommands.Size() > 0; + return _subcommands ? _subcommands.Size() > 0 : false; } // Function Description: // - attempt to get the name of this command from the provided json object. @@ -153,6 +156,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // will only be marked iterable on the first pass. if (const auto nestedCommandsJson{ json[JsonKey(CommandsKey)] }) { + result->_subcommands = winrt::single_threaded_map(); // Initialize our list of subcommands. auto nestedWarnings = Command::LayerJson(result->_subcommands, nestedCommandsJson); // It's possible that the nested commands have some warnings diff --git a/src/cascadia/TerminalSettingsModel/Command.h b/src/cascadia/TerminalSettingsModel/Command.h index 77ccb356e65..8cbd9e5fd0c 100644 --- a/src/cascadia/TerminalSettingsModel/Command.h +++ b/src/cascadia/TerminalSettingsModel/Command.h @@ -48,8 +48,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation static std::vector LayerJson(Windows::Foundation::Collections::IMap& commands, const Json::Value& json); - bool HasNestedCommands(); - Windows::Foundation::Collections::IMapView NestedCommands(); + bool HasNestedCommands() const; + Windows::Foundation::Collections::IMapView NestedCommands() const; winrt::Windows::UI::Xaml::Data::INotifyPropertyChanged::PropertyChanged_revoker propertyChangedRevoker; @@ -63,7 +63,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation private: Json::Value _originalJson; - Windows::Foundation::Collections::IMap _subcommands; + Windows::Foundation::Collections::IMap _subcommands{ nullptr }; static std::vector _expandCommand(Command* const expandable, Windows::Foundation::Collections::IVectorView profiles, diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp index f751bffd347..a4db069bfa7 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp @@ -94,7 +94,7 @@ winrt::com_ptr GlobalAppSettings::Copy() const globals->_unparsedDefaultProfile = _unparsedDefaultProfile; globals->_defaultProfile = _defaultProfile; globals->_keymap = _keymap->Copy(); - std::for_each(_keybindingsWarnings.begin(), _keybindingsWarnings.end(), [globals](auto& warning) { globals->_keybindingsWarnings.push_back(warning); }); + std::copy(_keybindingsWarnings.begin(), _keybindingsWarnings.end(), std::back_inserter(globals->_keybindingsWarnings)); for (auto kv : _colorSchemes) { From 8498b227c97c036044c15c5cdc64e27eafd11587 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 16 Oct 2020 14:34:27 -0700 Subject: [PATCH 4/4] move comment back --- src/cascadia/TerminalSettingsModel/Command.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp index d9f7dcd3ef1..1169101844f 100644 --- a/src/cascadia/TerminalSettingsModel/Command.cpp +++ b/src/cascadia/TerminalSettingsModel/Command.cpp @@ -156,8 +156,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // will only be marked iterable on the first pass. if (const auto nestedCommandsJson{ json[JsonKey(CommandsKey)] }) { - result->_subcommands = winrt::single_threaded_map(); // Initialize our list of subcommands. + result->_subcommands = winrt::single_threaded_map(); auto nestedWarnings = Command::LayerJson(result->_subcommands, nestedCommandsJson); // It's possible that the nested commands have some warnings warnings.insert(warnings.end(), nestedWarnings.begin(), nestedWarnings.end());