diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 4172374af6d..6c82b718fb9 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -147,11 +147,11 @@ const UnicodeStorage& ROW::GetUnicodeStorage() const noexcept // Arguments: // - it - custom console iterator to use for seeking input data. bool() false when it becomes invalid while seeking. // - index - column in row to start writing at -// - setWrap - set the wrap flags if we hit the end of the row while writing and there's still more data in the iterator. +// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data in the iterator. // - limitRight - right inclusive column ID for the last write in this row. (optional, will just write to the end of row if nullopt) // Return Value: // - iterator to first cell that was not written to this row. -OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, const bool setWrap, std::optional limitRight) +OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, const std::optional wrap, std::optional limitRight) { THROW_HR_IF(E_INVALIDARG, index >= _charRow.size()); THROW_HR_IF(E_INVALIDARG, limitRight.value_or(0) >= _charRow.size()); @@ -202,10 +202,15 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, co ++it; } - // If we're asked to set the wrap status and we just filled the last column with some text, set wrap status on the row. - if (setWrap && fillingLastColumn) + // If we're asked to (un)set the wrap status and we just filled the last column with some text... + // NOTE: + // - wrap = std::nullopt --> don't change the wrap value + // - wrap = true --> we're filling cells as a steam, consider this a wrap + // - wrap = false --> we're filling cells as a block, unwrap + if (wrap.has_value() && fillingLastColumn) { - _charRow.SetWrapForced(true); + // set wrap status on the row to parameter's value. + _charRow.SetWrapForced(wrap.value()); } } else diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index 27d2a390928..787e7339d95 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -57,7 +57,7 @@ class ROW final UnicodeStorage& GetUnicodeStorage() noexcept; const UnicodeStorage& GetUnicodeStorage() const noexcept; - OutputCellIterator WriteCells(OutputCellIterator it, const size_t index, const bool setWrap, std::optional limitRight = std::nullopt); + OutputCellIterator WriteCells(OutputCellIterator it, const size_t index, const std::optional wrap = std::nullopt, std::optional limitRight = std::nullopt); friend bool operator==(const ROW& a, const ROW& b) noexcept; diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index 0c964b848e9..fb498d25112 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -44,7 +44,7 @@ COLORREF TextAttribute::CalculateRgbBackground(std::basic_string_view COLORREF TextAttribute::_GetRgbForeground(std::basic_string_view colorTable, COLORREF defaultColor) const noexcept { - return _foreground.GetColor(colorTable, defaultColor, _isBold); + return _foreground.GetColor(colorTable, defaultColor, IsBold()); } // Routine Description: @@ -155,11 +155,6 @@ void TextAttribute::SetColor(const COLORREF rgbColor, const bool fIsForeground) } } -bool TextAttribute::IsBold() const noexcept -{ - return _isBold; -} - bool TextAttribute::_IsReverseVideo() const noexcept { return WI_IsFlagSet(_wAttrLegacy, COMMON_LVB_REVERSE_VIDEO); @@ -215,6 +210,11 @@ void TextAttribute::Debolden() noexcept _SetBoldness(false); } +void TextAttribute::SetExtendedAttributes(const ExtendedAttributes attrs) noexcept +{ + _extendedAttrs = attrs; +} + // Routine Description: // - swaps foreground and background color void TextAttribute::Invert() noexcept @@ -224,7 +224,7 @@ void TextAttribute::Invert() noexcept void TextAttribute::_SetBoldness(const bool isBold) noexcept { - _isBold = isBold; + WI_UpdateFlag(_extendedAttrs, ExtendedAttributes::Bold, isBold); } void TextAttribute::SetDefaultForeground() noexcept diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index eb8cc6111df..213c908e8f2 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -27,6 +27,8 @@ Revision History: #include "WexTestClass.h" #endif +#pragma pack(push, 1) + class TextAttribute final { public: @@ -34,7 +36,7 @@ class TextAttribute final _wAttrLegacy{ 0 }, _foreground{}, _background{}, - _isBold{ false } + _extendedAttrs{ ExtendedAttributes::Normal } { } @@ -42,7 +44,7 @@ class TextAttribute final _wAttrLegacy{ gsl::narrow_cast(wLegacyAttr & META_ATTRS) }, _foreground{ gsl::narrow_cast(wLegacyAttr & FG_ATTRS) }, _background{ gsl::narrow_cast((wLegacyAttr & BG_ATTRS) >> 4) }, - _isBold{ false } + _extendedAttrs{ ExtendedAttributes::Normal } { // If we're given lead/trailing byte information with the legacy color, strip it. WI_ClearAllFlags(_wAttrLegacy, COMMON_LVB_SBCSDBCS); @@ -53,7 +55,7 @@ class TextAttribute final _wAttrLegacy{ 0 }, _foreground{ rgbForeground }, _background{ rgbBackground }, - _isBold{ false } + _extendedAttrs{ ExtendedAttributes::Normal } { } @@ -62,7 +64,7 @@ class TextAttribute final 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); + return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0); } // Method Description: @@ -85,7 +87,7 @@ class TextAttribute final 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); + return (fg | bg | meta) | (IsBold() ? FOREGROUND_INTENSITY : 0); } COLORREF CalculateRgbForeground(std::basic_string_view colorTable, @@ -131,7 +133,18 @@ class TextAttribute final friend constexpr bool operator!=(const WORD& legacyAttr, const TextAttribute& attr) noexcept; bool IsLegacy() const noexcept; - bool IsBold() const noexcept; + + constexpr bool IsBold() const noexcept + { + return WI_IsFlagSet(_extendedAttrs, ExtendedAttributes::Bold); + } + + constexpr ExtendedAttributes GetExtendedAttributes() const noexcept + { + return _extendedAttrs; + } + + void SetExtendedAttributes(const ExtendedAttributes attrs) noexcept; void SetForeground(const COLORREF rgbForeground) noexcept; void SetBackground(const COLORREF rgbBackground) noexcept; @@ -159,7 +172,7 @@ class TextAttribute final WORD _wAttrLegacy; TextColor _foreground; TextColor _background; - bool _isBold; + ExtendedAttributes _extendedAttrs; #ifdef UNIT_TESTING friend class TextBufferTests; @@ -169,6 +182,13 @@ class TextAttribute final #endif }; +#pragma pack(pop) +// 2 for _wAttrLegacy +// 4 for _foreground +// 4 for _background +// 1 for _extendedAttrs +static_assert(sizeof(TextAttribute) <= 11 * sizeof(BYTE), "We should only need 11B for an entire TextColor. Any more than that is just waste"); + enum class TextAttributeBehavior { Stored, // use contained text attribute @@ -181,7 +201,7 @@ constexpr bool operator==(const TextAttribute& a, const TextAttribute& b) noexce return a._wAttrLegacy == b._wAttrLegacy && a._foreground == b._foreground && a._background == b._background && - a._isBold == b._isBold; + a._extendedAttrs == b._extendedAttrs; } constexpr bool operator!=(const TextAttribute& a, const TextAttribute& b) noexcept diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 1c51fde4bf0..fdec52c363a 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -318,10 +318,12 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt) // Arguments: // - givenIt - Iterator representing output cell data to write // - target - the row/column to start writing the text to +// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data // Return Value: // - The final position of the iterator OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt, - const COORD target) + const COORD target, + const std::optional wrap) { // Make mutable copy so we can walk. auto it = givenIt; @@ -336,7 +338,8 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt, while (it && size.IsInBounds(lineTarget)) { // Attempt to write as much data as possible onto this line. - it = WriteLine(it, lineTarget, true); + // NOTE: if wrap = true/false, we want to set the line's wrap to true/false (respectively) if we reach the end of the line + it = WriteLine(it, lineTarget, wrap); // Move to the next line down. lineTarget.X = 0; @@ -351,13 +354,13 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt, // Arguments: // - givenIt - The iterator that will dereference into cell data to insert // - target - Coordinate targeted within output buffer -// - setWrap - Whether we should try to set the wrap flag if we write up to the end of the line and have more data +// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data in the iterator. // - limitRight - Optionally restrict the right boundary for writing (e.g. stop writing earlier than the end of line) // Return Value: // - The iterator, but advanced to where we stopped writing. Use to find input consumed length or cells written length. OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt, const COORD target, - const bool setWrap, + const std::optional wrap, std::optional limitRight) { // If we're not in bounds, exit early. @@ -368,7 +371,7 @@ OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt, // Get the row and write the cells ROW& row = GetRowByOffset(target.Y); - const auto newIt = row.WriteCells(givenIt, target.X, setWrap, limitRight); + const auto newIt = row.WriteCells(givenIt, target.X, wrap, limitRight); // Take the cell distance written and notify that it needs to be repainted. const auto written = newIt.GetCellDistance(givenIt); diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 6f461eeefe8..ced8f0904bd 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -89,11 +89,12 @@ class TextBuffer final OutputCellIterator Write(const OutputCellIterator givenIt); OutputCellIterator Write(const OutputCellIterator givenIt, - const COORD target); + const COORD target, + const std::optional wrap = true); OutputCellIterator WriteLine(const OutputCellIterator givenIt, const COORD target, - const bool setWrap = false, + const std::optional setWrap = std::nullopt, const std::optional limitRight = std::nullopt); bool InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr); diff --git a/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp b/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp index 03dab6c849a..b73b9adee1b 100644 --- a/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp +++ b/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp @@ -103,7 +103,7 @@ bool TerminalDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTy isForeground = false; } - if (typeOpt == DispatchTypes::GraphicsOptions::RGBColor && cOptions >= 5) + if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint && cOptions >= 5) { *pcOptionsConsumed = 5; // ensure that each value fits in a byte @@ -115,7 +115,7 @@ bool TerminalDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTy fSuccess = _terminalApi.SetTextRgbColor(color, isForeground); } - else if (typeOpt == DispatchTypes::GraphicsOptions::Xterm256Index && cOptions >= 3) + else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index && cOptions >= 3) { *pcOptionsConsumed = 3; if (rgOptions[2] <= 255) // ensure that the provided index is on the table diff --git a/src/host/_output.cpp b/src/host/_output.cpp index 6a0a9ff7013..8d9fad7640a 100644 --- a/src/host/_output.cpp +++ b/src/host/_output.cpp @@ -290,7 +290,10 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region) try { const OutputCellIterator it(character, lengthToWrite); - const auto done = screenInfo.Write(it, startingCoordinate); + + // when writing to the buffer, specifically unset wrap if we get to the last column. + // a fill operation should UNSET wrap in that scenario. See GH #1126 for more details. + const auto done = screenInfo.Write(it, startingCoordinate, false); cellsModified = done.GetInputDistance(it); // Notify accessibility diff --git a/src/host/ft_host/API_FillOutputTests.cpp b/src/host/ft_host/API_FillOutputTests.cpp index 6505dfc746e..d0b9f2fa387 100644 --- a/src/host/ft_host/API_FillOutputTests.cpp +++ b/src/host/ft_host/API_FillOutputTests.cpp @@ -58,4 +58,125 @@ class FillOutputTests &charsWritten)); VERIFY_ARE_EQUAL(1u, charsWritten); } + + TEST_METHOD(UnsetWrap) + { + // WARNING: If this test suddenly decides to start failing, + // this is because the wrap registry key is not set. + // TODO GH #2859: Get/Set Registry Key for Wrap + + HANDLE hConsole = GetStdOutputHandle(); + DWORD charsWritten = 0; + + CONSOLE_SCREEN_BUFFER_INFOEX sbiex = { 0 }; + sbiex.cbSize = sizeof(sbiex); + VERIFY_WIN32_BOOL_SUCCEEDED(GetConsoleScreenBufferInfoEx(hConsole, &sbiex)); + + const auto consoleWidth = sbiex.dwSize.X; + + std::wstring input(consoleWidth + 2, L'a'); + std::wstring filled(consoleWidth, L'b'); + + // Write until a wrap occurs + VERIFY_WIN32_BOOL_SUCCEEDED(WriteConsoleW(hConsole, + input.data(), + gsl::narrow_cast(input.size()), + &charsWritten, + nullptr)); + + // Verify wrap occurred + std::unique_ptr bufferText = std::make_unique(consoleWidth); + DWORD readSize = 0; + VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole, + bufferText.get(), + consoleWidth, + { 0, 0 }, + &readSize)); + + WEX::Common::String expected(input.c_str(), readSize); + WEX::Common::String actual(bufferText.get(), readSize); + VERIFY_ARE_EQUAL(expected, actual); + + bufferText = std::make_unique(2); + VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole, + bufferText.get(), + 2, + { 0, 1 }, + &readSize)); + + VERIFY_ARE_EQUAL(2u, readSize); + expected = WEX::Common::String(input.c_str(), readSize); + actual = WEX::Common::String(bufferText.get(), readSize); + VERIFY_ARE_EQUAL(expected, actual); + + // Fill Console Line with 'b's + VERIFY_WIN32_BOOL_SUCCEEDED(FillConsoleOutputCharacterW(hConsole, + L'b', + consoleWidth, + { 2, 0 }, + &charsWritten)); + + // Verify first line is full of 'a's then 'b's + bufferText = std::make_unique(consoleWidth); + VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole, + bufferText.get(), + consoleWidth, + { 0, 0 }, + &readSize)); + + expected = WEX::Common::String(input.c_str(), 2); + actual = WEX::Common::String(bufferText.get(), 2); + VERIFY_ARE_EQUAL(expected, actual); + + expected = WEX::Common::String(filled.c_str(), consoleWidth - 2); + actual = WEX::Common::String(&bufferText[2], readSize - 2); + VERIFY_ARE_EQUAL(expected, actual); + + // Verify second line is still has 'a's that wrapped over + bufferText = std::make_unique(2); + VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole, + bufferText.get(), + static_cast(2), + { 0, 0 }, + &readSize)); + + VERIFY_ARE_EQUAL(2u, readSize); + expected = WEX::Common::String(input.c_str(), 2); + actual = WEX::Common::String(bufferText.get(), readSize); + VERIFY_ARE_EQUAL(expected, actual); + + // Resize to be smaller by 2 + sbiex.srWindow.Right -= 2; + sbiex.dwSize.X -= 2; + VERIFY_WIN32_BOOL_SUCCEEDED(SetConsoleScreenBufferInfoEx(hConsole, &sbiex)); + + // Verify first line is full of 'a's then 'b's + bufferText = std::make_unique(consoleWidth - 2); + VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole, + bufferText.get(), + consoleWidth - static_cast(2), + { 0, 0 }, + &readSize)); + + expected = WEX::Common::String(input.c_str(), 2); + actual = WEX::Common::String(bufferText.get(), 2); + VERIFY_ARE_EQUAL(expected, actual); + + expected = WEX::Common::String(filled.c_str(), consoleWidth - 4); + actual = WEX::Common::String(&bufferText[2], readSize - 2); + VERIFY_ARE_EQUAL(expected, actual); + + // Verify second line is still has 'a's ('b's didn't wrap over) + bufferText = std::make_unique(static_cast(2)); + VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole, + bufferText.get(), + static_cast(2), + { 0, 0 }, + &readSize)); + + VERIFY_ARE_EQUAL(2u, readSize); + expected = WEX::Common::String(input.c_str(), 2); + actual = WEX::Common::String(bufferText.get(), readSize); + VERIFY_ARE_EQUAL(expected, actual); + } }; diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 16aa6633a71..6d979633b56 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -649,31 +649,42 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont LOG_IF_FAILED(ConsoleImeResizeCompStrView()); - COORD WindowOrigin; - WindowOrigin.X = 0; - WindowOrigin.Y = 0; + // Attempt to "snap" the viewport to the cursor position. If the cursor + // is not in the current viewport, we'll try and move the viewport so + // that the cursor is visible. + // microsoft/terminal#1222 - Use the "virtual" viewport here, so that + // when the console is in terminal-scrolling mode, the viewport snaps + // back to the virtual viewport's location. + const SMALL_RECT currentViewport = gci.IsTerminalScrolling() ? + buffer.GetVirtualViewport().ToInclusive() : + buffer.GetViewport().ToInclusive(); + COORD delta{ 0 }; { - const SMALL_RECT currentViewport = buffer.GetViewport().ToInclusive(); if (currentViewport.Left > position.X) { - WindowOrigin.X = position.X - currentViewport.Left; + delta.X = position.X - currentViewport.Left; } else if (currentViewport.Right < position.X) { - WindowOrigin.X = position.X - currentViewport.Right; + delta.X = position.X - currentViewport.Right; } if (currentViewport.Top > position.Y) { - WindowOrigin.Y = position.Y - currentViewport.Top; + delta.Y = position.Y - currentViewport.Top; } else if (currentViewport.Bottom < position.Y) { - WindowOrigin.Y = position.Y - currentViewport.Bottom; + delta.Y = position.Y - currentViewport.Bottom; } } - RETURN_IF_NTSTATUS_FAILED(buffer.SetViewportOrigin(false, WindowOrigin, true)); + COORD newWindowOrigin{ 0 }; + newWindowOrigin.X = currentViewport.Left + delta.X; + newWindowOrigin.Y = currentViewport.Top + delta.Y; + // SetViewportOrigin will worry about clamping these values to the + // buffer for us. + RETURN_IF_NTSTATUS_FAILED(buffer.SetViewportOrigin(true, newWindowOrigin, true)); return S_OK; } @@ -970,6 +981,39 @@ void DoSrvPrivateBoldText(SCREEN_INFORMATION& screenInfo, const bool bolded) buffer.SetAttributes(attrs); } +// Method Description: +// - Retrieves the active ExtendedAttributes (italic, underline, etc.) of the +// given screen buffer. Text written to this buffer will be written with these +// attributes. +// Arguments: +// - screenInfo: The buffer to get the extended attrs from. +// Return Value: +// - the currently active ExtendedAttributes. +ExtendedAttributes DoSrvPrivateGetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo) +{ + auto& buffer = screenInfo.GetActiveBuffer(); + auto attrs = buffer.GetAttributes(); + return attrs.GetExtendedAttributes(); +} + +// Method Description: +// - Sets the active ExtendedAttributes (italic, underline, etc.) of the given +// screen buffer. Text written to this buffer will be written with these +// attributes. +// Arguments: +// - screenInfo: The buffer to set the extended attrs for. +// - extendedAttrs: The new ExtendedAttributes to use +// Return Value: +// - +void DoSrvPrivateSetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo, + const ExtendedAttributes extendedAttrs) +{ + auto& buffer = screenInfo.GetActiveBuffer(); + auto attrs = buffer.GetAttributes(); + attrs.SetExtendedAttributes(extendedAttrs); + buffer.SetAttributes(attrs); +} + // Routine Description: // - Sets the codepage used for translating text when calling A versions of functions affecting the output buffer. // Arguments: @@ -1378,12 +1422,25 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool coordDestination.X = 0; coordDestination.Y = viewport.Top + 1; - Status = NTSTATUS_FROM_HRESULT(ServiceLocator::LocateGlobals().api.ScrollConsoleScreenBufferWImpl(screenInfo, - srScroll, - coordDestination, - srScroll, - UNICODE_SPACE, - screenInfo.GetAttributes().GetLegacyAttributes())); + // Here we previously called to ScrollConsoleScreenBufferWImpl to + // perform the scrolling operation. However, that function only + // accepts a WORD for the fill attributes. That means we'd lose + // 256/RGB fidelity for fill attributes. So instead, we'll just call + // ScrollRegion ourselves, with the same params that + // ScrollConsoleScreenBufferWImpl would have. + // See microsoft/terminal#832, #2702 for more context. + try + { + LockConsole(); + auto Unlock = wil::scope_exit([&] { UnlockConsole(); }); + ScrollRegion(screenInfo, + srScroll, + srScroll, + coordDestination, + UNICODE_SPACE, + screenInfo.GetAttributes()); + } + CATCH_LOG(); } } return Status; @@ -1406,18 +1463,44 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool // Make sure the cursor doesn't move outside the viewport. screenInfo.GetViewport().Clamp(clampedPos); - // Make sure the cursor stays inside the margins, but only if it started there - if (screenInfo.AreMarginsSet() && screenInfo.IsCursorInMargins(cursor.GetPosition())) + // Make sure the cursor stays inside the margins + if (screenInfo.AreMarginsSet()) { - try + const auto margins = screenInfo.GetAbsoluteScrollMargins().ToInclusive(); + + const auto cursorY = cursor.GetPosition().Y; + + const auto lo = margins.Top; + const auto hi = margins.Bottom; + + // See microsoft/terminal#2929 - If the cursor is _below_ the top + // margin, it should stay below the top margin. If it's _above_ the + // bottom, it should stay above the bottom. Cursor movements that stay + // outside the margins shouldn't necessarily be affected. For example, + // moving up while below the bottom margin shouldn't just jump straight + // to the bottom margin. See + // ScreenBufferTests::CursorUpDownOutsideMargins for a test of that + // behavior. + const bool cursorBelowTop = cursorY >= lo; + const bool cursorAboveBottom = cursorY <= hi; + + if (cursorBelowTop) + { + try + { + clampedPos.Y = std::max(clampedPos.Y, lo); + } + CATCH_RETURN(); + } + + if (cursorAboveBottom) { - const auto margins = screenInfo.GetAbsoluteScrollMargins().ToInclusive(); - const auto v = clampedPos.Y; - const auto lo = margins.Top; - const auto hi = margins.Bottom; - clampedPos.Y = std::clamp(v, lo, hi); + try + { + clampedPos.Y = std::min(clampedPos.Y, hi); + } + CATCH_RETURN(); } - CATCH_RETURN(); } cursor.SetPosition(clampedPos); diff --git a/src/host/getset.h b/src/host/getset.h index 129bb8cd123..349c3c6b87c 100644 --- a/src/host/getset.h +++ b/src/host/getset.h @@ -60,6 +60,9 @@ void DoSrvPrivateSetConsoleRGBTextAttribute(SCREEN_INFORMATION& screenInfo, void DoSrvPrivateBoldText(SCREEN_INFORMATION& screenInfo, const bool bolded); +ExtendedAttributes DoSrvPrivateGetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo); +void DoSrvPrivateSetExtendedTextAttributes(SCREEN_INFORMATION& screenInfo, const ExtendedAttributes attrs); + [[nodiscard]] NTSTATUS DoSrvPrivateEraseAll(SCREEN_INFORMATION& screenInfo); void DoSrvSetCursorStyle(SCREEN_INFORMATION& screenInfo, diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index 9f1211ce9d9..f45fc8b1deb 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -273,6 +273,32 @@ BOOL ConhostInternalGetSet::PrivateBoldText(const bool bolded) return TRUE; } +// Method Description: +// - Retrieves the currently active ExtendedAttributes. See also +// DoSrvPrivateGetExtendedTextAttributes +// Arguments: +// - pAttrs: Recieves the ExtendedAttributes value. +// Return Value: +// - TRUE if successful (see DoSrvPrivateGetExtendedTextAttributes). FALSE otherwise. +BOOL ConhostInternalGetSet::PrivateGetExtendedTextAttributes(ExtendedAttributes* const pAttrs) +{ + *pAttrs = DoSrvPrivateGetExtendedTextAttributes(_io.GetActiveOutputBuffer()); + return TRUE; +} + +// Method Description: +// - Sets the active ExtendedAttributes of the active screen buffer. Text +// written to this buffer will be written with these attributes. +// Arguments: +// - extendedAttrs: The new ExtendedAttributes to use +// Return Value: +// - TRUE if successful (see DoSrvPrivateSetExtendedTextAttributes). FALSE otherwise. +BOOL ConhostInternalGetSet::PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) +{ + DoSrvPrivateSetExtendedTextAttributes(_io.GetActiveOutputBuffer(), attrs); + return TRUE; +} + // Routine Description: // - Connects the WriteConsoleInput API call directly into our Driver Message servicing call inside Conhost.exe // Arguments: diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index cc626e054ad..5cfb7bf8bbd 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -88,6 +88,8 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: const bool fIsForeground) override; BOOL PrivateBoldText(const bool bolded) override; + BOOL PrivateGetExtendedTextAttributes(ExtendedAttributes* const pAttrs) override; + BOOL PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) override; BOOL PrivateWriteConsoleInputW(_Inout_ std::deque>& events, _Out_ size_t& eventsWritten) override; diff --git a/src/host/screenInfo.cpp b/src/host/screenInfo.cpp index dc54614e9c0..ec384715803 100644 --- a/src/host/screenInfo.cpp +++ b/src/host/screenInfo.cpp @@ -2599,14 +2599,17 @@ OutputCellIterator SCREEN_INFORMATION::Write(const OutputCellIterator it) // Arguments: // - it - Iterator representing output cell data to write. // - target - The position to start writing at +// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data // Return Value: // - the iterator at its final position // Note: // - will throw exception on error. OutputCellIterator SCREEN_INFORMATION::Write(const OutputCellIterator it, - const COORD target) + const COORD target, + const std::optional wrap) { - return _textBuffer->Write(it, target); + // NOTE: if wrap = true/false, we want to set the line's wrap to true/false (respectively) if we reach the end of the line + return _textBuffer->Write(it, target, wrap); } // Routine Description: diff --git a/src/host/screenInfo.hpp b/src/host/screenInfo.hpp index d44a16e05d6..3c5c95b867e 100644 --- a/src/host/screenInfo.hpp +++ b/src/host/screenInfo.hpp @@ -131,7 +131,8 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console OutputCellIterator Write(const OutputCellIterator it); OutputCellIterator Write(const OutputCellIterator it, - const COORD target); + const COORD target, + const std::optional wrap = true); OutputCellIterator WriteRect(const OutputCellIterator it, const Microsoft::Console::Types::Viewport viewport); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index aa9195ce322..3281c08543c 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -105,6 +105,8 @@ class ScreenBufferTests TEST_METHOD(EraseAllTests); + TEST_METHOD(OutputNULTest); + TEST_METHOD(VtResize); TEST_METHOD(VtResizeComprehensive); TEST_METHOD(VtResizeDECCOLM); @@ -171,7 +173,7 @@ class ScreenBufferTests TEST_METHOD(DeleteLinesInMargins); TEST_METHOD(ReverseLineFeedInMargins); - TEST_METHOD(InsertDeleteLines256Colors); + TEST_METHOD(ScrollLines256Colors); TEST_METHOD(SetOriginMode); @@ -179,9 +181,18 @@ class ScreenBufferTests TEST_METHOD(RestoreDownAltBufferWithTerminalScrolling); + TEST_METHOD(SnapCursorWithTerminalScrolling); + TEST_METHOD(ClearAlternateBuffer); TEST_METHOD(InitializeTabStopsInVTMode); + + TEST_METHOD(TestExtendedTextAttributes); + TEST_METHOD(TestExtendedTextAttributesWithColors); + + TEST_METHOD(CursorUpDownAcrossMargins); + TEST_METHOD(CursorUpDownOutsideMargins); + TEST_METHOD(CursorUpDownExactlyAtMargins); }; void ScreenBufferTests::SingleAlternateBufferCreationTest() @@ -367,14 +378,14 @@ void ScreenBufferTests::TestReverseLineFeed() cursor.SetPosition({ 0, 5 }); VERIFY_SUCCEEDED(screenInfo.SetViewportOrigin(true, { 0, 5 }, true)); - stateMachine.ProcessString(L"ABCDEFGH", 9); - VERIFY_ARE_EQUAL(cursor.GetPosition().X, 9); + stateMachine.ProcessString(L"ABCDEFGH"); + VERIFY_ARE_EQUAL(cursor.GetPosition().X, 8); VERIFY_ARE_EQUAL(cursor.GetPosition().Y, 5); VERIFY_ARE_EQUAL(screenInfo.GetViewport().Top(), 5); LOG_IF_FAILED(DoSrvPrivateReverseLineFeed(screenInfo)); - VERIFY_ARE_EQUAL(cursor.GetPosition().X, 9); + VERIFY_ARE_EQUAL(cursor.GetPosition().X, 8); VERIFY_ARE_EQUAL(cursor.GetPosition().Y, 5); viewport = screenInfo.GetViewport(); VERIFY_ARE_EQUAL(viewport.Top(), 5); @@ -810,6 +821,45 @@ void ScreenBufferTests::EraseAllTests() viewport.BottomInclusive())); } +void ScreenBufferTests::OutputNULTest() +{ + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si._textBuffer->GetCursor(); + + VERIFY_ARE_EQUAL(0, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y); + + Log::Comment(NoThrowString().Format( + L"Writing a single NUL")); + stateMachine.ProcessString(L"\0", 1); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y); + + Log::Comment(NoThrowString().Format( + L"Writing many NULs")); + stateMachine.ProcessString(L"\0\0\0\0\0\0\0\0", 8); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y); + + Log::Comment(NoThrowString().Format( + L"Testing a single NUL followed by real text")); + stateMachine.ProcessString(L"\0foo", 4); + VERIFY_ARE_EQUAL(3, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\n"); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + + Log::Comment(NoThrowString().Format( + L"Writing NULs in between other strings")); + stateMachine.ProcessString(L"\0foo\0bar\0", 9); + VERIFY_ARE_EQUAL(6, cursor.GetPosition().X); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); +} + void ScreenBufferTests::VtResize() { // Run this test in isolation - for one reason or another, this breaks other tests. @@ -3994,10 +4044,10 @@ void ScreenBufferTests::ReverseLineFeedInMargins() } } -void ScreenBufferTests::InsertDeleteLines256Colors() +void ScreenBufferTests::ScrollLines256Colors() { BEGIN_TEST_METHOD_PROPERTIES() - TEST_METHOD_PROPERTY(L"Data:insert", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:scrollType", L"{0, 1, 2}") TEST_METHOD_PROPERTY(L"Data:colorStyle", L"{0, 1, 2}") END_TEST_METHOD_PROPERTIES(); @@ -4007,16 +4057,22 @@ void ScreenBufferTests::InsertDeleteLines256Colors() const int Use256Color = 1; const int UseRGBColor = 2; - bool insert; + // scrollType will be used to control whether we use InsertLines, + // DeleteLines, or ReverseIndex to scroll the contents of the buffer. + const int InsertLines = 0; + const int DeleteLines = 1; + const int ReverseLineFeed = 2; + + int scrollType; int colorStyle; - VERIFY_SUCCEEDED(TestData::TryGetValue(L"insert", insert), L"whether to insert(true) or delete(false) lines"); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"scrollType", scrollType), L"controls whether to use InsertLines, DeleteLines ot ReverseLineFeed"); VERIFY_SUCCEEDED(TestData::TryGetValue(L"colorStyle", colorStyle), L"controls whether to use the 16 color table, 256 table, or RGB colors"); // This test is largely taken from repro code from // https://github.com/microsoft/terminal/issues/832#issuecomment-507447272 Log::Comment( L"Sets the attributes to a 256/RGB color, then scrolls some lines with" - L" DL. Verifies the rows are cleared with the attributes we'd expect."); + L" IL/DL/RI. Verifies the rows are cleared with the attributes we'd expect."); auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); auto& si = gci.GetActiveOutputBuffer(); @@ -4052,8 +4108,22 @@ void ScreenBufferTests::InsertDeleteLines256Colors() // Move to home stateMachine.ProcessString(L"\x1b[H"); - // Insert/Delete 10 lines - stateMachine.ProcessString(insert ? L"\x1b[10L" : L"\x1b[10M"); + // Insert/Delete/Reverse Index 10 lines + std::wstring scrollSeq = L""; + if (scrollType == InsertLines) + { + scrollSeq = L"\x1b[10L"; + } + if (scrollType == DeleteLines) + { + scrollSeq = L"\x1b[10M"; + } + if (scrollType == ReverseLineFeed) + { + // This is 10 "Reverse Index" commands, which don't accept a parameter. + scrollSeq = L"\x1bM\x1bM\x1bM\x1bM\x1bM\x1bM\x1bM\x1bM\x1bM\x1bM"; + } + stateMachine.ProcessString(scrollSeq); Log::Comment(NoThrowString().Format( L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); @@ -4288,6 +4358,81 @@ void ScreenBufferTests::RestoreDownAltBufferWithTerminalScrolling() } } +void ScreenBufferTests::SnapCursorWithTerminalScrolling() +{ + // This is a test for microsoft/terminal#1222. Refer to that issue for more + // context + + auto& g = ServiceLocator::LocateGlobals(); + CONSOLE_INFORMATION& gci = g.getConsoleInformation(); + gci.SetTerminalScrolling(true); + gci.LockConsole(); // Lock must be taken to manipulate buffer. + auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); + + auto& si = gci.GetActiveOutputBuffer(); + auto& cursor = si.GetTextBuffer().GetCursor(); + const auto originalView = si._viewport; + si._virtualBottom = originalView.BottomInclusive(); + + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"originalView=%s", VerifyOutputTraits::ToString(originalView.ToInclusive()).GetBuffer())); + + Log::Comment(NoThrowString().Format( + L"First set the viewport somewhere lower in the buffer, as if the text " + L"was output there. Manually move the cursor there as well, so the " + L"cursor is within that viewport.")); + const COORD secondWindowOrigin{ 0, 10 }; + VERIFY_SUCCEEDED(si.SetViewportOrigin(true, secondWindowOrigin, true)); + si.GetTextBuffer().GetCursor().SetPosition(secondWindowOrigin); + + const auto secondView = si._viewport; + const auto secondVirtualBottom = si._virtualBottom; + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"secondView=%s", VerifyOutputTraits::ToString(secondView.ToInclusive()).GetBuffer())); + + VERIFY_ARE_EQUAL(10, secondView.Top()); + VERIFY_ARE_EQUAL(originalView.Height() + 10, secondView.BottomExclusive()); + VERIFY_ARE_EQUAL(originalView.Height() + 10 - 1, secondVirtualBottom); + + Log::Comment(NoThrowString().Format( + L"Emulate scrolling upwards with the mouse (not moving the virtual view)")); + + const COORD thirdWindowOrigin{ 0, 2 }; + VERIFY_SUCCEEDED(si.SetViewportOrigin(true, thirdWindowOrigin, false)); + + const auto thirdView = si._viewport; + const auto thirdVirtualBottom = si._virtualBottom; + + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"thirdView=%s", VerifyOutputTraits::ToString(thirdView.ToInclusive()).GetBuffer())); + + VERIFY_ARE_EQUAL(2, thirdView.Top()); + VERIFY_ARE_EQUAL(originalView.Height() + 2, thirdView.BottomExclusive()); + VERIFY_ARE_EQUAL(secondVirtualBottom, thirdVirtualBottom); + + Log::Comment(NoThrowString().Format( + L"Call SetConsoleCursorPosition to snap to the cursor")); + VERIFY_SUCCEEDED(g.api.SetConsoleCursorPositionImpl(si, secondWindowOrigin)); + + const auto fourthView = si._viewport; + const auto fourthVirtualBottom = si._virtualBottom; + + Log::Comment(NoThrowString().Format( + L"cursor=%s", VerifyOutputTraits::ToString(cursor.GetPosition()).GetBuffer())); + Log::Comment(NoThrowString().Format( + L"thirdView=%s", VerifyOutputTraits::ToString(fourthView.ToInclusive()).GetBuffer())); + + VERIFY_ARE_EQUAL(10, fourthView.Top()); + VERIFY_ARE_EQUAL(originalView.Height() + 10, fourthView.BottomExclusive()); + VERIFY_ARE_EQUAL(secondVirtualBottom, fourthVirtualBottom); +} + void ScreenBufferTests::ClearAlternateBuffer() { // This is a test for microsoft/terminal#1189. Refer to that issue for more @@ -4403,3 +4548,500 @@ void ScreenBufferTests::InitializeTabStopsInVTMode() VERIFY_IS_TRUE(gci.GetActiveOutputBuffer().AreTabsSet()); } + +void ScreenBufferTests::TestExtendedTextAttributes() +{ + // This is a test for microsoft/terminal#2554. Refer to that issue for more + // context. + + // We're going to set every possible combination of extended attributes via + // VT, then disable them, and make sure that they are all always represented + // internally correctly. + + // 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:italics", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:blink", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:invisible", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:crossedOut", L"{false, true}") + END_TEST_METHOD_PROPERTIES() + + bool bold, italics, blink, invisible, crossedOut; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"bold", bold)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"blink", blink)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"crossedOut", crossedOut)); + + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = tbi.GetCursor(); + + ExtendedAttributes expectedAttrs{ ExtendedAttributes::Normal }; + std::wstring vtSeq = L""; + + // Collect up a VT sequence to set the state given the method properties + if (bold) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::Bold); + vtSeq += L"\x1b[1m"; + } + if (italics) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::Italics); + vtSeq += L"\x1b[3m"; + } + if (blink) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::Blinking); + vtSeq += L"\x1b[5m"; + } + if (invisible) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::Invisible); + vtSeq += L"\x1b[8m"; + } + if (crossedOut) + { + WI_SetFlag(expectedAttrs, ExtendedAttributes::CrossedOut); + vtSeq += L"\x1b[9m"; + } + + // Helper lambda to write a VT sequence, then an "X", then check that the + // attributes of the "X" match what we think they should be. + auto validate = [&](const ExtendedAttributes expectedAttrs, + const std::wstring& vtSequence) { + auto cursorPos = cursor.GetPosition(); + + // Convert the vtSequence to something printable. Lets not set these + // attrs on the test console + std::wstring debugString = vtSequence; + { + size_t start_pos = 0; + while ((start_pos = debugString.find(L"\x1b", start_pos)) != std::string::npos) + { + debugString.replace(start_pos, 1, L"\\x1b"); + start_pos += 4; + } + } + + Log::Comment(NoThrowString().Format( + L"Testing string:\"%s\"", debugString.c_str())); + Log::Comment(NoThrowString().Format( + L"Expecting attrs:0x%02x", expectedAttrs)); + + stateMachine.ProcessString(vtSequence); + stateMachine.ProcessString(L"X"); + + auto iter = tbi.GetCellDataAt(cursorPos); + auto currentExtendedAttrs = iter->TextAttr().GetExtendedAttributes(); + VERIFY_ARE_EQUAL(expectedAttrs, currentExtendedAttrs); + }; + + // Check setting all the states collected above + validate(expectedAttrs, vtSeq); + + // One-by-one, turn off each of these states with VT, then check that the + // state matched. + if (bold) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::Bold); + vtSeq = L"\x1b[22m"; + validate(expectedAttrs, vtSeq); + } + if (italics) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::Italics); + vtSeq = L"\x1b[23m"; + validate(expectedAttrs, vtSeq); + } + if (blink) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::Blinking); + vtSeq = L"\x1b[25m"; + validate(expectedAttrs, vtSeq); + } + if (invisible) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::Invisible); + vtSeq = L"\x1b[28m"; + validate(expectedAttrs, vtSeq); + } + if (crossedOut) + { + WI_ClearFlag(expectedAttrs, ExtendedAttributes::CrossedOut); + vtSeq = L"\x1b[29m"; + validate(expectedAttrs, vtSeq); + } + + stateMachine.ProcessString(L"\x1b[0m"); +} + +void ScreenBufferTests::TestExtendedTextAttributesWithColors() +{ + // This is a test for microsoft/terminal#2554. Refer to that issue for more + // context. + + // We're going to set every possible combination of extended attributes via + // VT, then set assorted colors, then disable extended attrs, then reset + // colors, in various ways, and make sure that they are all always + // represented internally correctly. + + // 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:italics", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:blink", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:invisible", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:crossedOut", L"{false, true}") + TEST_METHOD_PROPERTY(L"Data:setForegroundType", L"{0, 1, 2, 3}") + TEST_METHOD_PROPERTY(L"Data:setBackgroundType", L"{0, 1, 2, 3}") + END_TEST_METHOD_PROPERTIES() + + // colorStyle will be used to control whether we use a color from the 16 + // color table, a color from the 256 color table, or a pure RGB color. + const int UseDefault = 0; + const int Use16Color = 1; + const int Use256Color = 2; + const int UseRGBColor = 3; + + bool bold, italics, blink, invisible, crossedOut; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"bold", bold)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"italics", italics)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"blink", blink)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"invisible", invisible)); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"crossedOut", crossedOut)); + + int setForegroundType, setBackgroundType; + VERIFY_SUCCEEDED(TestData::TryGetValue(L"setForegroundType", setForegroundType), L"controls whether to use the 16 color table, 256 table, or RGB colors"); + VERIFY_SUCCEEDED(TestData::TryGetValue(L"setBackgroundType", setBackgroundType), L"controls whether to use the 16 color table, 256 table, or RGB colors"); + + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = tbi.GetCursor(); + + TextAttribute expectedAttr{ si.GetAttributes() }; + ExtendedAttributes expectedExtendedAttrs{ ExtendedAttributes::Normal }; + std::wstring vtSeq = L""; + + // Collect up a VT sequence to set the state given the method properties + if (bold) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Bold); + vtSeq += L"\x1b[1m"; + } + if (italics) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Italics); + vtSeq += L"\x1b[3m"; + } + if (blink) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Blinking); + vtSeq += L"\x1b[5m"; + } + if (invisible) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::Invisible); + vtSeq += L"\x1b[8m"; + } + if (crossedOut) + { + WI_SetFlag(expectedExtendedAttrs, ExtendedAttributes::CrossedOut); + vtSeq += L"\x1b[9m"; + } + + // Prepare the foreground attributes + if (setForegroundType == UseDefault) + { + expectedAttr.SetDefaultForeground(); + vtSeq += L"\x1b[39m"; + } + else if (setForegroundType == Use16Color) + { + expectedAttr.SetIndexedAttributes({ static_cast(2) }, std::nullopt); + vtSeq += L"\x1b[32m"; + } + else if (setForegroundType == Use256Color) + { + expectedAttr.SetForeground(gci.GetColorTableEntry(20)); + vtSeq += L"\x1b[38;5;20m"; + } + else if (setForegroundType == UseRGBColor) + { + expectedAttr.SetForeground(RGB(1, 2, 3)); + vtSeq += L"\x1b[38;2;1;2;3m"; + } + + // Prepare the background attributes + if (setBackgroundType == UseDefault) + { + expectedAttr.SetDefaultBackground(); + vtSeq += L"\x1b[49m"; + } + else if (setBackgroundType == Use16Color) + { + expectedAttr.SetIndexedAttributes(std::nullopt, { static_cast(2) }); + vtSeq += L"\x1b[42m"; + } + else if (setBackgroundType == Use256Color) + { + expectedAttr.SetBackground(gci.GetColorTableEntry(20)); + vtSeq += L"\x1b[48;5;20m"; + } + else if (setBackgroundType == UseRGBColor) + { + expectedAttr.SetBackground(RGB(1, 2, 3)); + vtSeq += L"\x1b[48;2;1;2;3m"; + } + + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + + // Helper lambda to write a VT sequence, then an "X", then check that the + // attributes of the "X" match what we think they should be. + auto validate = [&](const TextAttribute attr, + const std::wstring& vtSequence) { + auto cursorPos = cursor.GetPosition(); + + // Convert the vtSequence to something printable. Lets not set these + // attrs on the test console + std::wstring debugString = vtSequence; + { + size_t start_pos = 0; + while ((start_pos = debugString.find(L"\x1b", start_pos)) != std::string::npos) + { + debugString.replace(start_pos, 1, L"\\x1b"); + start_pos += 4; + } + } + + Log::Comment(NoThrowString().Format( + L"Testing string:\"%s\"", debugString.c_str())); + Log::Comment(NoThrowString().Format( + L"Expecting attrs:0x%02x", VerifyOutputTraits::ToString(attr).GetBuffer())); + + stateMachine.ProcessString(vtSequence); + stateMachine.ProcessString(L"X"); + + auto iter = tbi.GetCellDataAt(cursorPos); + const TextAttribute currentAttrs = iter->TextAttr(); + VERIFY_ARE_EQUAL(attr, currentAttrs); + }; + + // Check setting all the states collected above + validate(expectedAttr, vtSeq); + + // One-by-one, turn off each of these states with VT, then check that the + // state matched. + if (bold) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Bold); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[22m"; + validate(expectedAttr, vtSeq); + } + if (italics) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Italics); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[23m"; + validate(expectedAttr, vtSeq); + } + if (blink) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Blinking); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[25m"; + validate(expectedAttr, vtSeq); + } + if (invisible) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::Invisible); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[28m"; + validate(expectedAttr, vtSeq); + } + if (crossedOut) + { + WI_ClearFlag(expectedExtendedAttrs, ExtendedAttributes::CrossedOut); + expectedAttr.SetExtendedAttributes(expectedExtendedAttrs); + vtSeq = L"\x1b[29m"; + validate(expectedAttr, vtSeq); + } + + stateMachine.ProcessString(L"\x1b[0m"); +} + +void ScreenBufferTests::CursorUpDownAcrossMargins() +{ + // Test inspired by: https://github.com/microsoft/terminal/issues/2929 + // echo -e "\e[6;19r\e[24H\e[99AX\e[1H\e[99BY\e[r" + // This does the following: + // * sets the top and bottom DECSTBM margins to 6 and 19 + // * moves to line 24 (i.e. below the bottom margin) + // * executes the CUU sequence with a count of 99, to move up 99 lines + // * writes out X + // * moves to line 1 (i.e. above the top margin) + // * executes the CUD sequence with a count of 99, to move down 99 lines + // * writes out Y + + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + VERIFY_IS_TRUE(si.GetViewport().BottomInclusive() > 24); + + // Set some scrolling margins + stateMachine.ProcessString(L"\x1b[6;19r"); + stateMachine.ProcessString(L"\x1b[24H"); + VERIFY_ARE_EQUAL(23, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[99A"); + VERIFY_ARE_EQUAL(5, cursor.GetPosition().Y); + stateMachine.ProcessString(L"X"); + { + auto iter = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"X", iter->Chars()); + } + stateMachine.ProcessString(L"\x1b[1H"); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[99B"); + VERIFY_ARE_EQUAL(18, cursor.GetPosition().Y); + stateMachine.ProcessString(L"Y"); + { + auto iter = tbi.GetCellDataAt({ 0, 18 }); + VERIFY_ARE_EQUAL(L"Y", iter->Chars()); + } + stateMachine.ProcessString(L"\x1b[r"); +} + +void ScreenBufferTests::CursorUpDownOutsideMargins() +{ + // Test inspired by the CursorUpDownAcrossMargins test. + // echo -e "\e[6;19r\e[24H\e[1AX\e[1H\e[1BY\e[r" + // This does the following: + // * sets the top and bottom DECSTBM margins to 6 and 19 + // * moves to line 24 (i.e. below the bottom margin) + // * executes the CUU sequence with a count of 1, to move up 1 lines (still below margins) + // * writes out X + // * moves to line 1 (i.e. above the top margin) + // * executes the CUD sequence with a count of 1, to move down 1 lines (still above margins) + // * writes out Y + + // This test is different becasue the end location of the vertical movement + // should not be within the margins at all. We should not clamp this + // movement to be within the margins. + + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + VERIFY_IS_TRUE(si.GetViewport().BottomInclusive() > 24); + + // Set some scrolling margins + stateMachine.ProcessString(L"\x1b[6;19r"); + stateMachine.ProcessString(L"\x1b[24H"); + VERIFY_ARE_EQUAL(23, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[1A"); + VERIFY_ARE_EQUAL(22, cursor.GetPosition().Y); + stateMachine.ProcessString(L"X"); + { + auto iter = tbi.GetCellDataAt({ 0, 22 }); + VERIFY_ARE_EQUAL(L"X", iter->Chars()); + } + stateMachine.ProcessString(L"\x1b[1H"); + VERIFY_ARE_EQUAL(0, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[1B"); + VERIFY_ARE_EQUAL(1, cursor.GetPosition().Y); + stateMachine.ProcessString(L"Y"); + { + auto iter = tbi.GetCellDataAt({ 0, 1 }); + VERIFY_ARE_EQUAL(L"Y", iter->Chars()); + } + stateMachine.ProcessString(L"\x1b[r"); +} + +void ScreenBufferTests::CursorUpDownExactlyAtMargins() +{ + // Test inspired by the CursorUpDownAcrossMargins test. + // echo -e "\e[6;19r\e[19H\e[1B1\e[1A2\e[6H\e[1A3\e[1B4\e[r" + // This does the following: + // * sets the top and bottom DECSTBM margins to 6 and 19 + // * moves to line 19 (i.e. on the bottom margin) + // * executes the CUD sequence with a count of 1, to move down 1 lines (still on the margin) + // * writes out 1 + // * executes the CUU sequence with a count of 1, to move up 1 lines (now inside margins) + // * writes out 2 + // * moves to line 6 (i.e. on the top margin) + // * executes the CUU sequence with a count of 1, to move up 1 lines (still on the margin) + // * writes out 3 + // * executes the CUD sequence with a count of 1, to move down 1 lines (still above margins) + // * writes out 4 + + // This test is different becasue the starting location for these scroll + // operations is _exactly_ on the margins + + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + auto& cursor = si.GetTextBuffer().GetCursor(); + + VERIFY_IS_TRUE(si.GetViewport().BottomInclusive() > 24); + + // Set some scrolling margins + stateMachine.ProcessString(L"\x1b[6;19r"); + + stateMachine.ProcessString(L"\x1b[19;1H"); + VERIFY_ARE_EQUAL(18, cursor.GetPosition().Y); + stateMachine.ProcessString(L"\x1b[1B"); + VERIFY_ARE_EQUAL(18, cursor.GetPosition().Y); + stateMachine.ProcessString(L"1"); + { + auto iter = tbi.GetCellDataAt({ 0, 18 }); + VERIFY_ARE_EQUAL(L"1", iter->Chars()); + } + + stateMachine.ProcessString(L"\x1b[1A"); + VERIFY_ARE_EQUAL(17, cursor.GetPosition().Y); + stateMachine.ProcessString(L"2"); + { + auto iter = tbi.GetCellDataAt({ 1, 17 }); + VERIFY_ARE_EQUAL(L"2", iter->Chars()); + } + + stateMachine.ProcessString(L"\x1b[6;1H"); + VERIFY_ARE_EQUAL(5, cursor.GetPosition().Y); + + stateMachine.ProcessString(L"\x1b[1A"); + VERIFY_ARE_EQUAL(5, cursor.GetPosition().Y); + stateMachine.ProcessString(L"3"); + { + auto iter = tbi.GetCellDataAt({ 0, 5 }); + VERIFY_ARE_EQUAL(L"3", iter->Chars()); + } + + stateMachine.ProcessString(L"\x1b[1B"); + VERIFY_ARE_EQUAL(6, cursor.GetPosition().Y); + stateMachine.ProcessString(L"4"); + { + auto iter = tbi.GetCellDataAt({ 1, 6 }); + VERIFY_ARE_EQUAL(L"4", iter->Chars()); + } + + stateMachine.ProcessString(L"\x1b[r"); +} diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp index 4a31e44697b..226b4ced32d 100644 --- a/src/host/ut_host/TextBufferTests.cpp +++ b/src/host/ut_host/TextBufferTests.cpp @@ -77,6 +77,8 @@ class TextBufferTests TEST_METHOD(TestWrapFlag); + TEST_METHOD(TestWrapThroughWriteLine); + TEST_METHOD(TestDoubleBytePadFlag); void DoBoundaryTest(PWCHAR const pwszInputString, @@ -197,6 +199,78 @@ void TextBufferTests::TestWrapFlag() VERIFY_IS_FALSE(Row.GetCharRow().WasWrapForced()); } +void TextBufferTests::TestWrapThroughWriteLine() +{ + TextBuffer& textBuffer = GetTbi(); + + auto VerifyWrap = [&](bool expected) { + ROW& Row = textBuffer._GetFirstRow(); + + if (expected) + { + VERIFY_IS_TRUE(Row.GetCharRow().WasWrapForced()); + } + else + { + VERIFY_IS_FALSE(Row.GetCharRow().WasWrapForced()); + } + }; + + // Construct string for testing + const auto width = textBuffer.GetSize().Width(); + std::wstring chars = L""; + for (auto i = 0; i < width; i++) + { + chars.append(L"a"); + } + const auto lineOfText = std::move(chars); + + Log::Comment(L"Case 1 : Implicit wrap (false)"); + { + TextAttribute expectedAttr(FOREGROUND_RED); + OutputCellIterator it(lineOfText, expectedAttr); + + textBuffer.WriteLine(it, { 0, 0 }); + VerifyWrap(false); + } + + Log::Comment(L"Case 2 : wrap = true"); + { + TextAttribute expectedAttr(FOREGROUND_RED); + OutputCellIterator it(lineOfText, expectedAttr); + + textBuffer.WriteLine(it, { 0, 0 }, true); + VerifyWrap(true); + } + + Log::Comment(L"Case 3: wrap = nullopt (remain as TRUE)"); + { + TextAttribute expectedAttr(FOREGROUND_RED); + OutputCellIterator it(lineOfText, expectedAttr); + + textBuffer.WriteLine(it, { 0, 0 }, std::nullopt); + VerifyWrap(true); + } + + Log::Comment(L"Case 4: wrap = false"); + { + TextAttribute expectedAttr(FOREGROUND_RED); + OutputCellIterator it(lineOfText, expectedAttr); + + textBuffer.WriteLine(it, { 0, 0 }, false); + VerifyWrap(false); + } + + Log::Comment(L"Case 5: wrap = nullopt (remain as false)"); + { + TextAttribute expectedAttr(FOREGROUND_RED); + OutputCellIterator it(lineOfText, expectedAttr); + + textBuffer.WriteLine(it, { 0, 0 }, std::nullopt); + VerifyWrap(false); + } +} + void TextBufferTests::TestDoubleBytePadFlag() { TextBuffer& textBuffer = GetTbi(); diff --git a/src/host/ut_host/VtRendererTests.cpp b/src/host/ut_host/VtRendererTests.cpp index 9aebf916c3b..437338cf546 100644 --- a/src/host/ut_host/VtRendererTests.cpp +++ b/src/host/ut_host/VtRendererTests.cpp @@ -121,12 +121,13 @@ class Microsoft::Console::Render::VtRendererTest TEST_METHOD(TestResize); + TEST_METHOD(TestCursorVisibility); + void Test16Colors(VtEngine* engine); std::deque qExpectedInput; bool WriteCallback(const char* const pch, size_t const cch); void TestPaint(VtEngine& engine, std::function pfn); - void TestPaintXterm(XtermEngine& engine, std::function pfn); Viewport SetUpViewport(); }; @@ -173,38 +174,6 @@ void VtRendererTest::TestPaint(VtEngine& engine, std::function pfn) VERIFY_SUCCEEDED(engine.EndPaint()); } -// Function Description: -// - Small helper to do a series of testing wrapped by StartPaint/EndPaint calls -// Also expects \x1b[?25l and \x1b[?25h on start/stop, for cursor visibility -// Arguments: -// - engine: the engine to operate on -// - pfn: A function pointer to some test code to run. -// Return Value: -// - -void VtRendererTest::TestPaintXterm(XtermEngine& engine, std::function pfn) -{ - HRESULT hr = engine.StartPaint(); - pfn(); - // If we didn't have anything to do on this frame, still execute our - // callback, but don't check for the following ?25h - if (hr != S_FALSE) - { - // If the engine has decided that it needs to disble the cursor, it'll - // insert ?25l to the front of the buffer (which won't hit this - // callback) and write ?25h to the end of the frame - if (engine._needToDisableCursor) - { - qExpectedInput.push_back("\x1b[?25h"); - } - } - - VERIFY_SUCCEEDED(engine.EndPaint()); - - VERIFY_ARE_EQUAL(qExpectedInput.size(), - static_cast(0), - L"Done painting, there shouldn't be any output we're still expecting"); -} - void VtRendererTest::VtSequenceHelperTests() { wil::unique_hfile hFile = wil::unique_hfile(INVALID_HANDLE_VALUE); @@ -298,7 +267,7 @@ void VtRendererTest::Xterm256TestInvalidate() L"Make sure that scrolling only invalidates part of the viewport, and sends the right sequences")); COORD scrollDelta = { 0, 1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled one down, only top line is invalid. ----")); invalid = view.ToExclusive(); @@ -314,7 +283,7 @@ void VtRendererTest::Xterm256TestInvalidate() scrollDelta = { 0, 3 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled three down, only top 3 lines are invalid. ----")); invalid = view.ToExclusive(); @@ -328,7 +297,7 @@ void VtRendererTest::Xterm256TestInvalidate() scrollDelta = { 0, -1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled one up, only bottom line is invalid. ----")); invalid = view.ToExclusive(); @@ -343,7 +312,7 @@ void VtRendererTest::Xterm256TestInvalidate() scrollDelta = { 0, -3 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled three up, only bottom 3 lines are invalid. ----")); invalid = view.ToExclusive(); @@ -363,7 +332,7 @@ void VtRendererTest::Xterm256TestInvalidate() VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); scrollDelta = { 0, 2 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled three down, only top 3 lines are invalid. ----")); invalid = view.ToExclusive(); @@ -421,25 +390,41 @@ void VtRendererTest::Xterm256TestColors() L"These values were picked for ease of formatting raw COLORREF values.")); qExpectedInput.push_back("\x1b[38;2;1;2;3m"); qExpectedInput.push_back("\x1b[48;2;5;6;7m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x00030201, 0x00070605, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x00030201, + 0x00070605, + 0, + ExtendedAttributes::Normal, + false)); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"----Change only the BG----")); qExpectedInput.push_back("\x1b[48;2;7;8;9m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x00030201, 0x00090807, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x00030201, + 0x00090807, + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the FG----")); qExpectedInput.push_back("\x1b[38;2;10;11;12m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x000c0b0a, 0x00090807, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x000c0b0a, + 0x00090807, + 0, + ExtendedAttributes::Normal, + false)); }); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Make sure that color setting persists across EndPaint/StartPaint")); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x000c0b0a, 0x00090807, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(0x000c0b0a, + 0x00090807, + 0, + ExtendedAttributes::Normal, + false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); @@ -451,41 +436,69 @@ void VtRendererTest::Xterm256TestColors() L"Begin by setting the default colors - FG,BG = BRIGHT_WHITE,DARK_BLACK")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"----Change only the BG----")); qExpectedInput.push_back("\x1b[41m"); // Background DARK_RED - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the FG----")); qExpectedInput.push_back("\x1b[37m"); // Foreground DARK_WHITE - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to something not in the table----")); qExpectedInput.push_back("\x1b[48;2;1;1;1m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + 0x010101, + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to the 'Default' background----")); qExpectedInput.push_back("\x1b[49m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Back to defaults----")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); }); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Make sure that color setting persists across EndPaint/StartPaint")); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); } @@ -557,8 +570,6 @@ void VtRendererTest::Xterm256TestCursor() L"----move to the start of this line (y stays the same)----")); qExpectedInput.push_back("\r"); VERIFY_SUCCEEDED(engine->_MoveCursor({ 0, 1 })); - - qExpectedInput.push_back("\x1b[?25h"); }); TestPaint(*engine, [&]() { @@ -629,7 +640,7 @@ void VtRendererTest::XtermTestInvalidate() L"Make sure that invalidating anything only invalidates that portion")); SMALL_RECT invalid = { 1, 1, 1, 1 }; VERIFY_SUCCEEDED(engine->Invalidate(&invalid)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { VERIFY_ARE_EQUAL(invalid, engine->_invalidRect.ToExclusive()); }); @@ -637,7 +648,7 @@ void VtRendererTest::XtermTestInvalidate() L"Make sure that scrolling only invalidates part of the viewport, and sends the right sequences")); COORD scrollDelta = { 0, 1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled one down, only top line is invalid. ----")); invalid = view.ToExclusive(); @@ -652,7 +663,7 @@ void VtRendererTest::XtermTestInvalidate() scrollDelta = { 0, 3 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled three down, only top 3 lines are invalid. ----")); invalid = view.ToExclusive(); @@ -666,7 +677,7 @@ void VtRendererTest::XtermTestInvalidate() scrollDelta = { 0, -1 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled one up, only bottom line is invalid. ----")); invalid = view.ToExclusive(); @@ -681,7 +692,7 @@ void VtRendererTest::XtermTestInvalidate() scrollDelta = { 0, -3 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled three up, only bottom 3 lines are invalid. ----")); invalid = view.ToExclusive(); @@ -701,7 +712,7 @@ void VtRendererTest::XtermTestInvalidate() VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); scrollDelta = { 0, 2 }; VERIFY_SUCCEEDED(engine->InvalidateScroll(&scrollDelta)); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"---- Scrolled three down, only top 3 lines are invalid. ----")); invalid = view.ToExclusive(); @@ -758,41 +769,65 @@ void VtRendererTest::XtermTestColors() L"Begin by setting the default colors - FG,BG = BRIGHT_WHITE,DARK_BLACK")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"----Change only the BG----")); qExpectedInput.push_back("\x1b[41m"); // Background DARK_RED - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the FG----")); qExpectedInput.push_back("\x1b[37m"); // Foreground DARK_WHITE - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to something not in the table----")); qExpectedInput.push_back("\x1b[40m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, ExtendedAttributes::Normal, false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to the 'Default' background----")); qExpectedInput.push_back("\x1b[40m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Back to defaults----")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); }); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Make sure that color setting persists across EndPaint/StartPaint")); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); } @@ -864,8 +899,6 @@ void VtRendererTest::XtermTestCursor() L"----move to the start of this line (y stays the same)----")); qExpectedInput.push_back("\r"); VERIFY_SUCCEEDED(engine->_MoveCursor({ 0, 1 })); - - qExpectedInput.push_back("\x1b[?25h"); }); TestPaint(*engine, [&]() { @@ -996,40 +1029,64 @@ void VtRendererTest::WinTelnetTestColors() L"Begin by setting the default colors - FG,BG = BRIGHT_WHITE,DARK_BLACK")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"----Change only the BG----")); qExpectedInput.push_back("\x1b[41m"); // Background DARK_RED - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the FG----")); qExpectedInput.push_back("\x1b[37m"); // Foreground DARK_WHITE - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[4], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[4], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to something not in the table----")); qExpectedInput.push_back("\x1b[40m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], 0x010101, 0, ExtendedAttributes::Normal, false)); Log::Comment(NoThrowString().Format( L"----Change only the BG to the 'Default' background----")); qExpectedInput.push_back("\x1b[40m"); // Background DARK_BLACK - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[7], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); Log::Comment(NoThrowString().Format( L"----Back to defaults----")); qExpectedInput.push_back("\x1b[m"); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); }); TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Make sure that color setting persists across EndPaint/StartPaint")); qExpectedInput.push_back(EMPTY_CALLBACK_SENTINEL); - VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], g_ColorTable[0], 0, false, false)); + VERIFY_SUCCEEDED(engine->UpdateDrawingBrushes(g_ColorTable[15], + g_ColorTable[0], + 0, + ExtendedAttributes::Normal, + false)); WriteCallback(EMPTY_CALLBACK_SENTINEL, 1); // This will make sure nothing was written to the callback }); } @@ -1130,14 +1187,14 @@ void VtRendererTest::TestWrapping() Viewport view = SetUpViewport(); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Make sure the cursor is at 0,0")); qExpectedInput.push_back("\x1b[H"); VERIFY_SUCCEEDED(engine->_MoveCursor({ 0, 0 })); }); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { Log::Comment(NoThrowString().Format( L"Painting a line that wrapped, then painting another line, and " L"making sure we don't manually move the cursor between those paints.")); @@ -1196,9 +1253,125 @@ void VtRendererTest::TestResize() VERIFY_SUCCEEDED(engine->UpdateViewport(newView.ToInclusive())); - TestPaintXterm(*engine, [&]() { + TestPaint(*engine, [&]() { VERIFY_ARE_EQUAL(newView, engine->_invalidRect); VERIFY_IS_FALSE(engine->_firstPaint); VERIFY_IS_FALSE(engine->_suppressResizeRepaint); }); } + +void VtRendererTest::TestCursorVisibility() +{ + Viewport view = SetUpViewport(); + wil::unique_hfile hFile = wil::unique_hfile(INVALID_HANDLE_VALUE); + auto engine = std::make_unique(std::move(hFile), _shutdownEvent, p, view, g_ColorTable, static_cast(COLOR_TABLE_SIZE)); + auto pfn = std::bind(&VtRendererTest::WriteCallback, this, std::placeholders::_1, std::placeholders::_2); + engine->SetTestCallback(pfn); + + // Verify the first paint emits a clear + qExpectedInput.push_back("\x1b[2J"); + VERIFY_IS_TRUE(engine->_firstPaint); + VERIFY_IS_FALSE(engine->_lastCursorIsVisible); + VERIFY_IS_TRUE(engine->_nextCursorIsVisible); + TestPaint(*engine, [&]() { + // During StartPaint, we'll mark the cursor as off. make sure that happens. + VERIFY_IS_FALSE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_firstPaint); + }); + + // The cursor wasn't painted in the last frame. + VERIFY_IS_FALSE(engine->_lastCursorIsVisible); + VERIFY_IS_FALSE(engine->_nextCursorIsVisible); + + COORD origin{ 0, 0 }; + + VERIFY_ARE_NOT_EQUAL(origin, engine->_lastText); + + IRenderEngine::CursorOptions options{}; + options.coordCursor = origin; + + // Frame 1: Paint the cursor at the home position. At the end of the frame, + // the cursor should be on. Because we're moving the cursor with CUP, we + // need to disable the cursor during this frame. + TestPaint(*engine, [&]() { + VERIFY_IS_FALSE(engine->_lastCursorIsVisible); + VERIFY_IS_FALSE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_needToDisableCursor); + + Log::Comment(NoThrowString().Format(L"Make sure the cursor is at 0,0")); + qExpectedInput.push_back("\x1b[H"); + VERIFY_SUCCEEDED(engine->PaintCursor(options)); + + VERIFY_IS_TRUE(engine->_nextCursorIsVisible); + VERIFY_IS_TRUE(engine->_needToDisableCursor); + + qExpectedInput.push_back("\x1b[?25h"); + }); + + VERIFY_IS_TRUE(engine->_lastCursorIsVisible); + VERIFY_IS_TRUE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_needToDisableCursor); + + // Frame 2: Paint the cursor again at the home position. At the end of the + // frame, the cursor should be on, the same as before. We aren't moving the + // cursor during this frame, so _needToDisableCursor will stay false. + TestPaint(*engine, [&]() { + VERIFY_IS_TRUE(engine->_lastCursorIsVisible); + VERIFY_IS_FALSE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_needToDisableCursor); + + Log::Comment(NoThrowString().Format(L"If we just paint the cursor again at the same position, the cursor should not need to be disabled")); + VERIFY_SUCCEEDED(engine->PaintCursor(options)); + + VERIFY_IS_TRUE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_needToDisableCursor); + }); + + VERIFY_IS_TRUE(engine->_lastCursorIsVisible); + VERIFY_IS_TRUE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_needToDisableCursor); + + // Frame 3: Paint the cursor at 2,2. At the end of the frame, the cursor + // should be on, the same as before. Because we're moving the cursor with + // CUP, we need to disable the cursor during this frame. + TestPaint(*engine, [&]() { + VERIFY_IS_TRUE(engine->_lastCursorIsVisible); + VERIFY_IS_FALSE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_needToDisableCursor); + + Log::Comment(NoThrowString().Format(L"Move the cursor to 2,2")); + qExpectedInput.push_back("\x1b[3;3H"); + + options.coordCursor = { 2, 2 }; + + VERIFY_SUCCEEDED(engine->PaintCursor(options)); + + VERIFY_IS_TRUE(engine->_lastCursorIsVisible); + VERIFY_IS_TRUE(engine->_nextCursorIsVisible); + VERIFY_IS_TRUE(engine->_needToDisableCursor); + + // Because _needToDisableCursor is true, we'll insert a ?25l at the + // start of the frame. Unfortunately, we can't test to make sure that + // it's there, but we can ensure that the matching ?25h is printed: + qExpectedInput.push_back("\x1b[?25h"); + }); + + VERIFY_IS_TRUE(engine->_lastCursorIsVisible); + VERIFY_IS_TRUE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_needToDisableCursor); + + // Frame 4: Don't paint the cursor. At the end of the frame, the cursor + // should be off. + Log::Comment(NoThrowString().Format(L"Painting without calling PaintCursor will hide the cursor")); + TestPaint(*engine, [&]() { + VERIFY_IS_TRUE(engine->_lastCursorIsVisible); + VERIFY_IS_FALSE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_needToDisableCursor); + + qExpectedInput.push_back("\x1b[?25l"); + }); + + VERIFY_IS_FALSE(engine->_lastCursorIsVisible); + VERIFY_IS_FALSE(engine->_nextCursorIsVisible); + VERIFY_IS_FALSE(engine->_needToDisableCursor); +} diff --git a/src/inc/conattrs.hpp b/src/inc/conattrs.hpp index 5e532519080..a168b173cb9 100644 --- a/src/inc/conattrs.hpp +++ b/src/inc/conattrs.hpp @@ -8,6 +8,21 @@ Licensed under the MIT license. #define BG_ATTRS (BACKGROUND_BLUE | BACKGROUND_GREEN | BACKGROUND_RED | BACKGROUND_INTENSITY) #define META_ATTRS (COMMON_LVB_LEADING_BYTE | COMMON_LVB_TRAILING_BYTE | COMMON_LVB_GRID_HORIZONTAL | COMMON_LVB_GRID_LVERTICAL | COMMON_LVB_GRID_RVERTICAL | COMMON_LVB_REVERSE_VIDEO | COMMON_LVB_UNDERSCORE) +enum class ExtendedAttributes : BYTE +{ + Normal = 0x00, + Bold = 0x01, + Italics = 0x02, + Blinking = 0x04, + Invisible = 0x08, + CrossedOut = 0x10, + // TODO:GH#2916 add support for these to the parser as well. + Underlined = 0x20, // _technically_ different from LVB_UNDERSCORE, see TODO:GH#2915 + DoublyUnderlined = 0x40, // Included for completeness, but not currently supported. + Faint = 0x80, // Included for completeness, but not currently supported. +}; +DEFINE_ENUM_FLAG_OPERATORS(ExtendedAttributes); + WORD FindNearestTableIndex(const COLORREF Color, _In_reads_(cColorTable) const COLORREF* const ColorTable, const WORD cColorTable); diff --git a/src/interactivity/onecore/BgfxEngine.cpp b/src/interactivity/onecore/BgfxEngine.cpp index 02eb42c5777..b73c769c7a5 100644 --- a/src/interactivity/onecore/BgfxEngine.cpp +++ b/src/interactivity/onecore/BgfxEngine.cpp @@ -196,7 +196,7 @@ BgfxEngine::BgfxEngine(PVOID SharedViewBase, LONG DisplayHeight, LONG DisplayWid [[nodiscard]] HRESULT BgfxEngine::UpdateDrawingBrushes(COLORREF const /*colorForeground*/, COLORREF const /*colorBackground*/, const WORD legacyColorAttribute, - const bool /*isBold*/, + const ExtendedAttributes /*extendedAttrs*/, bool const /*isSettingDefaultBrushes*/) noexcept { _currentLegacyColorAttribute = legacyColorAttribute; diff --git a/src/interactivity/onecore/BgfxEngine.hpp b/src/interactivity/onecore/BgfxEngine.hpp index 19a255503f3..9ce46e35708 100644 --- a/src/interactivity/onecore/BgfxEngine.hpp +++ b/src/interactivity/onecore/BgfxEngine.hpp @@ -60,7 +60,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(COLORREF const colorForeground, COLORREF const colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, bool const isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateDpi(int const iDpi) noexcept override; diff --git a/src/propsheet/PropSheetHandler.cpp b/src/propsheet/PropSheetHandler.cpp index d4b93c886e2..c6f2c5e412d 100644 --- a/src/propsheet/PropSheetHandler.cpp +++ b/src/propsheet/PropSheetHandler.cpp @@ -94,6 +94,12 @@ class ConsolePropertySheetHandler WrlFinal : public RuntimeClassfIsV2Console = GetConsoleBoolValue(CONSOLE_REGISTRY_FORCEV2, TRUE); + InitRegistryValues(gpStateInfo); gpStateInfo->Defaults = TRUE; GetRegistryValues(gpStateInfo); diff --git a/src/propsheet/console.cpp b/src/propsheet/console.cpp index cf3683dc8e0..6a40487514f 100644 --- a/src/propsheet/console.cpp +++ b/src/propsheet/console.cpp @@ -97,7 +97,10 @@ void SaveConsoleSettingsIfNeeded(const HWND hwnd) if (gpStateInfo->LinkTitle != NULL) { SetGlobalRegistryValues(); - if (!NT_SUCCESS(ShortcutSerialization::s_SetLinkValues(gpStateInfo, g_fEastAsianSystem, g_fForceV2))) + if (!NT_SUCCESS(ShortcutSerialization::s_SetLinkValues(gpStateInfo, + g_fEastAsianSystem, + g_fForceV2, + gpStateInfo->fIsV2Console))) { WCHAR szMessage[MAX_PATH + 100]; WCHAR awchBuffer[MAX_PATH] = { 0 }; diff --git a/src/propsheet/console.h b/src/propsheet/console.h index 8094ce31dba..7a98f78c28e 100644 --- a/src/propsheet/console.h +++ b/src/propsheet/console.h @@ -225,3 +225,5 @@ const unsigned int TERMINAL_PAGE_INDEX = 4; // number of property sheet pages static const int V1_NUMBER_OF_PAGES = 4; static const int NUMBER_OF_PAGES = 5; + +BOOL GetConsoleBoolValue(__in PCWSTR pszValueName, __in BOOL fDefault); diff --git a/src/propsheet/globals.cpp b/src/propsheet/globals.cpp index b7c59171cc9..01de6df00e9 100644 --- a/src/propsheet/globals.cpp +++ b/src/propsheet/globals.cpp @@ -9,6 +9,9 @@ LONG gcxScreen; LONG gcyScreen; BOOL g_fForceV2; +// If we didn't launch as a v2 console window, we don't want to persist v2 +// settings when we close, as they'll get zero'd. Use this to track the initial +// launch state. BOOL g_fEditKeys; BYTE g_bPreviewOpacity = 0x00; //sentinel value for initial test on dialog entry. Once initialized, won't be less than TRANSPARENCY_RANGE_MIN diff --git a/src/propsheet/registry.cpp b/src/propsheet/registry.cpp index 2914ae0998f..5c7a501d169 100644 --- a/src/propsheet/registry.cpp +++ b/src/propsheet/registry.cpp @@ -946,52 +946,59 @@ VOID SetRegistryValues( SetGlobalRegistryValues(); - // Save cursor type and color - dwValue = pStateInfo->CursorType; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_CURSORTYPE, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); + // Only save the "Terminal" settings if we launched as a v2 propsheet. The + // v1 console doesn't know anything about these settings, and their value + // will be incorrectly zero'd if we save in this state. + // See microsoft/terminal#2319 for more details. + if (gpStateInfo->fIsV2Console) + { + // Save cursor type and color + dwValue = pStateInfo->CursorType; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_CURSORTYPE, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); - dwValue = pStateInfo->CursorColor; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_CURSORCOLOR, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); + dwValue = pStateInfo->CursorColor; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_CURSORCOLOR, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); - dwValue = pStateInfo->InterceptCopyPaste; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_INTERCEPTCOPYPASTE, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); + dwValue = pStateInfo->InterceptCopyPaste; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_INTERCEPTCOPYPASTE, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); - dwValue = pStateInfo->TerminalScrolling; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_TERMINALSCROLLING, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); - dwValue = pStateInfo->DefaultForeground; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_DEFAULTFOREGROUND, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); - dwValue = pStateInfo->DefaultBackground; - LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, - hTitleKey, - CONSOLE_REGISTRY_DEFAULTBACKGROUND, - REG_DWORD, - (BYTE*)&dwValue, - sizeof(dwValue))); + dwValue = pStateInfo->TerminalScrolling; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_TERMINALSCROLLING, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); + dwValue = pStateInfo->DefaultForeground; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_DEFAULTFOREGROUND, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); + dwValue = pStateInfo->DefaultBackground; + LOG_IF_FAILED(RegistrySerialization::s_UpdateValue(hConsoleKey, + hTitleKey, + CONSOLE_REGISTRY_DEFAULTBACKGROUND, + REG_DWORD, + (BYTE*)&dwValue, + sizeof(dwValue))); + } // // Close the registry keys diff --git a/src/propslib/ShortcutSerialization.cpp b/src/propslib/ShortcutSerialization.cpp index a514de7c04d..39d21cc1a70 100644 --- a/src/propslib/ShortcutSerialization.cpp +++ b/src/propslib/ShortcutSerialization.cpp @@ -409,16 +409,19 @@ void ShortcutSerialization::s_GetLinkTitle(_In_ PCWSTR pwszShortcutFilename, return (SUCCEEDED(hr)) ? STATUS_SUCCESS : STATUS_UNSUCCESSFUL; } -/** -Writes the console properties out to the link it was opened from. -Arguments: - pStateInfo - pointer to structure containing information -Return Value: - A status code if something failed or S_OK -*/ +// Function Description: +// - Writes the console properties out to the link it was opened from. +// Arguments: +// - pStateInfo: pointer to structure containing information +// - writeTerminalSettings: If true, persist the "Terminal" properties only +// present in the v2 console. This should be false if called from a v11 +// console. See GH#2319 +// Return Value: +// - A status code if something failed or S_OK [[nodiscard]] NTSTATUS ShortcutSerialization::s_SetLinkValues(_In_ PCONSOLE_STATE_INFO pStateInfo, const BOOL fEastAsianSystem, - const BOOL fForceV2) + const BOOL fForceV2, + const bool writeTerminalSettings) { IShellLinkW* psl; IPersistFile* ppf; @@ -488,12 +491,21 @@ Return Value: s_SetLinkPropertyBoolValue(pps, PKEY_Console_CtrlKeyShortcutsDisabled, pStateInfo->fCtrlKeyShortcutsDisabled); s_SetLinkPropertyBoolValue(pps, PKEY_Console_LineSelection, pStateInfo->fLineSelection); s_SetLinkPropertyByteValue(pps, PKEY_Console_WindowTransparency, pStateInfo->bWindowTransparency); - s_SetLinkPropertyDwordValue(pps, PKEY_Console_CursorType, pStateInfo->CursorType); - s_SetLinkPropertyDwordValue(pps, PKEY_Console_CursorColor, pStateInfo->CursorColor); s_SetLinkPropertyBoolValue(pps, PKEY_Console_InterceptCopyPaste, pStateInfo->InterceptCopyPaste); - s_SetLinkPropertyDwordValue(pps, PKEY_Console_DefaultForeground, pStateInfo->DefaultForeground); - s_SetLinkPropertyDwordValue(pps, PKEY_Console_DefaultBackground, pStateInfo->DefaultBackground); - s_SetLinkPropertyBoolValue(pps, PKEY_Console_TerminalScrolling, pStateInfo->TerminalScrolling); + + // Only save the "Terminal" settings if we launched as a v2 + // propsheet. The v1 console doesn't know anything about + // these settings, and their value will be incorrectly + // zero'd if we save in this state. + // See microsoft/terminal#2319 for more details. + if (writeTerminalSettings) + { + s_SetLinkPropertyDwordValue(pps, PKEY_Console_CursorType, pStateInfo->CursorType); + s_SetLinkPropertyDwordValue(pps, PKEY_Console_CursorColor, pStateInfo->CursorColor); + s_SetLinkPropertyDwordValue(pps, PKEY_Console_DefaultForeground, pStateInfo->DefaultForeground); + s_SetLinkPropertyDwordValue(pps, PKEY_Console_DefaultBackground, pStateInfo->DefaultBackground); + s_SetLinkPropertyBoolValue(pps, PKEY_Console_TerminalScrolling, pStateInfo->TerminalScrolling); + } hr = pps->Commit(); pps->Release(); } diff --git a/src/propslib/ShortcutSerialization.hpp b/src/propslib/ShortcutSerialization.hpp index 27183b09c81..a21e57386f7 100644 --- a/src/propslib/ShortcutSerialization.hpp +++ b/src/propslib/ShortcutSerialization.hpp @@ -24,7 +24,7 @@ Revision History: class ShortcutSerialization { public: - [[nodiscard]] static NTSTATUS s_SetLinkValues(_In_ PCONSOLE_STATE_INFO pStateInfo, const BOOL fEastAsianSystem, const BOOL fForceV2); + [[nodiscard]] static NTSTATUS s_SetLinkValues(_In_ PCONSOLE_STATE_INFO pStateInfo, const BOOL fEastAsianSystem, const BOOL fForceV2, const bool writeTerminalSettings); [[nodiscard]] static NTSTATUS s_GetLinkConsoleProperties(_Inout_ PCONSOLE_STATE_INFO pStateInfo); [[nodiscard]] static NTSTATUS s_GetLinkValues(_Inout_ PCONSOLE_STATE_INFO pStateInfo, _Out_ BOOL* const pfReadConsoleProperties, diff --git a/src/renderer/base/renderer.cpp b/src/renderer/base/renderer.cpp index e434c5d0725..64e12c32ec6 100644 --- a/src/renderer/base/renderer.cpp +++ b/src/renderer/base/renderer.cpp @@ -872,11 +872,11 @@ void Renderer::_PaintSelection(_In_ IRenderEngine* const pEngine) const COLORREF rgbForeground = _pData->GetForegroundColor(textAttributes); const COLORREF rgbBackground = _pData->GetBackgroundColor(textAttributes); const WORD legacyAttributes = textAttributes.GetLegacyAttributes(); - const bool isBold = textAttributes.IsBold(); + const auto extendedAttrs = textAttributes.GetExtendedAttributes(); // The last color needs to be each engine's responsibility. If it's local to this function, // then on the next engine we might not update the color. - RETURN_IF_FAILED(pEngine->UpdateDrawingBrushes(rgbForeground, rgbBackground, legacyAttributes, isBold, isSettingDefaultBrushes)); + RETURN_IF_FAILED(pEngine->UpdateDrawingBrushes(rgbForeground, rgbBackground, legacyAttributes, extendedAttrs, isSettingDefaultBrushes)); return S_OK; } diff --git a/src/renderer/dx/DxRenderer.cpp b/src/renderer/dx/DxRenderer.cpp index 3d52a4b68d1..4c11cdfc533 100644 --- a/src/renderer/dx/DxRenderer.cpp +++ b/src/renderer/dx/DxRenderer.cpp @@ -1218,14 +1218,14 @@ enum class CursorPaintType // - colorForeground - Foreground brush color // - colorBackground - Background brush color // - legacyColorAttribute - -// - isBold - +// - extendedAttrs - // - isSettingDefaultBrushes - Lets us know that these are the default brushes to paint the swapchain background or selection // Return Value: // - S_OK or relevant DirectX error. [[nodiscard]] HRESULT DxEngine::UpdateDrawingBrushes(COLORREF const colorForeground, COLORREF const colorBackground, const WORD /*legacyColorAttribute*/, - const bool /*isBold*/, + const ExtendedAttributes /*extendedAttrs*/, bool const isSettingDefaultBrushes) noexcept { _foregroundColor = _ColorFFromColorRef(colorForeground); diff --git a/src/renderer/dx/DxRenderer.hpp b/src/renderer/dx/DxRenderer.hpp index f0960120316..1a2dd48fa32 100644 --- a/src/renderer/dx/DxRenderer.hpp +++ b/src/renderer/dx/DxRenderer.hpp @@ -80,7 +80,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(COLORREF const colorForeground, COLORREF const colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, bool const isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateDpi(int const iDpi) noexcept override; diff --git a/src/renderer/gdi/gdirenderer.hpp b/src/renderer/gdi/gdirenderer.hpp index 079c099ddfa..4840a181985 100644 --- a/src/renderer/gdi/gdirenderer.hpp +++ b/src/renderer/gdi/gdirenderer.hpp @@ -56,7 +56,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) noexcept override; diff --git a/src/renderer/gdi/state.cpp b/src/renderer/gdi/state.cpp index b55dc12f0e8..8ab7d3b8ed3 100644 --- a/src/renderer/gdi/state.cpp +++ b/src/renderer/gdi/state.cpp @@ -176,7 +176,7 @@ GdiEngine::~GdiEngine() // - colorForeground - Foreground Color // - colorBackground - Background colo // - legacyColorAttribute - -// - isBold - +// - extendedAttrs - // - isSettingDefaultBrushes - Lets us know that the default brushes are being set so we can update the DC background // and the hung app background painting color // Return Value: @@ -184,7 +184,7 @@ GdiEngine::~GdiEngine() [[nodiscard]] HRESULT GdiEngine::UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD /*legacyColorAttribute*/, - const bool /*isBold*/, + const ExtendedAttributes /*extendedAttrs*/, const bool isSettingDefaultBrushes) noexcept { RETURN_IF_FAILED(_FlushBufferLines()); diff --git a/src/renderer/inc/IRenderEngine.hpp b/src/renderer/inc/IRenderEngine.hpp index 2d321381318..aff56a5bb58 100644 --- a/src/renderer/inc/IRenderEngine.hpp +++ b/src/renderer/inc/IRenderEngine.hpp @@ -105,7 +105,7 @@ namespace Microsoft::Console::Render [[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept = 0; [[nodiscard]] virtual HRESULT UpdateFont(const FontInfoDesired& FontInfoDesired, _Out_ FontInfo& FontInfo) noexcept = 0; diff --git a/src/renderer/vt/VtSequences.cpp b/src/renderer/vt/VtSequences.cpp index 2990331a26a..a629c697b3e 100644 --- a/src/renderer/vt/VtSequences.cpp +++ b/src/renderer/vt/VtSequences.cpp @@ -345,3 +345,91 @@ using namespace Microsoft::Console::Render; { return _Write("\x1b[24m"); } + +// Method Description: +// - Writes a sequence to tell the terminal to start italicizing text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_BeginItalics() noexcept +{ + return _Write("\x1b[3m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to stop italicizing text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_EndItalics() noexcept +{ + return _Write("\x1b[23m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to start blinking text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_BeginBlink() noexcept +{ + return _Write("\x1b[5m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to stop blinking text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_EndBlink() noexcept +{ + return _Write("\x1b[25m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to start marking text as invisible +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_BeginInvisible() noexcept +{ + return _Write("\x1b[8m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to stop marking text as invisible +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_EndInvisible() noexcept +{ + return _Write("\x1b[28m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to start crossing-out text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_BeginCrossedOut() noexcept +{ + return _Write("\x1b[9m"); +} + +// Method Description: +// - Writes a sequence to tell the terminal to stop crossing-out text +// Arguments: +// - +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT VtEngine::_EndCrossedOut() noexcept +{ + return _Write("\x1b[29m"); +} diff --git a/src/renderer/vt/WinTelnetEngine.cpp b/src/renderer/vt/WinTelnetEngine.cpp index 614c66a3be6..c273232fd4a 100644 --- a/src/renderer/vt/WinTelnetEngine.cpp +++ b/src/renderer/vt/WinTelnetEngine.cpp @@ -29,6 +29,7 @@ WinTelnetEngine::WinTelnetEngine(_In_ wil::unique_hfile hPipe, // - colorBackground: The RGB Color to use to paint the background of the text. // - legacyColorAttribute: A console attributes bit field specifying the brush // colors we should use. +// - extendedAttrs - extended text attributes (italic, underline, etc.) to use. // - isSettingDefaultBrushes: indicates if we should change the background color of // the window. Unused for VT // Return Value: @@ -36,10 +37,14 @@ WinTelnetEngine::WinTelnetEngine(_In_ wil::unique_hfile hPipe, [[nodiscard]] HRESULT WinTelnetEngine::UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD /*legacyColorAttribute*/, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool /*isSettingDefaultBrushes*/) noexcept { - return VtEngine::_16ColorUpdateDrawingBrushes(colorForeground, colorBackground, isBold, _ColorTable, _cColorTable); + return VtEngine::_16ColorUpdateDrawingBrushes(colorForeground, + colorBackground, + WI_IsFlagSet(extendedAttrs, ExtendedAttributes::Bold), + _ColorTable, + _cColorTable); } // Routine Description: diff --git a/src/renderer/vt/WinTelnetEngine.hpp b/src/renderer/vt/WinTelnetEngine.hpp index d025d985cd8..8c27c221833 100644 --- a/src/renderer/vt/WinTelnetEngine.hpp +++ b/src/renderer/vt/WinTelnetEngine.hpp @@ -34,7 +34,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT ScrollFrame() noexcept override; diff --git a/src/renderer/vt/Xterm256Engine.cpp b/src/renderer/vt/Xterm256Engine.cpp index 7a2781d5f76..af8738a5650 100644 --- a/src/renderer/vt/Xterm256Engine.cpp +++ b/src/renderer/vt/Xterm256Engine.cpp @@ -14,7 +14,8 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, const Viewport initialViewport, _In_reads_(cColorTable) const COLORREF* const ColorTable, const WORD cColorTable) : - XtermEngine(std::move(hPipe), shutdownEvent, colorProvider, initialViewport, ColorTable, cColorTable, false) + XtermEngine(std::move(hPipe), shutdownEvent, colorProvider, initialViewport, ColorTable, cColorTable, false), + _lastExtendedAttrsState{ ExtendedAttributes::Normal } { } @@ -26,6 +27,7 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, // - colorBackground: The RGB Color to use to paint the background of the text. // - legacyColorAttribute: A console attributes bit field specifying the brush // colors we should use. +// - extendedAttrs - extended text attributes (italic, underline, etc.) to use. // - isSettingDefaultBrushes: indicates if we should change the background color of // the window. Unused for VT // Return Value: @@ -33,7 +35,7 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, [[nodiscard]] HRESULT Xterm256Engine::UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool /*isSettingDefaultBrushes*/) noexcept { //When we update the brushes, check the wAttrs to see if the LVB_UNDERSCORE @@ -42,11 +44,70 @@ Xterm256Engine::Xterm256Engine(_In_ wil::unique_hfile hPipe, // We have to do this here, instead of in PaintBufferGridLines, because // we'll have already painted the text by the time PaintBufferGridLines // is called. + // TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE RETURN_IF_FAILED(_UpdateUnderline(legacyColorAttribute)); + // Only do extended attributes in xterm-256color, as to not break telnet.exe. + RETURN_IF_FAILED(_UpdateExtendedAttrs(extendedAttrs)); + return VtEngine::_RgbUpdateDrawingBrushes(colorForeground, colorBackground, - isBold, + WI_IsFlagSet(extendedAttrs, ExtendedAttributes::Bold), _ColorTable, _cColorTable); } + +// Routine Description: +// - Write a VT sequence to either start or stop underlining text. +// Arguments: +// - legacyColorAttribute: A console attributes bit field containing information +// about the underlining state of the text. +// Return Value: +// - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. +[[nodiscard]] HRESULT Xterm256Engine::_UpdateExtendedAttrs(const ExtendedAttributes extendedAttrs) noexcept +{ + // Helper lambda to check if a state (attr) has changed since it's last + // value (lastState), and appropriately start/end that state with the given + // begin/end functions. + auto updateFlagAndState = [extendedAttrs, this](const ExtendedAttributes attr, + std::function beginFn, + std::function endFn) -> HRESULT { + const bool flagSet = WI_AreAllFlagsSet(extendedAttrs, attr); + const bool lastState = WI_AreAllFlagsSet(_lastExtendedAttrsState, attr); + if (flagSet != lastState) + { + if (flagSet) + { + RETURN_IF_FAILED(beginFn(this)); + } + else + { + RETURN_IF_FAILED(endFn(this)); + } + WI_SetAllFlags(_lastExtendedAttrsState, attr); + } + return S_OK; + }; + + auto hr = updateFlagAndState(ExtendedAttributes::Italics, + &Xterm256Engine::_BeginItalics, + &Xterm256Engine::_EndItalics); + RETURN_IF_FAILED(hr); + + hr = updateFlagAndState(ExtendedAttributes::Blinking, + &Xterm256Engine::_BeginBlink, + &Xterm256Engine::_EndBlink); + RETURN_IF_FAILED(hr); + + hr = updateFlagAndState(ExtendedAttributes::Invisible, + &Xterm256Engine::_BeginInvisible, + &Xterm256Engine::_EndInvisible); + RETURN_IF_FAILED(hr); + + hr = updateFlagAndState(ExtendedAttributes::CrossedOut, + &Xterm256Engine::_BeginCrossedOut, + &Xterm256Engine::_EndCrossedOut); + RETURN_IF_FAILED(hr); + + return S_OK; +} diff --git a/src/renderer/vt/Xterm256Engine.hpp b/src/renderer/vt/Xterm256Engine.hpp index 7ad288b3426..729ed12434c 100644 --- a/src/renderer/vt/Xterm256Engine.hpp +++ b/src/renderer/vt/Xterm256Engine.hpp @@ -35,10 +35,16 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept override; private: + [[nodiscard]] HRESULT _UpdateExtendedAttrs(const ExtendedAttributes extendedAttrs) noexcept; + + // We're only using Italics, Blinking, Invisible and Crossed Out for now + // See GH#2916 for adding a more complete implementation. + ExtendedAttributes _lastExtendedAttrsState; + #ifdef UNIT_TESTING friend class VtRendererTest; #endif diff --git a/src/renderer/vt/XtermEngine.cpp b/src/renderer/vt/XtermEngine.cpp index b84166cb107..b48dab303af 100644 --- a/src/renderer/vt/XtermEngine.cpp +++ b/src/renderer/vt/XtermEngine.cpp @@ -22,7 +22,9 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, _fUseAsciiOnly(fUseAsciiOnly), _previousLineWrapped(false), _usingUnderLine(false), - _needToDisableCursor(false) + _needToDisableCursor(false), + _lastCursorIsVisible(false), + _nextCursorIsVisible(true) { // Set out initial cursor position to -1, -1. This will force our initial // paint to manually move the cursor to 0, 0, not just ignore it. @@ -45,6 +47,11 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, _trace.TraceLastText(_lastText); + // Prep us to think that the cursor is not visible this frame. If it _is_ + // visible, then PaintCursor will be called, and we'll set this to true + // during the frame. + _nextCursorIsVisible = false; + if (_firstPaint) { // MSFT:17815688 @@ -73,14 +80,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, if (!_quickReturn) { - if (!_WillWriteSingleChar()) - { - // MSFT:TODO:20331739 - // Make sure to match the cursor visibility in the terminal to the console's - // // Turn off cursor - // RETURN_IF_FAILED(_HideCursor()); - } - else + if (_WillWriteSingleChar()) { // Don't re-enable the cursor. _quickReturn = true; @@ -99,22 +99,38 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // - S_OK if we succeeded, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT XtermEngine::EndPaint() noexcept { - // MSFT:TODO:20331739 - // Make sure to match the cursor visibility in the terminal to the console's - // if (!_quickReturn) - // { - // // Turn on cursor - // RETURN_IF_FAILED(_ShowCursor()); - // } - // If during the frame we determined that the cursor needed to be disabled, // then insert a cursor off at the start of the buffer, and re-enable // the cursor here. if (_needToDisableCursor) { - _buffer.insert(0, "\x1b[25l"); + // If the cursor was previously visible, let's hide it for this frame, + // by prepending a cursor off. + if (_lastCursorIsVisible) + { + _buffer.insert(0, "\x1b[25l"); + _lastCursorIsVisible = false; + } + // If the cursor was NOT previously visible, then that's fine! we don't + // need to worry, it's already off. + } + + // If the cursor is moving from off -> on (including cases where we just + // disabled if for this frame), show the cursor at the end of the frame + if (_nextCursorIsVisible && !_lastCursorIsVisible) + { RETURN_IF_FAILED(_ShowCursor()); } + // Otherwise, if the cursor previously was visible, and it should be hidden + // (on -> off), hide it at the end of the frame. + else if (!_nextCursorIsVisible && _lastCursorIsVisible) + { + RETURN_IF_FAILED(_HideCursor()); + } + + // Update our tracker of what we thought the last cursor state of the + // terminal was. + _lastCursorIsVisible = _nextCursorIsVisible; RETURN_IF_FAILED(VtEngine::EndPaint()); @@ -156,6 +172,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // - colorBackground: The RGB Color to use to paint the background of the text. // - legacyColorAttribute: A console attributes bit field specifying the brush // colors we should use. +// - extendedAttrs - extended text attributes (italic, underline, etc.) to use. // - isSettingDefaultBrushes: indicates if we should change the background color of // the window. Unused for VT // Return Value: @@ -163,7 +180,7 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, [[nodiscard]] HRESULT XtermEngine::UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool /*isSettingDefaultBrushes*/) noexcept { //When we update the brushes, check the wAttrs to see if the LVB_UNDERSCORE @@ -172,10 +189,35 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe, // We have to do this here, instead of in PaintBufferGridLines, because // we'll have already painted the text by the time PaintBufferGridLines // is called. - + // TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE RETURN_IF_FAILED(_UpdateUnderline(legacyColorAttribute)); // The base xterm mode only knows about 16 colors - return VtEngine::_16ColorUpdateDrawingBrushes(colorForeground, colorBackground, isBold, _ColorTable, _cColorTable); + return VtEngine::_16ColorUpdateDrawingBrushes(colorForeground, + colorBackground, + WI_IsFlagSet(extendedAttrs, ExtendedAttributes::Bold), + _ColorTable, + _cColorTable); +} + +// Routine Description: +// - Draws the cursor on the screen +// Arguments: +// - options - Options that affect the presentation of the cursor +// Return Value: +// - S_OK or suitable HRESULT error from writing pipe. +[[nodiscard]] HRESULT XtermEngine::PaintCursor(const IRenderEngine::CursorOptions& options) noexcept +{ + // PaintCursor is only called when the cursor is in fact visible in a single + // frame. When this is called, mark _nextCursorIsVisible as true. At the end + // of the frame, we'll decide to either turn the cursor on or not, based + // upon the previous state. + + // When this method is not called during a frame, it's because the cursor + // was not visible. In that case, at the end of the frame, + // _nextCursorIsVisible will still be false (from when we set it during + // StartPaint) + _nextCursorIsVisible = true; + return VtEngine::PaintCursor(options); } // Routine Description: diff --git a/src/renderer/vt/XtermEngine.hpp b/src/renderer/vt/XtermEngine.hpp index 0078030a5a5..2a482501d85 100644 --- a/src/renderer/vt/XtermEngine.hpp +++ b/src/renderer/vt/XtermEngine.hpp @@ -40,10 +40,12 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT StartPaint() noexcept override; [[nodiscard]] HRESULT EndPaint() noexcept override; + [[nodiscard]] HRESULT PaintCursor(const CursorOptions& options) noexcept override; + [[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT PaintBufferLine(std::basic_string_view const clusters, const COORD coord, @@ -61,6 +63,8 @@ namespace Microsoft::Console::Render bool _previousLineWrapped; bool _usingUnderLine; bool _needToDisableCursor; + bool _lastCursorIsVisible; + bool _nextCursorIsVisible; [[nodiscard]] HRESULT _MoveCursor(const COORD coord) noexcept override; diff --git a/src/renderer/vt/vtrenderer.hpp b/src/renderer/vt/vtrenderer.hpp index d95ecd5d87c..d2c7804a9fa 100644 --- a/src/renderer/vt/vtrenderer.hpp +++ b/src/renderer/vt/vtrenderer.hpp @@ -64,12 +64,12 @@ namespace Microsoft::Console::Render const COORD coordTarget) noexcept override; [[nodiscard]] HRESULT PaintSelection(const SMALL_RECT rect) noexcept override; - [[nodiscard]] HRESULT PaintCursor(const CursorOptions& options) noexcept override; + [[nodiscard]] virtual HRESULT PaintCursor(const CursorOptions& options) noexcept override; [[nodiscard]] virtual HRESULT UpdateDrawingBrushes(const COLORREF colorForeground, const COLORREF colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, const bool isSettingDefaultBrushes) noexcept = 0; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& pfiFontInfoDesired, _Out_ FontInfo& pfiFontInfo) noexcept override; @@ -172,9 +172,20 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT _ResizeWindow(const short sWidth, const short sHeight) noexcept; [[nodiscard]] HRESULT _BeginUnderline() noexcept; - [[nodiscard]] HRESULT _EndUnderline() noexcept; + [[nodiscard]] HRESULT _BeginItalics() noexcept; + [[nodiscard]] HRESULT _EndItalics() noexcept; + + [[nodiscard]] HRESULT _BeginBlink() noexcept; + [[nodiscard]] HRESULT _EndBlink() noexcept; + + [[nodiscard]] HRESULT _BeginInvisible() noexcept; + [[nodiscard]] HRESULT _EndInvisible() noexcept; + + [[nodiscard]] HRESULT _BeginCrossedOut() noexcept; + [[nodiscard]] HRESULT _EndCrossedOut() noexcept; + [[nodiscard]] HRESULT _RequestCursor() noexcept; [[nodiscard]] virtual HRESULT _MoveCursor(const COORD coord) noexcept = 0; diff --git a/src/renderer/wddmcon/WddmConRenderer.cpp b/src/renderer/wddmcon/WddmConRenderer.cpp index 150d4a1e6e7..23804ec31f2 100644 --- a/src/renderer/wddmcon/WddmConRenderer.cpp +++ b/src/renderer/wddmcon/WddmConRenderer.cpp @@ -307,7 +307,7 @@ bool WddmConEngine::IsInitialized() [[nodiscard]] HRESULT WddmConEngine::UpdateDrawingBrushes(COLORREF const /*colorForeground*/, COLORREF const /*colorBackground*/, const WORD legacyColorAttribute, - const bool /*isBold*/, + const ExtendedAttributes /*extendedAttrs*/, bool const /*isSettingDefaultBrushes*/) noexcept { _currentLegacyColorAttribute = legacyColorAttribute; diff --git a/src/renderer/wddmcon/WddmConRenderer.hpp b/src/renderer/wddmcon/WddmConRenderer.hpp index 714e977e3d3..c214d585a8e 100644 --- a/src/renderer/wddmcon/WddmConRenderer.hpp +++ b/src/renderer/wddmcon/WddmConRenderer.hpp @@ -52,7 +52,7 @@ namespace Microsoft::Console::Render [[nodiscard]] HRESULT UpdateDrawingBrushes(COLORREF const colorForeground, COLORREF const colorBackground, const WORD legacyColorAttribute, - const bool isBold, + const ExtendedAttributes extendedAttrs, bool const isSettingDefaultBrushes) noexcept override; [[nodiscard]] HRESULT UpdateFont(const FontInfoDesired& fiFontInfoDesired, FontInfo& fiFontInfo) noexcept override; [[nodiscard]] HRESULT UpdateDpi(int const iDpi) noexcept override; diff --git a/src/terminal/adapter/DispatchTypes.hpp b/src/terminal/adapter/DispatchTypes.hpp index e76349ce0c0..17f26c66d85 100644 --- a/src/terminal/adapter/DispatchTypes.hpp +++ b/src/terminal/adapter/DispatchTypes.hpp @@ -13,21 +13,28 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes Scrollback = 3 }; + // TODO:GH#2916 add support for DoublyUnderlined, Faint(2) to the adapter as well. enum GraphicsOptions : unsigned int { Off = 0, BoldBright = 1, - RGBColor = 2, - // 2 is also Faint, decreased intensity (ISO 6429). + // The 2 and 5 entries here are for BOTH the extended graphics options, + // as well as the Faint/Blink options. + RGBColorOrFaint = 2, // 2 is also Faint, decreased intensity (ISO 6429). + Italics = 3, Underline = 4, - Xterm256Index = 5, - // 5 is also Blink (appears as Bold). - // the 2 and 5 entries here are only for the extended graphics options - // as we do not currently support those features individually + BlinkOrXterm256Index = 5, // 5 is also Blink (appears as Bold). Negative = 7, + Invisible = 8, + CrossedOut = 9, + DoublyUnderlined = 21, UnBold = 22, + NotItalics = 23, NoUnderline = 24, - Positive = 27, + Steady = 25, // _not_ blink + Positive = 27, // _not_ inverse + Visible = 28, // _not_ invisible + NotCrossedOut = 29, ForegroundBlack = 30, ForegroundRed = 31, ForegroundGreen = 32, diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 7123e695087..d539eb8c551 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -6,19 +6,7 @@ #include "adaptDispatch.hpp" #include "conGetSet.hpp" #include "../../types/inc/Viewport.hpp" - -// Inspired from RETURN_IF_WIN32_BOOL_FALSE -// WIL doesn't include a RETURN_IF_FALSE, and RETURN_IF_WIN32_BOOL_FALSE -// will actually return the value of GLE. -#define RETURN_IF_FALSE(b) \ - do \ - { \ - BOOL __boolRet = wil::verify_bool(b); \ - if (!__boolRet) \ - { \ - return b; \ - } \ - } while (0, 0) +#include "../../types/inc/utils.hpp" using namespace Microsoft::Console::Types; using namespace Microsoft::Console::VirtualTerminal; @@ -504,15 +492,15 @@ bool AdaptDispatch::_InsertDeleteHelper(_In_ unsigned int const uiCount, const b { // We'll be doing short math on the distance since all console APIs use shorts. So check that we can successfully convert the uint into a short first. SHORT sDistance; - RETURN_IF_FALSE(SUCCEEDED(UIntToShort(uiCount, &sDistance))); + RETURN_BOOL_IF_FALSE(SUCCEEDED(UIntToShort(uiCount, &sDistance))); // get current cursor, attributes CONSOLE_SCREEN_BUFFER_INFOEX csbiex = { 0 }; csbiex.cbSize = sizeof(CONSOLE_SCREEN_BUFFER_INFOEX); // Make sure to reset the viewport (with MoveToBottom )to where it was // before the user scrolled the console output - RETURN_IF_FALSE(_conApi->MoveToBottom()); - RETURN_IF_FALSE(_conApi->GetConsoleScreenBufferInfoEx(&csbiex)); + RETURN_BOOL_IF_FALSE(_conApi->MoveToBottom()); + RETURN_BOOL_IF_FALSE(_conApi->GetConsoleScreenBufferInfoEx(&csbiex)); const auto cursor = csbiex.dwCursorPosition; // Rectangle to cut out of the existing buffer. This is inclusive. diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index a4fc702c5ce..255dd2c2e6e 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -170,10 +170,12 @@ namespace Microsoft::Console::VirtualTerminal bool _SetBoldColorHelper(const DispatchTypes::GraphicsOptions option); bool _SetDefaultColorHelper(const DispatchTypes::GraphicsOptions option); + bool _SetExtendedTextAttributeHelper(const DispatchTypes::GraphicsOptions option); static bool s_IsXtermColorOption(const DispatchTypes::GraphicsOptions opt); static bool s_IsRgbColorOption(const DispatchTypes::GraphicsOptions opt); static bool s_IsBoldColorOption(const DispatchTypes::GraphicsOptions opt) noexcept; static bool s_IsDefaultColorOption(const DispatchTypes::GraphicsOptions opt) noexcept; + static bool s_IsExtendedTextAttribute(const DispatchTypes::GraphicsOptions opt) noexcept; }; } diff --git a/src/terminal/adapter/adaptDispatchGraphics.cpp b/src/terminal/adapter/adaptDispatchGraphics.cpp index f676e08adac..85076d08238 100644 --- a/src/terminal/adapter/adaptDispatchGraphics.cpp +++ b/src/terminal/adapter/adaptDispatchGraphics.cpp @@ -5,6 +5,7 @@ #include "adaptDispatch.hpp" #include "conGetSet.hpp" +#include "../../types/inc/utils.hpp" #define ENABLE_INTSAFE_SIGNED_FUNCTIONS #include @@ -36,7 +37,9 @@ void AdaptDispatch::s_DisableAllColors(_Inout_ WORD* const pAttr, const bool fIs // Arguments: // - pAttr - Pointer to font attributes field to adjust // - wApplyThis - Color values to apply to the low or high word of the font attributes field. -// - fIsForeground - TRUE = foreground color. FALSE = background color. Specifies which half of the bit field to reset and then apply wApplyThis upon. +// - fIsForeground - TRUE = foreground color. FALSE = background color. +// Specifies which half of the bit field to reset and then apply wApplyThis +// upon. // Return Value: // - void AdaptDispatch::s_ApplyColors(_Inout_ WORD* const pAttr, const WORD wApplyThis, const bool fIsForeground) @@ -62,7 +65,9 @@ void AdaptDispatch::s_ApplyColors(_Inout_ WORD* const pAttr, const WORD wApplyTh // Routine Description: // - Helper to apply the actual flags to each text attributes field. -// - Placed as a helper so it can be recursive/re-entrant for some of the convenience flag methods that perform similar/multiple operations in one command. +// - Placed as a helper so it can be recursive/re-entrant for some of the +// convenience flag methods that perform similar/multiple operations in one +// command. // Arguments: // - opt - Graphics option sent to us by the parser/requestor. // - pAttr - Pointer to the font attribute field to adjust @@ -83,6 +88,7 @@ void AdaptDispatch::_SetGraphicsOptionHelper(const DispatchTypes::GraphicsOption _fChangedMetaAttrs = true; break; case DispatchTypes::GraphicsOptions::Underline: + // TODO:GH#2915 Treat underline separately from LVB_UNDERSCORE *pAttr |= COMMON_LVB_UNDERSCORE; _fChangedMetaAttrs = true; break; @@ -261,6 +267,30 @@ void AdaptDispatch::_SetGraphicsOptionHelper(const DispatchTypes::GraphicsOption } } +// Routine Description: +// Returns true if the GraphicsOption represents an extended text attribute. +// These include things such as Underlined, Italics, Blinking, etc. +// Return Value: +// - true if the opt is the indicator for an extended text attribute, false otherwise. +bool AdaptDispatch::s_IsExtendedTextAttribute(const DispatchTypes::GraphicsOptions opt) noexcept +{ + // TODO:GH#2916 add support for DoublyUnderlined, Faint(RGBColorOrFaint). + // These two are currently partially implemented as other things: + // * Faint is approximately the opposite of bold does, though it's much + // [more complicated]( + // https://github.com/microsoft/terminal/issues/2916#issuecomment-535860910) + // and less supported/used. + // * Doubly underlined should exist in a trinary state with Underlined + return opt == DispatchTypes::GraphicsOptions::Italics || + opt == DispatchTypes::GraphicsOptions::NotItalics || + opt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index || + opt == DispatchTypes::GraphicsOptions::Steady || + opt == DispatchTypes::GraphicsOptions::Invisible || + opt == DispatchTypes::GraphicsOptions::Visible || + opt == DispatchTypes::GraphicsOptions::CrossedOut || + opt == DispatchTypes::GraphicsOptions::NotCrossedOut; +} + // Routine Description: // Returns true if the GraphicsOption represents an extended color option. // These are followed by up to 4 more values which compose the entire option. @@ -338,7 +368,7 @@ bool AdaptDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTypes *pfIsForeground = false; } - if (typeOpt == DispatchTypes::GraphicsOptions::RGBColor && cOptions >= 5) + if (typeOpt == DispatchTypes::GraphicsOptions::RGBColorOrFaint && cOptions >= 5) { *pcOptionsConsumed = 5; // ensure that each value fits in a byte @@ -350,7 +380,7 @@ bool AdaptDispatch::_SetRgbColorsHelper(_In_reads_(cOptions) const DispatchTypes fSuccess = !!_conApi->SetConsoleRGBTextAttribute(*prgbColor, *pfIsForeground); } - else if (typeOpt == DispatchTypes::GraphicsOptions::Xterm256Index && cOptions >= 3) + else if (typeOpt == DispatchTypes::GraphicsOptions::BlinkOrXterm256Index && cOptions >= 3) { *pcOptionsConsumed = 3; if (rgOptions[2] <= 255) // ensure that the provided index is on the table @@ -374,31 +404,92 @@ bool AdaptDispatch::_SetDefaultColorHelper(const DispatchTypes::GraphicsOptions { const bool fg = option == GraphicsOptions::Off || option == GraphicsOptions::ForegroundDefault; const bool bg = option == GraphicsOptions::Off || option == GraphicsOptions::BackgroundDefault; + bool success = _conApi->PrivateSetDefaultAttributes(fg, bg); + if (success && fg && bg) { // If we're resetting both the FG & BG, also reset the meta attributes (underline) // as well as the boldness success = _conApi->PrivateSetLegacyAttributes(0, false, false, true) && - _conApi->PrivateBoldText(false); + _conApi->PrivateBoldText(false) && + _conApi->PrivateSetExtendedTextAttributes(ExtendedAttributes::Normal); } return success; } +// Method Description: +// - Sets the attributes for extended text attributes. Retrieves the current +// extended attrs from the console, modifies them according to the new +// GraphicsOption, and the sets them again. +// - Notably does _not_ handle Bold, Faint, Underline, DoublyUnderlined, or +// NoUnderline. Those should be handled in TODO:GH#2916. +// Arguments: +// - opt: the graphics option to set +// Return Value: +// - True if handled successfully. False otherwise. +bool AdaptDispatch::_SetExtendedTextAttributeHelper(const DispatchTypes::GraphicsOptions opt) +{ + ExtendedAttributes attrs{ ExtendedAttributes::Normal }; + + RETURN_BOOL_IF_FALSE(_conApi->PrivateGetExtendedTextAttributes(&attrs)); + + switch (opt) + { + case DispatchTypes::GraphicsOptions::Italics: + WI_SetFlag(attrs, ExtendedAttributes::Italics); + break; + case DispatchTypes::GraphicsOptions::NotItalics: + WI_ClearFlag(attrs, ExtendedAttributes::Italics); + break; + case DispatchTypes::GraphicsOptions::BlinkOrXterm256Index: + WI_SetFlag(attrs, ExtendedAttributes::Blinking); + break; + case DispatchTypes::GraphicsOptions::Steady: + WI_ClearFlag(attrs, ExtendedAttributes::Blinking); + break; + case DispatchTypes::GraphicsOptions::Invisible: + WI_SetFlag(attrs, ExtendedAttributes::Invisible); + break; + case DispatchTypes::GraphicsOptions::Visible: + WI_ClearFlag(attrs, ExtendedAttributes::Invisible); + break; + case DispatchTypes::GraphicsOptions::CrossedOut: + WI_SetFlag(attrs, ExtendedAttributes::CrossedOut); + break; + case DispatchTypes::GraphicsOptions::NotCrossedOut: + WI_ClearFlag(attrs, ExtendedAttributes::CrossedOut); + break; + // TODO:GH#2916 add support for the following + // case DispatchTypes::GraphicsOptions::DoublyUnderlined: + // case DispatchTypes::GraphicsOptions::RGBColorOrFaint: + // case DispatchTypes::GraphicsOptions::DoublyUnderlined: + } + + return _conApi->PrivateSetExtendedTextAttributes(attrs); +} + // Routine Description: -// - SGR - Modifies the graphical rendering options applied to the next characters written into the buffer. -// - Options include colors, invert, underlines, and other "font style" type options. +// - SGR - Modifies the graphical rendering options applied to the next +// characters written into the buffer. +// - Options include colors, invert, underlines, and other "font style" +// type options. + // Arguments: -// - rgOptions - An array of options that will be applied from 0 to N, in order, one at a time by setting or removing flags in the font style properties. +// - rgOptions - An array of options that will be applied from 0 to N, in order, +// one at a time by setting or removing flags in the font style properties. // - cOptions - The count of options (a.k.a. the N in the above line of comments) // Return Value: // - True if handled successfully. False otherwise. -bool AdaptDispatch::SetGraphicsRendition(_In_reads_(cOptions) const DispatchTypes::GraphicsOptions* const rgOptions, const size_t cOptions) +bool AdaptDispatch::SetGraphicsRendition(_In_reads_(cOptions) const DispatchTypes::GraphicsOptions* const rgOptions, + const size_t cOptions) { - // We use the private function here to get just the default color attributes as a performance optimization. - // Calling the public GetConsoleScreenBufferInfoEx costs a lot of performance time/power in a tight loop - // because it has to fill the Largest Window Size by asking the OS and wastes time memcpying colors and other data - // we do not need to resolve this Set Graphics Rendition request. + // We use the private function here to get just the default color attributes + // as a performance optimization. Calling the public + // GetConsoleScreenBufferInfoEx costs a lot of performance time/power in a + // tight loop because it has to fill the Largest Window Size by asking the + // OS and wastes time memcpying colors and other data we do not need to + // resolve this Set Graphics Rendition request. WORD attr; bool fSuccess = !!_conApi->PrivateGetConsoleScreenBufferAttributes(&attr); @@ -416,6 +507,10 @@ bool AdaptDispatch::SetGraphicsRendition(_In_reads_(cOptions) const DispatchType { fSuccess = _SetBoldColorHelper(rgOptions[i]); } + else if (s_IsExtendedTextAttribute(opt)) + { + fSuccess = _SetExtendedTextAttributeHelper(rgOptions[i]); + } else if (s_IsRgbColorOption(opt)) { COLORREF rgbColor; @@ -424,14 +519,21 @@ bool AdaptDispatch::SetGraphicsRendition(_In_reads_(cOptions) const DispatchType size_t cOptionsConsumed = 0; // _SetRgbColorsHelper will call the appropriate ConApi function - fSuccess = _SetRgbColorsHelper(&(rgOptions[i]), cOptions - i, &rgbColor, &fIsForeground, &cOptionsConsumed); + fSuccess = _SetRgbColorsHelper(&(rgOptions[i]), + cOptions - i, + &rgbColor, + &fIsForeground, + &cOptionsConsumed); i += (cOptionsConsumed - 1); // cOptionsConsumed includes the opt we're currently on. } else { _SetGraphicsOptionHelper(opt, &attr); - fSuccess = !!_conApi->PrivateSetLegacyAttributes(attr, _fChangedForeground, _fChangedBackground, _fChangedMetaAttrs); + fSuccess = !!_conApi->PrivateSetLegacyAttributes(attr, + _fChangedForeground, + _fChangedBackground, + _fChangedMetaAttrs); // Make sure we un-bold if (fSuccess && opt == DispatchTypes::GraphicsOptions::Off) diff --git a/src/terminal/adapter/conGetSet.hpp b/src/terminal/adapter/conGetSet.hpp index 37b85596cab..b337fe2c7d5 100644 --- a/src/terminal/adapter/conGetSet.hpp +++ b/src/terminal/adapter/conGetSet.hpp @@ -52,6 +52,8 @@ namespace Microsoft::Console::VirtualTerminal const bool fIsForeground) = 0; virtual BOOL SetConsoleRGBTextAttribute(const COLORREF rgbColor, const bool fIsForeground) = 0; virtual BOOL PrivateBoldText(const bool bolded) = 0; + virtual BOOL PrivateGetExtendedTextAttributes(ExtendedAttributes* const pAttrs) = 0; + virtual BOOL PrivateSetExtendedTextAttributes(const ExtendedAttributes attrs) = 0; virtual BOOL PrivateWriteConsoleInputW(_Inout_ std::deque>& events, _Out_ size_t& eventsWritten) = 0; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index b2a6f4bb88c..09258aca34b 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -339,6 +339,18 @@ class TestGetSet final : public ConGetSet return !!_fPrivateBoldTextResult; } + BOOL PrivateGetExtendedTextAttributes(ExtendedAttributes* const /*pAttrs*/) + { + Log::Comment(L"PrivateGetExtendedTextAttributes MOCK called..."); + return true; + } + + BOOL PrivateSetExtendedTextAttributes(const ExtendedAttributes /*attrs*/) + { + Log::Comment(L"PrivateSetExtendedTextAttributes MOCK called..."); + return true; + } + BOOL PrivateWriteConsoleInputW(_Inout_ std::deque>& events, _Out_ size_t& eventsWritten) override { @@ -2630,7 +2642,7 @@ class AdapterTest Log::Comment(L"Test 1: Change Foreground"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)2; // Green _testGetSet->_wExpectedAttribute = FOREGROUND_GREEN; _testGetSet->_iExpectedXtermTableEntry = 2; @@ -2640,7 +2652,7 @@ class AdapterTest Log::Comment(L"Test 2: Change Background"); rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)9; // Bright Red _testGetSet->_wExpectedAttribute = FOREGROUND_GREEN | BACKGROUND_RED | BACKGROUND_INTENSITY; _testGetSet->_iExpectedXtermTableEntry = 9; @@ -2650,7 +2662,7 @@ class AdapterTest Log::Comment(L"Test 3: Change Foreground to RGB color"); rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)42; // Arbitrary Color _testGetSet->_iExpectedXtermTableEntry = 42; _testGetSet->_fExpectedIsForeground = true; @@ -2659,7 +2671,7 @@ class AdapterTest Log::Comment(L"Test 4: Change Background to RGB color"); rgOptions[0] = DispatchTypes::GraphicsOptions::BackgroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)142; // Arbitrary Color _testGetSet->_iExpectedXtermTableEntry = 142; _testGetSet->_fExpectedIsForeground = false; @@ -2671,7 +2683,7 @@ class AdapterTest // to have its own color table and translate the pre-existing RGB BG into a legacy BG. // Fortunately, the ft_api:RgbColorTests IS smart enough to test that. rgOptions[0] = DispatchTypes::GraphicsOptions::ForegroundExtended; - rgOptions[1] = DispatchTypes::GraphicsOptions::Xterm256Index; + rgOptions[1] = DispatchTypes::GraphicsOptions::BlinkOrXterm256Index; rgOptions[2] = (DispatchTypes::GraphicsOptions)9; // Bright Red _testGetSet->_wExpectedAttribute = FOREGROUND_RED | FOREGROUND_INTENSITY | BACKGROUND_RED | BACKGROUND_INTENSITY; _testGetSet->_iExpectedXtermTableEntry = 9; diff --git a/src/terminal/parser/OutputStateMachineEngine.cpp b/src/terminal/parser/OutputStateMachineEngine.cpp index a17699a5247..66daa8fd23d 100644 --- a/src/terminal/parser/OutputStateMachineEngine.cpp +++ b/src/terminal/parser/OutputStateMachineEngine.cpp @@ -42,8 +42,27 @@ ITermDispatch& OutputStateMachineEngine::Dispatch() noexcept // - true iff we successfully dispatched the sequence. bool OutputStateMachineEngine::ActionExecute(const wchar_t wch) { + // microsoft/terminal#1825 - VT applications expect to be able to write NUL + // and have _nothing_ happen. Filter the NULs here, so they don't fill the + // buffer with empty spaces. + if (wch == AsciiChars::NUL) + { + return true; + } + _dispatch->Execute(wch); _ClearLastChar(); + + if (wch == AsciiChars::BEL) + { + // microsoft/terminal#2952 + // If we're attached to a terminal, let's also pass the BEL through. + if (_pfnFlushToTerminal != nullptr) + { + _pfnFlushToTerminal(); + } + } + return true; } diff --git a/src/terminal/parser/stateMachine.cpp b/src/terminal/parser/stateMachine.cpp index 99b49a58653..0a656342b7f 100644 --- a/src/terminal/parser/stateMachine.cpp +++ b/src/terminal/parser/stateMachine.cpp @@ -25,7 +25,8 @@ StateMachine::StateMachine(IStateMachineEngine* const pEngine) : // rgusParams Initialized below _sOscNextChar(0), _sOscParam(0), - _currRunLength(0) + _currRunLength(0), + _fProcessingIndividually(false) { ZeroMemory(_pwchOscStringBuffer, sizeof(_pwchOscStringBuffer)); ZeroMemory(_rgusParams, sizeof(_rgusParams)); @@ -1346,20 +1347,16 @@ void StateMachine::ProcessString(const wchar_t* const rgwch, const size_t cch) _pwchSequenceStart = rgwch; _currRunLength = 0; - // This should be static, because if one string starts a sequence, and the next finishes it, - // we want the partial sequence state to persist. - static bool s_fProcessIndividually = false; - for (size_t cchCharsRemaining = cch; cchCharsRemaining > 0; cchCharsRemaining--) { - if (s_fProcessIndividually) + if (_fProcessingIndividually) { // If we're processing characters individually, send it to the state machine. ProcessCharacter(*_pwchCurr); _pwchCurr++; if (_state == VTStates::Ground) // Then check if we're back at ground. If we are, the next character (pwchCurr) { // is the start of the next run of characters that might be printable. - s_fProcessIndividually = false; + _fProcessingIndividually = false; _pwchSequenceStart = _pwchCurr; _currRunLength = 0; } @@ -1371,13 +1368,13 @@ void StateMachine::ProcessString(const wchar_t* const rgwch, const size_t cch) FAIL_FAST_IF(!(_pwchSequenceStart + _currRunLength <= rgwch + cch)); _pEngine->ActionPrintString(_pwchSequenceStart, _currRunLength); // ... print all the chars leading up to it as part of the run... _trace.DispatchPrintRunTrace(_pwchSequenceStart, _currRunLength); - s_fProcessIndividually = true; // begin processing future characters individually... + _fProcessingIndividually = true; // begin processing future characters individually... _currRunLength = 0; _pwchSequenceStart = _pwchCurr; ProcessCharacter(*_pwchCurr); // ... Then process the character individually. if (_state == VTStates::Ground) // If the character took us right back to ground, start another run after it. { - s_fProcessIndividually = false; + _fProcessingIndividually = false; _pwchSequenceStart = _pwchCurr + 1; _currRunLength = 0; } @@ -1391,14 +1388,33 @@ void StateMachine::ProcessString(const wchar_t* const rgwch, const size_t cch) } // If we're at the end of the string and have remaining un-printed characters, - if (!s_fProcessIndividually && _currRunLength > 0) + if (!_fProcessingIndividually && _currRunLength > 0) { // print the rest of the characters in the string _pEngine->ActionPrintString(_pwchSequenceStart, _currRunLength); _trace.DispatchPrintRunTrace(_pwchSequenceStart, _currRunLength); } - else if (s_fProcessIndividually) - { + else if (_fProcessingIndividually) + { + // One of the "weird things" in VT input is the case of something like + // alt+[. In VT, that's encoded as `\x1b[`. However, that's + // also the start of a CSI, and could be the start of a longer sequence, + // there's no way to know for sure. For an alt+[ keypress, + // the parser originally would just sit in the `CsiEntry` state after + // processing it, which would pollute the following keypress (e.g. + // alt+[, A would be processed like `\x1b[A`, + // which is _wrong_). + // + // Fortunately, for VT input, each keystroke comes in as an individual + // write operation. So, if at the end of processing a string for the + // InputEngine, we find that we're not in the Ground state, that implies + // that we've processed some input, but not dispatched it yet. This + // block at the end of `ProcessString` will then re-process the + // undispatched string, but it will ensure that it dispatches on the + // last character of the string. For our previous `\x1b[` scenario, that + // means we'll make sure to call `_ActionEscDispatch('[')`., which will + // properly decode the string as alt+[. + if (_pEngine->FlushAtEndOfString()) { // Reset our state, and put all but the last char in again. @@ -1413,25 +1429,31 @@ void StateMachine::ProcessString(const wchar_t* const rgwch, const size_t cch) switch (_state) { case VTStates::Ground: - return _ActionExecute(*pwch); + _ActionExecute(*pwch); + break; case VTStates::Escape: case VTStates::EscapeIntermediate: - return _ActionEscDispatch(*pwch); + _ActionEscDispatch(*pwch); + break; case VTStates::CsiEntry: case VTStates::CsiIntermediate: case VTStates::CsiIgnore: case VTStates::CsiParam: - return _ActionCsiDispatch(*pwch); + _ActionCsiDispatch(*pwch); + break; case VTStates::OscParam: case VTStates::OscString: case VTStates::OscTermination: - return _ActionOscDispatch(*pwch); + _ActionOscDispatch(*pwch); + break; case VTStates::Ss3Entry: case VTStates::Ss3Param: - return _ActionSs3Dispatch(*pwch); - default: - return; + _ActionSs3Dispatch(*pwch); + break; } + // microsoft/terminal#2746: Make sure to return to the ground state + // after dispatching the characters + _EnterGround(); } } } diff --git a/src/terminal/parser/stateMachine.hpp b/src/terminal/parser/stateMachine.hpp index ce6980191e5..a4e71a69bd9 100644 --- a/src/terminal/parser/stateMachine.hpp +++ b/src/terminal/parser/stateMachine.hpp @@ -150,5 +150,9 @@ namespace Microsoft::Console::VirtualTerminal const wchar_t* _pwchCurr; const wchar_t* _pwchSequenceStart; size_t _currRunLength; + + // This is tracked per state machine instance so that separate calls to Process* + // can start and finish a sequence. + bool _fProcessingIndividually; }; } diff --git a/src/terminal/parser/ut_parser/InputEngineTest.cpp b/src/terminal/parser/ut_parser/InputEngineTest.cpp index fcb67d59a1e..27fbd25cb3e 100644 --- a/src/terminal/parser/ut_parser/InputEngineTest.cpp +++ b/src/terminal/parser/ut_parser/InputEngineTest.cpp @@ -230,6 +230,7 @@ class Microsoft::Console::VirtualTerminal::InputEngineTest TEST_METHOD(AltBackspaceTest); TEST_METHOD(AltCtrlDTest); TEST_METHOD(AltIntermediateTest); + TEST_METHOD(AltBackspaceEnterTest); friend class TestInteractDispatch; }; @@ -826,3 +827,54 @@ void InputEngineTest::AltIntermediateTest() Log::Comment(NoThrowString().Format(L"Processing \"\\x05\"")); stateMachine->ProcessString(seq); } + +void InputEngineTest::AltBackspaceEnterTest() +{ + // Created as a test for microsoft/terminal#2746. See that issue for mode + // details. We're going to send an Alt+Backspace to conpty, followed by an + // enter. The enter should be processed as just a single VK_ENTER, not a + // alt+enter. + + TestState testState; + auto pfn = std::bind(&TestState::TestInputCallback, &testState, std::placeholders::_1); + + auto inputEngine = std::make_unique(new TestInteractDispatch(pfn, &testState)); + auto _stateMachine = std::make_unique(inputEngine.release()); + VERIFY_IS_NOT_NULL(_stateMachine); + testState._stateMachine = _stateMachine.get(); + + INPUT_RECORD inputRec; + + inputRec.EventType = KEY_EVENT; + inputRec.Event.KeyEvent.bKeyDown = TRUE; + inputRec.Event.KeyEvent.dwControlKeyState = LEFT_ALT_PRESSED; + inputRec.Event.KeyEvent.wRepeatCount = 1; + inputRec.Event.KeyEvent.wVirtualKeyCode = VK_BACK; + inputRec.Event.KeyEvent.wVirtualScanCode = static_cast(MapVirtualKeyW(VK_BACK, MAPVK_VK_TO_VSC)); + inputRec.Event.KeyEvent.uChar.UnicodeChar = L'\x08'; + + // First, expect a alt+backspace. + testState.vExpectedInput.push_back(inputRec); + + std::wstring seq = L"\x1b\x7f"; + Log::Comment(NoThrowString().Format(L"Processing \"\\x1b\\x7f\"")); + _stateMachine->ProcessString(seq); + + // Ensure the state machine has correctly returned to the ground state + VERIFY_ARE_EQUAL(StateMachine::VTStates::Ground, _stateMachine->_state); + + inputRec.Event.KeyEvent.wVirtualKeyCode = VK_RETURN; + inputRec.Event.KeyEvent.dwControlKeyState = 0; + inputRec.Event.KeyEvent.wVirtualScanCode = static_cast(MapVirtualKeyW(VK_RETURN, MAPVK_VK_TO_VSC)); + inputRec.Event.KeyEvent.uChar.UnicodeChar = L'\x0d'; //maybe \xa + + // Then, expect a enter + testState.vExpectedInput.push_back(inputRec); + + seq = L"\x0d"; + Log::Comment(NoThrowString().Format(L"Processing \"\\x0d\"")); + _stateMachine->ProcessString(seq); + + // Ensure the state machine has correctly returned to the ground state + VERIFY_ARE_EQUAL(StateMachine::VTStates::Ground, _stateMachine->_state); +} diff --git a/src/terminal/parser/ut_parser/Parser.UnitTests-common.vcxproj b/src/terminal/parser/ut_parser/Parser.UnitTests-common.vcxproj deleted file mode 100644 index 24754a9dd5b..00000000000 --- a/src/terminal/parser/ut_parser/Parser.UnitTests-common.vcxproj +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - Create - - - - - - - - - - ..;%(AdditionalIncludeDirectories) - - - diff --git a/src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj b/src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj index 57daef497c5..65773c0f48c 100644 --- a/src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj +++ b/src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj @@ -2,14 +2,28 @@ - + + + + + + + + Create + + + + ..;%(AdditionalIncludeDirectories) + + + {18d09a24-8240-42d6-8cb6-236eee820263} diff --git a/src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj.filters b/src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj.filters index 6aeb1ba04c5..ecc61100147 100644 --- a/src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj.filters +++ b/src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj.filters @@ -15,7 +15,13 @@ - + + Source Files + + + Source Files + + Source Files diff --git a/src/terminal/parser/ut_parser/StateMachineTest.cpp b/src/terminal/parser/ut_parser/StateMachineTest.cpp new file mode 100644 index 00000000000..7235b1ecce0 --- /dev/null +++ b/src/terminal/parser/ut_parser/StateMachineTest.cpp @@ -0,0 +1,113 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" +#include "WexTestClass.h" +#include "../../inc/consoletaeftemplates.hpp" + +#include "stateMachine.hpp" + +using namespace WEX::Common; +using namespace WEX::Logging; +using namespace WEX::TestExecution; + +namespace Microsoft +{ + namespace Console + { + namespace VirtualTerminal + { + class StateMachineTest; + class TestStateMachineEngine; + }; + }; +}; + +using namespace Microsoft::Console::VirtualTerminal; + +class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStateMachineEngine +{ +public: + bool ActionExecute(const wchar_t /* wch */) override { return true; }; + bool ActionExecuteFromEscape(const wchar_t /* wch */) override { return true; }; + bool ActionPrint(const wchar_t /* wch */) override { return true; }; + bool ActionPrintString(const wchar_t* const /* rgwch */, + size_t const /* cch */) override { return true; }; + + bool ActionPassThroughString(const wchar_t* const /* rgwch */, + size_t const /* cch */) override { return true; }; + + bool ActionEscDispatch(const wchar_t /* wch */, + const unsigned short /* cIntermediate */, + const wchar_t /* wchIntermediate */) override { return true; }; + + bool ActionClear() override { return true; }; + + bool ActionIgnore() override { return true; }; + + bool ActionOscDispatch(const wchar_t /* wch */, + const unsigned short /* sOscParam */, + wchar_t* const /* pwchOscStringBuffer */, + const unsigned short /* cchOscString */) override { return true; }; + + bool ActionSs3Dispatch(const wchar_t /* wch */, + const unsigned short* const /* rgusParams */, + const unsigned short /* cParams */) override { return true; }; + + bool FlushAtEndOfString() const override { return false; }; + bool DispatchControlCharsFromEscape() const override { return false; }; + bool DispatchIntermediatesFromEscape() const override { return false; }; + + // ActionCsiDispatch is the only method that's actually implemented. + bool ActionCsiDispatch(const wchar_t /* wch */, + const unsigned short /* cIntermediate */, + const wchar_t /* wchIntermediate */, + _In_reads_(cParams) const unsigned short* const rgusParams, + const unsigned short cParams) override + { + csiParams.emplace(rgusParams, rgusParams + cParams); + return true; + } + + // This will only be populated if ActionCsiDispatch is called. + std::optional> csiParams; +}; + +class Microsoft::Console::VirtualTerminal::StateMachineTest +{ + TEST_CLASS(StateMachineTest); + + TEST_CLASS_SETUP(ClassSetup) + { + return true; + } + + TEST_CLASS_CLEANUP(ClassCleanup) + { + return true; + } + + TEST_METHOD(TwoStateMachinesDoNotInterfereWithEachother); +}; + +void StateMachineTest::TwoStateMachinesDoNotInterfereWithEachother() +{ + auto firstEnginePtr{ std::make_unique() }; + // this dance is required because StateMachine presumes to take ownership of its engine. + const auto& firstEngine{ *firstEnginePtr.get() }; + StateMachine firstStateMachine{ firstEnginePtr.release() }; + + auto secondEnginePtr{ std::make_unique() }; + const auto& secondEngine{ *secondEnginePtr.get() }; + StateMachine secondStateMachine{ secondEnginePtr.release() }; + + firstStateMachine.ProcessString(L"\x1b[12"); // partial sequence + secondStateMachine.ProcessString(L"\x1b[3C"); // full sequence on second parser + firstStateMachine.ProcessString(L";34m"); // completion to previous partial sequence on first parser + + std::vector expectedFirstCsi{ 12u, 34u }; + std::vector expectedSecondCsi{ 3u }; + + VERIFY_ARE_EQUAL(expectedFirstCsi, firstEngine.csiParams); + VERIFY_ARE_EQUAL(expectedSecondCsi, secondEngine.csiParams); +} diff --git a/src/terminal/parser/ut_parser/sources b/src/terminal/parser/ut_parser/sources index 32867cc0ff3..5e7f6c61e11 100644 --- a/src/terminal/parser/ut_parser/sources +++ b/src/terminal/parser/ut_parser/sources @@ -23,6 +23,7 @@ SOURCES = \ $(SOURCES) \ OutputEngineTest.cpp \ InputEngineTest.cpp \ + StateMachineTest.cpp \ # The InputEngineTest requires VTRedirMapVirtualKeyW, which means we need the # ServiceLocator, which means we need the entire host and all it's dependencies, diff --git a/src/types/inc/utils.hpp b/src/types/inc/utils.hpp index ff1c8dd1781..6dfa216ab4d 100644 --- a/src/types/inc/utils.hpp +++ b/src/types/inc/utils.hpp @@ -11,6 +11,19 @@ Author(s): - Mike Griese (migrie) 12-Jun-2018 --*/ +// Inspired from RETURN_IF_WIN32_BOOL_FALSE +// WIL doesn't include a RETURN_BOOL_IF_FALSE, and RETURN_IF_WIN32_BOOL_FALSE +// will actually return the value of GLE. +#define RETURN_BOOL_IF_FALSE(b) \ + do \ + { \ + BOOL __boolRet = wil::verify_bool(b); \ + if (!__boolRet) \ + { \ + return __boolRet; \ + } \ + } while (0, 0) + namespace Microsoft::Console::Utils { bool IsValidHandle(const HANDLE handle) noexcept;