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

Refactor the SGR implementation in AdaptDispatch #5758

Merged
merged 11 commits into from
May 8, 2020
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
3 changes: 3 additions & 0 deletions .github/actions/spell-check/dictionary/apis.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
bitfield
bitfields
href
IBox
ICustom
IMap
Expand Down
20 changes: 0 additions & 20 deletions src/buffer/out/AttrRow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,26 +190,6 @@ bool ATTR_ROW::SetAttrToEnd(const UINT iStart, const TextAttribute attr)
return SUCCEEDED(InsertAttrRuns({ &run, 1 }, iStart, _cchRowWidth - 1, _cchRowWidth));
}

// Routine Description:
// - Replaces all runs in the row with the given wToBeReplacedAttr with the new
// attribute wReplaceWith. This method is used for replacing specifically
// legacy attributes.
// Arguments:
// - wToBeReplacedAttr - the legacy attribute to replace in this row.
// - wReplaceWith - the new value for the matching runs' attributes.
// Return Value:
// <none>
void ATTR_ROW::ReplaceLegacyAttrs(_In_ WORD wToBeReplacedAttr, _In_ WORD wReplaceWith) noexcept
{
TextAttribute ToBeReplaced;
ToBeReplaced.SetFromLegacy(wToBeReplacedAttr);

TextAttribute ReplaceWith;
ReplaceWith.SetFromLegacy(wReplaceWith);

ReplaceAttrs(ToBeReplaced, ReplaceWith);
}

// Method Description:
// - Replaces all runs in the row with the given toBeReplacedAttr with the new
// attribute replaceWith.
Expand Down
1 change: 0 additions & 1 deletion src/buffer/out/AttrRow.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class ATTR_ROW final
size_t* const pApplies) const;

bool SetAttrToEnd(const UINT iStart, const TextAttribute attr);
void ReplaceLegacyAttrs(const WORD wToBeReplacedAttr, const WORD wReplaceWith) noexcept;
void ReplaceAttrs(const TextAttribute& toBeReplacedAttr, const TextAttribute& replaceWith) noexcept;

void Resize(const size_t newWidth);
Expand Down
26 changes: 0 additions & 26 deletions src/buffer/out/OutputCell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,6 @@ OutputCell::OutputCell(const std::wstring_view charData,
_setFromStringView(charData);
}

OutputCell::OutputCell(const CHAR_INFO& charInfo) :
_text{ UNICODE_INVALID },
_dbcsAttribute{},
_textAttribute{ InvalidTextAttribute },
_behavior{ TextAttributeBehavior::Stored }
{
_setFromCharInfo(charInfo);
}

OutputCell::OutputCell(const OutputCellView& cell)
{
_setFromOutputCellView(cell);
Expand Down Expand Up @@ -84,23 +75,6 @@ void OutputCell::_setFromBehavior(const TextAttributeBehavior behavior)
THROW_HR_IF(E_INVALIDARG, behavior == TextAttributeBehavior::Stored);
}

void OutputCell::_setFromCharInfo(const CHAR_INFO& charInfo)
{
_text = charInfo.Char.UnicodeChar;

if (WI_IsFlagSet(charInfo.Attributes, COMMON_LVB_LEADING_BYTE))
{
_dbcsAttribute.SetLeading();
}
else if (WI_IsFlagSet(charInfo.Attributes, COMMON_LVB_TRAILING_BYTE))
{
_dbcsAttribute.SetTrailing();
}
_textAttribute.SetFromLegacy(charInfo.Attributes);

_behavior = TextAttributeBehavior::Stored;
}

void OutputCell::_setFromStringView(const std::wstring_view view)
{
_text = view;
Expand Down
2 changes: 0 additions & 2 deletions src/buffer/out/OutputCell.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ class OutputCell final
const DbcsAttribute dbcsAttribute,
const TextAttribute textAttribute);

OutputCell(const CHAR_INFO& charInfo);
OutputCell(const OutputCellView& view);

OutputCell(const OutputCell&) = default;
Expand Down Expand Up @@ -86,7 +85,6 @@ class OutputCell final
TextAttributeBehavior _behavior;

