Skip to content

Commit

Permalink
Add support for all the line feed control sequences (#3271)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

This adds support for the `FF` (form feed) and `VT` (vertical tab) [control characters](https://vt100.net/docs/vt510-rm/chapter4.html#T4-1), as well as the [`NEL` (Next Line)](https://vt100.net/docs/vt510-rm/NEL.html) and [`IND` (Index)](https://vt100.net/docs/vt510-rm/IND.html) escape sequences.

## References

#976 discusses the conflict between VT100 Index sequence and the VT52 cursor back sequence.

## PR Checklist
* [x] Closes #3189
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Requires documentation to be updated
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #3189

## Detailed Description of the Pull Request / Additional comments

I've added a `LineFeed` method to the `ITermDispatch` interface, with an enum parameter specifying the required line feed type (i.e. with carriage return, without carriage return, or dependent on the [`LNM` mode](https://vt100.net/docs/vt510-rm/LNM.html)). The output state machine can then call that method to handle the various line feed control characters (parsed in the `ActionExecute` method), as well the `NEL` and `IND` escape sequences (parsed in the `ActionEscDispatch` method).

The `AdaptDispatch` implementation of `LineFeed` then forwards the call to a new `PrivateLineFeed` method in the `ConGetSet` interface, which simply takes a bool parameter specifying whether a carriage return is required or not. In the case of mode-dependent line feeds, the `AdaptDispatch` implementation determines whether the return is necessary or not, based on the existing _AutoReturnOnNewLine_ setting (which I'm obtaining via another new `PrivateGetLineFeedMode` method).

Ultimately we'll want to support changing the mode via the [`LNM` escape sequence](https://vt100.net/docs/vt510-rm/LNM.html), but there's no urgent need for that now. And using the existing _AutoReturnOnNewLine_ setting as a substitute for the mode gives us backwards compatible behaviour, since that will be true for the Windows shells (which expect a linefeed to also generate a carriage return), and false in a WSL bash shell (which won't want the carriage return by default).

As for the actual `PrivateLineFeed` implementation, that is just a simplified version of how the line feed would previously have been executed in the `WriteCharsLegacy` function. This includes setting the cursor to "On" (with `Cursor::SetIsOn`), potentially clearing the wrap property of the line being left (with `CharRow::SetWrapForced` false), and then setting the new position using `AdjustCursorPosition` with the _fKeepCursorVisible_ parameter set to false.

I'm unsure whether the `SetIsOn` call is really necessary, and I think the way the forced wrap is handled needs a rethink in general, but for now this should at least be compatible with the existing behaviour.

Finally, in order to make this all work in the _Windows Terminal_ app, I also had to add a basic implementation of the `ITermDispatch::LineFeed` method in the `TerminalDispatch` class. There is currently no need to support mode-specific line feeds here, so this simply forwards a `\n` or `\r\n` to the `Execute` method, which is ultimately handled by the `Terminal::_WriteBuffer` implementation.

## Validation Steps Performed

I've added output engine tests which confirm that the various control characters and escape sequences trigger the dispatch method correctly. Then I've added adapter tests which confirm the various dispatch options trigger the `PrivateLineFeed` API correctly. And finally I added some screen buffer tests that check the actual results of the `NEL` and `IND` sequences, which covers both forms of the `PrivateLineFeed` API (i.e. with and without a carriage return).

I've also run the _Test of cursor movements_ in the [Vttest](https://invisible-island.net/vttest/) utility, and confirmed that screens 1, 2, and 5 are now working correctly. The first two depend on `NEL` and `IND` being supported, and screen 5 requires the `VT` control character.
  • Loading branch information
j4james authored and msftbot[bot] committed Jan 15, 2020
1 parent 3e6b4b5 commit 701b421
Show file tree
Hide file tree
Showing 19 changed files with 340 additions and 6 deletions.
21 changes: 21 additions & 0 deletions src/cascadia/TerminalCore/TerminalDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,27 @@ try
}
CATCH_LOG_RETURN_FALSE()

bool TerminalDispatch::LineFeed(const DispatchTypes::LineFeedType lineFeedType) noexcept
try
{
switch (lineFeedType)
{
case DispatchTypes::LineFeedType::DependsOnMode:
// There is currently no need for mode-specific line feeds in the Terminal,
// so for now we just treat them as a line feed without carriage return.
case DispatchTypes::LineFeedType::WithoutReturn:
Execute(L'\n');
return true;
case DispatchTypes::LineFeedType::WithReturn:
Execute(L'\r');
Execute(L'\n');
return true;
default:
return false;
}
}
CATCH_LOG_RETURN_FALSE()

bool TerminalDispatch::EraseCharacters(const size_t numChars) noexcept
try
{
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalCore/TerminalDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class TerminalDispatch : public Microsoft::Console::VirtualTerminal::TermDispatc
bool CursorBackward(const size_t distance) noexcept override;
bool CursorUp(const size_t distance) noexcept override;

bool LineFeed(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::LineFeedType lineFeedType) noexcept override;

bool EraseCharacters(const size_t numChars) noexcept override;
bool SetWindowTitle(std::wstring_view title) noexcept override;

Expand Down
29 changes: 29 additions & 0 deletions src/host/getset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1347,6 +1347,35 @@ void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool
return Status;
}

// Routine Description:
// - A private API call for performing a line feed, possibly preceded by carriage return.
// Moves the cursor down one line, and possibly also to the leftmost column.
// Parameters:
// - screenInfo - A pointer to the screen buffer that should perform the line feed.
// - withReturn - Set to true if a carriage return should be performed as well.
// Return value:
// - STATUS_SUCCESS if handled successfully. Otherwise, an appropriate status code indicating the error.
[[nodiscard]] NTSTATUS DoSrvPrivateLineFeed(SCREEN_INFORMATION& screenInfo, const bool withReturn)
{
auto& textBuffer = screenInfo.GetTextBuffer();
auto cursorPosition = textBuffer.GetCursor().GetPosition();

// We turn the cursor on before an operation that might scroll the viewport, otherwise
// that can result in an old copy of the cursor being left behind on the screen.
textBuffer.GetCursor().SetIsOn(true);

// Since we are explicitly moving down a row, clear the wrap status on the row we're leaving
textBuffer.GetRowByOffset(cursorPosition.Y).GetCharRow().SetWrapForced(false);

cursorPosition.Y += 1;
if (withReturn)
{
cursorPosition.X = 0;
}

return AdjustCursorPosition(screenInfo, cursorPosition, FALSE, nullptr);
}

// Routine Description:
// - A private API call for performing a "Reverse line feed", essentially, the opposite of '\n'.
// Moves the cursor up one line, and tries to keep its position in the line
Expand Down
1 change: 1 addition & 0 deletions src/host/getset.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ void DoSrvPrivateShowCursor(SCREEN_INFORMATION& screenInfo, const bool show) noe
void DoSrvPrivateAllowCursorBlinking(SCREEN_INFORMATION& screenInfo, const bool fEnable);

[[nodiscard]] NTSTATUS DoSrvPrivateSetScrollingRegion(SCREEN_INFORMATION& screenInfo, const SMALL_RECT& scrollMargins);
[[nodiscard]] NTSTATUS DoSrvPrivateLineFeed(SCREEN_INFORMATION& screenInfo, const bool withReturn);
[[nodiscard]] NTSTATUS DoSrvPrivateReverseLineFeed(SCREEN_INFORMATION& screenInfo);
[[nodiscard]] HRESULT DoSrvMoveCursorVertically(SCREEN_INFORMATION& screenInfo, const short lines);

Expand Down
25 changes: 25 additions & 0 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,31 @@ bool ConhostInternalGetSet::PrivateSetScrollingRegion(const SMALL_RECT& scrollMa
return NT_SUCCESS(DoSrvPrivateSetScrollingRegion(_io.GetActiveOutputBuffer(), scrollMargins));
}

// Method Description:
// - Retrieves the current Line Feed/New Line (LNM) mode.
// Arguments:
// - None
// Return Value:
// - true if a line feed also produces a carriage return. false otherwise.
bool ConhostInternalGetSet::PrivateGetLineFeedMode() const
{
const auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
return gci.IsReturnOnNewlineAutomatic();
}

// Routine Description:
// - Connects the PrivateLineFeed call directly into our Driver Message servicing call inside Conhost.exe
// PrivateLineFeed is an internal-only "API" call that the vt commands can execute,
// but it is not represented as a function call on our public API surface.
// Arguments:
// - withReturn - Set to true if a carriage return should be performed as well.
// Return Value:
// - true if successful (see DoSrvPrivateLineFeed). false otherwise.
bool ConhostInternalGetSet::PrivateLineFeed(const bool withReturn)
{
return NT_SUCCESS(DoSrvPrivateLineFeed(_io.GetActiveOutputBuffer(), withReturn));
}

// Routine Description:
// - Connects the PrivateReverseLineFeed call directly into our Driver Message servicing call inside Conhost.exe
// PrivateReverseLineFeed is an internal-only "API" call that the vt commands can execute,
Expand Down
2 changes: 2 additions & 0 deletions src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::

bool PrivateSetScrollingRegion(const SMALL_RECT& scrollMargins) override;

bool PrivateGetLineFeedMode() const override;
bool PrivateLineFeed(const bool withReturn) override;
bool PrivateReverseLineFeed() override;

bool MoveCursorVertically(const ptrdiff_t lines) override;
Expand Down
89 changes: 89 additions & 0 deletions src/host/ut_host/ScreenBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ class ScreenBufferTests
TEST_METHOD(DeleteLinesInMargins);
TEST_METHOD(ReverseLineFeedInMargins);

TEST_METHOD(LineFeedEscapeSequences);

TEST_METHOD(ScrollLines256Colors);

TEST_METHOD(SetOriginMode);
Expand Down Expand Up @@ -4264,6 +4266,8 @@ void ScreenBufferTests::ReverseLineFeedInMargins()
_CommonScrollingSetup();
// Set the top scroll margin to the top of the screen
stateMachine.ProcessString(L"\x1b[1;5r");
// 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"); });
// Move to column 5 of line 1, the top of the screen
stateMachine.ProcessString(L"\x1b[1;5H");
// Execute a reverse line feed (RI)
Expand Down Expand Up @@ -4292,6 +4296,91 @@ void ScreenBufferTests::ReverseLineFeedInMargins()
}
}

void ScreenBufferTests::LineFeedEscapeSequences()
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:withReturn", L"{true, false}")
END_TEST_METHOD_PROPERTIES()

bool withReturn;
VERIFY_SUCCEEDED(TestData::TryGetValue(L"withReturn", withReturn));

auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
auto& si = gci.GetActiveOutputBuffer().GetActiveBuffer();
auto& stateMachine = si.GetStateMachine();
auto& cursor = si.GetTextBuffer().GetCursor();

std::wstring escapeSequence;
if (withReturn)
{
Log::Comment(L"Testing line feed with carriage return (NEL).");
escapeSequence = L"\033E";
}
else
{
Log::Comment(L"Testing line feed without carriage return (IND).");
escapeSequence = L"\033D";
}

// Set the viewport to a reasonable size.
const auto view = Viewport::FromDimensions({ 0, 0 }, { 80, 25 });
si.SetViewport(view, true);

// We'll place the cursor in the center of the line.
// If we are performing a line feed with carriage return,
// the cursor should move to the leftmost column.
const short initialX = view.Width() / 2;
const short expectedX = withReturn ? 0 : initialX;

{
Log::Comment(L"Starting at the top of viewport");
const short initialY = 0;
const short expectedY = initialY + 1;
const short expectedViewportTop = si.GetViewport().Top();
cursor.SetPosition(COORD{ initialX, initialY });
stateMachine.ProcessString(escapeSequence);

VERIFY_ARE_EQUAL(expectedX, cursor.GetPosition().X);
VERIFY_ARE_EQUAL(expectedY, cursor.GetPosition().Y);
VERIFY_ARE_EQUAL(expectedViewportTop, si.GetViewport().Top());
}

{
Log::Comment(L"Starting at the bottom of viewport");
const short initialY = si.GetViewport().BottomInclusive();
const short expectedY = initialY + 1;
const short expectedViewportTop = si.GetViewport().Top() + 1;
cursor.SetPosition(COORD{ initialX, initialY });
stateMachine.ProcessString(escapeSequence);

VERIFY_ARE_EQUAL(expectedX, cursor.GetPosition().X);
VERIFY_ARE_EQUAL(expectedY, cursor.GetPosition().Y);
VERIFY_ARE_EQUAL(expectedViewportTop, si.GetViewport().Top());
}

{
Log::Comment(L"Starting at the bottom of the scroll margins");
// Set the margins to rows 5 to 10.
stateMachine.ProcessString(L"\x1b[5;10r");
// 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"); });

const short initialY = si.GetViewport().Top() + 9;
const short expectedY = initialY;
const short expectedViewportTop = si.GetViewport().Top();
_FillLine(initialY, L'Q', {});
cursor.SetPosition(COORD{ initialX, initialY });
stateMachine.ProcessString(escapeSequence);

VERIFY_ARE_EQUAL(expectedX, cursor.GetPosition().X);
VERIFY_ARE_EQUAL(expectedY, cursor.GetPosition().Y);
VERIFY_ARE_EQUAL(expectedViewportTop, si.GetViewport().Top());
// Verify the line of Qs has been scrolled up.
VERIFY_IS_TRUE(_ValidateLineContains(initialY - 1, L'Q', {}));
VERIFY_IS_TRUE(_ValidateLineContains(initialY, L' ', si.GetAttributes()));
}
}

