Add support for the "overline" graphic rendition attribute (#6754)

## Summary of the Pull Request This PR adds support for the `SGR 53` and `SGR 55` escapes sequences, which enable and disable the ANSI _overline_ graphic rendition attribute, the equivalent of the console character attribute `COMMON_LVB_GRID_HORIZONTAL`. When a character is output with this attribute set, a horizontal line is rendered at the top of the character cell. ## PR Checklist * [x] Closes #6000 * [x] CLA signed. * [x] Tests added/passed * [ ] Documentation updated. * [ ] Schema updated. * [x] I've discussed this with core contributors already. ## Detailed Description of the Pull Request / Additional comments To start with, I added `SetOverline` and `IsOverlined` methods to the `TextAttribute` class, to set and get the legacy `COMMON_LVB_GRID_HORIZONTAL` attribute. Technically there was already an `IsTopHorizontalDisplayed` method, but I thought it more readable to add a separate `IsOverlined` as an alias for that. Then it was just a matter of adding calls to set and reset the attribute in response to the `SGR 53` and `SGR 55` sequences in the `SetGraphicsRendition` methods of the two dispatchers. The actual rendering was already taken care of by the `PaintBufferGridLines` method in the rendering engines. The only other change required was to update the `_UpdateExtendedAttrs` method in the `Xterm256Engine` of the VT renderer, to ensure the attribute state would be forwarded to the Windows Terminal over conpty. ## Validation Steps Performed I've extended the existing SGR unit tests to cover the new attribute in the `AdapterTest`, the `OutputEngineTest`, and the `VtRendererTest`. I've also manually tested the `SGR 53` and `SGR 55` sequences to confirm that they do actually render (or remove) an overline on the characters being output.
microsoft · Jul 6, 2020 · 70a7ccc · 70a7ccc
1 parent 0651fcf
commit 70a7ccc
Show file tree

Hide file tree

Showing 12 changed files with 69 additions and 5 deletions.
diff --git a/.github/actions/spell-check/dictionary/dictionary.txt b/.github/actions/spell-check/dictionary/dictionary.txt
@@ -284908,6 +284908,7 @@ overliing
 overliking
 overlimit
 overline
+overlined
 overling
 overlinger
 overlinked

diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp
@@ -223,6 +223,11 @@ bool TextAttribute::IsUnderlined() const noexcept
     return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_UNDERSCORE);
 }
 
+bool TextAttribute::IsOverlined() const noexcept
+{
+    return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL);
+}
+
 bool TextAttribute::IsReverseVideo() const noexcept
 {
     return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO);
@@ -259,6 +264,11 @@ void TextAttribute::SetUnderline(bool isUnderlined) noexcept
     WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_UNDERSCORE, isUnderlined);
 }
 
+void TextAttribute::SetOverline(bool isOverlined) noexcept
+{
+    WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL, isOverlined);
+}
+
 void TextAttribute::SetReverseVideo(bool isReversed) noexcept
 {
     WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO, isReversed);

diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp
@@ -95,6 +95,7 @@ class TextAttribute final
     bool IsInvisible() const noexcept;
     bool IsCrossedOut() const noexcept;
     bool IsUnderlined() const noexcept;
+    bool IsOverlined() const noexcept;
     bool IsReverseVideo() const noexcept;
 
     void SetBold(bool isBold) noexcept;
@@ -103,6 +104,7 @@ class TextAttribute final
     void SetInvisible(bool isInvisible) noexcept;
     void SetCrossedOut(bool isCrossedOut) noexcept;
     void SetUnderline(bool isUnderlined) noexcept;
+    void SetOverline(bool isOverlined) noexcept;
     void SetReverseVideo(bool isReversed) noexcept;
 
     ExtendedAttributes GetExtendedAttributes() const noexcept;

