Skip to content

Add support for horizontal scrolling sequences #15368

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

Merged
merged 8 commits into from
May 25, 2023
Merged

Add support for horizontal scrolling sequences #15368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fa105e8
Hook up the DECIC/DECDC sequences.
j4james Feb 23, 2023
549b4fb
Implement the DECIC/DECIC operations.
j4james Feb 23, 2023
b93b60d
Hook up the DECBI/DECFI sequences.
j4james Feb 23, 2023
9182d02
Implement the DECBI/DECFI operations.
j4james Feb 23, 2023
25c9534
Add some unit tests.
j4james Apr 23, 2023
b67320a
Update device attributes report.
j4james May 16, 2023
ad19d42
Add controls and test text to spelling dictionary.
j4james May 16, 2023
830bdd8
Remove unnecessary int32_t casts.
j4james May 22, 2023
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
11 changes: 11 additions & 0 deletions .github/actions/spelling/expect/expect.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ DECANM
DECARM
DECAUPSS
DECAWM
DECBI
DECBKM
DECCARA
DECCIR
Expand All @@ -418,13 +419,16 @@ DECCKSR
DECCOLM
DECCRA
DECCTR
DECDC
DECDHL
decdld
DECDMAC
DECDWL
DECEKBD
DECERA
DECFI
DECFRA
DECIC
DECID
DECINVM
DECKPAM
Expand Down Expand Up @@ -571,6 +575,7 @@ EDITTEXT
EDITUPDATE
edputil
Efast
efghijklmn
EHsc
EINS
EJO
Expand Down Expand Up @@ -971,6 +976,8 @@ KLF
KLMNO
KLMNOPQRST
KLMNOPQRSTQQQQQ
KLMNOPQRSTUVWXY
KLMNOPQRSTY
KOK
KPRIORITY
KVM
Expand Down Expand Up @@ -1134,6 +1141,7 @@ mmsystem
MNC
MNOPQ
MNOPQR
MNOPQRSTUVWXY
MODALFRAME
MODERNCORE
MONITORINFO
Expand Down Expand Up @@ -2044,6 +2052,7 @@ USRDLL
utr
UVWX
UVWXY
UVWXYZ
uwa
uwp
uxtheme
Expand Down Expand Up @@ -2304,7 +2313,9 @@ YOffset
YSubstantial
YVIRTUALSCREEN
YWalk
Zab
zabcd
Zabcdefghijklmn
Zabcdefghijklmnopqrstuvwxyz
ZCmd
ZCtrl
Expand Down
112 changes: 112 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 @@ -186,6 +186,7 @@ class ScreenBufferTests
TEST_METHOD(InsertReplaceMode);
TEST_METHOD(InsertChars);
TEST_METHOD(DeleteChars);
TEST_METHOD(HorizontalScrollOperations);
TEST_METHOD(ScrollingWideCharsHorizontally);

TEST_METHOD(EraseScrollbackTests);
Expand Down Expand Up @@ -4346,6 +4347,117 @@ void ScreenBufferTests::DeleteChars()
}
}

void ScreenBufferTests::HorizontalScrollOperations()
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:op", L"{0, 1, 2, 3}")
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding cleanup annotations, you can also use method isolation. It's maybe not worth changing now, but worth considering in the future :)

I don't know what downsides exist... so that could be a problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It depends how much cleanup is involved, but I usually try to avoid method isolation if I can help it, because I thought it made the tests quite a bit slower. It also makes them more painful to debug, because you can only step through a single iteration and then it complains about TAEF needing to start a second test host (I don't know if there's a workaround for that which I'm just not aware of).

END_TEST_METHOD_PROPERTIES();

enum Op : int
{
DECIC,
DECDC,
DECFI,
DECBI
} op;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"op", (int&)op));

auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer().GetActiveBuffer();
auto& stateMachine = si.GetStateMachine();
WI_SetFlag(si.OutputMode, ENABLE_VIRTUAL_TERMINAL_PROCESSING);

// Set the buffer width to 40, with a centered viewport of 20.
const auto bufferWidth = 40;
const auto bufferHeight = si.GetBufferSize().Height();
const auto viewportStart = 10;
const auto viewportEnd = viewportStart + 20;
VERIFY_SUCCEEDED(si.ResizeScreenBuffer({ bufferWidth, bufferHeight }, false));
si.SetViewport(Viewport::FromExclusive({ viewportStart, 0, viewportEnd, 25 }), true);

// Set the margin area to columns 10 to 29 and rows 14 to 19 (zero based).
stateMachine.ProcessString(L"\x1b[?69h");
stateMachine.ProcessString(L"\x1b[11;30s");
stateMachine.ProcessString(L"\x1b[15;20r");
// Make sure we clear the margins on exit so they can't break other tests.
auto clearMargins = wil::scope_exit([&] {
stateMachine.ProcessString(L"\x1b[r");
stateMachine.ProcessString(L"\x1b[s");
stateMachine.ProcessString(L"\x1b[?69l");
});

// Fill the buffer with text. Red on Blue.
const auto bufferChars = L"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmn";
const auto bufferAttr = TextAttribute{ FOREGROUND_RED | BACKGROUND_BLUE };
_FillLines(0, 25, bufferChars, bufferAttr);

// Set the attributes that will be used to fill the revealed area.
auto fillAttr = TextAttribute{ RGB(12, 34, 56), RGB(78, 90, 12) };
fillAttr.SetCrossedOut(true);
fillAttr.SetReverseVideo(true);
fillAttr.SetUnderlined(true);
si.SetAttributes(fillAttr);
// But note that the meta attributes are expected to be cleared.
auto expectedFillAttr = fillAttr;
expectedFillAttr.SetStandardErase();

