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

Fix clearing marks #17144

Merged
merged 9 commits into from May 2, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/buffer/out/textBuffer.cpp
Expand Up @@ -1189,6 +1189,8 @@ void TextBuffer::ClearScrollback(const til::CoordType start, const til::CoordTyp
return;
}

ClearMarksInRange(til::point{ 0, 0 }, til::point{ _width, height });
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved

// Our goal is to move the viewport to the absolute start of the underlying memory buffer so that we can
// MEM_DECOMMIT the remaining memory. _firstRow is used to make the TextBuffer behave like a circular buffer.
// The start parameter is relative to the _firstRow. The trick to get the content to the absolute start
Expand All @@ -1204,8 +1206,6 @@ void TextBuffer::ClearScrollback(const til::CoordType start, const til::CoordTyp
{
GetMutableRowByOffset(y).Reset(_initialAttributes);
}

ClearMarksInRange(til::point{ 0, height }, til::point{ _width, _height });
}

// Routine Description:
Expand Down
139 changes: 122 additions & 17 deletions src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
Expand Up @@ -239,12 +239,14 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final
TEST_METHOD(MultilinePromptRegions);
TEST_METHOD(ManyMultilinePromptsWithTrailingSpaces);
TEST_METHOD(ReflowPromptRegions);
TEST_METHOD(ClearMarksTest);

private:
bool _writeCallback(const char* const pch, const size_t cch);
void _flushFirstFrame();
void _resizeConpty(const til::CoordType sx, const til::CoordType sy);
void _clearConpty();
void _clear(int clearBufferMethod, SCREEN_INFORMATION& si);

[[nodiscard]] std::tuple<TextBuffer*, TextBuffer*> _performResize(const til::size newSize);

Expand Down Expand Up @@ -2720,9 +2722,6 @@ void ConptyRoundtripTests::ClsAndClearHostClearsScrollbackTest()
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:clearBufferMethod", L"{0, 1, 2}")
END_TEST_METHOD_PROPERTIES();
constexpr auto ClearLikeCls = 0;
constexpr auto ClearLikeClearHost = 1;
constexpr auto ClearWithVT = 2;
INIT_TEST_PROPERTY(int, clearBufferMethod, L"Controls whether we clear the buffer like cmd or like powershell");

Log::Comment(L"This test checks the shims for cmd.exe and powershell.exe. "
Expand Down Expand Up @@ -2789,6 +2788,31 @@ void ConptyRoundtripTests::ClsAndClearHostClearsScrollbackTest()
VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());

_clear(clearBufferMethod, si);

VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());

Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());

Log::Comment(L"========== Checking the host buffer state (after) ==========");
verifyBuffer(*hostTb, si.GetViewport().ToExclusive(), true);

Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"========== Checking the terminal buffer state (after) ==========");
verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true);
}

void ConptyRoundtripTests::_clear(int clearBufferMethod, SCREEN_INFORMATION& si)
{
constexpr auto ClearLikeCls = 0;
constexpr auto ClearLikeClearHost = 1;
constexpr auto ClearWithVT = 2;

auto& sm = si.GetStateMachine();

if (clearBufferMethod == ClearLikeCls)
{
// Execute the cls, EXACTLY LIKE CMD.
Expand Down Expand Up @@ -2840,20 +2864,6 @@ void ConptyRoundtripTests::ClsAndClearHostClearsScrollbackTest()

sm.ProcessString(L"\x1b[3J");
}

VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());

Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());

Log::Comment(L"========== Checking the host buffer state (after) ==========");
verifyBuffer(*hostTb, si.GetViewport().ToExclusive(), true);

Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"========== Checking the terminal buffer state (after) ==========");
verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true);
}

void ConptyRoundtripTests::TestResizeWithCookedRead()
Expand Down Expand Up @@ -4945,3 +4955,98 @@ void ConptyRoundtripTests::ReflowPromptRegions()
Log::Comment(L"========== Checking the terminal buffer state (after) ==========");
verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true, true);
}

