Tweaks: normalize TextAttribute method names (adjective form) (#6951)

## Summary of the Pull Request

Text can have various attributes, such as "bold", "italic", "underlined", etc.  The TextAttribute class embodies this. It has methods to set/query these attributes.

This change tweaks a few of the method names to make them match. I.e. for an imaginary text property "Foo", we should have methods along the lines of:

```
IsFoo
SetFoo(bool isFoo)
```

And variations should match: we should have "Foo" and "OverFoo", not "Fooey" and "OverFoo".

I chose to standardize on the adjective form, since that's what we are closest to already. The attributes I attacked here are:

SetItalic**s** --> SetItalic
SetUnderline --> SetUnderline**d**
SetOverline --> SetOverline**d**

("italic" is an adjective; "italics" is a plural noun, representing letters or words in an italic typeface)

And I also added methods for "DoublyUnderlined" for good measure.

I stopped short of renaming the GraphicsOptions enum values to match, too; but I'd be willing to do that in a follow-up change if people wanted it.

## Validation Steps Performed
It builds, and tests still pass.
This commit is contained in:
Dan Thompson 2020-07-17 08:50:23 -07:00 committed by GitHub
parent efb1fddb99
commit 1f8264d86b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 52 additions and 52 deletions

View file

@ -270,7 +270,7 @@ void TextAttribute::SetFaint(bool isFaint) noexcept
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Faint, isFaint);
}
void TextAttribute::SetItalics(bool isItalic) noexcept
void TextAttribute::SetItalic(bool isItalic) noexcept
{
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Italics, isItalic);
}
@ -290,13 +290,13 @@ void TextAttribute::SetCrossedOut(bool isCrossedOut) noexcept
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::CrossedOut, isCrossedOut);
}
void TextAttribute::SetUnderline(bool isUnderlined) noexcept
void TextAttribute::SetUnderlined(bool isUnderlined) noexcept
{
// TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE
WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_UNDERSCORE, isUnderlined);
}
void TextAttribute::SetOverline(bool isOverlined) noexcept
void TextAttribute::SetOverlined(bool isOverlined) noexcept
{
WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_GRID_HORIZONTAL, isOverlined);
}

View file

@ -100,12 +100,12 @@ public:
void SetBold(bool isBold) noexcept;
void SetFaint(bool isFaint) noexcept;
void SetItalics(bool isItalic) noexcept;
void SetItalic(bool isItalic) noexcept;
void SetBlinking(bool isBlinking) noexcept;
void SetInvisible(bool isInvisible) noexcept;
void SetCrossedOut(bool isCrossedOut) noexcept;
void SetUnderline(bool isUnderlined) noexcept;
void SetOverline(bool isOverlined) noexcept;
void SetUnderlined(bool isUnderlined) noexcept;
void SetOverlined(bool isOverlined) noexcept;
void SetReverseVideo(bool isReversed) noexcept;
ExtendedAttributes GetExtendedAttributes() const noexcept;
@ -218,11 +218,12 @@ namespace WEX
static WEX::Common::NoThrowString ToString(const TextAttribute& attr)
{
return WEX::Common::NoThrowString().Format(
L"{FG:%s,BG:%s,bold:%d,wLegacy:(0x%04x)}",
L"{FG:%s,BG:%s,bold:%d,wLegacy:(0x%04x),ext:(0x%02x)}",
VerifyOutputTraits<TextColor>::ToString(attr._foreground).GetBuffer(),
VerifyOutputTraits<TextColor>::ToString(attr._background).GetBuffer(),
attr.IsBold(),
attr._wAttrLegacy);
attr._wAttrLegacy,
static_cast<DWORD>(attr._extendedAttrs));
}
};
}

View file

@ -787,7 +787,7 @@ const Cursor& TextBuffer::GetCursor() const noexcept
return _currentAttributes;
}
void TextBuffer::SetCurrentAttributes(const TextAttribute currentAttributes) noexcept
void TextBuffer::SetCurrentAttributes(const TextAttribute& currentAttributes) noexcept
{
_currentAttributes = currentAttributes;
}

View file