void ScreenBufferTests::ScrollLines256Colors()
{
BEGIN_TEST_METHOD_PROPERTIES()
Expand Down
7 changes: 7 additions & 0 deletions src/terminal/adapter/DispatchTypes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,13 @@ namespace Microsoft::Console::VirtualTerminal::DispatchTypes
SteadyBar = 6
};

enum class LineFeedType : unsigned int
{
WithReturn,
WithoutReturn,
DependsOnMode
};

constexpr short s_sDECCOLMSetColumns = 132;
constexpr short s_sDECCOLMResetColumns = 80;

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 EnableCursorBlinking(const bool enable) = 0; // ATT610
virtual bool SetOriginMode(const bool relativeMode) = 0; // DECOM
virtual bool SetTopBottomScrollingMargins(const size_t topMargin, const size_t bottomMargin) = 0; // DECSTBM
virtual bool LineFeed(const DispatchTypes::LineFeedType lineFeedType) = 0; // IND, NEL
virtual bool ReverseLineFeed() = 0; // RI
virtual bool SetWindowTitle(std::wstring_view title) = 0; // OscWindowTitle
virtual bool UseAlternateScreenBuffer() = 0; // ASBSET
Expand Down
22 changes: 22 additions & 0 deletions src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,28 @@ bool AdaptDispatch::SetTopBottomScrollingMargins(const size_t topMargin,
return _DoSetTopBottomScrollingMargins(topMargin, bottomMargin) && CursorPosition(1, 1);
}

