Change AdjustIndistinguishableColors to an enum setting instead of a …

…boolean setting (#13512) ## Summary of the Pull Request `AdjustIndistinguishableColors` can now be set to: - `Never`: Never adjust the colors - `Indexed`: Only adjust colors that are part of the color scheme - `Always`: Always adjust the colors ## References #13343 ## PR Checklist * [ ] Closes #xxx * [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA * [ ] Tests added/passed * [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx * [ ] Schema updated. * [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx ## Detailed Description of the Pull Request / Additional comments For legacy purposes, `true` and `false` map to `Indexed` and `Never` respectively ## Validation Steps Performed Setting still works
microsoft · Jul 22, 2022 · d3ae00e · d3ae00e
1 parent 5ea25f1
commit d3ae00e
Show file tree

Hide file tree

Showing 19 changed files with 118 additions and 17 deletions.
diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json
@@ -207,8 +207,14 @@
           "type": "string"
         },
         "adjustIndistinguishableColors": {
-          "description": "When set to true, we will (when necessary) adjust the foreground color to make it more visible, based on the background color.",
-          "type": "boolean"
+          "default": "never",
+          "description": "Setting to adjust the foreground color to make it more visible, based on the background color. When set to \"indexed\", we will only adjust the colors if they came from the color scheme. Other possible values are \"never\" and \"always\".",
+          "enum": [
+            "never",
+            "indexed",
+            "always"
+          ],
+          "type": "string"
         },
         "experimental.retroTerminalEffect": {
           "description": "When set to true, enable retro terminal effects when unfocused. This is an experimental feature, and its continued existence is not guaranteed.",
@@ -2276,8 +2282,14 @@
           ]
         },
         "adjustIndistinguishableColors": {
-          "description": "When set to true, we will (when necessary) adjust the foreground color to make it more visible, based on the background color.",
-          "type": "boolean"
+          "default": "never",
+          "description": "Setting to adjust the foreground color to make it more visible, based on the background color. When set to \"indexed\", we will only adjust the colors if they came from the color scheme. Other possible values are \"never\" and \"always\".",
+          "enum": [
+            "never",
+            "indexed",
+            "always"
+          ],
+          "type": "string"
         },
         "scrollbarState": {
           "default": "visible",

diff --git a/src/cascadia/TerminalCore/ICoreAppearance.idl b/src/cascadia/TerminalCore/ICoreAppearance.idl
@@ -13,6 +13,13 @@ namespace Microsoft.Terminal.Core
         EmptyBox
     };
 
+    enum AdjustTextMode
+    {
+        Never,
+        Indexed,
+        Always
+    };
+
     // TerminalCore declares its own Color struct to avoid depending
     // on Windows.UI.Color and to avoid passing around unclothed uint32s.
     // It is supported by til::color for conversions in and out of WinRT land.
@@ -108,6 +115,6 @@ namespace Microsoft.Terminal.Core
         UInt32 CursorHeight;
         Boolean IntenseIsBold;
         Boolean IntenseIsBright;
-        Boolean AdjustIndistinguishableColors;
+        AdjustTextMode AdjustIndistinguishableColors;
     };
 }
diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp
@@ -168,7 +168,22 @@ void Terminal::UpdateAppearance(const ICoreAppearance& appearance)
 {
     _renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBold, appearance.IntenseIsBold());
     _renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, appearance.IntenseIsBright());
-    _renderSettings.SetRenderMode(RenderSettings::Mode::DistinguishableColors, appearance.AdjustIndistinguishableColors());
+
+    switch (appearance.AdjustIndistinguishableColors())
+    {
+    case AdjustTextMode::Always:
+        _renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, false);
+        _renderSettings.SetRenderMode(RenderSettings::Mode::AlwaysDistinguishableColors, true);
+        break;
+    case AdjustTextMode::Indexed:
+        _renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, true);
+        _renderSettings.SetRenderMode(RenderSettings::Mode::AlwaysDistinguishableColors, false);
+        break;
+    case AdjustTextMode::Never:
+        _renderSettings.SetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors, false);
+        _renderSettings.SetRenderMode(RenderSettings::Mode::AlwaysDistinguishableColors, false);
+        break;
+    }
 
     const til::color newBackgroundColor{ appearance.DefaultBackground() };
     _renderSettings.SetColorAlias(ColorAlias::DefaultBackground, TextColor::DEFAULT_BACKGROUND, newBackgroundColor);

diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.cpp b/src/cascadia/TerminalSettingsEditor/Appearances.cpp
@@ -92,6 +92,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         InitializeComponent();
 
         INITIALIZE_BINDABLE_ENUM_SETTING(CursorShape, CursorStyle, winrt::Microsoft::Terminal::Core::CursorStyle, L"Profile_CursorShape", L"Content");
+        INITIALIZE_BINDABLE_ENUM_SETTING(AdjustIndistinguishableColors, AdjustIndistinguishableColors, winrt::Microsoft::Terminal::Core::AdjustTextMode, L"Profile_AdjustIndistinguishableColors", L"Content");
         INITIALIZE_BINDABLE_ENUM_SETTING_REVERSE_ORDER(BackgroundImageStretchMode, BackgroundImageStretchMode, winrt::Windows::UI::Xaml::Media::Stretch, L"Profile_BackgroundImageStretchMode", L"Content");
 
         // manually add Custom FontWeight option. Don't add it to the Map
@@ -268,6 +269,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
                 {
                     _PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentIntenseTextStyle" });
                 }
+                else if (settingName == L"AdjustIndistinguishableColors")
+                {
+                    _PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentAdjustIndistinguishableColors" });
+                }
                 // YOU THERE ADDING A NEW APPEARANCE SETTING
                 // Make sure you add a block like
                 //
@@ -298,6 +303,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             _PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"ShowAllFonts" });
             _PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"UsingMonospaceFont" });
             _PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentIntenseTextStyle" });
+            _PropertyChangedHandlers(*this, PropertyChangedEventArgs{ L"CurrentAdjustIndistinguishableColors" });
         }
     }
 

diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.h b/src/cascadia/TerminalSettingsEditor/Appearances.h
@@ -132,6 +132,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         WINRT_PROPERTY(Windows::Foundation::Collections::IObservableVector<Microsoft::Terminal::Settings::Editor::EnumEntry>, FontWeightList);
 
         GETSET_BINDABLE_ENUM_SETTING(CursorShape, Microsoft::Terminal::Core::CursorStyle, Appearance().CursorShape);
+        GETSET_BINDABLE_ENUM_SETTING(AdjustIndistinguishableColors, Microsoft::Terminal::Core::AdjustTextMode, Appearance().AdjustIndistinguishableColors);
         WINRT_PROPERTY(Windows::Foundation::Collections::IObservableVector<Model::ColorScheme>, ColorSchemeList, nullptr);
 
         WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);

diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.idl b/src/cascadia/TerminalSettingsEditor/Appearances.idl
@@ -46,7 +46,7 @@ namespace Microsoft.Terminal.Settings.Editor
         OBSERVABLE_PROJECTED_APPEARANCE_SETTING(Windows.UI.Xaml.Media.Stretch, BackgroundImageStretchMode);
         OBSERVABLE_PROJECTED_APPEARANCE_SETTING(Microsoft.Terminal.Settings.Model.ConvergedAlignment, BackgroundImageAlignment);
         OBSERVABLE_PROJECTED_APPEARANCE_SETTING(Microsoft.Terminal.Settings.Model.IntenseStyle, IntenseTextStyle);
-        OBSERVABLE_PROJECTED_APPEARANCE_SETTING(Boolean, AdjustIndistinguishableColors);
+        OBSERVABLE_PROJECTED_APPEARANCE_SETTING(Microsoft.Terminal.Core.AdjustTextMode, AdjustIndistinguishableColors);
     }
 
     [default_interface] runtimeclass Appearances : Windows.UI.Xaml.Controls.UserControl, Windows.UI.Xaml.Data.INotifyPropertyChanged
@@ -65,6 +65,9 @@ namespace Microsoft.Terminal.Settings.Editor
         Boolean IsVintageCursor { get; };
         Windows.Foundation.Collections.IObservableVector<Microsoft.Terminal.Settings.Editor.EnumEntry> CursorShapeList { get; };
 
+        IInspectable CurrentAdjustIndistinguishableColors;
+        Windows.Foundation.Collections.IObservableVector<Microsoft.Terminal.Settings.Editor.EnumEntry> AdjustIndistinguishableColorsList { get; };
+
         Microsoft.Terminal.Settings.Model.ColorScheme CurrentColorScheme;
         Windows.Foundation.Collections.IObservableVector<Microsoft.Terminal.Settings.Model.ColorScheme> ColorSchemeList { get; };
 

