microsoft/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2025-12-10 00:48:23 -06:00
Code Issues Packages Projects Releases 138 Wiki Activity

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

Browse Source 
## 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.
This commit is contained in:
James Holderness 2020-07-06 15:11:17 +01:00 committed by GitHub
parent 0651fcff14
commit 70a7ccc120
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 69 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.github/actions/spell-check/dictionary/dictionary.txt vendored
View File

@ -284908,6 +284908,7 @@ overliing
overliking
overlimit
overline
overlined
overling
overlinger
overlinked

10
src/buffer/out/TextAttribute.cpp
View File

@ -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);

2
src/buffer/out/TextAttribute.hpp
View File

@ -95,6 +95,7 @@ public:
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 @@ public:
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;

6
src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp
View File

@ -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;

6
src/host/ut_host/VtRendererTests.cpp
View File

@ -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);

11
src/renderer/vt/VtSequences.cpp
View File

@ -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:

6
src/renderer/vt/Xterm256Engine.cpp
View File

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

1
src/renderer/vt/vtrenderer.hpp
View File

@ -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;

2
src/terminal/adapter/DispatchTypes.hpp
View File

@ -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,

6
src/terminal/adapter/adaptDispatchGraphics.cpp
View File

@ -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;

12
src/terminal/adapter/ut_adapter/adapterTest.cpp
View File

@ -1261,7 +1261,7 @@ public:
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 @@ public:
_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 @@ public:
_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 };

11
src/terminal/parser/ut_parser/OutputEngineTest.cpp
View File

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