Skip to content

Commit

Permalink
Rename the "Bold" SGR attribute as "Intense" (#12270)
Browse files Browse the repository at this point in the history
When we gave users the ability to configure how the `SGR 1` attribute
should be rendered, we described those options as "intense is bright"
and "intense is bold". Internally, though, we still referred to the `SGR 1`
attribute as bold. This PR renames all occurrences of "Bold" (when
referring to the `SGR 1` attribute) as "Intense", so the terminology is
more consistent now.

PR #10969 is where we decided on the wording to describe the `SGR 1`
attribute.

Specific changes include:
* `TextAttribute::IsBold` method renamed to `IsIntense`
* `TextAttribute::SetBold` method renamed to `SetIntense`
* `VtEngine::_SetBold` method renamed to `_SetIntense`
* `ExtendedAttributes::Bold` enum renamed to `Intense`
* `GraphicsOptions::BoldBright` enum renamed to `Intense`
* `GraphicsOptions::NotBoldOrFaint` enum renamed to `NotIntenseOrFaint`
* `SgrSaveRestoreStackOptions::Boldness` enum renamed to `Intense`

## Validation Steps Performed

I've checked that the code still compiles and the unit tests still run
successfully.

Closes #12252
  • Loading branch information
j4james committed Jan 27, 2022
1 parent a13b207 commit bcc38d0
Show file tree
Hide file tree
Showing 26 changed files with 188 additions and 187 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2508,6 +2508,7 @@ UNICRT
uninit
uninitialize
uninstall
unintense
Uniscribe
unittest
unittesting
Expand Down
12 changes: 6 additions & 6 deletions src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ TextAttribute TextAttribute::StripErroneousVT16VersionsOfLegacyDefaults(const Te
const auto bg{ attribute.GetBackground() };
auto copy{ attribute };
if (fg.IsIndex16() &&
attribute.IsBold() == WI_IsFlagSet(s_ansiDefaultForeground, FOREGROUND_INTENSITY) &&
attribute.IsIntense() == WI_IsFlagSet(s_ansiDefaultForeground, FOREGROUND_INTENSITY) &&
fg.GetIndex() == (s_ansiDefaultForeground & ~FOREGROUND_INTENSITY))
{
// We don't want to turn 1;37m into 39m (or even 1;39m), as this was meant to mimic a legacy color.
Expand All @@ -115,7 +115,7 @@ WORD TextAttribute::GetLegacyAttributes() const noexcept
const BYTE fgIndex = _foreground.GetLegacyIndex(s_legacyDefaultForeground);
const BYTE bgIndex = _background.GetLegacyIndex(s_legacyDefaultBackground);
const WORD metaAttrs = _wAttrLegacy & META_ATTRS;
const bool brighten = IsBold() && _foreground.CanBeBrightened();
const bool brighten = IsIntense() && _foreground.CanBeBrightened();
return fgIndex | (bgIndex << 4) | metaAttrs | (brighten ? FOREGROUND_INTENSITY : 0);
}

Expand Down Expand Up @@ -255,9 +255,9 @@ void TextAttribute::SetRightVerticalDisplayed(const bool isDisplayed) noexcept
WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_GRID_RVERTICAL, isDisplayed);
}

bool TextAttribute::IsBold() const noexcept
bool TextAttribute::IsIntense() const noexcept
{
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Bold);
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Intense);
}

bool TextAttribute::IsFaint() const noexcept
Expand Down Expand Up @@ -305,9 +305,9 @@ bool TextAttribute::IsReverseVideo() const noexcept
return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO);
}

void TextAttribute::SetBold(bool isBold) noexcept
void TextAttribute::SetIntense(bool isIntense) noexcept
{
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Bold, isBold);
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Intense, isIntense);
}

void TextAttribute::SetFaint(bool isFaint) noexcept
Expand Down
8 changes: 4 additions & 4 deletions src/buffer/out/TextAttribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class TextAttribute final
friend constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept;

bool IsLegacy() const noexcept;
bool IsBold() const noexcept;
bool IsIntense() const noexcept;
bool IsFaint() const noexcept;
bool IsItalic() const noexcept;
bool IsBlinking() const noexcept;
Expand All @@ -95,7 +95,7 @@ class TextAttribute final
bool IsOverlined() const noexcept;
bool IsReverseVideo() const noexcept;