diff --git a/src/cascadia/TerminalSettingsEditor/Appearances.xaml b/src/cascadia/TerminalSettingsEditor/Appearances.xaml
@@ -157,8 +157,11 @@
                                     HasSettingValue="{x:Bind Appearance.HasAdjustIndistinguishableColors, Mode=OneWay}"
                                     SettingOverrideSource="{x:Bind Appearance.AdjustIndistinguishableColorsOverrideSource, Mode=OneWay}"
                                     Visibility="{x:Bind ShowIndistinguishableColorsItem}">
-                <ToggleSwitch IsOn="{x:Bind Appearance.AdjustIndistinguishableColors, Mode=TwoWay}"
-                              Style="{StaticResource ToggleSwitchInExpanderStyle}" />
+                <ComboBox AutomationProperties.AccessibilityView="Content"
+                          ItemTemplate="{StaticResource EnumComboBoxTemplate}"
+                          ItemsSource="{x:Bind AdjustIndistinguishableColorsList, Mode=OneWay}"
+                          SelectedItem="{x:Bind CurrentAdjustIndistinguishableColors, Mode=TwoWay}"
+                          Style="{StaticResource ComboBoxSettingStyle}" />
             </local:SettingContainer>
         </StackPanel>
 

diff --git a/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw
@@ -758,6 +758,18 @@
     <value>Cursor shape</value>
     <comment>Header for a control to select the shape of the text cursor.</comment>
   </data>
+  <data name="Profile_AdjustIndistinguishableColorsNever.Content" xml:space="preserve">
+    <value>Never</value>
+    <comment>An option to choose from for the "adjust indistinguishable colors" setting. When selected, we will never adjust the text colors.</comment>
+  </data>
+  <data name="Profile_AdjustIndistinguishableColorsIndexed.Content" xml:space="preserve">
+    <value>Only for colors in the color scheme</value>
+    <comment>An option to choose from for the "adjust indistinguishable colors" setting. When selected, we will adjust the text colors for visibility only when the colors are part of this profile's color scheme's color table.</comment>
+  </data>
+  <data name="Profile_AdjustIndistinguishableColorsAlways.Content" xml:space="preserve">
+    <value>Always (More performance intensive)</value>
+    <comment>An option to choose from for the "adjust indistinguishable colors" setting. When selected, we will adjust the text colors for visibility.</comment>
+  </data>
   <data name="Profile_CursorShapeBar.Content" xml:space="preserve">
     <value>Bar ( ┃ )</value>
     <comment>{Locked="┃"} An option to choose from for the "cursor shape" setting. When selected, the cursor will look like a vertical bar. The character in the parentheses is used to show what it looks like.</comment>

diff --git a/src/cascadia/TerminalSettingsModel/EnumMappings.cpp b/src/cascadia/TerminalSettingsModel/EnumMappings.cpp
@@ -46,6 +46,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     DEFINE_ENUM_MAP(Microsoft::Terminal::Control::TextAntialiasingMode, TextAntialiasingMode);
     DEFINE_ENUM_MAP(Microsoft::Terminal::Core::CursorStyle, CursorStyle);
     DEFINE_ENUM_MAP(Microsoft::Terminal::Settings::Model::IntenseStyle, IntenseTextStyle);
+    DEFINE_ENUM_MAP(Microsoft::Terminal::Core::AdjustTextMode, AdjustIndistinguishableColors);
 
     // FontWeight is special because the JsonUtils::ConversionTrait for it
     // creates a FontWeight object, but we need to use the uint16_t value.

diff --git a/src/cascadia/TerminalSettingsModel/EnumMappings.h b/src/cascadia/TerminalSettingsModel/EnumMappings.h
@@ -43,6 +43,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         static winrt::Windows::Foundation::Collections::IMap<winrt::hstring, winrt::Microsoft::Terminal::Core::CursorStyle> CursorStyle();
         static winrt::Windows::Foundation::Collections::IMap<winrt::hstring, uint16_t> FontWeight();
         static winrt::Windows::Foundation::Collections::IMap<winrt::hstring, winrt::Microsoft::Terminal::Settings::Model::IntenseStyle> IntenseTextStyle();
+        static winrt::Windows::Foundation::Collections::IMap<winrt::hstring, winrt::Microsoft::Terminal::Core::AdjustTextMode> AdjustIndistinguishableColors();
     };
 }
 

