From 9eeea4a9d07e0f399ebb82f32ba99b7d3c7fbab4 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 10 Nov 2021 11:32:31 -0600 Subject: [PATCH] The simple PR feedback --- src/cascadia/TerminalApp/TerminalPage.cpp | 5 +++-- .../TerminalControl/ControlAppearance.h | 3 +-- src/cascadia/TerminalControl/ControlCore.h | 2 +- src/cascadia/TerminalControl/TermControl.h | 1 - src/cascadia/TerminalCore/ICoreAppearance.idl | 8 +++---- .../TerminalSettings.cpp | 21 ------------------- .../TerminalSettingsModel/TerminalSettings.h | 4 ---- .../TerminalSettings.idl | 4 +++- 8 files changed, 12 insertions(+), 36 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index a155a51a8..fa443d616 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -2126,8 +2126,9 @@ namespace winrt::TerminalApp::implementation TermControl TerminalPage::_InitControl(const TerminalSettingsCreateResult& settings, const ITerminalConnection& connection) { - // Give term control a child of the settings so that any overrides go in the child - // This way, when we do a settings reload we just update the parent and the overrides remain + // Do any initialization that needs to apply to _every_ TermControl we + // create here. + // TermControl will copy the settings out of the settings passed to it. TermControl term{ settings.DefaultSettings(), settings.UnfocusedSettings(), connection }; return term; } diff --git a/src/cascadia/TerminalControl/ControlAppearance.h b/src/cascadia/TerminalControl/ControlAppearance.h index 3356a0238..d39eb2578 100644 --- a/src/cascadia/TerminalControl/ControlAppearance.h +++ b/src/cascadia/TerminalControl/ControlAppearance.h @@ -29,8 +29,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation public: winrt::Microsoft::Terminal::Core::Color GetColorTableEntry(int32_t index) noexcept { - return _runtimeColorTable.at(index) ? *_runtimeColorTable.at(index) : - _ColorTable.at(index); + return til::coalesce_value(_runtimeColorTable.at(index), _ColorTable.at(index)); } void SetColorTableEntry(int32_t index, winrt::Microsoft::Terminal::Core::Color color) noexcept diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index c5f1fa694..fc9d6afeb 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -39,7 +39,7 @@ private: \ void name(const type newValue) { _runtime##name = newValue; } \ \ public: \ - type name() const { return _runtime##name ? *_runtime##name : setting; } + type name() const { return til::coalesce_value(_runtime##name, setting); } namespace winrt::Microsoft::Terminal::Control::implementation { diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 3d2dd025a..6afb9c61f 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -15,7 +15,6 @@ #include "SearchBoxControl.h" #include "ControlInteractivity.h" -#include "ControlSettings.h" namespace Microsoft::Console::VirtualTerminal { diff --git a/src/cascadia/TerminalCore/ICoreAppearance.idl b/src/cascadia/TerminalCore/ICoreAppearance.idl index f58e9a490..30813644f 100644 --- a/src/cascadia/TerminalCore/ICoreAppearance.idl +++ b/src/cascadia/TerminalCore/ICoreAppearance.idl @@ -59,10 +59,10 @@ namespace Microsoft.Terminal.Core Microsoft.Terminal.Core.Color CursorColor; - // Table: A WinRT struct doesn't allow pointers (like an array) in - // structs, but we very much would like this object to be a struct. So - // we'll call out each color individually. There's only 16, it's not - // that bad. + // Table: A WinRT struct doesn't allow pointers (READ: doesn't allow + // array members) in structs, but we very much would like this object to + // be a struct. So we'll call out each color individually. There's only + // 16, it's not that bad. Microsoft.Terminal.Core.Color Black; Microsoft.Terminal.Core.Color Red; Microsoft.Terminal.Core.Color Green; diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp b/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp index d345ef6cf..9528b0870 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/TerminalSettings.cpp @@ -206,27 +206,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation _Opacity = appearance.Opacity(); } - // Method Description: - // - Sets our parent to the provided TerminalSettings - // Arguments: - // - parent: our new parent - void TerminalSettings::SetParent(const Model::TerminalSettings& parent) - { - ClearParents(); - com_ptr parentImpl; - parentImpl.copy_from(get_self(parent)); - InsertParent(parentImpl); - } - - Model::TerminalSettings TerminalSettings::GetParent() - { - if (_parents.size() > 0) - { - return *_parents.at(0); - } - return nullptr; - } - // Method Description: // - Apply Profile settings, as well as any colors from our color scheme, if we have one. // Arguments: diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettings.h b/src/cascadia/TerminalSettingsModel/TerminalSettings.h index f35f0d6a5..46cc62f84 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettings.h +++ b/src/cascadia/TerminalSettingsModel/TerminalSettings.h @@ -64,10 +64,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation const Model::NewTerminalArgs& newTerminalArgs, const Control::IKeyBindings& keybindings); - Model::TerminalSettings GetParent(); - - void SetParent(const Model::TerminalSettings& parent); - void ApplyColorScheme(const Model::ColorScheme& scheme); // --------------------------- Core Settings --------------------------- diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettings.idl b/src/cascadia/TerminalSettingsModel/TerminalSettings.idl index d58ae24f1..950932130 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettings.idl +++ b/src/cascadia/TerminalSettingsModel/TerminalSettings.idl @@ -29,12 +29,14 @@ namespace Microsoft.Terminal.Settings.Model static TerminalSettingsCreateResult CreateWithProfile(CascadiaSettings appSettings, Profile profile, Microsoft.Terminal.Control.IKeyBindings keybindings); static TerminalSettingsCreateResult CreateWithNewTerminalArgs(CascadiaSettings appSettings, NewTerminalArgs newTerminalArgs, Microsoft.Terminal.Control.IKeyBindings keybindings); - void SetParent(TerminalSettings parent); TerminalSettings GetParent(); void ApplyColorScheme(ColorScheme scheme); ColorScheme AppliedColorScheme; + // The getters for these are already defined in IControlSettings. So + // we're just adding the setters here, because TerminalApp likes to be + // able to change these at runtime (e.g. when duplicating a pane). String Commandline { set; }; String StartingDirectory { set; }; String EnvironmentVariables { set; };