// Routine Description:
// - IND/NEL - Performs a line feed, possibly preceded by carriage return.
// Moves the cursor down one line, and possibly also to the leftmost column.
// Arguments:
// - lineFeedType - Specify whether a carriage return should be performed as well.
// Return Value:
// - True if handled successfully. False otherwise.
bool AdaptDispatch::LineFeed(const DispatchTypes::LineFeedType lineFeedType)
{
switch (lineFeedType)
{
case DispatchTypes::LineFeedType::DependsOnMode:
return _pConApi->PrivateLineFeed(_pConApi->PrivateGetLineFeedMode());
case DispatchTypes::LineFeedType::WithoutReturn:
return _pConApi->PrivateLineFeed(false);
case DispatchTypes::LineFeedType::WithReturn:
return _pConApi->PrivateLineFeed(true);
default:
return false;
}
}

// Routine Description:
// - RI - Performs a "Reverse line feed", essentially, the opposite of '\n'.
// Moves the cursor up one line, and tries to keep its position in the line
Expand Down
1 change: 1 addition & 0 deletions src/terminal/adapter/adaptDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ namespace Microsoft::Console::VirtualTerminal
bool SetOriginMode(const bool relativeMode) noexcept override; // DECOM
bool SetTopBottomScrollingMargins(const size_t topMargin,
const size_t bottomMargin) override; // DECSTBM
bool LineFeed(const DispatchTypes::LineFeedType lineFeedType) override; // IND, NEL
bool ReverseLineFeed() override; // RI
bool SetWindowTitle(const std::wstring_view title) override; // OscWindowTitle
bool UseAlternateScreenBuffer() override; // ASBSET
Expand Down
2 changes: 2 additions & 0 deletions src/terminal/adapter/conGetSet.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ namespace Microsoft::Console::VirtualTerminal
virtual bool PrivateAllowCursorBlinking(const bool enable) = 0;