diff --git a/src/cascadia/TerminalSettingsModel/EnumMappings.idl b/src/cascadia/TerminalSettingsModel/EnumMappings.idl
@@ -23,6 +23,7 @@ namespace Microsoft.Terminal.Settings.Model
         static Windows.Foundation.Collections.IMap<String, Windows.UI.Xaml.Media.Stretch> BackgroundImageStretchMode { get; };
         static Windows.Foundation.Collections.IMap<String, Microsoft.Terminal.Control.TextAntialiasingMode> TextAntialiasingMode { get; };
         static Windows.Foundation.Collections.IMap<String, Microsoft.Terminal.Core.CursorStyle> CursorStyle { get; };
+        static Windows.Foundation.Collections.IMap<String, Microsoft.Terminal.Core.AdjustTextMode> AdjustIndistinguishableColors { get; };
         static Windows.Foundation.Collections.IMap<String, UInt16> FontWeight { get; };
         static Windows.Foundation.Collections.IMap<String, Microsoft.Terminal.Settings.Model.IntenseStyle> IntenseTextStyle { get; };
     }

diff --git a/src/cascadia/TerminalSettingsModel/IAppearanceConfig.idl b/src/cascadia/TerminalSettingsModel/IAppearanceConfig.idl
@@ -51,7 +51,7 @@ namespace Microsoft.Terminal.Settings.Model
         INHERITABLE_APPEARANCE_SETTING(Boolean, RetroTerminalEffect);
         INHERITABLE_APPEARANCE_SETTING(String, PixelShaderPath);
         INHERITABLE_APPEARANCE_SETTING(IntenseStyle, IntenseTextStyle);
-        INHERITABLE_APPEARANCE_SETTING(Boolean, AdjustIndistinguishableColors);
+        INHERITABLE_APPEARANCE_SETTING(Microsoft.Terminal.Core.AdjustTextMode, AdjustIndistinguishableColors);
         INHERITABLE_APPEARANCE_SETTING(Double, Opacity);
     };
 }
diff --git a/src/cascadia/TerminalSettingsModel/MTSMSettings.h b/src/cascadia/TerminalSettingsModel/MTSMSettings.h
@@ -111,7 +111,7 @@ Author(s):
     X(hstring, ColorSchemeName, "colorScheme", L"Campbell")                                                                                                        \
     X(hstring, BackgroundImagePath, "backgroundImage")                                                                                                             \
     X(Model::IntenseStyle, IntenseTextStyle, "intenseTextStyle", Model::IntenseStyle::Bright)                                                                      \
-    X(bool, AdjustIndistinguishableColors, "adjustIndistinguishableColors", false)
+    X(Core::AdjustTextMode, AdjustIndistinguishableColors, "adjustIndistinguishableColors", Core::AdjustTextMode::Never)
 
 // Intentionally omitted Appearance settings:
 // * ForegroundKey, BackgroundKey, SelectionBackgroundKey, CursorColorKey: all optional colors

diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettings.h b/src/cascadia/TerminalSettingsModel/TerminalSettings.h
@@ -110,7 +110,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         INHERITABLE_SETTING(Model::TerminalSettings, bool, IntenseIsBold);
         INHERITABLE_SETTING(Model::TerminalSettings, bool, IntenseIsBright);
 
-        INHERITABLE_SETTING(Model::TerminalSettings, bool, AdjustIndistinguishableColors);
+        INHERITABLE_SETTING(Model::TerminalSettings, Microsoft::Terminal::Core::AdjustTextMode, AdjustIndistinguishableColors, Core::AdjustTextMode::Never);
 
         // ------------------------ End of Core Settings -----------------------
 

diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h
@@ -30,6 +30,34 @@ JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Core::CursorStyle)
     };
 };
 