diff --git a/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp b/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp
@@ -157,6 +157,12 @@ bool TerminalDispatch::SetGraphicsRendition(const std::basic_string_view<Dispatc
         case NoUnderline:
             attr.SetUnderline(false);
             break;
+        case Overline:
+            attr.SetOverline(true);
+            break;
+        case NoOverline:
+            attr.SetOverline(false);
+            break;
         case ForegroundBlack:
             attr.SetIndexedForeground(DARK_BLACK);
             break;

diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp
@@ -746,7 +746,7 @@ void VtRendererTest::Xterm256TestExtendedAttributes()
 void VtRendererTest::Xterm256TestAttributesAcrossReset()
 {
     BEGIN_TEST_METHOD_PROPERTIES()
-        TEST_METHOD_PROPERTY(L"Data:renditionAttribute", L"{1, 3, 4, 5, 7, 8, 9}")
+        TEST_METHOD_PROPERTY(L"Data:renditionAttribute", L"{1, 3, 4, 5, 7, 8, 9, 53}")
     END_TEST_METHOD_PROPERTIES()
 
     int renditionAttribute;
@@ -782,6 +782,10 @@ void VtRendererTest::Xterm256TestAttributesAcrossReset()
         Log::Comment(L"----Set Underline Attribute----");
         textAttributes.SetUnderline(true);
         break;
+    case GraphicsOptions::Overline:
+        Log::Comment(L"----Set Overline Attribute----");
+        textAttributes.SetOverline(true);
+        break;
     case GraphicsOptions::BlinkOrXterm256Index:
         Log::Comment(L"----Set Blink Attribute----");
         textAttributes.SetBlinking(true);

diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp
@@ -355,6 +355,17 @@ using namespace Microsoft::Console::Render;
     return _Write(isUnderlined ? "\x1b[4m" : "\x1b[24m");
 }
 
+// Method Description:
+// - Formats and writes a sequence to change the overline of the following text.
+// Arguments:
+// - isOverlined: If true, we'll overline the text. Otherwise we'll remove the overline.
+// Return Value:
+// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
+[[nodiscard]] HRESULT VtEngine::_SetOverline(const bool isOverlined) noexcept
+{
+    return _Write(isOverlined ? "\x1b[53m" : "\x1b[55m");
+}
+
 // Method Description:
 // - Formats and writes a sequence to change the italics of the following text.
 // Arguments:

diff --git a/src/renderer/vt/Xterm256Engine.cpp b/src/renderer/vt/Xterm256Engine.cpp
@@ -53,6 +53,12 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe,
         _lastTextAttributes.SetUnderline(textAttributes.IsUnderlined());
     }
 
