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.

Sign up for GitHub

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

Jump to bottom

Bugfix: CLS should clear current active buffer #2729

Merged
merged 2 commits into from
Sep 13, 2019
Merged

Bugfix: CLS should clear current active buffer #2729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c7eb97
Bugfix: cls won't clear alt buffer
carlos-zamora Sep 11, 2019
4e4480c
VT Mode detection checked by buffer, not context
carlos-zamora Sep 12, 2019
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
19 changes: 12 additions & 7 deletions src/host/getset.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,9 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont

CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();

const COORD coordScreenBufferSize = context.GetBufferSize().Dimensions();
auto& buffer = context.GetActiveBuffer();

const COORD coordScreenBufferSize = buffer.GetBufferSize().Dimensions();
// clang-format off
RETURN_HR_IF(E_INVALIDARG, (position.X >= coordScreenBufferSize.X ||
position.Y >= coordScreenBufferSize.Y ||
Expand All @@ -643,15 +645,15 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
// MSFT: 15813316 - Try to use this SetCursorPosition call to inherit the cursor position.
RETURN_IF_FAILED(gci.GetVtIo()->SetCursorPosition(position));

RETURN_IF_NTSTATUS_FAILED(context.SetCursorPosition(position, true));
RETURN_IF_NTSTATUS_FAILED(buffer.SetCursorPosition(position, true));

LOG_IF_FAILED(ConsoleImeResizeCompStrView());

COORD WindowOrigin;
WindowOrigin.X = 0;
WindowOrigin.Y = 0;
{
const SMALL_RECT currentViewport = context.GetViewport().ToInclusive();
const SMALL_RECT currentViewport = buffer.GetViewport().ToInclusive();
if (currentViewport.Left > position.X)
{
WindowOrigin.X = position.X - currentViewport.Left;
Expand All @@ -671,7 +673,7 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
}
}

RETURN_IF_NTSTATUS_FAILED(context.SetViewportOrigin(false, WindowOrigin, true));
RETURN_IF_NTSTATUS_FAILED(buffer.SetViewportOrigin(false, WindowOrigin, true));

return S_OK;
}
Expand Down Expand Up @@ -820,6 +822,8 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
LockConsole();
auto Unlock = wil::scope_exit([&] { UnlockConsole(); });

auto& buffer = context.GetActiveBuffer();

TextAttribute useThisAttr(fillAttribute);

// Here we're being a little clever - similar to FillConsoleOutputAttributeImpl
Expand All @@ -835,18 +839,19 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont
// this scenario is highly unlikely, and we can reasonably do this
// on their behalf.
// see MSFT:19853701
if (context.InVTMode())

if (buffer.InVTMode())
{
const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
const auto currentAttributes = context.GetAttributes();
const auto currentAttributes = buffer.GetAttributes();
const auto bufferLegacy = gci.GenerateLegacyAttributes(currentAttributes);
if (bufferLegacy == fillAttribute)
{
useThisAttr = currentAttributes;
}
}

ScrollRegion(context, source, clip, target, fillCharacter, useThisAttr);
ScrollRegion(buffer, source, clip, target, fillCharacter, useThisAttr);

return S_OK;
}
Expand Down
92 changes: 92 additions & 0 deletions src/host/ut_host/ScreenBufferTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ class ScreenBufferTests
TEST_METHOD(HardResetBuffer);

TEST_METHOD(RestoreDownAltBufferWithTerminalScrolling);

TEST_METHOD(ClearAlternateBuffer);
};

void ScreenBufferTests::SingleAlternateBufferCreationTest()
Expand Down Expand Up @@ -3710,6 +3712,8 @@ void ScreenBufferTests::RestoreDownAltBufferWithTerminalScrolling()
VERIFY_ARE_EQUAL(0, altBuffer._viewport.Top());
VERIFY_ARE_EQUAL(altBuffer._viewport.BottomInclusive(), altBuffer._virtualBottom);

auto useMain = wil::scope_exit([&] { altBuffer.UseMainScreenBuffer(); });

const COORD originalSize = originalView.Dimensions();
const COORD doubledSize = { originalSize.X * 2, originalSize.Y * 2 };

Expand Down Expand Up @@ -3746,3 +3750,91 @@ void ScreenBufferTests::RestoreDownAltBufferWithTerminalScrolling()
VERIFY_ARE_EQUAL(altBuffer._viewport.BottomInclusive(), altBuffer._virtualBottom);
}
}

