Skip to content

Commit

Permalink
Fix SGR indexed colors to distinguish Indexed256 color (and more) (#5834
Browse files Browse the repository at this point in the history
)

This PR introduces a new `ColorType` to allow us to distinguish between
`SGR` indexed colors from the 16 color table, the lower half of which
can be brightened, and the ISO/ITU indexed colors from the 256 color
table, which have a fixed brightness. Retaining the distinction between
these two types will enable us to forward the correct `SGR` sequences to
conpty when addressing issue #2661. 

The other benefit of retaining the color index (which we didn't
previously do for ISO/ITU colors) is that it ensures that the colors are
updated correctly when the color scheme is changed.

## References

* This is another step towards fixing the conpty narrowing bugs in issue
  #2661.
* This is technically a fix for issue #5384, but that won't be apparent
  until #2661 is complete.

## PR Checklist
* [x] Closes #1223
* [x] CLA signed. 
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already.

## Detailed Description of the Pull Request / Additional comments

The first part of this PR was the introduction of a new `ColorType` in
the `TextColor` class. Instead of just the one `IsIndex` type, there is
now an `IsIndex16` and an `IsIndex256`. `IsIndex16` covers the eight
original ANSI colors set with `SGR 3x` and `SGR 4x`, as well as the
brighter aixterm variants set with `SGR 9x` and `SGR 10x`. `IsIndex256`
covers the 256 ISO/ITU indexed colors set with `SGR 38;5` and `SGR
48;5`.

There are two reasons for this distinction. The first is that the ANSI
colors have the potential to be brightened by the `SGR 1` bold
attribute, while the ISO/ITO color do not. The second reason is that
when forwarding an attributes through conpty, we want to try and
preserve the original SGR sequence that generated each color (to the
extent that that is possible). By having the two separate types, we can
map the `IsIndex16` colors back to ANSI/aixterm values, and `IsIndex256`
to the ISO/ITU sequences.

In addition to the VT colors, we also have to deal with the legacy
colors set by the Windows console APIs, but we don't really need a
separate type for those. It seemed most appropriate to me to store them
as `IsIndex256` colors, since it doesn't make sense to have them
brightened by the `SGR 1` attribute (which is what would happen if they
were stored as `IsIndex16`). If a console app wanted a bright color it
would have selected one, so we shouldn't be messing with that choice.

The second part of the PR was the unification of the two color tables.
Originally we had a 16 color table for the legacy colors, and a separate
table for the 256 ISO/ITU colors. These have now been merged into one,
so color table lookups no longer need to decide which of the two tables
they should be referencing. I've also updated all the methods that took
a color table as a parameter to use a `basic_string_view` instead of
separate pointer and length variables, which I think makes them a lot
easier and safer to work with. 

With this new architecture in place, I could now update the
`AdaptDispatch` SGR implementation to store the ISO/ITU indexed colors
as `IsIndex256` values, where before they were mapped to RGB values
(which prevented them reflecting any color scheme changes). I could also
update the `TerminalDispatch` implementation to differentiate between
the two index types, so that the `SGR 1` brightening would only be
applied to the ANSI colors.

I've also done a bit of code refactoring to try and minimise any direct
access to the color tables, getting rid of a lot of places that were
copying tables with `memmove` operations. I'm hoping this will make it
easier for us to update the code in the future if we want to reorder the
table entries (which is likely a requirement for unifying the
`AdaptDispatch` and `TerminalDispatch` implementations). 

## Validation Steps Performed

For testing, I've just updated the existing unit tests to account for
the API changes. The `TextColorTests` required an extra parameter
specifying the index type when setting an index. And the `AdapterTest`
and `ScreenBufferTests` required the use of the new `SetIndexedXXX`
methods in order to be explicit about the index type, instead of relying
on the `TextAttribute` constructor and the old `SetForeground` and
`SetBackground` methods which didn't have a way to differentiate index
types.

I've manually tested the various console APIs
(`SetConsoleTextAttribute`, `ReadConsoleOutputAttribute`, and
`ReadConsoleOutput`), to make sure they are still setting and reading
the attributes as well as they used to. And I've tested the
`SetConsoleScreenBufferInfoEx` and `GetConsoleScreenBufferInfoEx` APIs
to make sure they can read and write the color table correctly. I've
also tested the color table in the properties dialog, made sure it was
saved and restored from the registry correctly, and similarly saved and
restored from a shortcut link.

Note that there are still a bunch of issues with the color table APIs,
but no new problems have been introduced by the changes in this PR, as
far as I could tell.