@ -118,7 +118,7 @@ public:
[[nodiscard]] TextAttribute GetCurrentAttributes() const noexcept;
void SetCurrentAttributes(const TextAttribute currentAttributes) noexcept;
void SetCurrentAttributes(const TextAttribute& currentAttributes) noexcept;
void Reset();

View file

@ -126,10 +126,10 @@ bool TerminalDispatch::SetGraphicsRendition(const gsl::span<const DispatchTypes:
attr.SetFaint(false);
break;
case Italics:
attr.SetItalics(true);
attr.SetItalic(true);
break;
case NotItalics:
attr.SetItalics(false);
attr.SetItalic(false);
break;
case BlinkOrXterm256Index:
attr.SetBlinking(true);
@ -156,16 +156,16 @@ bool TerminalDispatch::SetGraphicsRendition(const gsl::span<const DispatchTypes:
attr.SetReverseVideo(false);
break;
case Underline:
attr.SetUnderline(true);
attr.SetUnderlined(true);
break;
case NoUnderline:
attr.SetUnderline(false);
attr.SetUnderlined(false);
break;
case Overline:
attr.SetOverline(true);
attr.SetOverlined(true);
break;
case NoOverline:
attr.SetOverline(false);
attr.SetOverlined(false);
break;
case ForegroundBlack:
attr.SetIndexedForeground(DARK_BLACK);

View file

@ -1403,7 +1403,7 @@ void ScreenBufferTests::VtNewlinePastViewport()
auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) };
fillAttr.SetCrossedOut(true);
fillAttr.SetReverseVideo(true);
fillAttr.SetUnderline(true);
fillAttr.SetUnderlined(true);
si.SetAttributes(fillAttr);
// But note that the meta attributes are expected to be cleared.
auto expectedFillAttr = fillAttr;
@ -1480,7 +1480,7 @@ void ScreenBufferTests::VtNewlinePastEndOfBuffer()
auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) };
fillAttr.SetCrossedOut(true);
fillAttr.SetReverseVideo(true);
fillAttr.SetUnderline(true);
fillAttr.SetUnderlined(true);
si.SetAttributes(fillAttr);
// But note that the meta attributes are expected to be cleared.
auto expectedFillAttr = fillAttr;
@ -3382,7 +3382,7 @@ void ScreenBufferTests::ScrollOperations()
auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) };
fillAttr.SetCrossedOut(true);
fillAttr.SetReverseVideo(true);
fillAttr.SetUnderline(true);
fillAttr.SetUnderlined(true);
si.SetAttributes(fillAttr);
// But note that the meta attributes are expected to be cleared.
auto expectedFillAttr = fillAttr;
@ -3503,7 +3503,7 @@ void ScreenBufferTests::InsertChars()
auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) };
fillAttr.SetCrossedOut(true);
fillAttr.SetReverseVideo(true);
fillAttr.SetUnderline(true);
fillAttr.SetUnderlined(true);
si.SetAttributes(fillAttr);
// But note that the meta attributes are expected to be cleared.
auto expectedFillAttr = fillAttr;
@ -3663,7 +3663,7 @@ void ScreenBufferTests::DeleteChars()
auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) };
fillAttr.SetCrossedOut(true);
fillAttr.SetReverseVideo(true);
fillAttr.SetUnderline(true);
fillAttr.SetUnderlined(true);
si.SetAttributes(fillAttr);
// But note that the meta attributes are expected to be cleared.
auto expectedFillAttr = fillAttr;
@ -3895,7 +3895,7 @@ void ScreenBufferTests::EraseTests()
auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) };
fillAttr.SetCrossedOut(true);
fillAttr.SetReverseVideo(true);
fillAttr.SetUnderline(true);
fillAttr.SetUnderlined(true);
si.SetAttributes(fillAttr);
// But note that the meta attributes are expected to be cleared.
auto expectedFillAttr = fillAttr;
@ -5206,7 +5206,7 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
}
if (italics)
{
expectedAttr.SetItalics(true);
expectedAttr.SetItalic(true);
vtSeq += L"\x1b[3m";
}
if (blink)
@ -5315,7 +5315,7 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
}
if (italics)
{
expectedAttr.SetItalics(false);
expectedAttr.SetItalic(false);
vtSeq = L"\x1b[23m";
validate(expectedAttr, vtSeq);
}
@ -5789,7 +5789,7 @@ void ScreenBufferTests::ScreenAlignmentPattern()
// Set the initial attributes.
auto initialAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) };
initialAttr.SetReverseVideo(true);
initialAttr.SetUnderline(true);
initialAttr.SetUnderlined(true);
si.SetAttributes(initialAttr);
// Set some margins.

