When Conpty encounters a string we don't understand, immediately flus…

…h the frame

    ## Summary of the Pull Request

    When Conpty encounters a string we don't understand, immediately flush the frame.

    ## References

    This PR superceeds #2665. This solution is much simpler than what was proposed in that PR.
    As mentioned in #2665: "This might have some long-term consequences for #1173."

    ## PR Checklist
    * [x] Closes #2011
    * [x] Closes #4106
    * [x] I work here
    * [x] Tests added/passed
    * [n/a] Requires documentation to be updated

src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp

@@ -155,6 +155,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
 
     TEST_METHOD(PassthroughClearScrollback);
 
+    TEST_METHOD(PassthroughCursorShapeImmediately);
+
     TEST_METHOD(TestWrappingALongString);
     TEST_METHOD(TestAdvancedWrapping);
     TEST_METHOD(TestExactWrappingWithoutSpaces);
@@ -673,6 +675,36 @@ void ConptyRoundtripTests::MoveCursorAtEOL()
     verifyData1(termTb);
 }
 
+void ConptyRoundtripTests::PassthroughCursorShapeImmediately()
+{
+    // This is a test for GH#4106, and more indirectly, GH #2011.
+
+    Log::Comment(NoThrowString().Format(
+        L"Change the cursor shape with VT. This should immediately be flushed to the Terminal."));
+    VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
+
+    auto& g = ServiceLocator::LocateGlobals();
+    // auto& renderer = *g.pRender;
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& hostSm = si.GetStateMachine();
+    auto& hostTb = si.GetTextBuffer();
+    auto& termTb = *term->_buffer;
+
+    _flushFirstFrame();
+
+    _logConpty = true;
+
+    VERIFY_ARE_NOT_EQUAL(CursorType::VerticalBar, hostTb.GetCursor().GetType());
+    VERIFY_ARE_NOT_EQUAL(CursorType::VerticalBar, termTb.GetCursor().GetType());
+
+    expectedOutput.push_back("\x1b[5 q");
+    hostSm.ProcessString(L"\x1b[5 q");
+
+    VERIFY_ARE_EQUAL(CursorType::VerticalBar, hostTb.GetCursor().GetType());
+    VERIFY_ARE_EQUAL(CursorType::VerticalBar, termTb.GetCursor().GetType());
+}
+
 void ConptyRoundtripTests::PassthroughClearScrollback()
 {
     Log::Comment(NoThrowString().Format(

src/renderer/vt/WinTelnetEngine.cpp

@@ -111,5 +111,17 @@ WinTelnetEngine::WinTelnetEngine(_In_ wil::unique_hfile hPipe,
 // - S_OK or suitable HRESULT error from either conversion or writing pipe.
 [[nodiscard]] HRESULT WinTelnetEngine::WriteTerminalW(_In_ const std::wstring_view wstr) noexcept
 {
-    return VtEngine::_WriteTerminalAscii(wstr);
+    RETURN_IF_FAILED(VtEngine::_WriteTerminalAscii(wstr));
+    // GH#4106, GH#2011 - WriteTerminalW is only ever called by the
+    // StateMachine, when we've encountered a string we don't understand. When
+    // this happens, we usually don't actually trigger another frame, but we
+    // _do_ want this string to immediately be sent to the terminal. Since we
+    // only flush our buffer on actual frames, this means that strings we've
+    // decided to pass through would have gotten buffered here until the next
+    // actual frame is triggered.
+    //
+    // To fix this, flush here, so this string is sent to the connected terminal
+    // application.
+
+    return _Flush();
 }

src/renderer/vt/XtermEngine.cpp

@@ -477,9 +477,21 @@ XtermEngine::XtermEngine(_In_ wil::unique_hfile hPipe,
 // - S_OK or suitable HRESULT error from either conversion or writing pipe.
 [[nodiscard]] HRESULT XtermEngine::WriteTerminalW(const std::wstring_view wstr) noexcept
 {
-    return _fUseAsciiOnly ?
-               VtEngine::_WriteTerminalAscii(wstr) :
-               VtEngine::_WriteTerminalUtf8(wstr);
+    RETURN_IF_FAILED(_fUseAsciiOnly ?
+                         VtEngine::_WriteTerminalAscii(wstr) :
+                         VtEngine::_WriteTerminalUtf8(wstr));
+    // GH#4106, GH#2011 - WriteTerminalW is only ever called by the
+    // StateMachine, when we've encountered a string we don't understand. When
+    // this happens, we usually don't actually trigger another frame, but we
+    // _do_ want this string to immediately be sent to the terminal. Since we
+    // only flush our buffer on actual frames, this means that strings we've
+    // decided to pass through would have gotten buffered here until the next
+    // actual frame is triggered.
+    //
+    // To fix this, flush here, so this string is sent to the connected terminal
+    // application.
+
+    return _Flush();
 }
 
 // Method Description:

src/terminal/adapter/adaptDispatch.cpp

@@ -1847,13 +1847,6 @@ bool AdaptDispatch::EnableAlternateScroll(const bool enabled)
 // True if handled successfully. False otherwise.
 bool AdaptDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle)
 {
-    bool isPty = false;
-    _pConApi->IsConsolePty(isPty);
-    if (isPty)
-    {
-        return false;
-    }
-
     CursorType actualType = CursorType::Legacy;
     bool fEnableBlinking = false;
 
@@ -1894,6 +1887,15 @@ bool AdaptDispatch::SetCursorStyle(const DispatchTypes::CursorStyle cursorStyle)
         success = _pConApi->PrivateAllowCursorBlinking(fEnableBlinking);
     }
 
+    // If we're a conpty, always return false, so that this cursor state will be
+    // sent to the connected terminal
+    bool isPty = false;
+    _pConApi->IsConsolePty(isPty);
+    if (isPty)
+    {
+        return false;
+    }
+
     return success;
 }