auto& cursor = si.GetTextBuffer().GetCursor();
switch (op)
{
case DECIC:
Log::Comment(L"Insert 4 columns in the middle of the margin area.");
cursor.SetPosition({ 20, 17 });
stateMachine.ProcessString(L"\x1b[4'}");
VERIFY_ARE_EQUAL(til::point(20, 17), cursor.GetPosition(), L"The cursor should not move.");
VERIFY_IS_TRUE(_ValidateLinesContain(10, 14, 20, L"KLMNOPQRST", bufferAttr),
L"The margin area left of the cursor position should remain unchanged.");
VERIFY_IS_TRUE(_ValidateLinesContain(20, 14, 20, L" ", expectedFillAttr),
L"4 blank columns should be inserted at the cursor position.");
VERIFY_IS_TRUE(_ValidateLinesContain(24, 14, 20, L"UVWXYZ", bufferAttr),
L"The area right of that should be scrolled right by 4 columns.");
break;
case DECDC:
Log::Comment(L"Delete 4 columns in the middle of the margin area.");
cursor.SetPosition({ 20, 17 });
stateMachine.ProcessString(L"\x1b[4'~");
VERIFY_ARE_EQUAL(til::point(20, 17), cursor.GetPosition(), L"The cursor should not move.");
VERIFY_IS_TRUE(_ValidateLinesContain(10, 14, 20, L"KLMNOPQRSTYZabcd", bufferAttr),
L"The area right of the cursor position should be scrolled left by 4 columns.");
VERIFY_IS_TRUE(_ValidateLinesContain(26, 14, 20, L" ", expectedFillAttr),
L"4 blank columns should be inserted at the right of the margin area.");
break;
case DECFI:
Log::Comment(L"Forward index 4 times, 2 columns before the right margin.");
cursor.SetPosition({ 27, 17 });
stateMachine.ProcessString(L"\x1b\x39\x1b\x39\x1b\x39\x1b\x39");
VERIFY_ARE_EQUAL(til::point(29, 17), cursor.GetPosition(), L"The cursor should not pass the right margin.");
VERIFY_IS_TRUE(_ValidateLinesContain(10, 14, 20, L"MNOPQRSTUVWXYZabcd", bufferAttr),
L"The margin area should scroll left by 2 columns.");
VERIFY_IS_TRUE(_ValidateLinesContain(28, 14, 20, L" ", expectedFillAttr),
L"2 blank columns should be inserted at the right of the margin area.");
break;
case DECBI:
Log::Comment(L"Back index 4 times, 2 columns before the left margin.");
cursor.SetPosition({ 12, 17 });
stateMachine.ProcessString(L"\x1b\x36\x1b\x36\x1b\x36\x1b\x36");
VERIFY_ARE_EQUAL(til::point(10, 17), cursor.GetPosition(), L"The cursor should not pass the left margin.");
VERIFY_IS_TRUE(_ValidateLinesContain(10, 14, 20, L" ", expectedFillAttr),
L"2 blank columns should be inserted at the left of the margin area.");
VERIFY_IS_TRUE(_ValidateLinesContain(12, 14, 20, L"KLMNOPQRSTUVWXYZab", bufferAttr),
L"The rest of the margin area should scroll right by 2 columns.");
break;
}

VERIFY_IS_TRUE(_ValidateLinesContain(0, 14, bufferChars, bufferAttr),
L"Content above the top margin should remain unchanged.");
VERIFY_IS_TRUE(_ValidateLinesContain(20, 25, bufferChars, bufferAttr),
L"Content below the bottom margin should remain unchanged.");
VERIFY_IS_TRUE(_ValidateLinesContain(0, 14, 20, L"ABCDEFGHIJ", bufferAttr),
L"Content before the left margin should remain unchanged.");
VERIFY_IS_TRUE(_ValidateLinesContain(30, 14, 20, L"efghijklmn", bufferAttr),
L"Content beyond the right margin should remain unchanged.");
}

void ScreenBufferTests::ScrollingWideCharsHorizontally()
{
// The point of this test is to make sure wide characters can be
Expand Down
4 changes: 4 additions & 0 deletions src/terminal/adapter/ITermDispatch.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch
virtual bool ScrollDown(const VTInt distance) = 0; // SD
virtual bool InsertLine(const VTInt distance) = 0; // IL
virtual bool DeleteLine(const VTInt distance) = 0; // DL
virtual bool InsertColumn(const VTInt distance) = 0; // DECIC
virtual bool DeleteColumn(const VTInt distance) = 0; // DECDC
virtual bool SetKeypadMode(const bool applicationMode) = 0; // DECKPAM, DECKPNM
virtual bool SetAnsiMode(const bool ansiMode) = 0; // DECANM
virtual bool SetTopBottomScrollingMargins(const VTInt topMargin, const VTInt bottomMargin) = 0; // DECSTBM
Expand All @@ -59,6 +61,8 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch
virtual bool CarriageReturn() = 0; // CR
virtual bool LineFeed(const DispatchTypes::LineFeedType lineFeedType) = 0; // IND, NEL, LF, FF, VT
virtual bool ReverseLineFeed() = 0; // RI
virtual bool BackIndex() = 0; // DECBI
virtual bool ForwardIndex() = 0; // DECFI
virtual bool SetWindowTitle(std::wstring_view title) = 0; // OscWindowTitle
virtual bool HorizontalTabSet() = 0; // HTS
virtual bool ForwardTab(const VTInt numTabs) = 0; // CHT, HT
Expand Down
Loading