virtual bool PrivateSetScrollingRegion(const SMALL_RECT& scrollMargins) = 0;
virtual bool PrivateGetLineFeedMode() const = 0;
virtual bool PrivateLineFeed(const bool withReturn) = 0;
virtual bool PrivateReverseLineFeed() = 0;
virtual bool SetConsoleTitleW(const std::wstring_view title) = 0;
virtual bool PrivateUseAlternateScreenBuffer() = 0;
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 @@ -48,6 +48,7 @@ class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Cons
bool EnableCursorBlinking(const bool /*enable*/) noexcept override { return false; } // ATT610
bool SetOriginMode(const bool /*relativeMode*/) noexcept override { return false; }; // DECOM
bool SetTopBottomScrollingMargins(const size_t /*topMargin*/, const size_t /*bottomMargin*/) noexcept override { return false; } // DECSTBM
bool LineFeed(const DispatchTypes::LineFeedType /*lineFeedType*/) noexcept override { return false; } // IND, NEL
bool ReverseLineFeed() noexcept override { return false; } // RI
bool SetWindowTitle(std::wstring_view /*title*/) noexcept override { return false; } // OscWindowTitle
bool UseAlternateScreenBuffer() noexcept override { return false; } // ASBSET
Expand Down
47 changes: 47 additions & 0 deletions src/terminal/adapter/ut_adapter/adapterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,24 @@ class TestGetSet final : public ConGetSet
return _privateSetScrollingRegionResult;
}