void SetBold(bool isBold) noexcept;
void SetIntense(bool isIntense) noexcept;
void SetFaint(bool isFaint) noexcept;
void SetItalic(bool isItalic) noexcept;
void SetBlinking(bool isBlinking) noexcept;
Expand Down Expand Up @@ -214,10 +214,10 @@ 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),ext:(0x%02x)}",
L"{FG:%s,BG:%s,intense:%d,wLegacy:(0x%04x),ext:(0x%02x)}",
VerifyOutputTraits<TextColor>::ToString(attr._foreground).GetBuffer(),
VerifyOutputTraits<TextColor>::ToString(attr._background).GetBuffer(),
attr.IsBold(),
attr.IsIntense(),
attr._wAttrLegacy,
static_cast<DWORD>(attr._extendedAttrs));
}
Expand Down
4 changes: 2 additions & 2 deletions src/buffer/out/TextColor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ void TextColor::SetDefault() noexcept
// - If brighten is true, and we've got a 16 color index in the "dark"
// portion of the color table (indices [0,7]), then we'll look up the
// bright version of this color (from indices [8,15]). This should be
// true for TextAttributes that are "Bold" and we're treating bold as
// bright (which is the default behavior of most terminals.)
// true for TextAttributes that are "intense" and we're treating intense
// as bright (which is the default behavior of most terminals.)
// * If we're a default color, we'll return the default color provided.
// Arguments:
// - colorTable: The table of colors we should use to look up the value of
Expand Down
36 changes: 18 additions & 18 deletions src/buffer/out/ut_textbuffer/TextAttributeTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class TextAttributeTests
TEST_METHOD(TestTextAttributeColorGetters);
TEST_METHOD(TestReverseDefaultColors);
TEST_METHOD(TestRoundtripDefaultColors);
TEST_METHOD(TestBoldAsBright);
TEST_METHOD(TestIntenseAsBright);

RenderSettings _renderSettings;
const COLORREF _defaultFg = RGB(1, 2, 3);
Expand Down Expand Up @@ -257,7 +257,7 @@ void TextAttributeTests::TestRoundtripDefaultColors()
TextAttribute::SetLegacyDefaultAttributes(FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE);
}

void TextAttributeTests::TestBoldAsBright()
void TextAttributeTests::TestIntenseAsBright()
{
const auto& colorTable = _renderSettings.GetColorTable();
const COLORREF darkBlack = til::at(colorTable, 0);
Expand All @@ -267,8 +267,8 @@ void TextAttributeTests::TestBoldAsBright()
TextAttribute attr{};

// verify that calculated foreground/background are the same as the direct
// values when not bold
VERIFY_IS_FALSE(attr.IsBold());
// values when not intense
VERIFY_IS_FALSE(attr.IsIntense());

VERIFY_ARE_EQUAL(_defaultFg, attr.GetForeground().GetColor(colorTable, _defaultFgIndex));
VERIFY_ARE_EQUAL(_defaultBg, attr.GetBackground().GetColor(colorTable, _defaultBgIndex));
Expand All @@ -277,46 +277,46 @@ void TextAttributeTests::TestBoldAsBright()
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, false);
VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), _renderSettings.GetAttributeColors(attr));

// with bold set, calculated foreground/background values shouldn't change for the default colors.
attr.SetBold(true);
VERIFY_IS_TRUE(attr.IsBold());
// with intense set, calculated foreground/background values shouldn't change for the default colors.
attr.SetIntense(true);
VERIFY_IS_TRUE(attr.IsIntense());
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, true);
VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), _renderSettings.GetAttributeColors(attr));
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, false);
VERIFY_ARE_EQUAL(std::make_pair(_defaultFg, _defaultBg), _renderSettings.GetAttributeColors(attr));

attr.SetIndexedForeground(TextColor::DARK_BLACK);
VERIFY_IS_TRUE(attr.IsBold());
VERIFY_IS_TRUE(attr.IsIntense());

