From 9733a9ff2a1be98d09b31dd4b07d285e14c30292 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 21 Jan 2020 16:48:10 -0600 Subject: [PATCH 1/3] This at a minimum passes the sequences through --- src/terminal/adapter/adaptDispatch.cpp | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index fa991e31ee5..9ac9cd32623 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -554,7 +554,14 @@ bool AdaptDispatch::EraseInDisplay(const DispatchTypes::EraseType eraseType) // by moving the current contents of the viewport into the scrollback. if (eraseType == DispatchTypes::EraseType::Scrollback) { - return _EraseScrollback(); + const bool eraseScrollbackResult = _EraseScrollback(); + // GH#2715 - If this succeeded, but we're in a conpty, return `false` to + // make the state machine propogate this ED sequence to the connected + // terminal application. While we're in conpty mode, we don't really + // have a scrollback, but the attached terminal might. + bool isPty = false; + _pConApi->IsConsolePty(isPty); + return eraseScrollbackResult && (!isPty); } else if (eraseType == DispatchTypes::EraseType::All) { @@ -1460,6 +1467,17 @@ bool AdaptDispatch::HardReset() // delete all current tab stops and reapply _pConApi->PrivateSetDefaultTabStops(); + // GH#2715 - If all this succeeded, but we're in a conpty, return `false` to + // make the state machine propogate this RIS sequence to the connected + // terminal application. We've reset our state, but the connected terminal + // might need to do more. + bool isPty = false; + _pConApi->IsConsolePty(isPty); + if (isPty) + { + return false; + } + return success; } From 0b87ee3b4366acf31e36478ca6941ecb8554a768 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 27 Jan 2020 11:39:43 -0600 Subject: [PATCH 2/3] Stashing this for now - I need the tests configured as they are in #4354 for this to be testable --- .../ConptyRoundtripTests.cpp | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index badc0548e9b..0104c1dc258 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -103,6 +103,9 @@ class ConptyRoundtripTests auto pfn = std::bind(&ConptyRoundtripTests::_writeCallback, this, std::placeholders::_1, std::placeholders::_2); _pVtRenderEngine->SetTestCallback(pfn); + // TODO: configure the OutputStateMachine's _pfnFlushToTerminal + // Use OutputStateMachineEngine::SetTerminalConnection + g.pRender->AddRenderEngine(_pVtRenderEngine.get()); gci.GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get()); @@ -129,6 +132,7 @@ class ConptyRoundtripTests TEST_METHOD(SimpleWriteOutputTest); TEST_METHOD(WriteTwoLinesUsesNewline); TEST_METHOD(WriteAFewSimpleLines); + TEST_METHOD(PassthroughClearScrollback); private: bool _writeCallback(const char* const pch, size_t const cch); @@ -364,3 +368,70 @@ void ConptyRoundtripTests::WriteAFewSimpleLines() verifyData(termTb); } + +void ConptyRoundtripTests::PassthroughClearScrollback() +{ + Log::Comment(NoThrowString().Format( + L"Write more lines of outout. We should use \r\n to move the cursor")); + 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(); + + const auto hostView = si.GetViewport(); + for (auto i = 0; i < 2 * hostView.Height(); i++) + { + hostSm.ProcessString(L"X\n"); + + expectedOutput.push_back("X"); + expectedOutput.push_back("\r\n"); + } + + // hostSm.ProcessString(L"AAA\n"); + // hostSm.ProcessString(L"BBB\n"); + // hostSm.ProcessString(L"\n"); + // hostSm.ProcessString(L"CCC"); + // auto verifyData = [](TextBuffer& tb) { + // _verifyExpectedString(tb, L"AAA", { 0, 0 }); + // _verifyExpectedString(tb, L"BBB", { 0, 1 }); + // _verifyExpectedString(tb, L" ", { 0, 2 }); + // _verifyExpectedString(tb, L"CCC", { 0, 3 }); + // }; + + // verifyData(hostTb); + + // expectedOutput.push_back("AAA"); + // expectedOutput.push_back("\r\n"); + // expectedOutput.push_back("BBB"); + // expectedOutput.push_back("\r\n"); + // // Here, we're going to emit 3 spaces. The region that got invalidated was a + // // rectangle from 0,0 to 3,3, so the vt renderer will try to render the + // // region in between BBB and CCC as well, because it got included in the + // // rectangle Or() operation. + // // This behavior should not be seen as binding - if a future optimization + // // breaks this test, it wouldn't be the worst. + // expectedOutput.push_back(" "); + // expectedOutput.push_back("\r\n"); + // expectedOutput.push_back("CCC"); + + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + // verifyData(termTb); + + const auto termFirstView = term->GetViewport(); + for (short y = 0; y < 2 * termFirstView.Height(); y++) + { + _verifyExpectedString(termTb, L"X ", { 0, y }); + } + + // TODO: Make sure that a \e[3J comes through conpty here and clears the terminal scrollback. + + // This might need my other branch tbh +} From 4f5ab8010965fd161806eaab8efb7d0e88ddc54a Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 31 Jan 2020 16:43:08 -0600 Subject: [PATCH 3/3] This fixes the test for #2715 --- .../ConptyRoundtripTests.cpp | 82 ++++++++++--------- 1 file changed, 44 insertions(+), 38 deletions(-) diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index c3c782132e7..a66f21a1ab8 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -115,9 +115,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final auto pfn = std::bind(&ConptyRoundtripTests::_writeCallback, this, std::placeholders::_1, std::placeholders::_2); _pVtRenderEngine->SetTestCallback(pfn); - // TODO: configure the OutputStateMachine's _pfnFlushToTerminal + // Configure the OutputStateMachine's _pfnFlushToTerminal // Use OutputStateMachineEngine::SetTerminalConnection - g.pRender->AddRenderEngine(_pVtRenderEngine.get()); gci.GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get()); @@ -356,58 +355,65 @@ void ConptyRoundtripTests::PassthroughClearScrollback() auto& gci = g.getConsoleInformation(); auto& si = gci.GetActiveOutputBuffer(); auto& hostSm = si.GetStateMachine(); - // auto& hostTb = si.GetTextBuffer(); auto& termTb = *term->_buffer; _flushFirstFrame(); + _logConpty = true; + const auto hostView = si.GetViewport(); - for (auto i = 0; i < 2 * hostView.Height(); i++) + const auto end = 2 * hostView.Height(); + for (auto i = 0; i < end; i++) { + Log::Comment(NoThrowString().Format(L"Writing line %d/%d", i, end)); + expectedOutput.push_back("X"); + if (i < hostView.BottomInclusive()) + { + expectedOutput.push_back("\r\n"); + } + else + { + // After we hit the bottom of the viewport, the newlines come in + // seperated for whatever reason. + expectedOutput.push_back("\r"); + expectedOutput.push_back("\n"); + expectedOutput.push_back(""); + } + hostSm.ProcessString(L"X\n"); - expectedOutput.push_back("X"); - expectedOutput.push_back("\r\n"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); } - // hostSm.ProcessString(L"AAA\n"); - // hostSm.ProcessString(L"BBB\n"); - // hostSm.ProcessString(L"\n"); - // hostSm.ProcessString(L"CCC"); - // auto verifyData = [](TextBuffer& tb) { - // _verifyExpectedString(tb, L"AAA", { 0, 0 }); - // _verifyExpectedString(tb, L"BBB", { 0, 1 }); - // _verifyExpectedString(tb, L" ", { 0, 2 }); - // _verifyExpectedString(tb, L"CCC", { 0, 3 }); - // }; - - // verifyData(hostTb); - - // expectedOutput.push_back("AAA"); - // expectedOutput.push_back("\r\n"); - // expectedOutput.push_back("BBB"); - // expectedOutput.push_back("\r\n"); - // // Here, we're going to emit 3 spaces. The region that got invalidated was a - // // rectangle from 0,0 to 3,3, so the vt renderer will try to render the - // // region in between BBB and CCC as well, because it got included in the - // // rectangle Or() operation. - // // This behavior should not be seen as binding - if a future optimization - // // breaks this test, it wouldn't be the worst. - // expectedOutput.push_back(" "); - // expectedOutput.push_back("\r\n"); - // expectedOutput.push_back("CCC"); - VERIFY_SUCCEEDED(renderer.PaintFrame()); - // verifyData(termTb); - + // Verify that we've printed height*2 lines of X's to the Terminal const auto termFirstView = term->GetViewport(); for (short y = 0; y < 2 * termFirstView.Height(); y++) { - _verifyExpectedString(termTb, L"X ", { 0, y }); + TestUtils::VerifyExpectedString(termTb, L"X ", { 0, y }); } - // TODO: Make sure that a \e[3J comes through conpty here and clears the terminal scrollback. + // Write a Erase Scrollback VT sequence to the host, it should come through to the Terminal + expectedOutput.push_back("\x1b[3J"); + hostSm.ProcessString(L"\x1b[3J"); + + _checkConptyOutput = false; - // This might need my other branch tbh + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + const auto termSecondView = term->GetViewport(); + VERIFY_ARE_EQUAL(0, termSecondView.Top()); + + // Verify the top of the Terminal veiwoprt contains the contents of the old viewport + for (short y = 0; y < termSecondView.BottomInclusive(); y++) + { + TestUtils::VerifyExpectedString(termTb, L"X ", { 0, y }); + } + + // Verify below the new viewport (the old viewport) has been cleared out + for (short y = termSecondView.BottomInclusive(); y < termFirstView.BottomInclusive(); y++) + { + TestUtils::VerifyExpectedString(termTb, std::wstring(TerminalViewWidth, L' '), { 0, y }); + } }