Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename the "Bold" SGR attribute as "Intense" #12270

Merged
2 commits merged into from
Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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