void ConptyRoundtripTests::ClearMarksTest()
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:clearBufferMethod", L"{0, 1, 2}")
END_TEST_METHOD_PROPERTIES();

INIT_TEST_PROPERTY(int, clearBufferMethod, L"Controls whether we clear the buffer like cmd or like powershell");

Log::Comment(L"This test checks the shims for cmd.exe and powershell.exe. "
L"Their build in commands for clearing the console buffer "
L"should work to clear the terminal buffer, not just the "
L"terminal viewport.");

auto& g = ServiceLocator::LocateGlobals();
auto& renderer = *g.pRender;
auto& gci = g.getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer();
auto* hostTb = &si.GetTextBuffer();
auto* termTb = term->_mainBuffer.get();

auto& sm = si.GetStateMachine();

_flushFirstFrame();

_checkConptyOutput = false;
_logConpty = false;

const auto hostView = si.GetViewport();
const auto end = 2 * hostView.Height();

auto writePrompt = [](StateMachine& stateMachine, const auto& path) {
// A prompt looks like:
// `PWSH C:\> `
//
// which is 10 characters for "C:\"
stateMachine.ProcessString(FTCS_D);
stateMachine.ProcessString(FTCS_A);
stateMachine.ProcessString(L"\x1b]9;9;");
stateMachine.ProcessString(path);
stateMachine.ProcessString(L"\x7");
stateMachine.ProcessString(L"PWSH ");
stateMachine.ProcessString(path);
stateMachine.ProcessString(L"> ");
stateMachine.ProcessString(FTCS_B);
};
auto writeCommand = [](StateMachine& stateMachine, const auto& cmd) {
stateMachine.ProcessString(cmd);
stateMachine.ProcessString(FTCS_C);
stateMachine.ProcessString(L"\r\n");
};

for (auto i = 0; i < end; i++)
{
writePrompt(sm, L"C:\\");
writeCommand(sm, L"Foo-bar");
sm.ProcessString(L"This is some text \r\n");
}
writePrompt(sm, L"C:\\");

auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& /*viewport*/, const bool afterClear = false) {
const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope;
const auto marks = tb.GetMarkExtents();
if (afterClear)
{
VERIFY_ARE_EQUAL(0u, marks.size());
}
else
{
VERIFY_IS_GREATER_THAN(marks.size(), 1u, L"There should be at least one mark");
}
};

Log::Comment(L"========== Checking the host buffer state (before) ==========");
verifyBuffer(*hostTb, si.GetViewport().ToExclusive());

Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());

Log::Comment(L"========== Checking the terminal buffer state (before) ==========");
verifyBuffer(*termTb, term->_mutableViewport.ToExclusive());

VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());

_clear(clearBufferMethod, si);

VERIFY_ARE_EQUAL(si.GetViewport().Dimensions(), si.GetBufferSize().Dimensions());
VERIFY_ARE_EQUAL(si.GetViewport(), si.GetBufferSize());

Log::Comment(L"Painting the frame");
VERIFY_SUCCEEDED(renderer.PaintFrame());
Log::Comment(L"========== Checking the terminal buffer state (after) ==========");
verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true);
}
4 changes: 0 additions & 4 deletions src/terminal/adapter/adaptDispatch.cpp
Expand Up @@ -3294,10 +3294,6 @@ bool AdaptDispatch::_EraseAll()
// Also reset the line rendition for the erased rows.
textBuffer.ResetLineRenditionRange(newViewportTop, newViewportBottom);

// Clear any marks that remain below the start of the
textBuffer.ClearMarksInRange(til::point{ 0, newViewportTop },
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
til::point{ bufferSize.Width(), bufferSize.Height() });

// GH#5683 - If this succeeded, but we're in a conpty, return `false` to
// make the state machine propagate this ED sequence to the connected
// terminal application. While we're in conpty mode, when the client
Expand Down