Log::Comment(L"Foreground should be bright black when bold is bright is enabled");
Log::Comment(L"Foreground should be bright black when intense is bright is enabled");
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, true);
VERIFY_ARE_EQUAL(std::make_pair(brightBlack, _defaultBg), _renderSettings.GetAttributeColors(attr));

Log::Comment(L"Foreground should be dark black when bold is bright is disabled");
Log::Comment(L"Foreground should be dark black when intense is bright is disabled");
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, false);
VERIFY_ARE_EQUAL(std::make_pair(darkBlack, _defaultBg), _renderSettings.GetAttributeColors(attr));

attr.SetIndexedBackground(TextColor::DARK_GREEN);
VERIFY_IS_TRUE(attr.IsBold());
VERIFY_IS_TRUE(attr.IsIntense());

Log::Comment(L"background should be unaffected by 'bold is bright'");
Log::Comment(L"background should be unaffected by 'intense is bright'");
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, true);
VERIFY_ARE_EQUAL(std::make_pair(brightBlack, darkGreen), _renderSettings.GetAttributeColors(attr));
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, false);
VERIFY_ARE_EQUAL(std::make_pair(darkBlack, darkGreen), _renderSettings.GetAttributeColors(attr));

attr.SetBold(false);
VERIFY_IS_FALSE(attr.IsBold());
Log::Comment(L"when not bold, 'bold is bright' changes nothing");
attr.SetIntense(false);
VERIFY_IS_FALSE(attr.IsIntense());
Log::Comment(L"when not intense, 'intense is bright' changes nothing");
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, true);
VERIFY_ARE_EQUAL(std::make_pair(darkBlack, darkGreen), _renderSettings.GetAttributeColors(attr));
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, false);
VERIFY_ARE_EQUAL(std::make_pair(darkBlack, darkGreen), _renderSettings.GetAttributeColors(attr));

Log::Comment(L"When set to a bright color, and bold, 'bold is bright' changes nothing");
attr.SetBold(true);
Log::Comment(L"When set to a bright color, and intense, 'intense is bright' changes nothing");
attr.SetIntense(true);
attr.SetIndexedForeground(TextColor::BRIGHT_BLACK);
VERIFY_IS_TRUE(attr.IsBold());
VERIFY_IS_TRUE(attr.IsIntense());
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, true);
VERIFY_ARE_EQUAL(std::make_pair(brightBlack, darkGreen), _renderSettings.GetAttributeColors(attr));
_renderSettings.SetRenderMode(RenderSettings::Mode::IntenseIsBright, false);
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,14 @@ bool TerminalDispatch::SetGraphicsRendition(const VTParameters options) noexcept
case BackgroundDefault:
attr.SetDefaultBackground();
break;
case BoldBright:
attr.SetBold(true);
case Intense:
attr.SetIntense(true);
break;
case RGBColorOrFaint:
attr.SetFaint(true);
break;
case NotBoldOrFaint:
attr.SetBold(false);
case NotIntenseOrFaint:
attr.SetIntense(false);
attr.SetFaint(false);
break;
case Italics:
Expand Down
36 changes: 18 additions & 18 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5076,7 +5076,7 @@ void ScreenBufferTests::TestExtendedTextAttributes()

// Run this test for each and every possible combination of states.
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:bold", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:intense", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:faint", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:italics", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:underlined", L"{false, true}")
Expand All @@ -5086,8 +5086,8 @@ void ScreenBufferTests::TestExtendedTextAttributes()
TEST_METHOD_PROPERTY(L"Data:crossedOut", L"{false, true}")
END_TEST_METHOD_PROPERTIES()

bool bold, faint, italics, underlined, doublyUnderlined, blink, invisible, crossedOut;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"bold", bold));
bool intense, faint, italics, underlined, doublyUnderlined, blink, invisible, crossedOut;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"intense", intense));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"faint", faint));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"underlined", underlined));
Expand All @@ -5107,9 +5107,9 @@ void ScreenBufferTests::TestExtendedTextAttributes()
std::wstring vtSeq = L"";