I've also done a bunch of manual tests of `OSC 4` to make sure it's
updating all the colors correctly (at least in conhost), and confirmed
that the test case in issue #1223 now works as expected.
  • Loading branch information
j4james authored May 27, 2020
1 parent e7c22fb commit fa7c1ab
Show file tree
Hide file tree
Showing 36 changed files with 328 additions and 376 deletions.
19 changes: 17 additions & 2 deletions src/buffer/out/TextAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ bool TextAttribute::IsLegacy() const noexcept
return _foreground.IsLegacy() && _background.IsLegacy();
}

bool TextAttribute::IsHighColor() const noexcept
{
return _foreground.IsHighColor() || _background.IsHighColor();
}

// Arguments:
// - None
// Return Value:
Expand Down Expand Up @@ -72,12 +77,22 @@ void TextAttribute::SetBackground(const COLORREF rgbBackground) noexcept

void TextAttribute::SetIndexedForeground(const BYTE fgIndex) noexcept
{
_foreground = TextColor(fgIndex);
_foreground = TextColor(fgIndex, false);
}

void TextAttribute::SetIndexedBackground(const BYTE bgIndex) noexcept
{
_background = TextColor(bgIndex);
_background = TextColor(bgIndex, false);
}

void TextAttribute::SetIndexedForeground256(const BYTE fgIndex) noexcept
{
_foreground = TextColor(fgIndex, true);
}

void TextAttribute::SetIndexedBackground256(const BYTE bgIndex) noexcept
{
_background = TextColor(bgIndex, true);
}

void TextAttribute::SetColor(const COLORREF rgbColor, const bool fIsForeground) noexcept
Expand Down
44 changes: 12 additions & 32 deletions src/buffer/out/TextAttribute.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ class TextAttribute final

constexpr TextAttribute(const WORD wLegacyAttr) noexcept :
_wAttrLegacy{ gsl::narrow_cast<WORD>(wLegacyAttr & META_ATTRS) },
_foreground{ gsl::narrow_cast<BYTE>(wLegacyAttr & FG_ATTRS) },
_background{ gsl::narrow_cast<BYTE>((wLegacyAttr & BG_ATTRS) >> 4) },
_foreground{ gsl::narrow_cast<BYTE>(wLegacyAttr & FG_ATTRS), true },
_background{ gsl::narrow_cast<BYTE>((wLegacyAttr & BG_ATTRS) >> 4), true },
_extendedAttrs{ ExtendedAttributes::Normal }
{
// If we're given lead/trailing byte information with the legacy color, strip it.
Expand All @@ -59,12 +59,13 @@ class TextAttribute final
{
}

constexpr WORD GetLegacyAttributes() const noexcept
WORD GetLegacyAttributes() const noexcept
{
const BYTE fg = (_foreground.GetIndex() & FG_ATTRS);
const BYTE bg = (_background.GetIndex() << 4) & BG_ATTRS;
const WORD meta = (_wAttrLegacy & META_ATTRS);
return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0);
const bool brighten = _foreground.IsIndex16() && IsBold();
return (fg | bg | meta) | (brighten ? FOREGROUND_INTENSITY : 0);
}

// Method Description:
Expand All @@ -79,15 +80,16 @@ class TextAttribute final
// the background not be a legacy style attribute.
// Return Value:
// - a WORD with legacy-style attributes for this textattribute.
constexpr WORD GetLegacyAttributes(const BYTE defaultFgIndex,
const BYTE defaultBgIndex) const noexcept
WORD GetLegacyAttributes(const BYTE defaultFgIndex,
const BYTE defaultBgIndex) const noexcept
{
const BYTE fgIndex = _foreground.IsLegacy() ? _foreground.GetIndex() : defaultFgIndex;
const BYTE bgIndex = _background.IsLegacy() ? _background.GetIndex() : defaultBgIndex;
const BYTE fg = (fgIndex & FG_ATTRS);
const BYTE bg = (bgIndex << 4) & BG_ATTRS;
const WORD meta = (_wAttrLegacy & META_ATTRS);
return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0);
const bool brighten = _foreground.IsIndex16() && IsBold();
return (fg | bg | meta) | (brighten ? FOREGROUND_INTENSITY : 0);
}