View file

@ -686,7 +686,7 @@ void VtRendererTest::Xterm256TestExtendedAttributes()
}
if (italics)
{
desiredAttrs.SetItalics(true);
desiredAttrs.SetItalic(true);
onSequences.push_back("\x1b[3m");
offSequences.push_back("\x1b[23m");
}
@ -788,15 +788,15 @@ void VtRendererTest::Xterm256TestAttributesAcrossReset()
break;
case GraphicsOptions::Italics:
Log::Comment(L"----Set Italics Attribute----");
textAttributes.SetItalics(true);
textAttributes.SetItalic(true);
break;
case GraphicsOptions::Underline:
Log::Comment(L"----Set Underline Attribute----");
textAttributes.SetUnderline(true);
textAttributes.SetUnderlined(true);
break;
case GraphicsOptions::Overline:
Log::Comment(L"----Set Overline Attribute----");
textAttributes.SetOverline(true);
textAttributes.SetOverlined(true);
break;
case GraphicsOptions::BlinkOrXterm256Index:
Log::Comment(L"----Set Blink Attribute----");
@ -1289,7 +1289,7 @@ void VtRendererTest::XtermTestAttributesAcrossReset()
break;
case GraphicsOptions::Underline:
Log::Comment(L"----Set Underline Attribute----");
textAttributes.SetUnderline(true);
textAttributes.SetUnderlined(true);
break;
case GraphicsOptions::Negative:
Log::Comment(L"----Set Negative Attribute----");

View file

@ -361,7 +361,7 @@ using namespace Microsoft::Console::Render;
// - isUnderlined: If true, we'll underline the text. Otherwise we'll remove the underline.
// Return Value:
// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
[[nodiscard]] HRESULT VtEngine::_SetUnderline(const bool isUnderlined) noexcept
[[nodiscard]] HRESULT VtEngine::_SetUnderlined(const bool isUnderlined) noexcept
{
return _Write(isUnderlined ? "\x1b[4m" : "\x1b[24m");
}
@ -372,7 +372,7 @@ using namespace Microsoft::Console::Render;
// - 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
[[nodiscard]] HRESULT VtEngine::_SetOverlined(const bool isOverlined) noexcept
{
return _Write(isOverlined ? "\x1b[53m" : "\x1b[55m");
}
@ -383,7 +383,7 @@ using namespace Microsoft::Console::Render;
// - isItalic: If true, we'll italicize the text. Otherwise we'll remove the italics.
// Return Value:
// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write.
[[nodiscard]] HRESULT VtEngine::_SetItalics(const bool isItalic) noexcept
[[nodiscard]] HRESULT VtEngine::_SetItalic(const bool isItalic) noexcept
{
return _Write(isItalic ? "\x1b[3m" : "\x1b[23m");
}

View file

@ -67,20 +67,20 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe,
if (textAttributes.IsUnderlined() != _lastTextAttributes.IsUnderlined())
{
RETURN_IF_FAILED(_SetUnderline(textAttributes.IsUnderlined()));
_lastTextAttributes.SetUnderline(textAttributes.IsUnderlined());
RETURN_IF_FAILED(_SetUnderlined(textAttributes.IsUnderlined()));
_lastTextAttributes.SetUnderlined(textAttributes.IsUnderlined());
}
if (textAttributes.IsOverlined() != _lastTextAttributes.IsOverlined())
{
RETURN_IF_FAILED(_SetOverline(textAttributes.IsOverlined()));
_lastTextAttributes.SetOverline(textAttributes.IsOverlined());
RETURN_IF_FAILED(_SetOverlined(textAttributes.IsOverlined()));
_lastTextAttributes.SetOverlined(textAttributes.IsOverlined());
}
if (textAttributes.IsItalic() != _lastTextAttributes.IsItalic())
{
RETURN_IF_FAILED(_SetItalics(textAttributes.IsItalic()));
_lastTextAttributes.SetItalics(textAttributes.IsItalic());
RETURN_IF_FAILED(_SetItalic(textAttributes.IsItalic()));
_lastTextAttributes.SetItalic(textAttributes.IsItalic());
}
if (textAttributes.IsBlinking() != _lastTextAttributes.IsBlinking())