bool PrivateGetLineFeedMode() const override
{
Log::Comment(L"PrivateGetLineFeedMode MOCK called...");
return _privateGetLineFeedModeResult;
}

bool PrivateLineFeed(const bool withReturn) override
{
Log::Comment(L"PrivateLineFeed MOCK called...");

if (_privateLineFeedResult)
{
VERIFY_ARE_EQUAL(_expectedLineFeedWithReturn, withReturn);
}

return _privateLineFeedResult;
}

bool PrivateReverseLineFeed() override
{
Log::Comment(L"PrivateReverseLineFeed MOCK called...");
Expand Down Expand Up @@ -893,6 +911,9 @@ class TestGetSet final : public ConGetSet
bool _privateAllowCursorBlinkingResult = false;
bool _enable = false; // for cursor blinking
bool _privateSetScrollingRegionResult = false;
bool _privateGetLineFeedModeResult = false;
bool _privateLineFeedResult = false;
bool _expectedLineFeedWithReturn = false;
bool _privateReverseLineFeedResult = false;

bool _setConsoleTitleWResult = false;
Expand Down Expand Up @@ -2109,6 +2130,32 @@ class AdapterTest
VERIFY_IS_FALSE(_pDispatch.get()->SetTopBottomScrollingMargins(srTestMargins.Top, srTestMargins.Bottom));
}

TEST_METHOD(LineFeedTest)
{
Log::Comment(L"Starting test...");

// All test cases need the LineFeed call to succeed.
_testGetSet->_privateLineFeedResult = TRUE;

Log::Comment(L"Test 1: Line feed without carriage return.");
_testGetSet->_expectedLineFeedWithReturn = false;
VERIFY_IS_TRUE(_pDispatch.get()->LineFeed(DispatchTypes::LineFeedType::WithoutReturn));

Log::Comment(L"Test 2: Line feed with carriage return.");
_testGetSet->_expectedLineFeedWithReturn = true;
VERIFY_IS_TRUE(_pDispatch.get()->LineFeed(DispatchTypes::LineFeedType::WithReturn));

Log::Comment(L"Test 3: Line feed depends on mode, and mode reset.");
_testGetSet->_privateGetLineFeedModeResult = false;
_testGetSet->_expectedLineFeedWithReturn = false;
VERIFY_IS_TRUE(_pDispatch.get()->LineFeed(DispatchTypes::LineFeedType::DependsOnMode));

Log::Comment(L"Test 4: Line feed depends on mode, and mode set.");
_testGetSet->_privateGetLineFeedModeResult = true;
_testGetSet->_expectedLineFeedWithReturn = true;
VERIFY_IS_TRUE(_pDispatch.get()->LineFeed(DispatchTypes::LineFeedType::DependsOnMode));
}

TEST_METHOD(TabSetClearTests)
{
Log::Comment(L"Starting test...");
Expand Down
Loading

0 comments on commit 701b421

Please sign in to comment.