void _setFromBehavior(const TextAttributeBehavior behavior);
void _setFromCharInfo(const CHAR_INFO& charInfo);
void _setFromStringView(const std::wstring_view view);
void _setFromOutputCellView(const OutputCellView& cell);
};
3 changes: 1 addition & 2 deletions src/buffer/out/OutputCellIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,7 @@ OutputCellView OutputCellIterator::s_GenerateView(const CHAR_INFO& charInfo) noe
dbcsAttr.SetTrailing();
}

TextAttribute textAttr;
textAttr.SetFromLegacy(charInfo.Attributes);
const TextAttribute textAttr(charInfo.Attributes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny nit: it took me a while looking at this to get over the "most vexing parse"; can we use {} universal init syntax?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm a fan of the {} syntax too, but most of the surrounding code is using () initialization, at least in the areas I was modifying. So if we want to change that, I think it's best to handle that in a separate code cleanup PR that replaces everything consistently.


const auto behavior = TextAttributeBehavior::Stored;
return OutputCellView(glyph, dbcsAttr, textAttr, behavior);
Expand Down
156 changes: 75 additions & 81 deletions src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ COLORREF TextAttribute::CalculateRgbForeground(std::basic_string_view<COLORREF>
COLORREF defaultFgColor,
COLORREF defaultBgColor) const noexcept
{
return _IsReverseVideo() ? _GetRgbBackground(colorTable, defaultBgColor) : _GetRgbForeground(colorTable, defaultFgColor);
return IsReverseVideo() ? _GetRgbBackground(colorTable, defaultBgColor) : _GetRgbForeground(colorTable, defaultFgColor);
}

// Routine Description:
Expand All @@ -31,7 +31,7 @@ COLORREF TextAttribute::CalculateRgbBackground(std::basic_string_view<COLORREF>
COLORREF defaultFgColor,
COLORREF defaultBgColor) const noexcept
{
return _IsReverseVideo() ? _GetRgbForeground(colorTable, defaultFgColor) : _GetRgbBackground(colorTable, defaultBgColor);
return IsReverseVideo() ? _GetRgbForeground(colorTable, defaultFgColor) : _GetRgbBackground(colorTable, defaultBgColor);
}

// Routine Description:
Expand Down Expand Up @@ -60,21 +60,6 @@ COLORREF TextAttribute::_GetRgbBackground(std::basic_string_view<COLORREF> color
return _background.GetColor(colorTable, defaultColor, false);
}

void TextAttribute::SetMetaAttributes(const WORD wMeta) noexcept
{
WI_UpdateFlagsInMask(_wAttrLegacy, META_ATTRS, wMeta);
WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS);
}

WORD TextAttribute::GetMetaAttributes() const noexcept
{
WORD wMeta = _wAttrLegacy;
WI_ClearAllFlags(wMeta, FG_ATTRS);
WI_ClearAllFlags(wMeta, BG_ATTRS);
WI_ClearAllFlags(wMeta, COMMON_LVB_SBCSDBCS);
return wMeta;
}

void TextAttribute::SetForeground(const COLORREF rgbForeground) noexcept
{
_foreground = TextColor(rgbForeground);
Expand All @@ -85,62 +70,14 @@ void TextAttribute::SetBackground(const COLORREF rgbBackground) noexcept
_background = TextColor(rgbBackground);
}

void TextAttribute::SetFromLegacy(const WORD wLegacy) noexcept
void TextAttribute::SetIndexedForeground(const BYTE fgIndex) noexcept
{
_wAttrLegacy = gsl::narrow_cast<WORD>(wLegacy & META_ATTRS);
WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS);
const BYTE fgIndex = gsl::narrow_cast<BYTE>(wLegacy & FG_ATTRS);
const BYTE bgIndex = gsl::narrow_cast<BYTE>(wLegacy & BG_ATTRS) >> 4;
_foreground = TextColor(fgIndex);
_background = TextColor(bgIndex);
}

void TextAttribute::SetLegacyAttributes(const WORD attrs,
const bool setForeground,
const bool setBackground,
const bool setMeta) noexcept
{
if (setForeground)
{
const BYTE fgIndex = gsl::narrow_cast<BYTE>(attrs & FG_ATTRS);
_foreground = TextColor(fgIndex);
}
if (setBackground)
{
const BYTE bgIndex = gsl::narrow_cast<BYTE>(attrs & BG_ATTRS) >> 4;
_background = TextColor(bgIndex);
}
if (setMeta)
{
SetMetaAttributes(attrs);
}
}

