Skip to content

Commit

Permalink
Add support for horizontal margin sequences (#15084)
Browse files Browse the repository at this point in the history
This PR introduces two new escapes sequences: `DECLRMM` (Left Right
Margin Mode), which enables the horizontal margin support, and `DECSLRM`
(Set Left and Right Margins), which does exactly what the name suggests.

A whole lot of existing operations have then been updated to take those
horizontal margins into account.

## Detailed Description of the Pull Request / Additional comments

The main complication was in figuring out in what way each operation is
affected, since there's a fair amount of variation.

* When writing text to the buffer, we need to wrap within the horizontal
margins, but only when the cursor is also within the vertical margins,
otherwise we just wrap within the boundaries of the screen.

* Not all cursor movement operations are constrained by the margins, but
for those that are, we clamp within both the vertical and horizontal
margins. But if the cursor is already outside the margins, it is just
clamped at the edges of the screen.

* The `ICH` and `DCH` operations are constrained by the horizontal
margins, but only when inside the vertical margins. And if the cursor is
outside the horizontal margins, these operations have no effect at all.

* The rectangular area operations are clamped within the horizontal
margins when in the origin mode, the same way they're clamped within the
vertical margins.

* The scrolling operations only scroll the area inside both horizontal
and vertical margins. This includes the `IL` and `DL` operations, but
they also won't have any effect at all unless the cursor is already
inside the margin area.

* `CR` returns to the left margin rather than the start of the line,
unless the cursor is already left of that margin, or outside the
vertical margins.

* `LF`, `VT`, `FF`, and `IND` only trigger a scroll at the bottom margin
if the cursor is already inside both vertical and horizontal margins.
The same rules apply to `RI` when triggering a scroll at the top margin.

Another thing worth noting is the change to the `ANSISYSSC` operation.
Since that shares the same escape sequence as `DECSLRM`, they can't both
be active at the same time. However, the latter is only meant to be
active when `DECLRMM` is set, so by default `ANSISYSC` will still work,
but it'll no longer apply once the `DECLRMM` mode is enabled.

## Validation Steps Performed

Thanks to @al20878, these operations have been extensively tested on a
number of DEC terminals and I've manually confirmed our implementation
matches their behavior.

I've also extended some of our existing unit tests to cover at least the
basic margin handling, although not all of the edge cases.

Closes #14876
  • Loading branch information
j4james authored May 15, 2023
1 parent 4628ceb commit b00b77a
Show file tree
Hide file tree
Showing 13 changed files with 1,025 additions and 351 deletions.
4 changes: 4 additions & 0 deletions .github/actions/spelling/expect/alphabet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ ABCDEFGHIJ
abcdefghijk
ABCDEFGHIJKLMNO
abcdefghijklmnop
ABCDEFGHIJKLMNOPQRS
ABCDEFGHIJKLMNOPQRST
ABCG
ABE
Expand All @@ -21,9 +22,12 @@ BBGGRR
efg
EFG
EFGh
KLMNOQQQQQQQQQQ
QQQQQQQQQQABCDEFGHIJ
QQQQQQQQQQABCDEFGHIJKLMNOPQRS
QQQQQQQQQQABCDEFGHIJKLMNOPQRSTQQQQQQQQQ
QQQQQQQQQQABCDEFGHIJKLMNOPQRSTQQQQQQQQQQ
QQQQQQQQQQABCDEFGHIJPQRST
QQQQQQQQQQABCDEFGHIJPQRSTQQQQQQQQQQ
qrstuvwxyz
qwerty
Expand Down
905 changes: 646 additions & 259 deletions src/host/ut_host/ScreenBufferTests.cpp

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/terminal/adapter/DispatchTypes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes
XTERM_EnableDECCOLMSupport = DECPrivateMode(40),
DECNKM_NumericKeypadMode = DECPrivateMode(66),
DECBKM_BackarrowKeyMode = DECPrivateMode(67),
DECLRMM_LeftRightMarginMode = DECPrivateMode(69),
VT200_MOUSE_MODE = DECPrivateMode(1000),
BUTTON_EVENT_MOUSE_MODE = DECPrivateMode(1002),
ANY_EVENT_MOUSE_MODE = DECPrivateMode(1003),
Expand Down
1 change: 1 addition & 0 deletions src/terminal/adapter/ITermDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch
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
virtual bool SetLeftRightScrollingMargins(const VTInt leftMargin, const VTInt rightMargin) = 0; // DECSLRM
virtual bool WarningBell() = 0; // BEL
virtual bool CarriageReturn() = 0; // CR
virtual bool LineFeed(const DispatchTypes::LineFeedType lineFeedType) = 0; // IND, NEL, LF, FF, VT
Expand Down
403 changes: 329 additions & 74 deletions src/terminal/adapter/adaptDispatch.cpp

Large diffs are not rendered by default.

12 changes: 11 additions & 1 deletion src/terminal/adapter/adaptDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ namespace Microsoft::Console::VirtualTerminal
bool SetAnsiMode(const bool ansiMode) override; // DECANM
bool SetTopBottomScrollingMargins(const VTInt topMargin,
const VTInt bottomMargin) override; // DECSTBM
bool SetLeftRightScrollingMargins(const VTInt leftMargin,
const VTInt rightMargin) override; // DECSLRM
bool WarningBell() override; // BEL
bool CarriageReturn() override; // CR
bool LineFeed(const DispatchTypes::LineFeedType lineFeedType) override; // IND, NEL, LF, FF, VT
Expand Down Expand Up @@ -163,6 +165,7 @@ namespace Microsoft::Console::VirtualTerminal
Origin,
Column,
AllowDECCOLM,
AllowDECSLRM,
RectangularChangeExtent
};
enum class ScrollDirection
Expand Down Expand Up @@ -201,6 +204,7 @@ namespace Microsoft::Console::VirtualTerminal