+// Type Description:
+// - Helper for converting a user-specified adjustTextMode value to its corresponding enum
+JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Core::AdjustTextMode)
+{
+    JSON_MAPPINGS(3) = {
+        pair_type{ "never", ValueType::Never },
+        pair_type{ "indexed", ValueType::Indexed },
+        pair_type{ "always", ValueType::Always },
+    };
+
+    // Override mapping parser to add boolean parsing
+    ::winrt::Microsoft::Terminal::Core::AdjustTextMode FromJson(const Json::Value& json)
+    {
+        if (json.isBool())
+        {
+            return json.asBool() ? ValueType::Indexed : ValueType::Never;
+        }
+        return EnumMapper::FromJson(json);
+    }
+
+    bool CanConvert(const Json::Value& json)
+    {
+        return EnumMapper::CanConvert(json) || json.isBool();
+    }
+
+    using EnumMapper::TypeDescription;
+};
+
 JSON_ENUM_MAPPER(::winrt::Windows::UI::Xaml::Media::Stretch)
 {
     static constexpr std::array<pair_type, 4> mappings = {

diff --git a/src/cascadia/inc/ControlProperties.h b/src/cascadia/inc/ControlProperties.h
@@ -13,7 +13,7 @@
     X(uint32_t, CursorHeight, DEFAULT_CURSOR_HEIGHT)                                                                      \
     X(bool, IntenseIsBold)                                                                                                \
     X(bool, IntenseIsBright, true)                                                                                        \
-    X(bool, AdjustIndistinguishableColors, true)
+    X(winrt::Microsoft::Terminal::Core::AdjustTextMode, AdjustIndistinguishableColors, winrt::Microsoft::Terminal::Core::AdjustTextMode::Never)
 
 // --------------------------- Control Appearance ---------------------------
 //  All of these settings are defined in IControlSettings.

diff --git a/src/renderer/base/RenderSettings.cpp b/src/renderer/base/RenderSettings.cpp
@@ -199,7 +199,7 @@ std::pair<COLORREF, COLORREF> RenderSettings::GetAttributeColors(const TextAttri
     // We want to nudge the foreground color to make it more perceivable only for the
     // default color pairs within the color table
     if (Feature_AdjustIndistinguishableText::IsEnabled() &&
-        GetRenderMode(Mode::DistinguishableColors) &&
+        GetRenderMode(Mode::IndexedDistinguishableColors) &&
         !dimFg &&
         !attr.IsInvisible() &&
         (fgTextColor.IsDefault() || fgTextColor.IsLegacy()) &&
@@ -252,6 +252,16 @@ std::pair<COLORREF, COLORREF> RenderSettings::GetAttributeColors(const TextAttri
             fg = bg;
         }
 
+        // We intentionally aren't _only_ checking for attr.IsInvisible here, because we also want to
+        // catch the cases where the fg was intentionally set to be the same as the bg. In either case,
+        // don't adjust the foreground.
+        if (Feature_AdjustIndistinguishableText::IsEnabled() &&
+            fg != bg &&
+            GetRenderMode(Mode::AlwaysDistinguishableColors))
+        {
+            fg = ColorFix::GetPerceivableColor(fg, bg);
+        }
+
         return { fg, bg };
     }
 }

diff --git a/src/renderer/inc/RenderSettings.hpp b/src/renderer/inc/RenderSettings.hpp
@@ -21,7 +21,8 @@ namespace Microsoft::Console::Render
         enum class Mode : size_t
         {
             BlinkAllowed,
-            DistinguishableColors,
+            IndexedDistinguishableColors,
+            AlwaysDistinguishableColors,
             IntenseIsBold,
             IntenseIsBright,
             ScreenReversed

diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp
@@ -2221,7 +2221,7 @@ bool AdaptDispatch::SetClipboard(const std::wstring_view content)
 bool AdaptDispatch::SetColorTableEntry(const size_t tableIndex, const DWORD dwColor)
 {
     _renderSettings.SetColorTableEntry(tableIndex, dwColor);
-    if (_renderSettings.GetRenderMode(RenderSettings::Mode::DistinguishableColors))
+    if (_renderSettings.GetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors))
     {
         // Re-calculate the adjusted colors now that one of the entries has been changed
         _renderSettings.MakeAdjustedColorArray();
@@ -2291,7 +2291,7 @@ bool AdaptDispatch::AssignColor(const DispatchTypes::ColorItem item, const VTInt
     case DispatchTypes::ColorItem::NormalText:
         _renderSettings.SetColorAliasIndex(ColorAlias::DefaultForeground, fgIndex);
         _renderSettings.SetColorAliasIndex(ColorAlias::DefaultBackground, bgIndex);
-        if (_renderSettings.GetRenderMode(RenderSettings::Mode::DistinguishableColors))
+        if (_renderSettings.GetRenderMode(RenderSettings::Mode::IndexedDistinguishableColors))
         {
             // Re-calculate the adjusted colors now that these aliases have been changed
             _renderSettings.MakeAdjustedColorArray();