// Method Description:
// - Sets the foreground and/or background to a particular index in the 256color
// table. If either parameter is nullptr, it's ignored.
// This method can be used to set the colors to indexes in the range [0, 255],
// as opposed to SetLegacyAttributes, which clamps them to [0,15]
// Arguments:
// - foreground: nullptr if we should ignore this attr, else a pointer to a byte
// value to use as an index into the 256-color table.
// - background: nullptr if we should ignore this attr, else a pointer to a byte
// value to use as an index into the 256-color table.
// Return Value:
// - <none>
void TextAttribute::SetIndexedAttributes(const std::optional<const BYTE> foreground,
const std::optional<const BYTE> background) noexcept
void TextAttribute::SetIndexedBackground(const BYTE bgIndex) noexcept
{
if (foreground)
{
const BYTE fgIndex = (*foreground) & 0xFF;
_foreground = TextColor(fgIndex);
}
if (background)
{
const BYTE bgIndex = (*background) & 0xFF;
_background = TextColor(bgIndex);
}
_background = TextColor(bgIndex);
}

void TextAttribute::SetColor(const COLORREF rgbColor, const bool fIsForeground) noexcept
Expand Down Expand Up @@ -195,33 +132,90 @@ void TextAttribute::SetRightVerticalDisplayed(const bool isDisplayed) noexcept
WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_GRID_RVERTICAL, isDisplayed);
}

void TextAttribute::Embolden() noexcept
bool TextAttribute::IsBold() const noexcept
{
_SetBoldness(true);
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Bold);
}

void TextAttribute::Debolden() noexcept
bool TextAttribute::IsItalic() const noexcept
{
_SetBoldness(false);
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Italics);
}

void TextAttribute::SetExtendedAttributes(const ExtendedAttributes attrs) noexcept
bool TextAttribute::IsBlinking() const noexcept
{
_extendedAttrs = attrs;
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Blinking);
}

// Routine Description:
// - swaps foreground and background color
void TextAttribute::Invert() noexcept
bool TextAttribute::IsInvisible() const noexcept
{
WI_ToggleFlag(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO);
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Invisible);
}

bool TextAttribute::IsCrossedOut() const noexcept
{
return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::CrossedOut);
}

void TextAttribute::_SetBoldness(const bool isBold) noexcept
bool TextAttribute::IsUnderlined() const noexcept
{
// TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE
return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_UNDERSCORE);
}

bool TextAttribute::IsReverseVideo() const noexcept
{
return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO);
}

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

void TextAttribute::SetItalics(bool isItalic) noexcept
{
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Italics, isItalic);
}

void TextAttribute::SetBlinking(bool isBlinking) noexcept
{
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Blinking, isBlinking);
}

void TextAttribute::SetInvisible(bool isInvisible) noexcept
{
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Invisible, isInvisible);
}

void TextAttribute::SetCrossedOut(bool isCrossedOut) noexcept
{
WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::CrossedOut, isCrossedOut);
}

void TextAttribute::SetUnderline(bool isUnderlined) noexcept
{
// TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE
WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_UNDERSCORE, isUnderlined);
}

void TextAttribute::SetReverseVideo(bool isReversed) noexcept
{
WI_UpdateFlag(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO, isReversed);
}

ExtendedAttributes TextAttribute::GetExtendedAttributes() const noexcept
{
return _extendedAttrs;
}

// Routine Description:
// - swaps foreground and background color
void TextAttribute::Invert() noexcept
{
WI_ToggleFlag(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO);
}

void TextAttribute::SetDefaultForeground() noexcept
{
_foreground = TextColor();
Expand Down Expand Up @@ -267,6 +261,6 @@ bool TextAttribute::BackgroundIsDefault() const noexcept
// requires for most erasing and filling operations.
void TextAttribute::SetStandardErase() noexcept
{
SetExtendedAttributes(ExtendedAttributes::Normal);
SetMetaAttributes(0);
_extendedAttrs = ExtendedAttributes::Normal;
_wAttrLegacy = 0;
}
Loading