COLORREF CalculateRgbForeground(std::basic_string_view<COLORREF> colorTable,
Expand Down Expand Up @@ -117,6 +119,7 @@ class TextAttribute final
friend constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept;

bool IsLegacy() const noexcept;
bool IsHighColor() const noexcept;
bool IsBold() const noexcept;
bool IsItalic() const noexcept;
bool IsBlinking() const noexcept;
Expand All @@ -139,6 +142,8 @@ class TextAttribute final
void SetBackground(const COLORREF rgbBackground) noexcept;
void SetIndexedForeground(const BYTE fgIndex) noexcept;
void SetIndexedBackground(const BYTE bgIndex) noexcept;
void SetIndexedForeground256(const BYTE fgIndex) noexcept;
void SetIndexedBackground256(const BYTE bgIndex) noexcept;
void SetColor(const COLORREF rgbColor, const bool fIsForeground) noexcept;

void SetDefaultForeground() noexcept;
Expand All @@ -149,11 +154,6 @@ class TextAttribute final

void SetStandardErase() noexcept;

constexpr bool IsRgb() const noexcept
{
return _foreground.IsRgb() || _background.IsRgb();
}

// This returns whether this attribute, if printed directly next to another attribute, for the space
// character, would look identical to the other one.
bool HasIdenticalVisualRepresentationForBlankSpace(const TextAttribute& other, const bool inverted = false) const noexcept
Expand Down Expand Up @@ -224,26 +224,6 @@ constexpr bool operator!=(const TextAttribute& a, const TextAttribute& b) noexce
return !(a == b);
}

constexpr bool operator==(const TextAttribute& attr, const WORD& legacyAttr) noexcept
{
return attr.GetLegacyAttributes() == legacyAttr;
}

constexpr bool operator!=(const TextAttribute& attr, const WORD& legacyAttr) noexcept
{
return !(attr == legacyAttr);
}

constexpr bool operator==(const WORD& legacyAttr, const TextAttribute& attr) noexcept
{
return attr == legacyAttr;
}

constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept
{
return !(attr == legacyAttr);
}

#ifdef UNIT_TESTING

#define LOG_ATTR(attr) (Log::Comment(NoThrowString().Format( \
Expand Down
67 changes: 45 additions & 22 deletions src/buffer/out/TextColor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,36 @@
#include "precomp.h"
#include "TextColor.h"

bool TextColor::IsLegacy() const noexcept
{
return IsIndex16() || (IsIndex256() && _index < 16);
}

bool TextColor::IsHighColor() const noexcept
{
return IsRgb() || (IsIndex256() && _index >= 16);
}

bool TextColor::IsIndex16() const noexcept
{
return _meta == ColorType::IsIndex16;
}

bool TextColor::IsIndex256() const noexcept
{
return _meta == ColorType::IsIndex256;
}

bool TextColor::IsDefault() const noexcept
{
return _meta == ColorType::IsDefault;
}

bool TextColor::IsRgb() const noexcept
{
return _meta == ColorType::IsRgb;
}

// Method Description:
// - Sets the color value of this attribute, and sets this color to be an RGB
// attribute.
Expand All @@ -23,11 +53,12 @@ void TextColor::SetColor(const COLORREF rgbColor) noexcept
// - Sets this TextColor to be a legacy-style index into the color table.
// Arguments:
// - index: the index of the colortable we should use for this TextColor.
// - isIndex256: is this a 256 color index (true) or a 16 color index (false).
// Return Value:
// - <none>
void TextColor::SetIndex(const BYTE index) noexcept
void TextColor::SetIndex(const BYTE index, const bool isIndex256) noexcept
{
_meta = ColorType::IsIndex;
_meta = isIndex256 ? ColorType::IsIndex256 : ColorType::IsIndex16;
_index = index;
}

Expand All @@ -48,15 +79,15 @@ void TextColor::SetDefault() noexcept
// * If we're an RGB color, we'll use that value.
// * If we're an indexed color table value, we'll use that index to look up
// our value in the provided color table.
// - If brighten is true, and the index is 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.)
// - 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.)
// * 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 a
// legacy attribute from
// - colorTable: The table of colors we should use to look up the value of
// an indexed attribute from.
// - defaultColor: The color value to use if we're a default attribute.
// - brighten: if true, we'll brighten a dark color table index.
// Return Value:
Expand Down Expand Up @@ -94,21 +125,13 @@ COLORREF TextColor::GetColor(std::basic_string_view<COLORREF> colorTable,
{
return _GetRGB();
}
else if (IsIndex16() && brighten)
{
return colorTable.at(_index | 8);
}
else
{
FAIL_FAST_IF(colorTable.size() < _index);
// If the color is already bright (it's in index [8,15] or it's a
// 256color value [16,255], then boldness does nothing.
if (brighten && _index < 8)
{
FAIL_FAST_IF(colorTable.size() < 16);
FAIL_FAST_IF(gsl::narrow_cast<size_t>(_index) + 8 > colorTable.size());
return colorTable.at(gsl::narrow_cast<size_t>(_index) + 8);
}
else
{
return colorTable.at(_index);
}
return colorTable.at(_index);
}
}