void _WriteToBuffer(const std::wstring_view string);
std::pair<int, int> _GetVerticalMargins(const til::rect& viewport, const bool absolute) noexcept;
std::pair<int, int> _GetHorizontalMargins(const til::CoordType bufferWidth) noexcept;
bool _CursorMovePosition(const Offset rowOffset, const Offset colOffset, const bool clampInMargins);
void _ApplyCursorMovementFlags(Cursor& cursor) noexcept;
void _FillRect(TextBuffer& textBuffer, const til::rect& fillRect, const wchar_t fillChar, const TextAttribute fillAttrs);
Expand All @@ -217,7 +221,12 @@ namespace Microsoft::Console::VirtualTerminal
void _ScrollMovement(const VTInt delta);

void _DoSetTopBottomScrollingMargins(const VTInt topMargin,
const VTInt bottomMargin);
const VTInt bottomMargin,
const bool homeCursor = false);
void _DoSetLeftRightScrollingMargins(const VTInt leftMargin,
const VTInt rightMargin,
const bool homeCursor = false);

void _DoLineFeed(TextBuffer& textBuffer, const bool withReturn, const bool wrapForced);

void _OperatingStatus() const;
Expand All @@ -239,6 +248,7 @@ namespace Microsoft::Console::VirtualTerminal

