From a1865b9cf795b9fdd997162304ce293015d20fd5 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Wed, 18 Jan 2023 20:26:04 +0000 Subject: [PATCH] Merge the PrintString functionality into AdaptDispatch (#14640) The main purpose of this PR was to merge the `ITerminalApi::PrintString` implementations into a shared method in `AdaptDispatch`, and avoid the VT code path depending on `WriteCharsLegacy`. But this refactoring has also fixed some bugs that existed in the original implementations. This helps to close the gap between the Conhost and Terminal (#13408). I started by taking the `WriteCharsLegacy` implementation, and stripping out everything that didn't apply to the VT code path. What was left was a fairly simple loop with the following steps: 1. Check if _delayed wrap_ is set, and if so, move to the next line. 2. Write out as much of the string as will fit on the current line. 3. If we reach the end of the line, set the _delayed wrap_ flag again. 4. Repeat the loop until the entire string has been output. But step 2 was a little more complicated than necessary because of its legacy history. It was copying the string into a temporary buffer, manually estimated how much of it would fit, and then passing on that partial buffer to the `TextBuffer::Write` method. In the new implementation, we just pass the entire string directly to `TextBuffer::WriteLine`, and that handles the clipping itself. The returned `OutputCellIterator` tells us how much of the string is left. This approach fixes some issues with wide characters, and should also cope better with future buffer enhancements. Another improvement from the new implementation is that the Terminal now handles delayed EOL wrap correctly. However, the downside of this is that it introduced a cursor-dropping bug that previously only affected conhost. I hadn't originally intended to fix that, but it became more of an issue now. The root cause was the fact that we called `cursor.StartDeferDrawing()` before outputting the text, and this was something I had adopted in the new implementation as well. But I've now removed that, and instead just call `cursor.SetIsOn(false)`. This seems to avoid the cursor droppings, and hopefully still has similar performance benefits. The other thing worth mentioning is that I've eliminated some special casing handling for the `ENABLE_VIRTUAL_TERMINAL_PROCESSING` mode and the `WC_DELAY_EOL_WRAP` flag in the `WriteCharsLegacy` function. They were only used for VT output, so aren't needed here anymore. ## Validation Steps Performed I've just been testing manually, writing out sample text both in ASCII and with wide Unicode chars. I've made sure it wraps correctly when exceeding the available space, but doesn't wrap when stopped at the last column, and with `DECAWM` disabled, it doesn't wrap at all. I've also confirmed that the test case from issue #12739 is now working correctly, and the cursor no longer disappears in Windows Terminal when writing to the last column (i.e. the delayed EOL wrap is working). Closes #780 Closes #6162 Closes #6555 Closes #12440 Closes #12739 --- src/cascadia/TerminalCore/Terminal.cpp | 77 --------------- src/cascadia/TerminalCore/Terminal.hpp | 5 +- src/cascadia/TerminalCore/TerminalApi.cpp | 14 +-- .../ConptyRoundtripTests.cpp | 2 +- .../TerminalApiTest.cpp | 2 +- src/host/_stream.cpp | 29 +----- src/host/cmdline.h | 2 +- src/host/outputStream.cpp | 45 +-------- src/host/outputStream.hpp | 3 +- src/terminal/adapter/ITerminalApi.hpp | 3 +- src/terminal/adapter/adaptDispatch.cpp | 96 +++++++++++++++++-- src/terminal/adapter/adaptDispatch.hpp | 1 + .../adapter/ut_adapter/adapterTest.cpp | 45 ++++----- 13 files changed, 129 insertions(+), 195 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 58443c863c5..7677910354d 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1078,83 +1078,6 @@ Viewport Terminal::_GetVisibleViewport() const noexcept size); } -// Writes a string of text to the buffer, then moves the cursor (and viewport) -// in accordance with the written text. -// This method is our proverbial `WriteCharsLegacy`, and great care should be made to -// keep it minimal and orderly, lest it become WriteCharsLegacy2ElectricBoogaloo -// TODO: MSFT 21006766 -// This needs to become stream logic on the buffer itself sooner rather than later -// because it's otherwise impossible to avoid the Electric Boogaloo-ness here. -// I had to make a bunch of hacks to get Japanese and emoji to work-ish. -void Terminal::_WriteBuffer(const std::wstring_view& stringView) -{ - auto& cursor = _activeBuffer().GetCursor(); - - // Defer the cursor drawing while we are iterating the string, for a better performance. - // We can not waste time displaying a cursor event when we know more text is coming right behind it. - cursor.StartDeferDrawing(); - - for (size_t i = 0; i < stringView.size(); i++) - { - const auto wch = stringView.at(i); - const auto cursorPosBefore = cursor.GetPosition(); - auto proposedCursorPosition = cursorPosBefore; - - // TODO: MSFT 21006766 - // This is not great but I need it demoable. Fix by making a buffer stream writer. - // - // If wch is a surrogate character we need to read 2 code units - // from the stringView to form a single code point. - const auto isSurrogate = wch >= 0xD800 && wch <= 0xDFFF; - const auto view = stringView.substr(i, isSurrogate ? 2 : 1); - const OutputCellIterator it{ view, _activeBuffer().GetCurrentAttributes() }; - const auto end = _activeBuffer().Write(it); - const auto cellDistance = end.GetCellDistance(it); - const auto inputDistance = end.GetInputDistance(it); - - if (inputDistance > 0) - { - // If "wch" was a surrogate character, we just consumed 2 code units above. - // -> Increment "i" by 1 in that case and thus by 2 in total in this iteration. - proposedCursorPosition.x += cellDistance; - i += gsl::narrow_cast(inputDistance - 1); - } - else - { - // If _WriteBuffer() is called with a consecutive string longer than the viewport/buffer width - // the call to _buffer->Write() will refuse to write anything on the current line. - // GetInputDistance() thus returns 0, which would in turn cause i to be - // decremented by 1 below and force the outer loop to loop forever. - // This if() basically behaves as if "\r\n" had been encountered above and retries the write. - // With well behaving shells during normal operation this safeguard should normally not be encountered. - proposedCursorPosition.x = 0; - proposedCursorPosition.y++; - - // Try the character again. - i--; - - // If we write the last cell of the row here, TextBuffer::Write will - // mark this line as wrapped for us. If the next character we - // process is a newline, the Terminal::CursorLineFeed will unmark - // this line as wrapped. - - // TODO: GH#780 - This should really be a _deferred_ newline. If - // the next character to come in is a newline or a cursor - // movement or anything, then we should _not_ wrap this line - // here. - } - - _AdjustCursorPosition(proposedCursorPosition); - } - - // Notify UIA of new text. - // It's important to do this here instead of in TextBuffer, because here you have access to the entire line of text, - // whereas TextBuffer writes it one character at a time via the OutputCellIterator. - _activeBuffer().TriggerNewTextNotification(stringView); - - cursor.EndDeferDrawing(); -} - void Terminal::_AdjustCursorPosition(const til::point proposedPosition) { #pragma warning(suppress : 26496) // cpp core checks wants this const but it's modified below. diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index c7a3b6b50a7..ec95bf8caf6 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -105,7 +105,6 @@ class Microsoft::Terminal::Core::Terminal final : #pragma region ITerminalApi // These methods are defined in TerminalApi.cpp - void PrintString(const std::wstring_view string) override; void ReturnResponse(const std::wstring_view response) override; Microsoft::Console::VirtualTerminal::StateMachine& GetStateMachine() noexcept override; TextBuffer& GetTextBuffer() noexcept override; @@ -117,7 +116,7 @@ class Microsoft::Terminal::Core::Terminal final : void SetScrollingRegion(const til::inclusive_rect& scrollMargins) noexcept override; void WarningBell() override; bool GetLineFeedMode() const noexcept override; - void LineFeed(const bool withReturn) override; + void LineFeed(const bool withReturn, const bool wrapForced) override; void SetWindowTitle(const std::wstring_view title) override; CursorType GetUserDefaultCursorStyle() const noexcept override; bool ResizeWindow(const til::CoordType width, const til::CoordType height) noexcept override; @@ -421,8 +420,6 @@ class Microsoft::Terminal::Core::Terminal final : Microsoft::Console::Types::Viewport _GetMutableViewport() const noexcept; Microsoft::Console::Types::Viewport _GetVisibleViewport() const noexcept; - void _WriteBuffer(const std::wstring_view& stringView); - void _AdjustCursorPosition(const til::point proposedPosition); void _NotifyScrollEvent() noexcept; diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index fa2d3c50d5e..c4ec65d62e5 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -20,12 +20,6 @@ TRACELOGGING_DEFINE_PROVIDER(g_hCTerminalCoreProvider, (0x103ac8cf, 0x97d2, 0x51aa, 0xb3, 0xba, 0x5f, 0xfd, 0x55, 0x28, 0xfa, 0x5f), TraceLoggingOptionMicrosoftTelemetry()); -// Print puts the text in the buffer and moves the cursor -void Terminal::PrintString(const std::wstring_view string) -{ - _WriteBuffer(string); -} - void Terminal::ReturnResponse(const std::wstring_view response) { if (_pfnWriteInput) @@ -92,13 +86,13 @@ bool Terminal::GetLineFeedMode() const noexcept return false; } -void Terminal::LineFeed(const bool withReturn) +void Terminal::LineFeed(const bool withReturn, const bool wrapForced) { auto cursorPos = _activeBuffer().GetCursor().GetPosition(); - // since we explicitly just moved down a row, clear the wrap status on the - // row we just came from - _activeBuffer().GetRowByOffset(cursorPos.y).SetWrapForced(false); + // If the line was forced to wrap, set the wrap status. + // When explicitly moving down a row, clear the wrap status. + _activeBuffer().GetRowByOffset(cursorPos.y).SetWrapForced(wrapForced); cursorPos.y++; if (withReturn) diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index fe21447b275..a81914229c1 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -3207,7 +3207,7 @@ void ConptyRoundtripTests::WrapNewLineAtBottom() // GH#5839 - // This test does expose a real bug when using WriteCharsLegacy to emit - // wrapped lines in conpty without WC_DELAY_EOL_WRAP. However, this fix has + // wrapped lines in conpty without delayed EOL wrap. However, this fix has // not yet been made, so for now, we need to just skip the cases that cause // this. if (writingMethod == PrintWithWriteCharsLegacy && paintEachNewline == PaintEveryLine) diff --git a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp index 10eca2c8ce4..683740c155b 100644 --- a/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp @@ -97,7 +97,7 @@ void TerminalApiTest::PrintStringOfSurrogatePairs() [](LPVOID data) -> DWORD { const auto& baton = *reinterpret_cast(data); Log::Comment(L"Writing data."); - baton.pTerm->PrintString(baton.text); + baton.pTerm->_stateMachine->ProcessString(baton.text); Log::Comment(L"Setting event."); SetEvent(baton.done); return 0; diff --git a/src/host/_stream.cpp b/src/host/_stream.cpp index c0ff3eae2ac..0d4af59ee36 100644 --- a/src/host/_stream.cpp +++ b/src/host/_stream.cpp @@ -350,12 +350,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; auto lpString = pwchRealUnicode; - auto coordScreenBufferSize = screenInfo.GetBufferSize().Dimensions(); - // In VT mode, the width at which we wrap is determined by the line rendition attribute. - if (WI_IsFlagSet(screenInfo.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING)) - { - coordScreenBufferSize.width = textBuffer.GetLineWidth(CursorPosition.y); - } + const auto coordScreenBufferSize = screenInfo.GetBufferSize().Dimensions(); static constexpr til::CoordType LOCAL_BUFFER_SIZE = 1024; WCHAR LocalBuffer[LOCAL_BUFFER_SIZE]; @@ -376,11 +371,6 @@ using Microsoft::Console::VirtualTerminal::StateMachine; Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY); CursorPosition = cursor.GetPosition(); - // In VT mode, we need to recalculate the width when moving to a new line. - if (WI_IsFlagSet(screenInfo.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING)) - { - coordScreenBufferSize.width = textBuffer.GetLineWidth(CursorPosition.y); - } } } @@ -570,22 +560,7 @@ using Microsoft::Console::VirtualTerminal::StateMachine; // WCL-NOTE: wrong place (typically inside another character). CursorPosition.x = XPosition; - // enforce a delayed newline if we're about to pass the end and the WC_DELAY_EOL_WRAP flag is set. - if (WI_IsFlagSet(dwFlags, WC_DELAY_EOL_WRAP) && CursorPosition.x >= coordScreenBufferSize.width && fWrapAtEOL) - { - // Our cursor position as of this time is going to remain on the last position in this column. - CursorPosition.x = coordScreenBufferSize.width - 1; - - // Update in the structures that we're still pointing to the last character in the row - cursor.SetPosition(CursorPosition); - - // Record for the delay comparison that we're delaying on the last character in the row - cursor.DelayEOLWrap(CursorPosition); - } - else - { - Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY); - } + Status = AdjustCursorPosition(screenInfo, CursorPosition, WI_IsFlagSet(dwFlags, WC_KEEP_CURSOR_VISIBLE), psScrollY); // WCL-NOTE: If we have processed the entire input string during our "fast one-line print" handler, // WCL-NOTE: we are done as there is nothing more to do. Neat! diff --git a/src/host/cmdline.h b/src/host/cmdline.h index b503ec21b15..32f3ee4e7b4 100644 --- a/src/host/cmdline.h +++ b/src/host/cmdline.h @@ -142,7 +142,7 @@ void RedrawCommandLine(COOKED_READ_DATA& cookedReadData); #define WC_LIMIT_BACKSPACE 0x10 //#define WC_NONDESTRUCTIVE_TAB 0x20 - This is not needed anymore, because the VT code handles tabs internally now. //#define WC_NEWLINE_SAVE_X 0x40 - This has been replaced with an output mode flag instead as it's line discipline behavior that may not necessarily be coupled with VT. -#define WC_DELAY_EOL_WRAP 0x80 +//#define WC_DELAY_EOL_WRAP 0x80 - This is not needed anymore, because the AdaptDispatch class handles all VT output. // Word delimiters bool IsWordDelim(const wchar_t wch); diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index 62ade57b79b..9f382cab2af 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -24,39 +24,6 @@ ConhostInternalGetSet::ConhostInternalGetSet(_In_ IIoProvider& io) : { } -// Routine Description: -// - Handles the print action from the state machine -// Arguments: -// - string - The string to be printed. -// Return Value: -// - -void ConhostInternalGetSet::PrintString(const std::wstring_view string) -{ - auto dwNumBytes = string.size() * sizeof(wchar_t); - - auto& cursor = _io.GetActiveOutputBuffer().GetTextBuffer().GetCursor(); - if (!cursor.IsOn()) - { - cursor.SetIsOn(true); - } - - // Defer the cursor drawing while we are iterating the string, for a better performance. - // We can not waste time displaying a cursor event when we know more text is coming right behind it. - cursor.StartDeferDrawing(); - const auto ntstatus = WriteCharsLegacy(_io.GetActiveOutputBuffer(), - string.data(), - string.data(), - string.data(), - &dwNumBytes, - nullptr, - _io.GetActiveOutputBuffer().GetTextBuffer().GetCursor().GetPosition().x, - WC_LIMIT_BACKSPACE | WC_DELAY_EOL_WRAP, - nullptr); - cursor.EndDeferDrawing(); - - THROW_IF_NTSTATUS_FAILED(ntstatus); -} - // - Sends a string response to the input stream of the console. // - Used by various commands where the program attached would like a reply to one of the commands issued. // - This will generate two "key presses" (one down, one up) for every character in the string and place them into the head of the console's input stream. @@ -207,20 +174,18 @@ bool ConhostInternalGetSet::GetLineFeedMode() const // - Performs a line feed, possibly preceded by carriage return. // Arguments: // - withReturn - Set to true if a carriage return should be performed as well. +// - wrapForced - Set to true is the line feed was the result of the line wrapping. // Return Value: // - -void ConhostInternalGetSet::LineFeed(const bool withReturn) +void ConhostInternalGetSet::LineFeed(const bool withReturn, const bool wrapForced) { auto& screenInfo = _io.GetActiveOutputBuffer(); auto& textBuffer = screenInfo.GetTextBuffer(); auto cursorPosition = textBuffer.GetCursor().GetPosition(); - // We turn the cursor on before an operation that might scroll the viewport, otherwise - // that can result in an old copy of the cursor being left behind on the screen. - textBuffer.GetCursor().SetIsOn(true); - - // Since we are explicitly moving down a row, clear the wrap status on the row we're leaving - textBuffer.GetRowByOffset(cursorPosition.y).SetWrapForced(false); + // If the line was forced to wrap, set the wrap status. + // When explicitly moving down a row, clear the wrap status. + textBuffer.GetRowByOffset(cursorPosition.y).SetWrapForced(wrapForced); cursorPosition.y += 1; if (withReturn) diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index bc033cd39d6..f673e9110ba 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -29,7 +29,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: public: ConhostInternalGetSet(_In_ Microsoft::Console::IIoProvider& io); - void PrintString(const std::wstring_view string) override; void ReturnResponse(const std::wstring_view response) override; Microsoft::Console::VirtualTerminal::StateMachine& GetStateMachine() override; @@ -47,7 +46,7 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: void WarningBell() override; bool GetLineFeedMode() const override; - void LineFeed(const bool withReturn) override; + void LineFeed(const bool withReturn, const bool wrapForced) override; void SetWindowTitle(const std::wstring_view title) override; diff --git a/src/terminal/adapter/ITerminalApi.hpp b/src/terminal/adapter/ITerminalApi.hpp index cd064edf80c..4705f382817 100644 --- a/src/terminal/adapter/ITerminalApi.hpp +++ b/src/terminal/adapter/ITerminalApi.hpp @@ -37,7 +37,6 @@ namespace Microsoft::Console::VirtualTerminal ITerminalApi& operator=(const ITerminalApi&) = delete; ITerminalApi& operator=(ITerminalApi&&) = delete; - virtual void PrintString(const std::wstring_view string) = 0; virtual void ReturnResponse(const std::wstring_view response) = 0; virtual StateMachine& GetStateMachine() = 0; @@ -55,7 +54,7 @@ namespace Microsoft::Console::VirtualTerminal virtual void SetScrollingRegion(const til::inclusive_rect& scrollMargins) = 0; virtual void WarningBell() = 0; virtual bool GetLineFeedMode() const = 0; - virtual void LineFeed(const bool withReturn) = 0; + virtual void LineFeed(const bool withReturn, const bool wrapForced) = 0; virtual void SetWindowTitle(const std::wstring_view title) = 0; virtual void UseAlternateScreenBuffer() = 0; virtual void UseMainScreenBuffer() = 0; diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 8bb756ccb72..40edfe1cf96 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -40,7 +40,7 @@ void AdaptDispatch::Print(const wchar_t wchPrintable) // a character is only output if the DEL is translated to something else. if (wchTranslated != AsciiChars::DEL) { - _api.PrintString({ &wchTranslated, 1 }); + _WriteToBuffer({ &wchTranslated, 1 }); } } @@ -61,14 +61,98 @@ void AdaptDispatch::PrintString(const std::wstring_view string) { buffer.push_back(_termOutput.TranslateKey(wch)); } - _api.PrintString(buffer); + _WriteToBuffer(buffer); } else { - _api.PrintString(string); + _WriteToBuffer(string); } } +void AdaptDispatch::_WriteToBuffer(const std::wstring_view string) +{ + auto& textBuffer = _api.GetTextBuffer(); + auto& cursor = textBuffer.GetCursor(); + auto cursorPosition = cursor.GetPosition(); + const auto wrapAtEOL = _api.GetAutoWrapMode(); + const auto attributes = textBuffer.GetCurrentAttributes(); + + // Turn off the cursor until we're done, so it isn't refreshed unnecessarily. + cursor.SetIsOn(false); + + // The width at which we wrap is determined by the line rendition attribute. + auto lineWidth = textBuffer.GetLineWidth(cursorPosition.y); + + auto stringPosition = string.cbegin(); + while (stringPosition < string.cend()) + { + if (cursor.IsDelayedEOLWrap() && wrapAtEOL) + { + const auto delayedCursorPosition = cursor.GetDelayedAtPosition(); + cursor.ResetDelayEOLWrap(); + // Only act on a delayed EOL if we didn't move the cursor to a + // different position from where the EOL was marked. + if (delayedCursorPosition == cursorPosition) + { + _api.LineFeed(true, true); + cursorPosition = cursor.GetPosition(); + // We need to recalculate the width when moving to a new line. + lineWidth = textBuffer.GetLineWidth(cursorPosition.y); + } + } + + const OutputCellIterator it(std::wstring_view{ stringPosition, string.cend() }, attributes); + const auto itEnd = textBuffer.WriteLine(it, cursorPosition, wrapAtEOL, lineWidth - 1); + + if (itEnd.GetInputDistance(it) == 0) + { + // If we haven't written anything out because there wasn't enough space, + // we move the cursor to the end of the line so that it's forced to wrap. + cursorPosition.x = lineWidth; + // But if wrapping is disabled, we also need to move to the next string + // position, otherwise we'll be stuck in this loop forever. + if (!wrapAtEOL) + { + stringPosition++; + } + } + else + { + const auto cellCount = itEnd.GetCellDistance(it); + const auto changedRect = til::rect{ cursorPosition, til::size{ cellCount, 1 } }; + _api.NotifyAccessibilityChange(changedRect); + + stringPosition += itEnd.GetInputDistance(it); + cursorPosition.x += cellCount; + } + + if (cursorPosition.x >= lineWidth) + { + // If we're past the end of the line, we need to clamp the cursor + // back into range, and if wrapping is enabled, set the delayed wrap + // flag. The wrapping only occurs once another character is output. + cursorPosition.x = lineWidth - 1; + cursor.SetPosition(cursorPosition); + if (wrapAtEOL) + { + cursor.DelayEOLWrap(cursorPosition); + } + } + else + { + cursor.SetPosition(cursorPosition); + } + } + + _ApplyCursorMovementFlags(cursor); + + // Notify UIA of new text. + // It's important to do this here instead of in TextBuffer, because here you + // have access to the entire line of text, whereas TextBuffer writes it one + // character at a time via the OutputCellIterator. + textBuffer.TriggerNewTextNotification(string); +} + // Routine Description: // - CUU - Handles cursor upward movement by given distance. // CUU and CUD are handled separately from other CUP sequences, because they are @@ -1892,13 +1976,13 @@ bool AdaptDispatch::LineFeed(const DispatchTypes::LineFeedType lineFeedType) switch (lineFeedType) { case DispatchTypes::LineFeedType::DependsOnMode: - _api.LineFeed(_api.GetLineFeedMode()); + _api.LineFeed(_api.GetLineFeedMode(), false); return true; case DispatchTypes::LineFeedType::WithoutReturn: - _api.LineFeed(false); + _api.LineFeed(false, false); return true; case DispatchTypes::LineFeedType::WithReturn: - _api.LineFeed(true); + _api.LineFeed(true, false); return true; default: return false; diff --git a/src/terminal/adapter/adaptDispatch.hpp b/src/terminal/adapter/adaptDispatch.hpp index bc5a0f04c25..76dbfc05f5b 100644 --- a/src/terminal/adapter/adaptDispatch.hpp +++ b/src/terminal/adapter/adaptDispatch.hpp @@ -193,6 +193,7 @@ namespace Microsoft::Console::VirtualTerminal std::optional background; }; + void _WriteToBuffer(const std::wstring_view string); std::pair _GetVerticalMargins(const til::rect& viewport, const bool absolute); bool _CursorMovePosition(const Offset rowOffset, const Offset colOffset, const bool clampInMargins); void _ApplyCursorMovementFlags(Cursor& cursor) noexcept; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index f4e81adf72a..45cd8715666 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -60,11 +60,6 @@ using namespace Microsoft::Console::VirtualTerminal; class TestGetSet final : public ITerminalApi { public: - void PrintString(const std::wstring_view string) override - { - _printed += string; - } - void ReturnResponse(const std::wstring_view response) override { Log::Comment(L"ReturnResponse MOCK called..."); @@ -148,7 +143,7 @@ class TestGetSet final : public ITerminalApi return _getLineFeedModeResult; } - void LineFeed(const bool withReturn) override + void LineFeed(const bool withReturn, const bool /*wrapForced*/) override { Log::Comment(L"LineFeed MOCK called..."); @@ -316,8 +311,6 @@ class TestGetSet final : public ITerminalApi _response.clear(); _retainResponse = false; - - _printed.clear(); } void PrepCursor(CursorX xact, CursorY yact) @@ -409,8 +402,6 @@ class TestGetSet final : public ITerminalApi til::inclusive_rect _expectedScrollRegion; til::inclusive_rect _activeScrollRegion; - std::wstring _printed; - til::point _expectedCursorPos; TextAttribute _expectedAttribute = {}; @@ -2534,44 +2525,50 @@ class AdapterTest setMacroText(2, L"Macro 2"); setMacroText(63, L"Macro 63"); + const auto getBufferOutput = [&]() { + const auto& textBuffer = _testGetSet->GetTextBuffer(); + const auto cursorPos = textBuffer.GetCursor().GetPosition(); + return textBuffer.GetRowByOffset(cursorPos.y).GetText().substr(0, cursorPos.x); + }; + Log::Comment(L"Simple macro invoke"); - _testGetSet->_printed.clear(); + _testGetSet->PrepData(); _stateMachine->ProcessString(L"\033[2*z"); - VERIFY_ARE_EQUAL(L"Macro 2", _testGetSet->_printed); + VERIFY_ARE_EQUAL(L"Macro 2", getBufferOutput()); Log::Comment(L"Default macro invoke"); - _testGetSet->_printed.clear(); + _testGetSet->PrepData(); _stateMachine->ProcessString(L"\033[*z"); - VERIFY_ARE_EQUAL(L"Macro 0", _testGetSet->_printed); + VERIFY_ARE_EQUAL(L"Macro 0", getBufferOutput()); Log::Comment(L"Maximum ID number"); - _testGetSet->_printed.clear(); + _testGetSet->PrepData(); _stateMachine->ProcessString(L"\033[63*z"); - VERIFY_ARE_EQUAL(L"Macro 63", _testGetSet->_printed); + VERIFY_ARE_EQUAL(L"Macro 63", getBufferOutput()); Log::Comment(L"Out of range ID number"); - _testGetSet->_printed.clear(); + _testGetSet->PrepData(); _stateMachine->ProcessString(L"\033[64*z"); - VERIFY_ARE_EQUAL(L"", _testGetSet->_printed); + VERIFY_ARE_EQUAL(L"", getBufferOutput()); Log::Comment(L"Only one ID parameter allowed"); - _testGetSet->_printed.clear(); + _testGetSet->PrepData(); _stateMachine->ProcessString(L"\033[2;0;1*z"); - VERIFY_ARE_EQUAL(L"Macro 2", _testGetSet->_printed); + VERIFY_ARE_EQUAL(L"Macro 2", getBufferOutput()); Log::Comment(L"DECDMAC ignored when inside a macro"); setMacroText(10, L"[\033P1;0;0!zReplace Macro 1\033\\]"); - _testGetSet->_printed.clear(); + _testGetSet->PrepData(); _stateMachine->ProcessString(L"\033[10*z"); _stateMachine->ProcessString(L"\033[1*z"); - VERIFY_ARE_EQUAL(L"[]Macro 1", _testGetSet->_printed); + VERIFY_ARE_EQUAL(L"[]Macro 1", getBufferOutput()); Log::Comment(L"Maximum recursive depth is 16"); setMacroText(0, L"<\033[1*z>"); setMacroText(1, L"[\033[0*z]"); - _testGetSet->_printed.clear(); + _testGetSet->PrepData(); _stateMachine->ProcessString(L"\033[0*z"); - VERIFY_ARE_EQUAL(L"<[<[<[<[<[<[<[<[]>]>]>]>]>]>]>]>", _testGetSet->_printed); + VERIFY_ARE_EQUAL(L"<[<[<[<[<[<[<[<[]>]>]>]>]>]>]>]>", getBufferOutput()); _pDispatch->_macroBuffer = nullptr; }