Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passthrough CSI 3 J in Conpty #4433

Merged
4 commits merged into from
Feb 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
auto pfn = std::bind(&ConptyRoundtripTests::_writeCallback, this, std::placeholders::_1, std::placeholders::_2);
_pVtRenderEngine->SetTestCallback(pfn);

// Configure the OutputStateMachine's _pfnFlushToTerminal
// Use OutputStateMachineEngine::SetTerminalConnection
g.pRender->AddRenderEngine(_pVtRenderEngine.get());
gci.GetActiveOutputBuffer().SetTerminalConnection(_pVtRenderEngine.get());

Expand Down Expand Up @@ -148,6 +150,7 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
TEST_METHOD(SimpleWriteOutputTest);
TEST_METHOD(WriteTwoLinesUsesNewline);
TEST_METHOD(WriteAFewSimpleLines);
TEST_METHOD(PassthroughClearScrollback);

private:
bool _writeCallback(const char* const pch, size_t const cch);
Expand Down Expand Up @@ -340,3 +343,77 @@ 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& termTb = *term->_buffer;

_flushFirstFrame();

_logConpty = true;

const auto hostView = si.GetViewport();
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");

VERIFY_SUCCEEDED(renderer.PaintFrame());
}

VERIFY_SUCCEEDED(renderer.PaintFrame());

// 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++)
{
TestUtils::VerifyExpectedString(termTb, L"X ", { 0, y });
}

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

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 });
}
}
20 changes: 19 additions & 1 deletion src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,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)
{
Expand Down Expand Up @@ -1505,6 +1512,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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really feels like a function that should have been returning the bool...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big +1. This is one of things I was hoping to clean up when moving to exceptions in place of return values for the error handling (which is still on my long term todo list). But some of these getters aren't even using the return value, so they don't actually need to have the exception handling in place - it should be an easy update.

if (isPty)
{
return false;
}

return success;
}

Expand Down