void _ReportSGRSetting() const;
void _ReportDECSTBMSetting();
void _ReportDECSLRMSetting();
void _ReportDECSCASetting() const;
void _ReportDECSACESetting() const;
void _ReportDECACSetting(const VTInt itemNumber) const;
Expand Down
1 change: 1 addition & 0 deletions src/terminal/adapter/termDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Cons
bool SetKeypadMode(const bool /*applicationMode*/) override { return false; } // DECKPAM, DECKPNM
bool SetAnsiMode(const bool /*ansiMode*/) override { return false; } // DECANM
bool SetTopBottomScrollingMargins(const VTInt /*topMargin*/, const VTInt /*bottomMargin*/) override { return false; } // DECSTBM
bool SetLeftRightScrollingMargins(const VTInt /*leftMargin*/, const VTInt /*rightMargin*/) override { return false; } // DECSLRM
bool WarningBell() override { return false; } // BEL
bool CarriageReturn() override { return false; } // CR
bool LineFeed(const DispatchTypes::LineFeedType /*lineFeedType*/) override { return false; } // IND, NEL, LF, FF, VT
Expand Down
18 changes: 17 additions & 1 deletion src/terminal/adapter/ut_adapter/adapterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1613,6 +1613,22 @@ class AdapterTest
requestSetting(L"r");
_testGetSet->ValidateInputEvent(L"\033P1$r1;25r\033\\");

Log::Comment(L"Requesting DECSLRM margins (5 to 10).");
_testGetSet->PrepData();
// We need to enable DECLRMM for horizontal margins to work.
_pDispatch->SetMode(DispatchTypes::DECLRMM_LeftRightMarginMode);
_pDispatch->SetLeftRightScrollingMargins(5, 10);
requestSetting(L"s");
_testGetSet->ValidateInputEvent(L"\033P1$r5;10s\033\\");

Log::Comment(L"Requesting DECSLRM margins (full width).");
_testGetSet->PrepData();
_pDispatch->SetLeftRightScrollingMargins(0, 0);
requestSetting(L"s");
_testGetSet->ValidateInputEvent(L"\033P1$r1;100s\033\\");
// Reset DECLRMM once we're done with horizontal margin testing.
_pDispatch->ResetMode(DispatchTypes::DECLRMM_LeftRightMarginMode);

Log::Comment(L"Requesting SGR attributes (default).");
_testGetSet->PrepData();
TextAttribute attribute = {};
Expand Down Expand Up @@ -1779,7 +1795,7 @@ class AdapterTest
// and DECRQM would not then be applicable.

BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:modeNumber", L"{1, 3, 5, 6, 7, 8, 12, 25, 40, 66, 67, 1000, 1002, 1003, 1004, 1005, 1006, 1007, 1049, 2004, 9001}")
TEST_METHOD_PROPERTY(L"Data:modeNumber", L"{1, 3, 5, 6, 7, 8, 12, 25, 40, 66, 67, 69, 1000, 1002, 1003, 1004, 1005, 1006, 1007, 1049, 2004, 9001}")
END_TEST_METHOD_PROPERTIES()