+    if (textAttributes.IsOverlined() != _lastTextAttributes.IsOverlined())
+    {
+        RETURN_IF_FAILED(_SetOverline(textAttributes.IsOverlined()));
+        _lastTextAttributes.SetOverline(textAttributes.IsOverlined());
+    }
+
     if (textAttributes.IsItalic() != _lastTextAttributes.IsItalic())
     {
         RETURN_IF_FAILED(_SetItalics(textAttributes.IsItalic()));

diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp
@@ -187,6 +187,7 @@ namespace Microsoft::Console::Render
 
         [[nodiscard]] HRESULT _SetBold(const bool isBold) noexcept;
         [[nodiscard]] HRESULT _SetUnderline(const bool isUnderlined) noexcept;
+        [[nodiscard]] HRESULT _SetOverline(const bool isUnderlined) noexcept;
         [[nodiscard]] HRESULT _SetItalics(const bool isItalic) noexcept;
         [[nodiscard]] HRESULT _SetBlinking(const bool isBlinking) noexcept;
         [[nodiscard]] HRESULT _SetInvisible(const bool isInvisible) noexcept;

diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp
@@ -55,6 +55,8 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes
         BackgroundWhite = 47,
         BackgroundExtended = 48,
         BackgroundDefault = 49,
+        Overline = 53,
+        NoOverline = 55,
         BrightForegroundBlack = 90,
         BrightForegroundRed = 91,
         BrightForegroundGreen = 92,

diff --git a/src/terminal/adapter/adaptDispatchGraphics.cpp b/src/terminal/adapter/adaptDispatchGraphics.cpp
@@ -166,6 +166,12 @@ bool AdaptDispatch::SetGraphicsRendition(const std::basic_string_view<DispatchTy
             case NoUnderline:
                 attr.SetUnderline(false);
                 break;
+            case Overline:
+                attr.SetOverline(true);
+                break;
+            case NoOverline:
+                attr.SetOverline(false);
+                break;
             case ForegroundBlack:
                 attr.SetIndexedForeground(DARK_BLACK);
                 break;

diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp
@@ -1261,7 +1261,7 @@ class AdapterTest
     TEST_METHOD(GraphicsSingleTests)
     {
         BEGIN_TEST_METHOD_PROPERTIES()
-            TEST_METHOD_PROPERTY(L"Data:uiGraphicsOptions", L"{0, 1, 4, 7, 24, 27, 30, 31, 32, 33, 34, 35, 36, 37, 39, 40, 41, 42, 43, 44, 45, 46, 47, 49, 90, 91, 92, 93, 94, 95, 96, 97, 100, 101, 102, 103, 104, 105, 106, 107}") // corresponds to options in DispatchTypes::GraphicsOptions
+            TEST_METHOD_PROPERTY(L"Data:uiGraphicsOptions", L"{0, 1, 4, 7, 24, 27, 30, 31, 32, 33, 34, 35, 36, 37, 39, 40, 41, 42, 43, 44, 45, 46, 47, 49, 53, 55, 90, 91, 92, 93, 94, 95, 96, 97, 100, 101, 102, 103, 104, 105, 106, 107}") // corresponds to options in DispatchTypes::GraphicsOptions
         END_TEST_METHOD_PROPERTIES()
 
         Log::Comment(L"Starting test...");
@@ -1295,6 +1295,11 @@ class AdapterTest
             _testGetSet->_attribute = TextAttribute{ 0 };
             _testGetSet->_expectedAttribute = TextAttribute{ COMMON_LVB_UNDERSCORE };
             break;
+        case DispatchTypes::GraphicsOptions::Overline:
+            Log::Comment(L"Testing graphics 'Overline'");
+            _testGetSet->_attribute = TextAttribute{ 0 };
+            _testGetSet->_expectedAttribute = TextAttribute{ COMMON_LVB_GRID_HORIZONTAL };
+            break;
         case DispatchTypes::GraphicsOptions::Negative:
             Log::Comment(L"Testing graphics 'Negative'");
             _testGetSet->_attribute = TextAttribute{ 0 };
@@ -1305,6 +1310,11 @@ class AdapterTest
             _testGetSet->_attribute = TextAttribute{ COMMON_LVB_UNDERSCORE };
             _testGetSet->_expectedAttribute = TextAttribute{ 0 };
             break;
+        case DispatchTypes::GraphicsOptions::NoOverline:
+            Log::Comment(L"Testing graphics 'No Overline'");
+            _testGetSet->_attribute = TextAttribute{ COMMON_LVB_GRID_HORIZONTAL };
+            _testGetSet->_expectedAttribute = TextAttribute{ 0 };
+            break;
         case DispatchTypes::GraphicsOptions::Positive:
             Log::Comment(L"Testing graphics 'Positive'");
             _testGetSet->_attribute = TextAttribute{ COMMON_LVB_REVERSE_VIDEO };

diff --git a/src/terminal/parser/ut_parser/OutputEngineTest.cpp b/src/terminal/parser/ut_parser/OutputEngineTest.cpp
@@ -1602,6 +1602,9 @@ class StateMachineExternalTest final
         mach.ProcessCharacter(L';');
         mach.ProcessCharacter(L'4');
         mach.ProcessCharacter(L'5');
+        mach.ProcessCharacter(L';');
+        mach.ProcessCharacter(L'5');
+        mach.ProcessCharacter(L'3');
         mach.ProcessCharacter(L'm');
         VERIFY_IS_TRUE(pDispatch->_setGraphics);
 
@@ -1610,7 +1613,8 @@ class StateMachineExternalTest final
         rgExpected[2] = DispatchTypes::GraphicsOptions::Negative;
         rgExpected[3] = DispatchTypes::GraphicsOptions::ForegroundBlack;
         rgExpected[4] = DispatchTypes::GraphicsOptions::BackgroundMagenta;
-        VerifyDispatchTypes({ rgExpected, 5 }, *pDispatch);
+        rgExpected[5] = DispatchTypes::GraphicsOptions::Overline;
+        VerifyDispatchTypes({ rgExpected, 6 }, *pDispatch);
 
         pDispatch->ClearState();
 
@@ -1822,7 +1826,7 @@ class StateMachineExternalTest final
 
         Log::Comment(L"Test 2: A couple of sequences all in one string");
 
-        mach.ProcessString(L"\x1b[1;4;7;30;45m\x1b[2J");
+        mach.ProcessString(L"\x1b[1;4;7;30;45;53m\x1b[2J");
         VERIFY_IS_TRUE(pDispatch->_setGraphics);
         VERIFY_IS_TRUE(pDispatch->_eraseDisplay);
 
@@ -1831,8 +1835,9 @@ class StateMachineExternalTest final
         rgExpected[2] = DispatchTypes::GraphicsOptions::Negative;
         rgExpected[3] = DispatchTypes::GraphicsOptions::ForegroundBlack;
         rgExpected[4] = DispatchTypes::GraphicsOptions::BackgroundMagenta;
+        rgExpected[5] = DispatchTypes::GraphicsOptions::Overline;
         expectedDispatchTypes = DispatchTypes::EraseType::All;
-        VerifyDispatchTypes({ rgExpected, 5 }, *pDispatch);
+        VerifyDispatchTypes({ rgExpected, 6 }, *pDispatch);
         VERIFY_ARE_EQUAL(expectedDispatchTypes, pDispatch->_eraseType);
 
         pDispatch->ClearState();