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

@@ -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<size_t>(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.

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;

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)

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)

src/cascadia/UnitTests_TerminalCore/TerminalApiTest.cpp

@@ -97,7 +97,7 @@ void TerminalApiTest::PrintStringOfSurrogatePairs()
         [](LPVOID data) -> DWORD {
             const auto& baton = *reinterpret_cast<Baton*>(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;

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!

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);

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:
-// - <none>
-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:
 // - <none>
-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)

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;
 

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;