// Collect up a VT sequence to set the state given the method properties
if (bold)
if (intense)
{
WI_SetFlag(expectedAttrs, ExtendedAttributes::Bold);
WI_SetFlag(expectedAttrs, ExtendedAttributes::Intense);
vtSeq += L"\x1b[1m";
}
if (faint)
Expand Down Expand Up @@ -5184,10 +5184,10 @@ void ScreenBufferTests::TestExtendedTextAttributes()

// One-by-one, turn off each of these states with VT, then check that the
// state matched.
if (bold || faint)
if (intense || faint)
{
// The bold and faint attributes share the same reset sequence.
WI_ClearAllFlags(expectedAttrs, ExtendedAttributes::Bold | ExtendedAttributes::Faint);
// The intense and faint attributes share the same reset sequence.
WI_ClearAllFlags(expectedAttrs, ExtendedAttributes::Intense | ExtendedAttributes::Faint);
vtSeq = L"\x1b[22m";
validate(expectedAttrs, vtSeq);
}
Expand Down Expand Up @@ -5238,7 +5238,7 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()

// Run this test for each and every possible combination of states.
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:bold", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:intense", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:faint", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:italics", L"{false, true}")
TEST_METHOD_PROPERTY(L"Data:underlined", L"{false, true}")
Expand All @@ -5257,8 +5257,8 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
const int Use256Color = 2;
const int UseRGBColor = 3;

bool bold, faint, italics, underlined, doublyUnderlined, blink, invisible, crossedOut;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"bold", bold));
bool intense, faint, italics, underlined, doublyUnderlined, blink, invisible, crossedOut;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"intense", intense));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"faint", faint));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics));
VERIFY_SUCCEEDED(TestData::TryGetValue(L"underlined", underlined));
Expand All @@ -5282,9 +5282,9 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()
std::wstring vtSeq = L"";

// Collect up a VT sequence to set the state given the method properties
if (bold)
if (intense)
{
expectedAttr.SetBold(true);
expectedAttr.SetIntense(true);
vtSeq += L"\x1b[1m";
}
if (faint)
Expand Down Expand Up @@ -5403,10 +5403,10 @@ void ScreenBufferTests::TestExtendedTextAttributesWithColors()

// One-by-one, turn off each of these states with VT, then check that the
// state matched.
if (bold || faint)
if (intense || faint)
{
// The bold and faint attributes share the same reset sequence.
expectedAttr.SetBold(false);
// The intense and faint attributes share the same reset sequence.
expectedAttr.SetIntense(false);
expectedAttr.SetFaint(false);
vtSeq = L"\x1b[22m";
validate(expectedAttr, vtSeq);
Expand Down Expand Up @@ -6222,7 +6222,7 @@ void ScreenBufferTests::TestWriteConsoleVTQuirkMode()
TextAttribute vtBrightWhiteOnBlackAttribute{};
vtBrightWhiteOnBlackAttribute.SetForeground(TextColor{ TextColor::DARK_WHITE, false });
vtBrightWhiteOnBlackAttribute.SetBackground(TextColor{ TextColor::DARK_BLACK, false });
vtBrightWhiteOnBlackAttribute.SetBold(true);
vtBrightWhiteOnBlackAttribute.SetIntense(true);

TextAttribute vtBrightWhiteOnDefaultAttribute{ vtBrightWhiteOnBlackAttribute }; // copy the above attribute
vtBrightWhiteOnDefaultAttribute.SetDefaultBackground();
Expand All @@ -6248,7 +6248,7 @@ void ScreenBufferTests::TestWriteConsoleVTQuirkMode()
vtWhiteOnBlack256Attribute.SetForeground(TextColor{ TextColor::DARK_WHITE, true });
vtWhiteOnBlack256Attribute.SetBackground(TextColor{ TextColor::DARK_BLACK, true });

// reset (disable bold from the last test) before setting both colors
// reset (disable intense from the last test) before setting both colors
seq = L"\x1b[m\x1b[38;5;7;48;5;0m"; // the quirk should *not* suppress this (!)
seqCb = 2 * seq.size();
VERIFY_SUCCEEDED(DoWriteConsole(&seq[0], &seqCb, mainBuffer, useQuirk, waiter));
Expand Down
Loading

0 comments on commit bcc38d0

Please sign in to comment.