View file

@ -155,8 +155,8 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
}
if (textAttributes.IsUnderlined() != _lastTextAttributes.IsUnderlined())
{
RETURN_IF_FAILED(_SetUnderline(textAttributes.IsUnderlined()));
_lastTextAttributes.SetUnderline(textAttributes.IsUnderlined());
RETURN_IF_FAILED(_SetUnderlined(textAttributes.IsUnderlined()));
_lastTextAttributes.SetUnderlined(textAttributes.IsUnderlined());
}
return S_OK;

View file

@ -187,9 +187,9 @@ namespace Microsoft::Console::Render
[[nodiscard]] HRESULT _SetBold(const bool isBold) noexcept;
[[nodiscard]] HRESULT _SetFaint(const bool isFaint) noexcept;
[[nodiscard]] HRESULT _SetUnderline(const bool isUnderlined) noexcept;
[[nodiscard]] HRESULT _SetOverline(const bool isUnderlined) noexcept;
[[nodiscard]] HRESULT _SetItalics(const bool isItalic) noexcept;
[[nodiscard]] HRESULT _SetUnderlined(const bool isUnderlined) noexcept;
[[nodiscard]] HRESULT _SetOverlined(const bool isOverlined) noexcept;
[[nodiscard]] HRESULT _SetItalic(const bool isItalic) noexcept;
[[nodiscard]] HRESULT _SetBlinking(const bool isBlinking) noexcept;
[[nodiscard]] HRESULT _SetInvisible(const bool isInvisible) noexcept;
[[nodiscard]] HRESULT _SetCrossedOut(const bool isCrossedOut) noexcept;

View file

@ -135,10 +135,10 @@ bool AdaptDispatch::SetGraphicsRendition(const gsl::span<const DispatchTypes::Gr
attr.SetFaint(false);
break;
case Italics:
attr.SetItalics(true);
attr.SetItalic(true);
break;
case NotItalics:
attr.SetItalics(false);
attr.SetItalic(false);
break;
case BlinkOrXterm256Index:
attr.SetBlinking(true);
@ -165,16 +165,16 @@ bool AdaptDispatch::SetGraphicsRendition(const gsl::span<const DispatchTypes::Gr
attr.SetReverseVideo(false);
break;
case Underline:
attr.SetUnderline(true);
attr.SetUnderlined(true);
break;
case NoUnderline:
attr.SetUnderline(false);
attr.SetUnderlined(false);
break;
case Overline:
attr.SetOverline(true);
attr.SetOverlined(true);
break;
case NoOverline:
attr.SetOverline(false);
attr.SetOverlined(false);
break;
case ForegroundBlack:
attr.SetIndexedForeground(DARK_BLACK);

View file

@ -81,7 +81,7 @@ public:
VERIFY_ARE_EQUAL(_expectedCursorPos, sbiex.dwCursorPosition);
VERIFY_ARE_EQUAL(_expectedScreenBufferSize, sbiex.dwSize);
VERIFY_ARE_EQUAL(_expectedScreenBufferViewport, sbiex.srWindow);
VERIFY_ARE_EQUAL(_expectedAttributes, sbiex.wAttributes);
VERIFY_ARE_EQUAL(_expectedAttribute, TextAttribute{ sbiex.wAttributes });
}
return _setConsoleScreenBufferInfoExResult;
}
@ -764,7 +764,6 @@ public:
COORD _expectedScreenBufferSize = { 0, 0 };
SMALL_RECT _expectedScreenBufferViewport{ 0, 0, 0, 0 };
WORD _expectedAttributes = 0;
bool _privateSetCursorKeysModeResult = false;
bool _privateSetKeypadModeResult = false;
bool _cursorKeysApplicationMode = false;