void ScreenBufferTests::ClearAlternateBuffer()
{
// This is a test for microsoft/terminal#1189. Refer to that issue for more
// context

CONSOLE_INFORMATION& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto& g = ServiceLocator::LocateGlobals();
gci.LockConsole(); // Lock must be taken to manipulate buffer.
auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); });

auto& siMain = gci.GetActiveOutputBuffer();
auto WriteText = [&](TextBuffer& tbi) {
// Write text to buffer
auto& stateMachine = siMain.GetStateMachine();
auto& cursor = tbi.GetCursor();
stateMachine.ProcessString(L"foo\nfoo");
VERIFY_ARE_EQUAL(cursor.GetPosition().X, 3);
VERIFY_ARE_EQUAL(cursor.GetPosition().Y, 1);
};

auto VerifyText = [&](TextBuffer& tbi) {
// Verify written text in buffer
{
auto iter00 = tbi.GetCellDataAt({ 0, 0 });
auto iter10 = tbi.GetCellDataAt({ 1, 0 });
auto iter20 = tbi.GetCellDataAt({ 2, 0 });
auto iter30 = tbi.GetCellDataAt({ 3, 0 });
auto iter01 = tbi.GetCellDataAt({ 0, 1 });
auto iter02 = tbi.GetCellDataAt({ 1, 1 });
auto iter03 = tbi.GetCellDataAt({ 2, 1 });
VERIFY_ARE_EQUAL(L"f", iter00->Chars());
VERIFY_ARE_EQUAL(L"o", iter10->Chars());
VERIFY_ARE_EQUAL(L"o", iter20->Chars());
VERIFY_ARE_EQUAL(L"\x20", iter30->Chars());
VERIFY_ARE_EQUAL(L"f", iter01->Chars());
VERIFY_ARE_EQUAL(L"o", iter02->Chars());
VERIFY_ARE_EQUAL(L"o", iter03->Chars());
}
};

WriteText(siMain.GetTextBuffer());
VerifyText(siMain.GetTextBuffer());

Log::Comment(L"Create an alternate buffer");
if (VERIFY_IS_TRUE(NT_SUCCESS(siMain.UseAlternateScreenBuffer())))
{
VERIFY_IS_NOT_NULL(siMain._psiAlternateBuffer);
auto& altBuffer = *siMain._psiAlternateBuffer;
VERIFY_ARE_EQUAL(0, altBuffer._viewport.Top());
VERIFY_ARE_EQUAL(altBuffer._viewport.BottomInclusive(), altBuffer._virtualBottom);

auto useMain = wil::scope_exit([&] { altBuffer.UseMainScreenBuffer(); });

WriteText(altBuffer.GetTextBuffer());
VerifyText(altBuffer.GetTextBuffer());

#pragma region Test ScrollConsoleScreenBufferWImpl()
// Clear text of alt buffer (same params as in CMD)
VERIFY_SUCCEEDED(g.api.ScrollConsoleScreenBufferWImpl(siMain,
{ 0, 0, 120, 9001 },
{ 0, -9001 },
std::nullopt,
L' ',
7));

// Verify text is now gone
VERIFY_ARE_EQUAL(L" ", altBuffer.GetTextBuffer().GetCellDataAt({ 0, 0 })->Chars());
#pragma endregion

#pragma region Test SetConsoleCursorPositionImpl()
// Reset cursor position as we do with CLS command (same params as in CMD)
VERIFY_SUCCEEDED(g.api.SetConsoleCursorPositionImpl(siMain, { 0 }));

// Verify state of alt buffer
auto& altBufferCursor = altBuffer.GetTextBuffer().GetCursor();
VERIFY_ARE_EQUAL(altBufferCursor.GetPosition().X, 0);
VERIFY_ARE_EQUAL(altBufferCursor.GetPosition().Y, 0);
#pragma endregion
}

// Verify state of main buffer is untouched
auto& cursor = siMain.GetTextBuffer().GetCursor();
VERIFY_ARE_EQUAL(cursor.GetPosition().X, 3);
VERIFY_ARE_EQUAL(cursor.GetPosition().Y, 1);

VerifyText(siMain.GetTextBuffer());
}