Expand Down
35 changes: 14 additions & 21 deletions src/buffer/out/TextColor.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ Revision History:

enum class ColorType : BYTE
{
IsIndex = 0x0,
IsDefault = 0x1,
IsRgb = 0x2
IsIndex256 = 0x0,
IsIndex16 = 0x1,
IsDefault = 0x2,
IsRgb = 0x3
};

struct TextColor
Expand All @@ -57,9 +58,9 @@ struct TextColor
{
}

constexpr TextColor(const BYTE wLegacyAttr) noexcept :
_meta{ ColorType::IsIndex },
_index{ wLegacyAttr },
constexpr TextColor(const BYTE index, const bool isIndex256) noexcept :
_meta{ isIndex256 ? ColorType::IsIndex256 : ColorType::IsIndex16 },
_index{ index },
_green{ 0 },
_blue{ 0 }
{
Expand All @@ -76,23 +77,15 @@ struct TextColor
friend constexpr bool operator==(const TextColor& a, const TextColor& b) noexcept;
friend constexpr bool operator!=(const TextColor& a, const TextColor& b) noexcept;

constexpr bool IsLegacy() const noexcept
{
return !(IsDefault() || IsRgb());
}

constexpr bool IsDefault() const noexcept
{
return _meta == ColorType::IsDefault;
}

constexpr bool IsRgb() const noexcept
{
return _meta == ColorType::IsRgb;
}
bool IsLegacy() const noexcept;
bool IsHighColor() const noexcept;
bool IsIndex16() const noexcept;
bool IsIndex256() const noexcept;
bool IsDefault() const noexcept;
bool IsRgb() const noexcept;

void SetColor(const COLORREF rgbColor) noexcept;
void SetIndex(const BYTE index) noexcept;
void SetIndex(const BYTE index, const bool isIndex256) noexcept;
void SetDefault() noexcept;

COLORREF GetColor(std::basic_string_view<COLORREF> colorTable,
Expand Down
8 changes: 4 additions & 4 deletions src/buffer/out/ut_textbuffer/TextColorTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ void TextColorTests::TestDefaultColor()

void TextColorTests::TestDarkIndexColor()
{
TextColor indexColor((BYTE)(7));
TextColor indexColor((BYTE)(7), false);

VERIFY_IS_FALSE(indexColor.IsDefault());
VERIFY_IS_TRUE(indexColor.IsLegacy());
Expand All @@ -104,7 +104,7 @@ void TextColorTests::TestDarkIndexColor()

void TextColorTests::TestBrightIndexColor()
{
TextColor indexColor((BYTE)(15));
TextColor indexColor((BYTE)(15), false);

VERIFY_IS_FALSE(indexColor.IsDefault());
VERIFY_IS_TRUE(indexColor.IsLegacy());
Expand Down Expand Up @@ -186,7 +186,7 @@ void TextColorTests::TestChangeColor()
color = rgbColor.GetColor(view, _defaultBg, true);
VERIFY_ARE_EQUAL(_defaultBg, color);

rgbColor.SetIndex(7);
rgbColor.SetIndex(7, false);
color = rgbColor.GetColor(view, _defaultFg, false);
VERIFY_ARE_EQUAL(_colorTable[7], color);

Expand All @@ -199,7 +199,7 @@ void TextColorTests::TestChangeColor()
color = rgbColor.GetColor(view, _defaultBg, true);
VERIFY_ARE_EQUAL(_colorTable[15], color);

rgbColor.SetIndex(15);
rgbColor.SetIndex(15, false);
color = rgbColor.GetColor(view, _defaultFg, false);
VERIFY_ARE_EQUAL(_colorTable[15], color);

Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalCore/ITerminalApi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ namespace Microsoft::Terminal::Core
virtual bool SetTextToDefaults(bool foreground, bool background) noexcept = 0;
virtual bool SetTextForegroundIndex(BYTE colorIndex) noexcept = 0;
virtual bool SetTextBackgroundIndex(BYTE colorIndex) noexcept = 0;
virtual bool SetTextForegroundIndex256(BYTE colorIndex) noexcept = 0;
virtual bool SetTextBackgroundIndex256(BYTE colorIndex) noexcept = 0;
virtual bool SetTextRgbColor(COLORREF color, bool foreground) noexcept = 0;
virtual bool BoldText(bool boldOn) noexcept = 0;
virtual bool UnderlineText(bool underlineOn) noexcept = 0;
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ class Microsoft::Terminal::Core::Terminal final :
bool SetTextToDefaults(bool foreground, bool background) noexcept override;
bool SetTextForegroundIndex(BYTE colorIndex) noexcept override;
bool SetTextBackgroundIndex(BYTE colorIndex) noexcept override;
bool SetTextForegroundIndex256(BYTE colorIndex) noexcept override;
bool SetTextBackgroundIndex256(BYTE colorIndex) noexcept override;
bool SetTextRgbColor(COLORREF color, bool foreground) noexcept override;
bool BoldText(bool boldOn) noexcept override;
bool UnderlineText(bool underlineOn) noexcept override;
Expand Down
16 changes: 16 additions & 0 deletions src/cascadia/TerminalCore/TerminalApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,22 @@ bool Terminal::SetTextBackgroundIndex(BYTE colorIndex) noexcept
return true;
}

bool Terminal::SetTextForegroundIndex256(BYTE colorIndex) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetIndexedForeground256(colorIndex);
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::SetTextBackgroundIndex256(BYTE colorIndex) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
attrs.SetIndexedBackground256(colorIndex);
_buffer->SetCurrentAttributes(attrs);
return true;
}

bool Terminal::SetTextRgbColor(COLORREF color, bool foreground) noexcept
{
TextAttribute attrs = _buffer->GetCurrentAttributes();
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ bool TerminalDispatch::_SetRgbColorsHelper(const std::basic_string_view<Dispatch
{
const auto tableIndex = til::at(options, 2);
success = isForeground ?
_terminalApi.SetTextForegroundIndex(gsl::narrow_cast<BYTE>(tableIndex)) :
_terminalApi.SetTextBackgroundIndex(gsl::narrow_cast<BYTE>(tableIndex));
_terminalApi.SetTextForegroundIndex256(gsl::narrow_cast<BYTE>(tableIndex)) :
_terminalApi.SetTextBackgroundIndex256(gsl::narrow_cast<BYTE>(tableIndex));
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
auto vtRenderEngine = std::make_unique<Xterm256Engine>(std::move(hFile),
gci,
initialViewport,
gci.GetColorTable(),
static_cast<WORD>(gci.GetColorTableSize()));
gci.Get16ColorTable());
auto pfn = std::bind(&ConptyRoundtripTests::_writeCallback, this, std::placeholders::_1, std::placeholders::_2);
vtRenderEngine->SetTestCallback(pfn);

Expand Down
Loading

1 comment on commit fa7c1ab

@github-actions
Copy link

Choose a reason for hiding this comment

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

New misspellings found, please review:

  • IStorage
To accept these changes, run the following commands
remove_obsolete_words=$(mktemp)
echo '#!/usr/bin/perl -ni
my $re=join "|", qw('"
AAD
abcd
abe
abec
acb
afae
afceeeaa
baac
bbd
bca
bcb
bcc
bda
bfb
bitfield
caa
carlos
cbb
dcf
ddb
dfa
eba
ebce
EBFB
ECFB
eeb
eee
Emoji
Emojis
fbb
fbd
FBE
fcc
fd
fdb
fdd
ffc
HREF
memcpying
OUTPATHROOT
storageitems
textblock
usr
vpack
zamora
"');
next if /^($re)(?:$| .*)/;
print;' > $remove_obsolete_words
chmod +x $remove_obsolete_words
for file in .github/actions/spell-check/whitelist/alphabet.txt .github/actions/spell-check/whitelist/web.txt .github/actions/spell-check/whitelist/whitelist.txt; do $remove_obsolete_words $file; done
rm $remove_obsolete_words
(
echo "
ABE
EEB
FDD
IStorage
"
) | sort -u -f | perl -ne 'next unless /./; print' > new_whitelist.txt && mv new_whitelist.txt '.github/actions/spell-check/whitelist/fa7c1abdf8f857be12af51cea7ac16ac10e95df1.txt'
✏️ Contributor please read this
  • If the items listed above are names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • If they're APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • If they're just things you're using, please add them to an appropriate file in .github/actions/spell-check/whitelist/.
  • If you need to use a specific token in one place and it shouldn't generally be used, you can
    add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

Please sign in to comment.