VTInt modeNumber;
Expand Down
13 changes: 7 additions & 6 deletions src/terminal/parser/OutputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -491,10 +491,15 @@ bool OutputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParamete
success = _dispatch->CursorPosition(parameters.at(0), parameters.at(1));
TermTelemetry::Instance().Log(TermTelemetry::Codes::CUP);
break;
case CsiActionCodes::DECSTBM_SetScrollingRegion:
case CsiActionCodes::DECSTBM_SetTopBottomMargins:
success = _dispatch->SetTopBottomScrollingMargins(parameters.at(0).value_or(0), parameters.at(1).value_or(0));
TermTelemetry::Instance().Log(TermTelemetry::Codes::DECSTBM);
break;
case CsiActionCodes::DECSLRM_SetLeftRightMargins:
// Note that this can also be ANSISYSSC, depending on the state of DECLRMM.
success = _dispatch->SetLeftRightScrollingMargins(parameters.at(0).value_or(0), parameters.at(1).value_or(0));
TermTelemetry::Instance().Log(TermTelemetry::Codes::DECSTBM);
break;
case CsiActionCodes::ICH_InsertCharacter:
success = _dispatch->InsertCharacter(parameters.at(0));
TermTelemetry::Instance().Log(TermTelemetry::Codes::ICH);
Expand Down Expand Up @@ -588,12 +593,8 @@ bool OutputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParamete
success = _dispatch->ScrollDown(parameters.at(0));
TermTelemetry::Instance().Log(TermTelemetry::Codes::SD);
break;
case CsiActionCodes::ANSISYSSC_CursorSave:
success = parameters.empty() && _dispatch->CursorSaveState();
TermTelemetry::Instance().Log(TermTelemetry::Codes::ANSISYSSC);
break;
case CsiActionCodes::ANSISYSRC_CursorRestore:
success = parameters.empty() && _dispatch->CursorRestoreState();
success = _dispatch->CursorRestoreState();
TermTelemetry::Instance().Log(TermTelemetry::Codes::ANSISYSRC);
break;
case CsiActionCodes::IL_InsertLine:
Expand Down
4 changes: 2 additions & 2 deletions src/terminal/parser/OutputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ namespace Microsoft::Console::VirtualTerminal
SGR_SetGraphicsRendition = VTID("m"),
DSR_DeviceStatusReport = VTID("n"),
DSR_PrivateDeviceStatusReport = VTID("?n"),
DECSTBM_SetScrollingRegion = VTID("r"),
ANSISYSSC_CursorSave = VTID("s"), // NOTE: Overlaps with DECLRMM/DECSLRM. Fix when/if implemented.
DECSTBM_SetTopBottomMargins = VTID("r"),
DECSLRM_SetLeftRightMargins = VTID("s"),
DTTERM_WindowManipulation = VTID("t"), // NOTE: Overlaps with DECSLPP. Fix when/if implemented.
ANSISYSRC_CursorRestore = VTID("u"),
DECREQTPARM_RequestTerminalParameters = VTID("x"),
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/parser/telemetry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,9 @@ void TermTelemetry::WriteFinalTraceLog() const
TraceLoggingUInt32(_uiTimesUsed[DL], "DL"),
TraceLoggingUInt32(_uiTimesUsed[SU], "SU"),
TraceLoggingUInt32(_uiTimesUsed[SD], "SD"),
TraceLoggingUInt32(_uiTimesUsed[ANSISYSSC], "ANSISYSSC"),
TraceLoggingUInt32(_uiTimesUsed[ANSISYSRC], "ANSISYSRC"),
TraceLoggingUInt32(_uiTimesUsed[DECSTBM], "DECSTBM"),
TraceLoggingUInt32(_uiTimesUsed[DECSLRM], "DECSLRM"),
TraceLoggingUInt32(_uiTimesUsed[NEL], "NEL"),
TraceLoggingUInt32(_uiTimesUsed[IND], "IND"),
TraceLoggingUInt32(_uiTimesUsed[RI], "RI"),
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/parser/telemetry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ namespace Microsoft::Console::VirtualTerminal
DCH,
SU,
SD,
ANSISYSSC,
ANSISYSRC,
IL,
DL,
DECSTBM,
DECSLRM,
NEL,
IND,
RI,
Expand Down
10 changes: 4 additions & 6 deletions src/terminal/parser/ut_parser/OutputEngineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1657,12 +1657,10 @@ class StateMachineExternalTest final

pDispatch->ClearState();

mach.ProcessCharacter(AsciiChars::ESC);
mach.ProcessCharacter(L'[');
mach.ProcessCharacter(L's');
VERIFY_IS_TRUE(pDispatch->_cursorSave);

pDispatch->ClearState();
// Note that CSI s is dispatched as SetLeftRightScrollingMargins rather
// than CursorSaveState, so we don't test that here. The CursorSaveState
// will only be triggered by this sequence (in AdaptDispatch) when the
// Left-Right-Margin mode (DECLRMM) is disabled.

mach.ProcessCharacter(AsciiChars::ESC);
mach.ProcessCharacter(L'[');
Expand Down

1 comment on commit b00b77a

@BSteffaniak
Copy link

Choose a reason for hiding this comment

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

This is a fun commit hash

Please sign in to comment.